Repository navigation
fix(queue): roll back PullWork RUNNING state when post-update Get fails (round-25 audit B.1) - #468
Merged
Merged
Conversation
lusoris
marked this pull request as ready for review
May 31, 2026 08:20
lusoris
enabled auto-merge (squash)
May 31, 2026 08:20
lusoris
marked this pull request as draft
May 31, 2026 09:01
auto-merge was automatically disabled
May 31, 2026 09:01
Pull request was converted to draft
lusoris
marked this pull request as ready for review
May 31, 2026 11:34
lusoris
enabled auto-merge (squash)
May 31, 2026 11:34
…ls (round-25 audit B.1)
lusoris
force-pushed
the
fix/queue-pullwork-rollback-on-get-failure
branch
from
May 31, 2026 11:36
d6b98c0 to
a577385
Compare
…tally reverted by rebase
1 of 6 tasks
lusoris
added a commit
that referenced
this pull request
May 31, 2026
…okForTest and #472 ListAll (both additions kept)
lusoris
added a commit
that referenced
this pull request
May 31, 2026
…nel (round-25 audit) Round 25 audit B.3 + B.4 fixes for the vmafx-controller. Re-applied cleanly on top of current master (PR #468 SetGetUnlockedHookForTest + PR #474 helm seccomp) after a previous quick-rebase left conflict markers in cmd/vmafx-controller/queue/queue.go and the AGENTS.md sidecar. B.3 — controllerServer.StreamJobs snapshot: * queue.Queue interface gains ListAll(ctx, statuses) ([]*Job, error). * SQLiteQueue implements it with a status-IN filter (parameterised, bounded to the 5-element Status enum) and per-row copy semantics. * grpc_server.go StreamJobs sends a single snapshot via ListAll, converting Job.Status with protoStatusToQueue/queueStatusToProto helpers kept in lockstep. B.4 — reaper goroutine stop signal: * nodes.NewRegistry(ctx, log) signature now takes a required context. * reaper goroutine selects on ctx.Done() + ticker.C and exits when the controller's shutdown context is cancelled. * registry gains Close() which cancels its own derived context. * main.go wires NewShutdownContext()'s context into NewRegistry and defers nodeRegistry.Close() after jobQueue.Close(). Tests, ADR, research digest, changelog fragment, AGENTS.md sidecar, rebase-notes entry, ADR index update — all in this PR. ADR-0962. Research: docs/research/0962-controller-streamjobs-reaper-fixes-2026-05-31.md. Conflict resolution notes: * cmd/vmafx-controller/queue/queue.go — both #468's SetGetUnlockedHookForTest and #472's ListAll/repeatCommaQ kept, with the missing closing brace for SetGetUnlockedHookForTest restored (previous botched rebase had sed-stripped the marker and left a syntax error). * cmd/vmafx-controller/AGENTS.md — kept both halves additively: governance/queue invariants from master plus rebase-sensitive invariants from this PR, with ADR-0962 added to the Governing ADRs table and entries split across the queue / nodes / grpc server / main sections. * docs/adr/README.md, docs/adr/_index_fragments/_order.txt, docs/rebase-notes.md — additive (both halves preserved).
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
1 participant
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.
Fix Round 25 audit B.1:
queue.PullWorkorphans jobs in RUNNING state when post-UPDATEgetUnlockedread fails.Bug: 3-step state mutation (FIFO pop → SQL
UPDATE jobs SET status=running→runningSet[matchID]insert) had no rollback whengetUnlockedfailed afterward. Job permanently stuck RUNNING until controller restart.Fix:
rollbackToPending(jobID)called ongetUnlockederror path. ExecutesUPDATE jobs SET status='pending', assigned_node=NULL WHERE id=?, removes fromrunningSet, re-prepends FIFO. CRITICAL-level log if rollback SQL itself fails.Test added:
TestPullWork_GetUnlockedFailure_RollsBackToPending— uses newSetGetUnlockedHookForTestcount-based escape hatch to inject failure, asserts SQL statuspending,assigned_nodeempty,RunningCount==0,PendingCount==1, retry succeeds.All 10 queue tests pass. gofmt + go vet clean.
no state delta: bug fix per CLAUDE.md §12 r8 (no ADR for bug fix); state.md row added in same PR for T-CONTROLLER-QUEUE-PULLWORK-ORPHAN-2026-05-31 is in the diff.ADR-0108 deliverables checklist
go test ./cmd/vmafx-controller/queue/... -v -run TestPullWork— 10/10 pass.🤖 Generated with Claude Code