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

Timeout and deadline propagation issues #249

Closed
jhump opened this issue Apr 1, 2024 · 0 comments · Fixed by #276
Closed

Timeout and deadline propagation issues #249

jhump opened this issue Apr 1, 2024 · 0 comments · Fixed by #276

Comments

@jhump
Copy link
Member

jhump commented Apr 1, 2024

Currently, this library relies wholly on the underlying HTTPClientInterface implementation handling timeouts. That poses two problems:

  1. The only implementation of HTTPClientInterface provided is based on okhttp, which does not enforce timeouts on full-duplex bidirectional operations after the stream is established.
  2. It should not be up to this layer to handle deadline propagation -- communicating the timeout to the server, so it can attenuate the deadline as it processes the request. That is a protocol concern (and Connect and gRPC/gRPC-Web both handle it slightly differently). Currently, timeout headers are never sent, so this client does not attempt to propagate deadlines to the server.

So to address these deficiencies:

  1. There needs to be a mechanism for configuring timeouts other than the HTTPClientInterface implementation. This could be in the form of default timeouts in ProtocolClientConfig; it could be some way to specify timeouts on a per-RPC basis; or it could be both.
  2. When such a timeout is configured, the protocol handling code needs to encode the timeout in a request header to communicate the deadline to the server.

The underlying HTTPClientInterface might still impose its own timeouts. So it's an open question as to whether this needs to be considered in the timeout handling. For example, if the user sets a 20 second timeout for an RPC, but the underlying HTTP client has a 10 second timeout configured, should the library take this into consideration and communicate a 10 second deadline to the server? If so, then we may need to add methods to HTTPClientInterface (or create sub-interfaces that can be optionally implemented) so that the framework can ask the client about its timeouts, in order to correctly consider them.

Alternatively, instead of the framework asking the client about its timeout, it could tell the client what timeout to use (similarly to how #14 proposes that the framework may need to tell the client what protocols to support, depending on the RPC protocol and URL scheme used for a particular operation).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant