fix: [sc-106110] Cap and jitter the auto-update retry backoff - #101
Merged
mlataza merged 4 commits intoAug 13, 2026
Merged
Conversation
The auto-update retry delay was computed as baseBackoff * (1 << attempt) with no ceiling and no jitter - the same defect already fixed for postback retries in sc-103965, which the update path never got. The unjittered schedule is the more immediate problem: every agent that fails against the release endpoint at the same moment retries at exactly base, 2*base, 4*base, so a GitHub API outage or rate limit turns the fleet into a synchronized retry storm that sustains the condition it is recovering from, and endpoints stay on old versions - missing shipped fixes - far longer than the outage lasted. The missing ceiling is the quieter half: at the production 5 minute base, attempt 10 alone would sleep over 42 hours, longer than the 48 hour check interval the retries are nested inside, and because maxRetries is injectable a large value overflows the shift into a negative time.Duration, so time.After fires immediately and the loop spins issuing HTTP requests as fast as it can. Extract the postback fix's schedule math into utils.JitteredBackoff rather than duplicating it, and point both retry paths at it. The doubling is performed by iterated multiplication with an early exit at the cap, so no intermediate value can overflow; jitter of up to +/-25% is applied after clamping and the result is re-clamped, so every slot is strictly positive, never exceeds the cap, and spreads across a fleet. postbackRetryBackoff keeps its own defaults and is otherwise unchanged in behavior. The update cap is the lower of a documented DefaultUpdateMaxRetryBackoff (1 hour) and a quarter of the check interval, so the retry budget always stays nested inside the interval it belongs to. The interval fraction matters for builds that shorten the interval via ldflags; at the production 48 hour interval the absolute ceiling is far lower and wins. The production schedule is unchanged where it matters - 5m, 10m, 20m, 40m - and then flattens at an hour instead of growing to 80 minutes, 42 hours, and eventually a negative sleep. The backoff wait still selects on the stop signal, so a service stop is never delayed by a pending retry. Tests assert every slot is positive and capped across the full attempt range for deliberately absurd maxRetries values (64, 128, 1000) and spot checks past 1<<20, that repeated computations of the same slot produce a distribution rather than one repeated value, that a short-interval build's whole retry budget fits inside its interval, and cover cap selection, base-above-cap clamping and non-positive inputs. The existing stop-during-backoff test now raises the cap explicitly so it still measures a stop interrupting a genuinely long wait.
… integration workflow The cap and the jitter are unit tested against the schedule function, which cannot show that a real agent facing an unavailable release endpoint now retries on bounded, spread-out slots instead of a synchronized schedule that can overflow into a busy-spin. Add a scenario that drives the retry loop on all three platforms against an endpoint that fails and recovers on demand. A GitHub outage is not something CI can arrange, and an unroutable address produces connection errors rather than the HTTP failure the ticket describes, so the scenario needs a stub the agent will actually query. The release endpoint is compiled in, so it gets a build-time-gated seam: releaseUrlOverrideFileStr names a file in the org's data directory whose contents replace the endpoint, and it is injected only into the integration binary. A released build leaves it empty and never looks for the file at all, so a shipped agent's update source stays fixed at build time and cannot be redirected by dropping a file on an endpoint. Unit tests pin both halves: the released build ignoring a planted override, and the integration build honoring a valid one while falling back to the compiled-in endpoint for a missing, empty or whitespace-only file - a fixture that failed to write the file leaves the agent updating normally rather than silently not updating at all. The fixture, test/stubrelease, answers over plain HTTP on loopback, so unlike the stub broker nothing has to be installed into the host trust store. It fails every request with 503 until its mode file flips, which it re-reads per request, so the endpoint can recover in the middle of a retry sequence. It logs every arrival with millisecond precision, which is the request-spacing record the ticket asks for, and serves the running agent's own version as tag_name so a recovered check ends at "No updates available" - the recovery is what is under test, not the installer. The scenario points the agent at the failing stub, installs the integration binary with auto-updates on, waits for three retries, flips the endpoint mid-sequence and asserts the update succeeds on a retry, then asserts the schedule itself: it prints an attempt/backoff/elapsed table and checks every slot is strictly positive, within the cap, jittered, growing, and never served back to back. Distinct backoff values alone would be weak evidence - the doubling produces those on its own until it reaches the ceiling - so the slots that do reach the ceiling must differ from each other, which is what an unjittered schedule cannot do. It then fails the endpoint again and asserts a service stop issued mid-backoff is honored in under 20s rather than waiting out the remaining budget. Every restore step runs under always(), so a failed assertion cannot leave the rest of the job running an agent pointed at a stub endpoint or leave the service stopped. The integration build's base backoff drops from 10s to 2s. With the 30s check interval the cap is a quarter of it (7.5s), so a 10s base was clamped on every slot and the doubling was unobservable; at 2s the schedule climbs 2s, 4s, then flattens at the cap, and both halves of the fix show up in one sequence. Verified locally as far as a macOS host allows: the fixture serves 503 and the release payload correctly, driving the real AutoUpdateRunner against it produced 1.87s, 3.48s, 7.5s, 6.29s, 7.49s, 6.41s - capped and jittered - and a mid-sequence flip logged "Update succeeded on retry: attempt=3". The analysis script passes on that real log and fails correctly on synthetic pre-fix logs: uncapped doubling, negative durations arriving back to back, and a capped but unjittered schedule. Every new PowerShell block parses under pwsh and actionlint is clean. The actions themselves - service managers, the WMI launch, elevation - can only run on the runners.
… a backoff The integration scenario proved the retry wait is interruptible - the stop was honored 276ms into a 2.17s backoff on Linux, 474ms and 541ms into comparable slots on Windows and macOS - but its assertion failed on all three platforms because nothing was written to the log. The run loop logs "Auto updater stopped" only when its own select observes the stop, i.e. while idle between checks. A stop observed inside the backoff wait returns through retryWithBackoff instead and exited silently, so the one path where a stop actually has to interrupt a pending sleep - the path the bounded, jittered schedule exists for, and the one an operator waiting on a service stop is watching - was the only one that left no record. Log it there too, exactly once, so both exits are observable. Unit tests cover both exits: a stop delivered while the runner is inside a backoff wait (verified to be inside it before the stop is issued, so it cannot be the idle select that answers) and a stop delivered while idle, each asserting the line appears exactly once. The workflow now waits for four retries rather than three before flipping the endpoint back to healthy. With three, only one slot had reached the ceiling, so the assertion that distinguishes a jittered schedule from a merely capped one - the capped slots must differ from each other - had a single sample and was skipped. Four puts two slots at the ceiling and still leaves ~15s of budget for the flip to land in.
…amping to it The integration scenario failed on Linux and Windows with both of its capped slots measuring exactly 7.5s, and passed on macOS - a 25% coin flip, not a flaky assertion. The jitter was applied and then clamped to maxBackoff, so every draw that came out above the ceiling landed on the ceiling exactly. At the cap the jitter is symmetric, which means roughly half of all capped slots collapsed onto a single value. That is the synchronization this ticket set out to remove, surviving in the one place it matters most. A brief failure is spread by the early slots, but a sustained outage walks every agent up to the ceiling and holds it there, and from then on about half the fleet retried at the same instant on every slot - the herd re-forms exactly when the endpoint is least able to absorb it. Reflect instead: a jittered value above the ceiling becomes 2*max - value, which lands in [0.75*max, max] and is still strictly positive, since the pre-jitter value is at most max and the jitter at most a quarter of it. Capped slots now spread across the band below the ceiling rather than piling onto it, and the documented guarantee - never above the cap, never zero or negative - is unchanged. The regression test samples a capped step 1000 times and fails if more than 1% land on exactly the cap; the clamped implementation scored around 500. This also benefits the postback retry schedule, which shares the helper. The reconnect backoff generator has its own copy of the clamp and the same behavior at its ceiling; it is left alone here to keep this change scoped to the update and postback paths, and is worth a follow-up.
mlataza
deleted the
bug/sc-106110/cap-and-jitter-the-auto-update-retry-backoff-to
branch
August 13, 2026 00:39
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.
Problem
The auto-update retry delay was
r.baseBackoff * (1 << attempt)— no ceiling, no jitter.Unjittered, every agent that fails against the release endpoint at the same moment retries at exactly
base,2*base,4*base, so a GitHub API outage or rate-limit response is sustained by the fleet trying to recover from it, and endpoints stay on old versions — missing fixes we have already shipped — far longer than the outage lasted.Uncapped, the production 5 minute base reaches a 42 hour sleep by attempt 10, longer than the 48 hour check interval the retries are nested inside, and because
maxRetriesis injectable a large value overflows the shift into a negativetime.Duration:time.Afterfires immediately and the loop issues HTTP requests as fast as it can.This is the defect class already fixed for postback retries in sc-103965; the update path never got the same treatment.
Changes
Cap and jitter (f580066). Extracted the postback fix's schedule math into
utils.JitteredBackoffrather than duplicating it, and pointed both retry paths at it. The doubling is iterated multiplication with an early exit at the cap, so no intermediate value can overflow; jitter of up to ±25% spreads each slot. The update cap is the lower of a documentedDefaultUpdateMaxRetryBackoff(1 hour) and a quarter of the check interval, so the retry budget always stays nested inside the interval it belongs to. The production schedule is unchanged where it matters — 5m, 10m, 20m, 40m — and then flattens at an hour.postbackRetryBackoffkeeps its own defaults and is otherwise unchanged.Integration coverage (9cd12bc). A scenario on all three platforms drives the retry loop against
test/stubrelease, a stub release endpoint that fails and recovers on demand. Reaching it needs a build-time-gated seam:releaseUrlOverrideFileStris injected only into the integration binary; released builds leave it empty and never look for the override file, so a shipped agent's update source stays fixed at build time.The scenario found two further defects, both fixed here:
Silent stop (15a1db0). The stop was prompt — honored 276ms into a 2.17s backoff on Linux — but nothing was logged.
Start()loggedAuto updater stoppedonly when its own select saw the stop; a stop observed inside the backoff wait returned throughretryWithBackoffand exited silently, so the one path where a stop must interrupt a pending sleep was the only one leaving no record.Jitter collapsing at the cap (b62503d). Jitter was applied and then clamped to
maxBackoff, so roughly half of every capped slot landed on exactly the ceiling. That is this ticket's synchronization surviving where it matters most: a sustained outage walks the whole fleet up to the ceiling and holds it there, and from then on about half of it retries at the same instant. A jittered value above the ceiling is now reflected back under it (2*max - value), which lands in[0.75*max, max]and stays strictly positive. The postback schedule shares the helper and benefits too.Verification
Unit tests assert every slot is strictly positive and capped across the full attempt range for absurd
maxRetries(64/128/1000, plus spot checks past1<<20), that repeated draws of one slot produce a distribution rather than a repeated value, that capped draws do not pile onto the ceiling (fails above 1% of 1000 samples; the clamped implementation scored ~500), that a short-interval build's whole budget fits inside its interval, and that both stop paths log exactly once.Integration test run: https://github.com/RewstApp/agent-smith-go/actions/runs/31653559872 — all 13 jobs green. Observed schedules against a 503 endpoint, cap 7.5s (a quarter of the integration build's 30s interval):
Every slot bounded by the cap, four distinct values per run, never served back to back, and the schedule recovered on a retry when the endpoint started serving a valid payload mid-sequence.
Follow-up
ReconnectTimeoutGenerator.Nexthas its own copy of the clamp and the same pile-up at its ceiling. Left alone to keep this change scoped to the update and postback paths; worth its own ticket.