jobs: clear claim for already-dead paused jobs #92121
Merged
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
Previously we only cleared the claim after the state machine returned and only if the status wasn't pause-requested or cancel-requested. This filter on status, however, was unnecessary. The job may still be in the cancel-requested or pause-requested state when we go to clear the claim because the transaction that resulted in the canceled context may not have completed. But, it is still fine to clear the claim. There are 1 of two cases: 1) Either the transaction that cancelled us fails and we are thus still in the state cancel-requested or paused-requested with no claim. This is fine. The adoption loop will adopt the job and move the state to paused or reverting, just with no context to cancel. 2) The transaction succeeds and we are in paused or reverting without a claim set. Just as we wanted. Here we remove the where clause to always clear the claim when we return from the state machine. In the case of (1), when processing the cancel-requested or paused-requested state the second time, we may still want the claim cleared. Here, we make sure it gets cleared even in the case where there is no running job that actually needs to be canceled. Fixes cockroachdb#92112 Release note: None
|
This change is |
ajwerner
approved these changes
Nov 22, 2022
|
bors r=ajwerner |
|
Build succeeded: |
This was referenced Dec 12, 2022
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.
Previously we only cleared the claim after the state machine returned and only if the status wasn't pause-requested or
cancel-requested. This filter on status, however, was unnecessary.
The job may still be in the cancel-requested or pause-requested state when we go to clear the claim because the transaction that resulted in the canceled context may not have completed. But, it is still fine to clear the claim. There are 1 of two cases:
Either the transaction that cancelled us fails and we are thus
still in the state cancel-requested or paused-requested with no
claim. This is fine. The claim-jobs loop will claim the job and we will then move
the state to paused or reverting, just with no context to cancel.
The transaction succeeds and we are in paused or reverting without
a claim set. Just as we wanted.
Here we remove the where clause to always clear the claim when we return from the state machine.
In the case of (1), when processing the cancel-requested or paused-requested state the second time, we may still want the claim cleared. Here, we make sure it gets cleared even in the case where there is no running job that actually needs to be canceled.
Fixes #92112
Epic: None
Release note: None