Repository navigation
DELETE cancels a failed deployment whose job still runs (#101, backend half) - #185
Open
dadamsncsa wants to merge 5 commits into
Open
dadamsncsa wants to merge 5 commits into
dadamsncsa wants to merge 5 commits into
Conversation
…#101) shutdown_deployment returned early for every failed row, so a job vec-inf had reported FAILED from a line in its log kept its nodes to the time limit (3277706 ran to TIMEOUT; 3282264 was still RUNNING after its shutdown request). A failed row whose job sacct shows pending or running is now cancelled through the same impersonated scancel as a running one and marked shutdown. A failed row whose job has ended, or that never had a job, is left as it is. Only a DELETE does this: the status sync still never cancels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
…acct's time From /code-review on PR 4. A DELETE on a failed row now logs a warning when sacct gives no state, since the job is then left alone, and sacct gets a 30 s timeout because it gates a request. The CHANGELOG says the UI still offers no Stop for a failed deployment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
…ound sacct only on DELETE From PR 4's re-review and fresh reader. When sacct couldn't answer, a DELETE on a failed row left a possibly live job running and still answered 200 (the #101 outcome); it now cancels unless sacct reports an ended state, since cancelling an ended job does no harm. That also covers live states the old set missed (STOPPED, REQUEUE_HOLD, SIGNALING). sacct's timeout is passed only by shutdown_deployment, so the status sync's sacct lookup is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
From the extra /code-review of 1bb4b8e. When sacct can't say and the job had in fact ended, scancel refuses it; the failure branch then overwrote the failed row's errorMessage with scancel's error. A failed row now keeps its reason (the scancel error is logged). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
…ob is gone From the /code-review of 3a5100e: the guard kept the failure reason for every scancel error on a failed row, so a live job that could not be cancelled (an impersonation error, say) looked like an ended one. The reason is now kept, and the row left unwritten, only when scancel says the job is already gone ("Invalid job id specified", "already completing or completed"); any other cancel failure is recorded on the row, as for a running one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
This branch has not been deployed
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.
Part of #101: its backend half (the kit half was #103). #101 stays open for the UI's missing Stop on a failed deployment and for Delta's single-node false FAILED (
cxip_ep_close ... Unable to free EP object, Qwen2.5-32B on 2 GPUs). Part of #60.Problem
ModelService.shutdown_deploymentreturned early for everyfailedrow. vec-inf reports FAILED from a line in a job's log while Slurm still runs the job. ADELETEon that row then cancelled nothing, and the job held its nodes to the time limit: 3277706 ran to TIMEOUT, and 3282264 was still RUNNING after its shutdown request.What changes
When the
DELETElands on afailedrow with a Slurm job id, the backend askssacctfor the job's state:sacctsays it ended (COMPLETED, or one of the states the status sync already counts as failed: FAILED, CANCELLED, TIMEOUT, NODE_FAIL, …), the row is left as it is;scancelas a running row, and the row becomesshutdown.Two more cases:
sacctgives no state (an error, no row, or no answer in 30 s), the job is cancelled anyway, with a warning. Ifscancelthen says the job is already gone, the row keeps its failure reason; any other cancel failure is recorded on the row, as for a running one;failedrow that never had a job (failed/awaiting-download) is left alone, and Slurm is not asked.Only a
DELETEdoes this. The status sync never cancels, so a false FAILED cannot kill a serving job. The UI still offers no Stop for afaileddeployment; API callers and the kit'slaunch-testsend theDELETE.Evidence (DeltaAI
gh-login03, at2f5edfa)tests/test_shutdown_failed_deployment.py: 16 passed. Each test sends the realDELETEthroughModelServiceandLLMInferenceClientin impersonate mode, with a recording impersonation wrapper and a fakesacct.--no-login-shell <user> -- scancel <job>). It endsshutdownand releases its GPUs.saccterroring, or hanging past the timeout, still cancels the job and logs the warning. A cancelscancelrefuses for a gone job leaves a failed row's reason as it was; one that fails otherwise (an impersonation error on a live job) is recorded.git archiveof the head, and its tests exit 1 (the unedited control exits 0):failedrestored to the early return: 5 failed;/simplify,/code-review, a re-review, a fresh reader and two more/code-reviewrounds; each finding is fixed or answered.2f5edfa(the fix for the last review's findings) has not been reviewed by an agent.Rollback
Revert the PR. It has no config or schema change.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2