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

core: enable retry by default #8402

Merged
merged 3 commits into from Aug 11, 2021
Merged

Conversation

dapengzhang0
Copy link
Member

@dapengzhang0 dapengzhang0 commented Aug 11, 2021

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 added the TODO:backport PR needs to be backported. Removed after backport complete label Aug 11, 2021
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer if we just disabled retries for most of ManagedChannelImplTest. It seems to reduce the test precision and brings in a lot more code that is really being testing elsewhere.

@dapengzhang0 dapengzhang0 enabled auto-merge (squash) August 11, 2021 21:06
@dapengzhang0
Copy link
Member Author

I think I'd prefer if we just disabled retries for most of ManagedChannelImplTest. It seems to reduce the test precision and brings in a lot more code that is really being testing elsewhere.

Done.

@dapengzhang0 dapengzhang0 merged commit bdf9a96 into grpc:master Aug 11, 2021
@dapengzhang0 dapengzhang0 deleted the enable-retry branch August 11, 2021 21:54
dapengzhang0 added a commit to dapengzhang0/grpc-java that referenced this pull request Aug 12, 2021
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 removed the TODO:backport PR needs to be backported. Removed after backport complete label Aug 12, 2021
dapengzhang0 added a commit that referenced this pull request Aug 12, 2021
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
Copy link
Member Author

Addressing #3982

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 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