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.

Sign up for GitHub

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

Jump to bottom

Timeouts for streams hanging prior to route determination #3853

Closed
htuch opened this issue Jul 13, 2018 · 0 comments
Closed

Timeouts for streams hanging prior to route determination #3853

htuch opened this issue Jul 13, 2018 · 0 comments
Assignees
@htuch
Labels
enhancement Feature requests. Not bugs or questions.

Comments

@htuch
Copy link
Member

htuch commented Jul 13, 2018

In #3841 (see also #1778), we introduce per-stream idle timeouts. However, these apply at the route level. In order to know the per-route timeout, we need to have decoded headers already. It's possible that streams may idle hang prior to request header completion.

A simple solution is to just do a global stream idle timeout as well, that can be overridden once the per-route idle timeout is determined.
@htuch htuch self-assigned this Jul 13, 2018
@htuch htuch added the enhancement Feature requests. Not bugs or questions. label Jul 13, 2018
@htuch htuch mentioned this issue Jul 13, 2018
Merged
htuch added a commit to htuch/envoy that referenced this issue Jul 17, 2018
@htuch
http: global connection manager per-stream idle timeouts.
9ecc481 
This is a followup to envoyproxy#3841, where we introduce HCM-wide stream idle timeouts. This has two effects:

1. We can now timeout immediately after stream creation, potentially before receiving request
   headers and routing.

2. A default timeout can be configured across all routes. This is overridable on a per-route basis.

The default and overriding semantics are explained in the docs. Also added as a bonus some docs
about how timeouts interact more generally in Envoy.

Fixes envoyproxy#3853.

Risk Level: Low. While there is some change to the per-route vs. HCM wide semantics for stream idle
  timeouts, it's not anticipated this feature is in common use yet (it's only a couple of days since
  landing), and the caveats in envoyproxy#3841 with the new 5 minute default timeout should already apply.
Testing: Unit/integration tests added.

Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch htuch mentioned this issue Jul 17, 2018
Merged
htuch added a commit that referenced this issue Jul 23, 2018
@htuch
http: global connection manager per-stream idle timeouts. (#3879)
672083c 
This is a followup to #3841, where we introduce HCM-wide stream idle timeouts. This has two effects:

1. We can now timeout immediately after stream creation, potentially before receiving request headers and routing.

2. A default timeout can be configured across all routes. This is overridable on a per-route basis.

The default and overriding semantics are explained in the docs. Also added as a bonus some docs
about how timeouts interact more generally in Envoy.

Fixes #3853.

Risk Level: Low. While there is some change to the per-route vs. HCM wide semantics for stream idle
timeouts, it's not anticipated this feature is in common use yet (it's only a couple of days since
landing), and the caveats in #3841 with the new 5 minute default timeout should already apply.
Testing: Unit/integration tests added.

Signed-off-by: Harvey Tuch <htuch@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@htuch htuch

Labels
enhancement Feature requests. Not bugs or questions.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@htuch