fix(time): prevent overflow in ExponentialDelayWithJitter backoff

[anxkhn]

Description

ExponentialDelayWithJitter computed baseDelay * (1 << attempt) and only then applied min(delay, maxDelay). A large attempt overflows the int64-nanosecond time.Duration before the cap is applied, so the wrapped negative or zero value passes the cap unchanged, delay > 0 is false, and the sleep is skipped.

The shift is now applied only when it cannot overflow time.Duration (gated on the leading-zero count of baseDelay); otherwise the product would exceed maxDelay anyway, so the cap is used directly. A non-positive baseDelay is also guarded, matching the existing guard in RandomDelayWithJitter. Behavior is unchanged for the existing small-attempt cases. Three regression cases cover the overflow regime.

Related Issue

Closes #4851

Motivation and Context

The scheduler passes attempt = host.ConcurrentRegisterCount, which is the number of concurrent in-flight registrations on a host rather than a retry counter (scheduler/service/service_v2.go:1325, base 30ms, max 1s). With those constants the overflow makes the register-peer backoff break once a host has roughly 39 or more concurrent registrations, disabling the thundering-herd mitigation in exactly the scenario it is meant to guard against.

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation Update (if none of the other choices apply)

Checklist

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.05%. Comparing base (0b3cca4) to head (6b9e1ed).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4852   +/-   ##
=======================================
  Coverage   28.04%   28.05%           
=======================================
  Files         232      232           
  Lines       23148    23151    +3     
=======================================
+ Hits         6491     6494    +3     
  Misses      16218    16218           
  Partials      439      439           

Flag	Coverage Δ
unittests	28.05% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/time/delay.go	85.71% <100.00%> (+1.71%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[pmady]

nice, the old code would wrap negative on large attempt values before the min() could cap it. using LeadingZeros to check if the shift is safe is clever avoids the overflow without needing big.Int or float math.

test coverage for attempt=39, 62, 100 is thorough. the only edge case i can think of is baseDelay=0 which your guard handles already

[anxkhn]

Thanks for taking a look, @pmady.

Yes, that is the crux: the multiply/shift happened before min(delay, maxDelay), so a wrapped negative or zero slipped past the cap and the sleep was skipped.

On the LeadingZeros64 check: for a positive baseDelay its top set bit sits at index 63 - LeadingZeros64(baseDelay), so baseDelay << attempt stays within a positive int64 exactly while attempt < LeadingZeros64(baseDelay). When that does not hold, the true product already exceeds any sane maxDelay, so clamping straight to maxDelay is the correct value, not a heuristic.

Good call on baseDelay <= 0: it maps to no delay, which also keeps this consistent with the existing guard in RandomDelayWithJitter.