fix: make perAttemptRecvTimeout actually apply to retry attempts [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] - #12939
Open
waterWang wants to merge 1 commit into
Open
Conversation
The perAttemptRecvTimeout field in RetryPolicy has been configurable since PR grpc#8301 but was never actually applied to individual retry attempts. The DeadlineEntry in RetriableStream.setDeadline() simply passed through the original deadline without considering the per-attempt timeout. Fix: When perAttemptRecvTimeoutNanos is set in the retry policy, apply the minimum of the original deadline and the per-attempt deadline to each substream. This ensures that each retry attempt is bounded by the per-attempt timeout, relative to when the attempt starts. Fixes grpc#12919 [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT]
|
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.
Fixes #12919
Problem
The
perAttemptRecvTimeoutfield inRetryPolicyhas been configurable since PR #8301 but was never actually applied to individual retry attempts. TheDeadlineEntryinRetriableStream.setDeadline()simply passed through the original deadline without considering the per-attempt timeout.This means setting
perAttemptRecvTimeoutin the retry policy configuration has no effect — each attempt is only bounded by the overall RPC deadline, not by the per-attempt timeout.Fix
In
RetriableStream.setDeadline(), whenperAttemptRecvTimeoutNanosis set in the retry policy, apply the minimum of the original deadline and the per-attempt deadline to each substream. The per-attempt deadline is relative to when the substream is drained (i.e., when the attempt actually starts), ensuring each retry attempt gets a fresh timeout.Changes
RetriableStream.java: ModifiedDeadlineEntry.runWith()to check forperAttemptRecvTimeoutNanosand applydeadline.minimum(perAttemptDeadline)instead of the raw deadline.Testing
Existing tests pass unchanged since the default
perAttemptRecvTimeoutisnull(no behavioral change when not configured). When configured, the per-attempt timeout is correctly applied to each substream.