-
Notifications
You must be signed in to change notification settings - Fork 414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(move-to-finished): consider addition of prioritized jobs when processing last active job #2176
fix(move-to-finished): consider addition of prioritized jobs when processing last active job #2176
Conversation
…cessing last active job
tests/test_worker.ts
Outdated
@@ -669,14 +669,44 @@ describe('workers', function () { | |||
const worker = new Worker(queueName, processor, { connection }); | |||
await worker.waitUntilReady(); | |||
|
|||
// wait for all jobs to enter the queue and then start processing | |||
await Promise.all([normalPriority, mediumPriority, highPriority]); | |||
|
|||
await processing; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by awaiting this line is sufficient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small typo
src/commands/moveToFinished-13.lua
Outdated
if jobId then | ||
if string.sub(jobId, 1, 2) == "0:" then | ||
rcall("LREM", KEYS[2], 1, jobId) | ||
|
||
-- If jobId is special ID 0:delay (delay grater than 0), then there is no job to process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grater -> greater
tests/test_worker.ts
Outdated
@@ -669,14 +669,44 @@ describe('workers', function () { | |||
const worker = new Worker(queueName, processor, { connection }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roggervalf I think that
await Promise.all([...normalPriority,...mediumPriority,...highPriority]);
should be added before creating a new worker, as you need to make sure that all jobs are entered before worker starts processing jobs.
When I run the test with this fixes the test failed as some jobs where added after the worker start processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, probably in case of dragonfly its needed, ok let me add it back
# [4.10.0](v4.9.0...v4.10.0) (2023-09-12) ### Bug Fixes * **move-to-finished:** consider addition of prioritized jobs when processing last active job ([#2176](#2176)) (python) ([4b01f35](4b01f35)) * **remove:** change error message when job is locked (python) ([#2175](#2175)) ([2f5628f](2f5628f)) ### Features * **flow-producer:** add addBulk method (python) ([#2174](#2174)) ([c67dfb4](c67dfb4))
No description provided.