Skip to content

fix: keep EqualJitter delays within [cap/2, cap) - #7

Merged
nodivbyzero merged 1 commit into
nodivbyzero:mainfrom
MrBeldum:fix-equal-jitter-window
Oct 5, 2026
Merged

nodivbyzero merged 1 commit into
nodivbyzero:mainfrom
MrBeldum:fix-equal-jitter-window

Conversation

@MrBeldum

@MrBeldum MrBeldum commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

EqualJitter is documented (on the constant and in doc.go) as cap/2 + rand[0, cap/2), which keeps every delay in [cap/2, cap). But calculateNextDelay used the whole backoff cap as the jitter window for both strategies. EqualJitter then added rand[0, cap) on top of the cap/2 base, so a delay could reach almost 1.5 × cap. The only thing holding it back was MaxDelay, so the overshoot showed up any time the exponential cap was still under MaxDelay. That covers every early retry with the default 30s ceiling.

Example: with WithInitialDelay(100ms) and WithJitter(EqualJitter), the first retry could wait up to ~150ms, the second up to ~300ms, and so on.

Change

  • For EqualJitter, the default jitter window is now the other half of the cap (cap - cap/2). WithMaxJitter can still shrink it. The base stays at cap/2, so TestDo_WithMaxJitter_EqualJitter_PreservesBase still holds.
  • FullJitter is unchanged.
  • Updated the step-7 comment. EqualJitter no longer goes over the cap, so the MaxDelay clamp is now only there for the 1ms floors.

Test

TestDo_EqualJitter_StaysWithinCap runs 200 rounds with MaxDelay far above the cap and checks that every RetryInfo.Delay lands in [cap/2, cap). It fails on main and passes with this change. go test -race ./... and go vet ./... pass.

@nodivbyzero nodivbyzero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution.
The fix looks good to me.

@nodivbyzero
nodivbyzero merged commit 59bdcd2 into nodivbyzero:main Oct 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants