supervisor: Don't use "safe" unlock methods #2032
Draft
Conversation
This file contains hidden or 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
Paddles uses this format. Signed-off-by: Zack Cerza <[email protected]>
This was simply faulty logic. One effect was that the supervisor would refuse to unlock nodes that were used by its own job. Signed-off-by: Zack Cerza <[email protected]>
Signed-off-by: Zack Cerza <[email protected]>
The supervisor knows when its job's nodes are no longer needed. Signed-off-by: Zack Cerza <[email protected]>
VallariAg
reviewed
Mar 7, 2025
| if job_config.get("unlock_on_failure", True): | ||
| log.info('Unlocking machines...') | ||
| lock_ops.unlock_safe(locked, job_config["owner"], job_config["name"], job_config["job_id"]) | ||
| lock_ops.unlock_many(locked, job_config["owner"]) |
There was a problem hiding this comment.
Why did we switch to unlock_many? It doesn't seem to have any checks before unlocking the nodes.
jmundack
requested review from
a team,
VallariAg and
amathuria
and removed request for
jmundack and
a team
VallariAg
approved these changes
Mar 10, 2025
|
@amathuria - mind taking a look when you have a few mins? |
|
I discovered a potential issue with jobs that run too long and get killed; I'll update once I've gotten to the bottom of it. |
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.
This fixes a bug where the supervisor would avoid locking its own nodes.
Example from job 8142206:
Note that the job IDs are the same.