fix(drain): consider checking parent jobs when draining #992
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roggervalf
commented
Jan 15, 2022
tests/test_queue.ts
Outdated
| expect(keys.length).to.be.eql(5); | ||
|
|
||
| const countAfterEmpty = await queue.count(); | ||
| expect(countAfterEmpty).to.be.eql(1); |
There was a problem hiding this comment.
this job is the parent that was moved to wait state, before it was in waiting-children state
roggervalf
commented
Jan 18, 2022
| rcall("ZREM", parentPrefix .. "waiting-children", parentId) | ||
|
|
||
| if rcall("HEXISTS", parentPrefix .. "meta", "paused") ~= 1 then | ||
| rcall("RPUSH", parentPrefix .. "wait", parentId) | ||
| if parentPrefix == baseKey then |
There was a problem hiding this comment.
we should only delete those records that belongs to the same queue that runs this script, on the contrary case, move it to wait or paused
Yes, this one is ready |
manast
reviewed
Jan 28, 2022
| ``` | ||
|
|
||
| {% hint style="info" %} | ||
| Locked jobs (in active state) could not be removed. An error would be thrown. |
There was a problem hiding this comment.
"could" -> "can", "would" -> "will"
manast
reviewed
Jan 28, 2022
|
|
||
| There are 2 possible cases: | ||
|
|
||
| 1. There are not pending dependencies; in this case the parent is moved to wait state, we may try to process this job. |
manast
reviewed
Jan 28, 2022
| There are 2 possible cases: | ||
|
|
||
| 1. There are not pending dependencies; in this case the parent is moved to wait state, we may try to process this job. | ||
| 2. There are pending dependencies; in this case the parent is kept in waiting-children state. |
There was a problem hiding this comment.
-> "in the waiting-children status"
manast
reviewed
Jan 28, 2022
|
|
||
| # Having pending dependencies | ||
|
|
||
| We may try to remove all its pending decendents first. |
There was a problem hiding this comment.
"decendents" -> "descendents"
manast
reviewed
Jan 28, 2022
| We may try to remove all its pending decendents first. | ||
|
|
||
| {% hint style="warning" %} | ||
| in case one of the children is locked, we would stop the deletion process. |
There was a problem hiding this comment.
"in" -> "In", "we would" -> "it will"
manast
reviewed
Jan 28, 2022
| ``` | ||
|
|
||
| {% hint style="warning" %} | ||
| Parent jobs that belong to the queue being drained will be kept in **waiting-children** state if they have pending children, but if they do not have any pending children they will just be removed. |
manast
reviewed
Jan 28, 2022
github-actions bot
pushed a commit
that referenced
this pull request
Jan 28, 2022
## [1.67.3](v1.67.2...v1.67.3) (2022-01-28) ### Bug Fixes * **drain:** consider checking parent jobs when draining ([#992](#992)) ([81b7221](81b7221))
|
🎉 This PR is included in version 1.67.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we call drain, we were not considering checking the parent dependencies, so those records could be moved to wait or paused depending on how many dependencies they still have, this way we don't end in an scenario where parents have dependencies but the actual child record is deleted