Skip to content

fix(toolkit-lib): retry backoff has no jitter, retries stay in lockstep - #1942

Open
polothy wants to merge 1 commit into
aws:mainfrom
polothy:fix/retry-backoff-jitter
Open

fix(toolkit-lib): retry backoff has no jitter, retries stay in lockstep#1942
polothy wants to merge 1 commit into
aws:mainfrom
polothy:fix/retry-backoff-jitter

Conversation

@polothy

@polothy polothy commented Sep 3, 2026

Copy link
Copy Markdown

Note

This PR was created by AI (Claude Code, Anthropic's agentic CLI), including the code, tests and this description. A human directed the work and reviewed the result, but the reasoning and wording below are the model's. Please review it on its merits rather than assuming human verification of every claim.

Related to #1777 (deliberately not using a closing keyword — that issue is about a different defect and should stay open). This is the secondary finding from its comment thread, and it stands alone: it reduces how often a request exhausts its retry budget under throttling, which is the precondition for the failure described there. It does not depend on, conflict with, or supersede #1941.

The problem

ConfiguredRetryStrategy does not layer a custom delay onto the SDK's backoff — it replaces it:

// @smithy/core/dist-cjs/submodules/retry/index.js:572
this.retryBackoffStrategy.computeNextBackoffDelay = (completedAttempt) => {
  const nextAttempt = completedAttempt + 1;
  return this.computeNextBackoffDelay(nextAttempt);   // <-- our function
};

The strategy it displaces is jittered (DefaultRetryBackoffStrategy, line 386). cappedExponentialBackoff was not:

return (attempt: number) => Math.min(baseMs * (2 ** attempt), capMs);

So every request on a toolkit client that started retrying waited exactly 2s, 4s, 8s, 15s, 15s, 15s. Requests throttled in the same instant retry in the same instants: a herd stays a herd, re-colliding at each retry point rather than dispersing, and spends its budget on collisions with itself. Stack-event pollers are phase-aligned by construction (stacks in one deploy start together, nothing de-phases them), so this is the common case at high --concurrency, not a corner case.

The change

One function, in packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk.ts:

const delay = Math.min(baseMs * (2 ** attempt), capMs);
return Math.floor(delay / 2 + Math.random() * (delay / 2));

The cap is still applied before the jitter, so the documented worst case remains the cap.

Jitter applies only between retries. The first attempt of every request is unaffected, so a call that succeeds — or fails non-retryably — first time sees no change at all.

Each delay moves from a fixed value d to a random point in [floor(d/2), d-1] (equal jitter; the top is d-1 rather than d because of Math.floor). Every possible delay is therefore less than or equal to the delay before this change — no request can wait longer than it does today, and the capped worst case that #1500 established is preserved exactly.

Both call sites of the helper are affected. The STS strategy (sdk.ts:677) keeps its own inline schedule deliberately: it is a low-volume credential-validity probe that is not part of any throttling herd, and its 3-attempt budget already bounds it at 400ms.

Base client config — ConfiguredRetryStrategy(7, cappedExponentialBackoff(300, 15_000))

Spread into every client the SDK class builds (S3, Lambda, SSM, ECR, CloudWatch Logs, Route 53, Secrets Manager, SFN, …).

Retry Was (fixed) Now (range)
1 600ms 300–599ms
2 1200ms 600–1199ms
3 2400ms 1200–2399ms
4 4800ms 2400–4799ms
5 9600ms 4800–9599ms
6 15000ms 7500–14999ms
Total across 6 retries 33.6s 16.8s min / 25.2s mean / 33.594s max

CloudFormation client — ConfiguredRetryStrategy(7, cappedExponentialBackoff(1000, 15_000))

The path in #1777.

Retry Was (fixed) Now (range)
1 2000ms 1000–1999ms
2 4000ms 2000–3999ms
3 8000ms 4000–7999ms
4 15000ms 7500–14999ms
5 15000ms 7500–14999ms
6 15000ms 7500–14999ms
Total across 6 retries 59.0s 29.5s min / 44.3s mean / 58.994s max

Why equal jitter rather than the SDK's full jitter

Full jitter (random * d, what smithy's DefaultRetryBackoffStrategy does and what #1777 proposed) would drop the CloudFormation mean to ~29.5s and allow a near-zero wait before the first retry. Equal jitter keeps the 59s worst case and a floor of half the scheduled delay, so the window a request has to ride out a throttling burst shrinks by 25% rather than 50%, while still dispersing simultaneous retries across d/2 — 7.5s at the cap, roughly 75 request slots against the 10 req/s DescribeStackEvents quota.

Switching to full jitter is a one-line change if reviewers prefer to match the SDK default.

How this compares to the SDK's own jitter

Same shape, one intentional deviation — smithy's default, verbatim:

// DefaultRetryBackoffStrategy, retry/index.js:386
const b = Math.random();
const t_i = b * Math.min(this.x * 2 ** i, MAXIMUM_RETRY_DELAY);
return Math.floor(t_i);
window mean near-instant retry possible
smithy (full jitter) [0, d) d/2 yes
this PR (equal jitter) [floor(d/2), d) 0.75d no

Both are exponential base-2, clamped, Math.random()-scaled and floored; only the multiplier's range differs. Full jitter disperses better but halves the mean, which would shorten the CloudFormation budget from ~59s of wall clock to ~29.5s — the opposite of what helps when the problem is exhausting that budget. Equal jitter still spreads simultaneous retries across d/2 (7.5s at the cap, ~75 request slots against the 10 req/s DescribeStackEvents quota) while keeping more of the window intact.

Switching to full jitter is a one-line change if maintainers prefer to match the SDK default exactly.

One related observation, pre-existing and not addressed here: StandardRetryStrategy calls setDelayBase(THROTTLING_RETRY_DELAY_BASE) (500ms, vs 100ms otherwise) when it classifies an error as throttling (retry/index.js:472). A computeNextBackoffDelay supplied to ConfiguredRetryStrategy closes over its own base and never reads this.x, so that adaptation has always been inert for the toolkit. It matters less than it sounds — the CloudFormation client's 1000ms base already exceeds smithy's throttling base — but reviewers weighing this PR's throttling rationale should know the SDK's own throttling adaptation is not in play.

Tests

test/api/aws-auth/sdk-retry.test.ts — the four existing tests keep their original expectations (2000, 4000, 8000, 15_000, 59_000) by mocking Math.random() to the top of the jitter window, so they still assert the schedule rather than jitter arithmetic. Three new tests cover the jitter itself: the floor at half the scheduled delay, a property check that 1,000 draws across attempts 1–10 all land in [floor(d/2), d), and that repeated draws for one attempt are not all identical.

Mutation-checked by reverting the body to the deterministic version: never waits less than half the scheduled delay, stays within the jitter window for every attempt and disperses callers that retry at the same moment all fail. (The schedule tests pass under that mutation by design — mocking the top of the window makes them agree with the deterministic implementation, which is what keeps them readable.)

The interval in the docstring was also verified against odd baseMs/capMs combinations (333/15001, 1/3, 7/7, 1001/9999), since this is a general helper and Math.floor makes the bounds inexact for odd delays.

All seven test/api/aws-auth suites pass (82 tests). eslint clean. No API Extractor change — the helper is exported only via lib/api/aws-auth/private.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

`ConfiguredRetryStrategy` replaces smithy's own jittered backoff with whatever
delay function it is handed, and `cappedExponentialBackoff` was deterministic.
Requests throttled at the same moment therefore retried at exactly the same
moments, so a herd of pollers stayed a herd instead of dispersing over the
retry window, and burned its retry budget on re-collisions.

Apply equal jitter - a random point in `[floor(d/2), d - 1]` - to the capped
delay. The capped worst case is unchanged; no request waits longer than it
does today.

Refs aws#1777

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@polothy
polothy deployed to automation September 3, 2026 22:44 — with GitHub Actions Active
@github-actions github-actions Bot added the p2 label Sep 3, 2026
@aws-cdk-automation
aws-cdk-automation requested a review from a team September 3, 2026 22:45
@polothy
polothy marked this pull request as ready for review September 3, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant