add configurable idle timeout #2632
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2632 +/- ##
=======================================
Coverage 76.92% 76.93%
=======================================
Files 71 71
Lines 5591 5605 +14
=======================================
+ Hits 4301 4312 +11
- Misses 1203 1205 +2
- Partials 87 88 +1
|
skriss
changed the title
skriss
added
release-note-action-required
cmd/contour/servecontext.go
Outdated
| // IdleTimeout defines how long the proxy should wait while there | ||
| // are no active requests before terminating an HTTP connection. Set | ||
| // to 0 to disable the timeout. | ||
| IdleTimeout time.Duration `yaml:"idle-timeout,omitempty"` |
There was a problem hiding this comment.
@youngnick @jpeach @stevesloka before merging this, I'm considering renaming this field and related code to ConnectionIdleTimeout to more explicitly differentiate from StreamIdleTimeout which I'm adding next. What do you all prefer? IdleTimeout and StreamIdleTimeout, or ConnectionIdleTimeout and StreamIdleTimeout?
There was a problem hiding this comment.
Envoy has separate stream and connection timeouts, so we should name this for whichever Envoy timeout it refers to. This helps a lot.
There was a problem hiding this comment.
This one is the connection idle timeout, but envoy just calls it "idle_timeout".
There was a problem hiding this comment.
I'll leave this as-is so I can move ahead, but can always circle back if desired.
There was a problem hiding this comment.
I think ConnectionIdleTimeout & StreamIdleTimeout like you suggested is good. It helps to better clarify the context a bit.
There was a problem hiding this comment.
OK @stevesloka I updated the name, mind taking a quick look before I merge?
There was a problem hiding this comment.
This one is the connection idle timeout, but envoy just calls it "idle_timeout".
Right. The doc I linked to explains that this is an alias to the stream timeout.
Signed-off-by: Steve Kriss <krisss@vmware.com>
Signed-off-by: Steve Kriss <krisss@vmware.com>
Signed-off-by: Steve Kriss <krisss@vmware.com>
updates #2225
I started with just one timeout setting chosen at random; I'll add the others if the approach looks OK.
I can see how adding additional tuneables one-by-one is not ideal (+268 LOC to add a single config parameter). I'm still paging in the context to see if there are other ideas/options for how to do this in a more general way.