Skip to content

[extended] Fix int overflow in exponential backoff rate limiter - #4957

Draft
yue9944882 wants to merge 1 commit into
kubernetes-client:masterfrom
yue9944882:fix/extended-expbackoff-int-overflow
Draft

yue9944882 wants to merge 1 commit into
kubernetes-client:masterfrom
yue9944882:fix/extended-expbackoff-int-overflow

Conversation

@yue9944882

Copy link
Copy Markdown
Member

Bug

ItemExponentialFailureRateLimiter.when() (extended module, work-queue rate limiters) computes the backoff multiplier as 1 << exp:

long exp = failures.computeIfAbsent(item, k -> new AtomicLong()).getAndIncrement();
long d = maxDelay.toMillis() >> exp;
return d > baseDelay.toMillis() ? baseDelay.multipliedBy(1 << exp) : maxDelay;

exp is a long, but 1 << exp is evaluated in int arithmetic (the shift result type follows the left operand, and the distance is masked to 5 bits). Once exp reaches 31, 1 << 31 overflows to Integer.MIN_VALUE (and wraps for higher counts). When maxDelay is large enough that the baseDelay * 2^exp branch is still selected at that failure count, baseDelay.multipliedBy(...) then returns a negative Duration — never a valid backoff.

Fix

Compute the multiplier in long arithmetic: 1L << exp. This matches the long type of exp and of Duration.multipliedBy(long), and is behavior-preserving for all exp < 31 (where 1 << exp == 1L << exp).

-    return d > baseDelay.toMillis() ? baseDelay.multipliedBy(1 << exp) : maxDelay;
+    return d > baseDelay.toMillis() ? baseDelay.multipliedBy(1L << exp) : maxDelay;

Verification

Added a focused regression test highFailureCountStaysPositive that drives the item to exp == 31 with a large maxDelay and asserts the backoff stays positive and equals baseDelay * 2^31.

  • Before fix: highFailureCountStaysPositive FAILS (negative duration).
  • After fix: all 5 tests in ItemExponentialFailureRateLimiterTest PASS.
./mvnw -pl extended -am test -Dtest=ItemExponentialFailureRateLimiterTest -Dsurefire.failIfNoSpecifiedTests=false
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0

Draft pending maintainer review.

@kubernetes-prow kubernetes-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 22, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yue9944882

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 22, 2026
ItemExponentialFailureRateLimiter.when() computed the backoff multiplier
as 1 << exp, an int shift. Once exp reaches 31 the shift overflows to
Integer.MIN_VALUE (and wraps thereafter), so baseDelay.multipliedBy(...)
returns a negative Duration whenever maxDelay is large enough that the
baseDelay*2^exp branch is still selected at that failure count.

Compute the multiplier in long arithmetic (1L << exp), matching the long
type of exp and of Duration.multipliedBy(long). Add a regression test that
fails before and passes after.
@yue9944882
yue9944882 force-pushed the fix/extended-expbackoff-int-overflow branch from 2264c25 to b02272f Compare October 22, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant