router: per data frame timeouts

[cdelguercio]

When using gRPC streams, the connection times out after the route's timeout_ms is hit. Is the proper solution to make a route for only my streaming endpoints and to set timeout_ms to 0? Seems very hacky to me and the docs don't mention much about handling gRPC streams.

[mattklein123]

Yeah, we don't have a great solution here today. Really the only solution right now for streams is to disable timeouts on the route. So yes, I think given the current code this is what you have to do. Ultimately, I would like to create a different type of timeout which operates at the granularity of data frames. So you can say something like: "reset if we don't get any data within X milliseconds." This would be better for streaming.

[cdelguercio]

Thanks for your quick reply!

[mattklein123]

I'm going to reopen this to track the feature request.

[jrajahalme]

@mattklein123 gRPC spec (https://tools.ietf.org/html/draft-kumar-rtgwg-grpc-protocol-00) defines a timeout header grpc-timeout:

   If Timeout is omitted a server should assume an infinite timeout.
   Client implementations are free to send a default minimum timeout
   based on their deployment requirements.

Maybe router filter should detect if the request is gRPC and set the request timeout accordingly, i.e., default to infinity but use the value from the grpc-timeout field if present? Strictly speaking it is a server timeout, so maybe the proxy timeout should be marginally longer to avoid timing out while the server might still respond?

[mattklein123]

@jrajahalme AFAICT from looking at that, that timeout value is also not sufficient for streaming APIs (if it is, it's not specified what the timeout actually means).

I don't have any issue with potentially supporting gRPC timeouts via header if there is a specific timeout header, but I think we should also definitely allow data frame timeouts to be specified in config since we can't trust headers for edge traffic, operators might want to override, etc.

If you are interested in implementing this please let me know. :)

[jrajahalme]

I'm experimenting with this. Currently looking at adding support for grpc-timeout header, including defaulting to infinity for gRPC requests that are missing grpc-timeout header. Envoy-specific timeout headers override these, but timeouts from route config currently don't. Should maybe add a route config option that turns this behavior on ('use_grpc_timeout')?

[jrajahalme]

Not sure if gRPC client libraries set grpc-timeout, though, but even the default infinity would help without splitting up routes/clusters for gRPC/other traffic.

[mattklein123]

@jrajahalme SGTM to try that out. I would still also like to implement data frame timeouts, so if you can do that at the same time that would be awesome.

[jrajahalme]

Added support for grpc-timeout header (#3564), but not for data frame timeouts, sorry.

[htuch]

I'll work on implementing this over the next week.