Stream idle timeout behavior on gRPC streaming?

[jrajahalme]

Based on the documentation and reading the code it seems stream idle timeout breaks gRPC streaming streams when no information has been exchanged on the stream for a time longer than the stream idle timeout in effect.

It seems that if max-grpc-timeout has been specified (maybe as unlimited), stream idle timeout should not fire before the grpc-timeout (specified in gRPC headers) fires.

[mattklein123]

It seems reasonable to allow gRPC specific timeouts to override Envoy timeouts. @jrajahalme can you put together a specific proposal for how it will all work before implementing? (In terms of what would override what, sanitizing, etc.).

[jrajahalme]

@mattklein123 We already override the connection manager level stream idle timeout with route level idle timeout, if available, at the end of ConnectionManagerImpl::ActiveStream::decodeHeaders(). Maybe we could check right after decoding the headers (very end of the same function) with Grpc::Common::hasGrpcContentType() if the request is a gRPC request, and simply disable the stream idle timeout if so, as gRPC requests have explicit request timeouts (and are bounded by the max-grpc-timeout)?

[mattklein123]

@jrajahalme I think this came up last time we did a similar change, but there are security concerns around allowing request headers to override timeouts. I.e., on external requests we can't do this. In general this makes sense to me but I think we are going to have to iterate in code review to make sure we get the sanitizing process correct. It might also need to be optional. How does gRPC today handle streaming API timeouts? What does max-grpc-timeout mean in this case?

[jrajahalme]

@mattklein123 Yeah, recall the back and forth on whether or not to sanitize at the edge. We ended up relying on max-grpc-timeout for this purpose. My proposal above was to disable stream-idle timeouts for gRPC streams, as those streams would still be bounded by max-grpc-timeout for the request timeout. AFAIK this would be equivalent of making the idle-timeout being set at max-grpc-timeout, or less, if a lower value is included in a grpc-timeout header.

[jrajahalme]

This can be accomplished with the gRPC route matcher, so no new functionality is needed.

[nvcnvn]

This can be accomplished with the gRPC route matcher, so no new functionality is needed.

Can you give an example for gRRC route matcher.
I'm facing similar issue but very new to Envoy.
Thanks