Skip to content

[core] Fix overflow on exponential backoff multiplication#62366

Merged
edoakes merged 7 commits into
ray-project:masterfrom
edoakes:eoakes/fix-overflow
Apr 6, 2026
Merged

[core] Fix overflow on exponential backoff multiplication#62366
edoakes merged 7 commits into
ray-project:masterfrom
edoakes:eoakes/fix-overflow

Conversation

@edoakes

@edoakes edoakes commented Apr 6, 2026

Copy link
Copy Markdown
Collaborator

We guarded against overflow by checking the result of the exponential, but there could still be an overflow when multiplying against base_ms.

edoakes added 5 commits April 6, 2026 10:17
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the ExponentialBackoff utility by implementing explicit overflow checks for both the exponential calculation and the multiplication with the base delay. Unit tests were updated to include scenarios where multiplication would overflow. Review feedback recommends using bitwise shifts instead of floating-point math for power-of-two calculations and optimizing the test suite by moving invariant logic out of loops.

Comment thread src/ray/util/exponential_backoff.cc
Comment thread src/ray/util/tests/exponential_backoff_test.cc Outdated
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes added the go add ONLY when ready to merge, run all tests label Apr 6, 2026
@edoakes edoakes changed the title [WIP][core] Fix exponential backoff Apr 6, 2026
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes marked this pull request as ready for review April 6, 2026 15:34
@edoakes edoakes requested a review from a team as a code owner April 6, 2026 15:34
@edoakes edoakes enabled auto-merge (squash) April 6, 2026 16:37
@edoakes edoakes merged commit 0d46b5e into ray-project:master Apr 6, 2026
7 checks passed
jeffreywang88 pushed a commit that referenced this pull request Apr 10, 2026
We guarded against overflow by checking the result of the exponential,
but there could still be an overflow when multiplying against `base_ms`.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
aslonnie added a commit that referenced this pull request Apr 11, 2026
## Description
- #62323
- #62330
- #62366

## Related issues
> Link related issues: "Fixes #1234", "Closes #1234", or "Related to
#1234".

## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: kourosh hakhamaneshi <31483498+kouroshHakha@users.noreply.github.com>
Co-authored-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Co-authored-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
Lucas61000 pushed a commit to Lucas61000/ray that referenced this pull request May 15, 2026
…t#62366)

We guarded against overflow by checking the result of the exponential,
but there could still be an overflow when multiplying against `base_ms`.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests

2 participants