Skip to content

router filter's global timeout timer starts later than the per try timeout one #6624

@mpuncel

Description

@mpuncel

Title: router filter's global timeout starts later than per try timeout

Description:
The router filter manages a per try timeout that puts an upper bound on the length of time an upstream request can take, and a global timeout that encompasses all upstream requests. However, there seems to be an oddity in that the global timeout timer is not started until after the full downstream request has been decoded (happens in onRequestComplete()) which can be later than when the per try timeout is armed (happens in onPoolReady()).

This is slightly problematic because of the fact that the per try timeout is disabled if it's larger than the global timeout (https://github.com/envoyproxy/envoy/blob/master/source/common/router/router.cc#L150). However they're not measuring the same thing, so it's possible that the per try timeout could still fire first even if it is higher than the global timeout, in particular if the request takes a long time to decode. It also means that a global timeout alone will not protect you from a slow downstream client whereas a per try timeout does a better job just because it starts earlier (although this can be worked around by using stream_idle_timeout and request_timeout in the http connection manager).

A related problem is this assert: https://github.com/envoyproxy/envoy/blob/master/source/common/router/router.cc#L965. I believe it's possible to hit it if a retry due to per try timeout occurs before the full request has been decoded.

And here's an integration test case demonstrating the problem: d5b1de6. The test case is identical to the GlobalTimeout one at the top of the file except it doesn't finish writing the request (i.e. end_stream is true)

Metadata

Metadata

Assignees

Labels

bugno stalebotDisables stalebot from closing an issue

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions