Skip to content

Fix Long overflow in Retry when backoff factor > 1.0#3422

Merged
dlmarion merged 1 commit intoapache:2.1from
dlmarion:3414-fix-retry-overflow
May 23, 2023
Merged

Fix Long overflow in Retry when backoff factor > 1.0#3422
dlmarion merged 1 commit intoapache:2.1from
dlmarion:3414-fix-retry-overflow

Conversation

@dlmarion
Copy link
Contributor

Closes #3414

@dlmarion dlmarion self-assigned this May 23, 2023
@dlmarion dlmarion linked an issue May 23, 2023 that may be closed by this pull request
@dlmarion dlmarion merged commit 487f2a3 into apache:2.1 May 23, 2023
@dlmarion dlmarion deleted the 3414-fix-retry-overflow branch May 23, 2023 13:08
@EdColeman
Copy link
Contributor

This may prevent the overflow, but I was reviewing this to see if the wait / backoff factor is what is expected when this was merged. Where

currentWait = Math.min(maxWait, initialWait + waitIncrement);

should it be something like

currentWait = Math.min(maxWait, currentWait + waitIncrement);

If you have an initial wait of 10 seconds and then you want to retry every second with a back off I would expect wait times like 10, 11, 13, 17,... But it appears that the code would be like 10, 21, 33 (or something), but basically backing off by the initialWait and the back-off increment.

@dlmarion
Copy link
Contributor Author

So, two bugs in our Retry object then? Maybe if we are going to touch this again, instead of fixing it we rip it out and replace it with one of:

https://curator.apache.org/apidocs/org/apache/curator/retry/ExponentialBackoffRetry.html
http://rholder.github.io/guava-retrying/
https://resilience4j.readme.io/v1.7.0/docs/retry

@ctubbsii ctubbsii added this to the 2.1.1 milestone Jul 12, 2024
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.

TableOperationsImpl.locate Retry can fail when backOffFactor > 1

4 participants