Support deadlines with "grpc-timeout" header

[ParkMyCar]

Motivation

This PR fixes issue #75, which is to implement support for the "grpc-timeout" header in tonic. Setting a timeout/deadline is an official part of the gRPC spec, and is a recommended practice.

Note

This code needs to be cleaned up a bit before merging, e.g. adding a set_duration(...) function to the generated client code, instead of making client::Grpc.inner public (and more). I put this PR up in this state to get some early feedback on the design of the timeout implementation.

Solution

Server

Requests are timed out by attempting to read a "grpc-timeout" value from the MetadataMap of a request. If we find a value for a timeout, then we wrap inner service call with a tokio::time::Timeout, using the duration we read previously.

Client

Taking inspiration from other gRPC clients we add a function to client::Grpc called set_duration(deadline: Duration) which sets a duration value on the inner struct. When any request is made with this client, we insert this duration value in the "grpc-timeout" header.

Testing

To make sure this works, and to prevent regressions, I added a cancelation_on_timeout test which sets a deadline on the client of 500ms, but the handler on the server pauses for 1,000ms before returning a value. We assert this request returns with the expected message and code.

Note

As part of the cleanup I'd also like to add a benchmark test to assert the timely cleanup of assets on timeout. e.g. in the current test, our request should only take 10s (ideally 1s) of milliseconds longer than the deadline.

[evoxmusic]

Well done @ParkMyCar for this PR. It was a missing feature and luckily you made it 🙏

[LucioFranco]

Hey @ParkMyCar I think this is a fine approach but I'd like to suggest a different one. The goal with the main client/grpc.rs and server/grpc.rs modules was to not have it 100% tied to tokio. This is to allow other runtimes to use it. By bringing in the timeout mechanics we unfortunately bind users to tokio. The solution I see is to instead of implementing this at the root level of the core grpc mechanisms support this at the transport level. From here we can implement a tower service similar to the current Timeout one but instead pulls the timeout from a header rather than a static value (or maybe both!). What do you think?

[ParkMyCar]

@LucioFranco, totally makes sense, thanks for the feedback!
I’m on vacation right now, but within the next week or so I’ll update the branch with an implementation at the Transport level

[ParkMyCar]

Just a heads up, I still intend on updating this PR, just haven't had time in the past couple weeks. If someone else is interested they should feel free 🙂

[LucioFranco]

@ParkMyCar thanks for the update! No rush :)

[ParkMyCar]

Opened a new PR #516, because I previously deleted the fork used to create this one and couldn't figure out how to update this one with a new fork, sorry if this causes unnecessary spam!