Populate enqueued_at when enqueuing jobs about verk HOT 5 CLOSED

Kia Ora!

I had some time to work on this. According to https://redis.io/commands/eval we can't get system time in Lua script itself:

In order to enforce this behavior in scripts Redis does the following:
Lua does not export commands to access the system time or other external state.
Redis will block the script with an error if a script calls a Redis command able to alter the data set after a Redis random command like RANDOMKEY, SRANDMEMBER, TIME. This means that if a script is read-only and does not modify the data set it is free to call those commands. Note that a random command does not necessarily mean a command that uses random numbers: any non-deterministic command is considered a random command (the best example in this regard is the TIME command).

127.0.0.1:6379> eval "local t = redis.call('TIME'); redis.call('LPUSH', 'test_queue', t[1]); return t[1]" 0
(error) ERR Error running script (call to f_23907c1ac8a9f92f2e36a750baa9b1ac885dc9d6): @user_script:1: @user_script: 1: Write commands not allowed after non deterministic commands

So it have to be passed to the script as argument, which is already done anyway - it's used for the sorted set score.

I did not want to make the script aware for the the difference between the retry and schedule sets, so we can either change it to set enqueued_at for all jobs that don't have this already (assuming these will be coming only from the schedule set), which I did in https://github.com/krasio/verk/tree/issues/109 or we can change the ScheduleManager to pass additional param when handling fetch_scheduled - https://github.com/krasio/verk/tree/issues/109-2.
The downside of the former approach is that after upgrade all jobs from the retry set will get their enqueued_at set to the time they were retried first time after upgrade.

Let me know what do you think and I'll send a pull request.

from verk.

Hey @krasio! I think it's fine. Can you set up a PR? Sorry for the delay

from verk.

Which one of the approaches will you prefer?

from verk.

Oops sorry I forgot to specify which one. I would simply populate enqueued_at when the data is missing. The downside is that people upgrading will see a wrong enqueued_at when there's a retry but after some time it will be fine. What do you reckon?

from verk.

I'm fine with that, will submit PR soon.

from verk.