interceptor: add HTTP keep-alive toggle#1571
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Persistent HTTP connections are not always helpful for all workloads. With synchronous/blocking backends, one interceptor can keep a connection open to a backend worker after a request finishes. Another interceptor may reuse that worker and wait until the first connection is closed, causing cross-replica blocking and higher latency. Add `KEDA_HTTP_DISABLE_KEEP_ALIVES` so operators can disable connection reuse when this behavior appears. Signed-off-by: Ihor Kalnytskyi <ihor@kalnytskyi.com>
0f41d13 to
918fcd9
Compare
There was a problem hiding this comment.
Pull request overview
Adds an interceptor configuration toggle to disable HTTP keep-alive connection reuse when it causes backend-side cross-replica blocking/latency.
Changes:
- Adds
KEDA_HTTP_DISABLE_KEEP_ALIVESto interceptor timeouts config. - Wires
DisableKeepAlivesinto the interceptor’s upstreamhttp.Transport. - Adds unit tests asserting
Connection: closebehavior when keep-alives are disabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| interceptor/proxy.go | Plumbs DisableKeepAlives into the outbound http.Transport used to reach backends. |
| interceptor/proxy_test.go | Adds tests + harness config to validate keep-alive disabling behavior. |
| interceptor/config/timeouts.go | Introduces DisableKeepAlives env-configured field (KEDA_HTTP_DISABLE_KEEP_ALIVES). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| transport := &http.Transport{ | ||
| Proxy: http.ProxyFromEnvironment, | ||
| DialContext: dialFunc, | ||
| ForceAttemptHTTP2: cfg.Timeouts.ForceHTTP2, | ||
| DisableKeepAlives: cfg.Timeouts.DisableKeepAlives, | ||
| MaxIdleConns: cfg.Timeouts.MaxIdleConns, |
There was a problem hiding this comment.
DisableKeepAlives is intended to prevent backend connection reuse, but ForceAttemptHTTP2 is still enabled when cfg.Timeouts.ForceHTTP2 is true. With HTTP/2, a single TCP connection is typically reused/multiplexed across requests, so disabling HTTP/1.1 keep-alives may not achieve the desired "no reuse" behavior. Consider making DisableKeepAlives override/disable ForceAttemptHTTP2 (and/or documenting/enforcing that these options are mutually exclusive) so the toggle reliably prevents reuse.
|
Hi @ikalnytskyi , thanks for your PR! I understand the solution you proposed but want to understand the problem as well as I'm not really sure about it right now. Before that: There is currently one issue where existing backends continue to receive more traffic due to TCP connection to them being reused than new backend pods e.g. after scaling up. Completely disabling connection reuse as done in this PR is quite a heavy solution and affects all InterceptorRoutes/HTTPScaledObjects with massive performance implications... Is this the problem that you have? Thanks! |
|
Hey @linkvt, Sorry for the delay in my response.
Let's assume we have two replicas (A and B) of keda-http-interceptor and a single replica of the upstream service.
This happens when the upstream server respects the
I agree it should be configurable at the I think saying the performance implications are massive is an exaggeration. It's really not that bad, especially without TLS. Nginx has used HTTP/1.0 for upstream proxying for years (i.e., no connection pooling by default), and many keep using it this way to date.
That's the workaround that I use: decrease it to 1ms here, and then configure the connection pool to a couple of seconds in my upstream server to prevent races when one party hasn't yet sent FIN/RST. I just feel like this is a workaround, and I'd rather make sure the requests are forwarded to upstream with |
Persistent HTTP connections are not always helpful for all workloads. With synchronous/blocking backends, one interceptor can keep a connection open to a backend worker after a request finishes. Another interceptor may reuse that worker and wait until the first connection is closed, causing cross-replica blocking and higher latency.
Add
KEDA_HTTP_DISABLE_KEEP_ALIVESso operators can disable connection reuse when this behavior appears.Checklist