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

Expose hyper BDP config #358

Closed
pranjalssh opened this issue May 20, 2020 · 4 comments · Fixed by #657
Closed

Expose hyper BDP config #358

pranjalssh opened this issue May 20, 2020 · 4 comments · Fixed by #657
Labels
A-tonic C-question Category: Further information is requested
Milestone

Comments

@pranjalssh
Copy link

pranjalssh commented May 20, 2020

Feature Request

Is it possible to support grpc retry configs and grpc channel args? These are quite frequently used in production.

My requests on a single tonic channel seem to fail at high concurrency(10s of thousands) and without these, it is painful to handle these correctly and optimally.

@LucioFranco
Copy link
Member

We support some of these but this mostly relies on hyper. @seanmonstar might be interested in hearing what other options you'd like.

@LucioFranco LucioFranco added A-tonic C-question Category: Further information is requested labels Jun 3, 2020
@seanmonstar
Copy link
Member

I believe retries could be added into tonic using tower::retry.

We'd need to be more specific about "channel args" to know what is desired.

@pranjalssh
Copy link
Author

pranjalssh commented Jun 5, 2020

Among the ones not currently implemented in Tonic, our production code uses
GRPC_ARG_HTTP2_BDP_PROBE
GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA
GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS
GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS

I think grpcio provides support for a fairly common list of grpc args
https://docs.rs/grpcio/0.5.3/grpcio/struct.ChannelBuilder.html

Retry configs are the big one though.

@seanmonstar
Copy link
Member

So hyper, the underlying HTTP library used in Tonic, has support for http2_adaptive_window, which uses BDP pings. Tonic could add a builder option to enable it. Tonic already has exposed some of the keep alive properties as well.

@LucioFranco LucioFranco added this to the 0.4 milestone Nov 27, 2020
@LucioFranco LucioFranco changed the title Support for retry configs and channel args Expose hyper BDP config Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tonic C-question Category: Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants