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

Support deadlines with the grpc-timeout header #75

Closed
thomasetter opened this issue Oct 15, 2019 · 7 comments · Fixed by #606
Closed

Support deadlines with the grpc-timeout header #75

thomasetter opened this issue Oct 15, 2019 · 7 comments · Fixed by #606
Labels
A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.
Milestone

Comments

@thomasetter
Copy link

The official protocol definition mentions the "grpc-timeout" header which is a way for the client to tell the server what deadline it has.
As I understand it, the timeout should be converted into a deadline as soon as the header is read (before the rest of the payload).

Using this, when the server is overloaded, requests can just be dropped with DEADLINE_EXCEEDED or CANCELLED (not sure which one should be picked on the server side) if it takes too long until they are processed.
E.g. in C++, There is the ServerContext::IsCancelled() function which can be used to exit early: Blog post on gRPC deadlines

How can this be achieved with tonic?
Are there features in tower/hyper which could be used?

@LucioFranco
Copy link
Member

Hi Yes, this should be possible by implementing a custom service layer that reads the initial headers and wraps its inner future with a timeout.

@LucioFranco LucioFranco added A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue. labels Oct 17, 2019
@ParkMyCar
Copy link

I think this would be pretty interesting to build, if no one else is already working on it, I can try to put up a PR in a bit?

Aside from timeouts, there's also cancelation. Is there a story around this yet in tonic?

@LucioFranco
Copy link
Member

I think the right method would be to create a version of https://github.com/tower-rs/tower/tree/master/tower/src/timeout that gets the timeout from a header rather than a static value.

@Maher4Ever
Copy link

@LucioFranco Hi, is there an update regarding cancellation support in tonic?

@LucioFranco
Copy link
Member

No updates from my end yet, its easy to support cancellation by dropping the task but we currently do not parse the headers for a timeout automatically.

@Sushisource
Copy link
Contributor

Sushisource commented Feb 3, 2021

Huge +1 here. In particular I need support for setting them on the client. Adding them manually appears to not work, since it looks like all the GRPC_RESERVED_HEADERS are silently stripped (which isn't so fun) as mentioned in #516. If there's anything I can do to help @LucioFranco I am happy to do so.

@liufuyang
Copy link
Contributor

I feel I need to set deadline or timeout on client as well, otherwise my Rust client cannot talk to our Java gRPC servers as they are build to require having deadline specified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants