Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport to v1.40.x] retry changes: retry stats impl, bug fix, enable retry by default #8406

Merged
merged 4 commits into from Aug 12, 2021

Conversation

dapengzhang0
Copy link
Member

This backports

#8403 xds: enable xDS retry by default
#8402 core: enable retry by default
#8401 core: fix retry flow control issue
#8362 all: implement retry stats

to v1.40.x

There has been an issue about flow control when retry is enabled.

Currently we call `masterListener.onReady()` whenever `substreamListener.onReady()` is called.

The user's `onReady()` implementation might do

```
while(observer.isReady()) {
  // send one more message.
}
```

However, currently if the `RetriableStream` is still draining, `isReady()` is false, and user's `onReady()` exits immediately. And because `substreamListener.onReady()` is already called, it may not be called again after drained.

This PR fixes the issue by

- Use a SerializeExecutor to call all `masterListener` callbacks.
- Once `RetriableStream` is drained, check `isReady()` and if so call `onReady()`.
- Once `substreamListener.onReady()` is called, check `isReady()` and only if so we call `masterListener.onReady()`.
Stabilize `enableRetry()` and `disableRetry()`.

Disable retry in `ManagedChannelImplTest` because each call attempt will fork the headers to a new instance, and add a ClientStreamTracer.Factory for bufferSizeLimit in CallOptions, which makes verification not straightforward.
@dapengzhang0 dapengzhang0 enabled auto-merge (rebase) August 12, 2021 17:44
@dapengzhang0 dapengzhang0 merged commit 2e2e41e into grpc:v1.40.x Aug 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants