fix(queue): different score purpose per state #2133
Conversation
src/commands/includes/cleanSet.lua
Outdated
| jobTS = getTimestamp(jobKey, attributes) | ||
| if (not jobTS or jobTS < timestamp) then | ||
| if (not jobTS or jobTS <= timestamp) then |
There was a problem hiding this comment.
I think better to consider <= as for completed and failed we can bring all the expected jobs in ZRANGEBYSCORE
There was a problem hiding this comment.
I need more context to understand this 😅
| @@ -2,11 +2,17 @@ | |||
| -- of items in a sorted set only run a single iteration. If we simply used | |||
| -- ZRANGE, we may take a long time traversing through jobs that are within the | |||
| -- grace period. | |||
| local function getJobsInZset(zsetKey, rangeStart, rangeEnd, maxTimestamp, limit) | |||
| local function getJobsInZset(zsetKey, rangeStart, rangeEnd, maxTimestamp, limit, useTimestampAsScore) | |||
There was a problem hiding this comment.
I am confused, why aren't we using rangeEnd anymore?
Btw, instead of sending useTimeAsScore, couldn't we just set the proper rangeEnd argument before calling this function?
There was a problem hiding this comment.
rangeStart is not used anymore either 🤔
src/commands/includes/cleanSet.lua
Outdated
| jobTS = getTimestamp(jobKey, attributes) | ||
| if (not jobTS or jobTS < timestamp) then | ||
| if (not jobTS or jobTS <= timestamp) then |
There was a problem hiding this comment.
I need more context to understand this 😅
|
#2133 (comment) just to be consistent with a perf change in last commit, when getting finished states, we are getting all the jobs in the period of time to be removed also including the time passed parameter, so the other states should consider it as well |
| @@ -34,13 +34,19 @@ local result | |||
| if ARGV[4] == "active" then | |||
| result = cleanList(KEYS[1], ARGV[1], rangeStart, rangeEnd, ARGV[2], false) | |||
| elseif ARGV[4] == "delayed" then | |||
| result = cleanSet(KEYS[1], ARGV[1], rangeStart, rangeEnd, ARGV[2], limit, {"processedOn", "timestamp"}) | |||
| rangeEnd = "+inf" | |||
There was a problem hiding this comment.
zsets uses +inf instead of -1 on lists
| -- * finishedOn says when the job was completed, but it isn't set unless the job has actually completed | ||
| jobTS = getTimestamp(jobKey, attributes) | ||
| if (not jobTS or jobTS < timestamp) then | ||
| if isFinished then |
There was a problem hiding this comment.
when clean is made for completed or failed states, we should be ok deleting job keys without timestamp check, as when we get these records we uses timestamp as one of the limits, this is why I'm using <= to be consistent on the other states
ref #2130
fixes #2124