Uh oh!
There was an error while loading. Please reload this page.
Fix logic to remove retries from callable chains. - #668
Conversation
Codecov Report
@@ Coverage Diff @@## master #668 +/- ##
============================================
+ Coverage 75.94% 75.94% +<.01% - Complexity 1014 1017 +3
============================================
Files 190 190 Lines 4452 4453 +1 Branches 346 346 ============================================
+ Hits 3381 3382 +1
Misses 915 915 Partials 156 156
Continue to review full report at Codecov.
|
| // RetryingCallable affects the rpc deadline of the initial attempt and the number of retries. | ||
| // If the rpc timeout is disabled and no further attempts can be made, then the retry | ||
| // infrastructure can be removed. | ||
| return retrySettings.getInitialRpcTimeout().isZero() |
There was a problem hiding this comment.
retrySettings.getInitialRpcTimeout().isZero() in && is suspicious. I have two questions:
- What does
initialRpcTimeout == 0means from sever side point of view (does it mean "no timeout", call takes as much time as it needs?) - Aren't
retrySettings.maxAttempts() == 1andretryableCodes.isEmtpy()themselves are enough to disable retries (why isinitialRpcTimeoutrelevant here)? I.e. why not just the following:
returnretrySettings.getMaxAttempts() == 1 || retryableCodes.isEmpty();Otherwise, if retrySettings.getInitialRpcTimeout().isZero() == false, but retrySettings.getMaxAttempts() == 1 || retryableCodes.isEmpty() == true, the whole expression gives false. Then, if something fails, the process goes into retrying infrastructure, but it immediately determines it as non-retriabel (one attempt is made, and one is already the maximum) and terminates.
There was a problem hiding this comment.
initialRpcTimeout doesn't have a meaning on the serverside. However, gax treats 0 as unset:
https://github.com/googleapis/gax-java/blob/master/gax/src/main/java/com/google/api/gax/rpc/AttemptCallable.java#L72-L75Ok I'll remove it
igorbernstein2
commented
Feb 14, 2019
updated, ptal |
No description provided.