Repository navigation
Keep terminated deploys terminated - #420
Merged
Priyanshu-u07 merged 3 commits intoOct 24, 2026
Merged
Conversation
SCHEDULING and DEPLOYING were still plain writes, so a terminate before placement was overwritten and later promoted to RUNNING. Use the guarded write and return when the deploy has ended. When an ended deploy deprovisions the node it just created, also mark that node's inventory row terminated so it does not stay ready. Signed-off-by: Priyanshu-u07 <connect.priyanshu8271@gmail.com>
Priyanshu-u07
approved these changes
Oct 24, 2026
Priyanshu-u07
left a comment
Collaborator
There was a problem hiding this comment.
Pushed two fixes to this branch:
- The placement path's SCHEDULING and DEPLOYING writes now use the guarded write and return if the deploy has ended, so a terminate before placement is no longer overwritten. Nothing has been created at that point, so there's nothing to clean up.
- When an ended deploy deprovisions the node it just created, its inventory row is now marked terminated instead of staying ready.
- Tests added for both.
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
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.
If you terminate a deployment while the worker is still provisioning it, the worker can set it back to RUNNING when it finishes.
update_statewas a plainUPDATE ... WHERE deployment_id=$1, and the worker wrote RUNNING (and FAILED) with it at the end of each provisioning path. There's a "state changed during provisioning" check in the job path, but after it the worker still doesupdate_endpoint,register_nodeandattach_runtimebefore writing RUNNING. The placement path writes RUNNING afterstrategy.deploywith no check. A terminate landing in that gap gets overwritten: the deploy shows RUNNING again, but terminate already released its GPU.Changes:
update_statetakes an optionalunless_inand skips the write if the current state is in it. It returns whether it wrote. Callers that don't pass it behave the same as before, so start/resume from TERMINATED still works.handle_deploy_requestedthe worker's state writes skip when the deploy is TERMINATING, TERMINATED or STOPPED.Testing:
test_terminate_during_provisioning_is_not_overwritten: it terminates duringattach_runtime. It fails on main (state ends up RUNNING) and passes here (state stays TERMINATED and the node is deprovisioned).Not covered: on the placement path, if terminate already ran, a model that
strategy.deployloaded on an existing node isn't unloaded here. The state now stays right, but that's a separate cleanup.