fix(scheduler): correct claim retry queue (ladder, give-up, native)#106
Merged
Conversation
The claim retry queue merged in #104 has three correctness bugs; fix them in place (config-gated and enabled by default, so this runs on every fleet host): 1. Backoff ladder never advances / give-up never fires. fireDue deleted the popped item from the index, so runOne's failure re-Add built a fresh item — attempts reset to 1 (stuck at schedule[0] ~30s) and firstFailure reset every fire (MaxAge never triggered). A job that couldn't be claimed retried every 30s forever, burning the shared installation token it was waiting on. Fix: fireDue keeps the item in the index across a fire. 2. Retries dropped after one attempt. With (1) half-fixed you get the opposite: retryHandler always returned nil, and runOne dropping on nil removed the item handleQueued had just re-Added synchronously — so a job self-healed at most once. Fix: retryHandler returns the real claim error (captured via a context pointer that enqueueRetryIfEligible fills in, which also suppresses the duplicate enqueue). runOne drops on success / advances on failure, with the rate-limit error class preserved so snap-to-reset still applies on later attempts. 3. Native macOS claim failures didn't self-heal — handleNativeMacOSJob was the one claim path not wired to the queue (Windows already routes through handleLocalJob). Wired it up. Tests: TestRetryQueue_StillFailingRetryStaysEnqueued drives the real fireDue->retryHandler->claim path (fails before this change); TestRetryQueue_LadderAdvancesThroughFirePath asserts the delay grows 30s->1m->2m and firstFailure is preserved. Full scheduler suite green, -race and go vet clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ChVRC9ZrvQarrctDwSy1S5
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.
The claim retry queue merged in #104 ships three correctness bugs. This patches them in place on current
main(the queue is config-gated but enabled by default, so it runs on every host). Supersedes my earlier #103, which went stale once #104/#105 movedmain.fireDuedeleted the popped item from the index, sorunOne's failure re-Addbuilt a fresh item —attemptsreset to 1 (stuck at ~30s) andfirstFailurereset every fire (MaxAge never triggered). An unclaimable job retried every 30s forever, burning the very shared token it was waiting on. Fix:fireDuekeeps the item in the index across a fire.retryHandleralways returned nil, andrunOnedropping on nil removed the itemhandleQueuedhad just re-Added synchronously. Fix:retryHandlerreturns the real claim error via a context pointer thatenqueueRetryIfEligiblefills in (also suppressing the duplicate enqueue); the rate-limit class is preserved so snap-to-reset still works.handleNativeMacOSJobwas the one claim path not wired to the queue. Wired it up.Tests
TestRetryQueue_StillFailingRetryStaysEnqueueddrives the realfireDue→retryHandler→claimpath and fails before this change.TestRetryQueue_LadderAdvancesThroughFirePathasserts the delay grows 30s→1m→2m andfirstFailureis preserved.-raceandgo vetclean.🤖 Generated with Claude Code