fix(gax): adjust RetryAlgorithm logic for timeout v. exception#13243
Draft
whowes wants to merge 4 commits into
Draft
fix(gax): adjust RetryAlgorithm logic for timeout v. exception#13243whowes wants to merge 4 commits into
whowes wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request modifies the RetryAlgorithm to short-circuit the retry check when an operation succeeds and the result algorithm indicates no retry is necessary. This change prevents the timing algorithm from erroneously throwing a CancellationException on successful operations. New unit tests were added to verify both the short-circuiting behavior and the standard path. Feedback suggests adding a missing assertion to one of the new test cases to explicitly verify the return value of the retry decision.
whowes
force-pushed
the
whowes/issue-12373-flaky
branch
from
32b949e to
f648950
Compare
whowes
force-pushed
the
whowes/issue-12373-flaky
branch
from
3cff2d3 to
54b9310
Compare
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current RetryAlgorithm logic is susceptible to an edge case when a short RPC deadline is set near the end of polling. If an exception results from a too-short deadline, the exception thrown by the associated Future will be an ExecutionException rather than CancellationException. This likely causes flakiness observed in #12373 and #3277.
This change ensures that shouldRetryBasedOnTiming executes even when shouldRetryBasedOnResult is false (e.g. due to non-retriable exception). This way a CancellationException will be thrown instead if the overall timeout has been reached.
Resolves #12373