envoy icon indicating copy to clipboard operation
envoy copied to clipboard

HCM supports configuring keep-alive headers

Open johnlanni opened this issue 1 year ago • 5 comments

Title: *HCM supports configuring keep-alive headers *

Description:

image

Doc link: https://hc.apache.org/httpcomponents-client-4.5.x/current/tutorial/html/connmgmt.html

When using a lower version (<5.2) of Java Apache HTTP client, the client will determine how long a connection can be reused based on the Keep-Alive header in the backend response.If the Keep-Alive header is not present in the response, HttpClient assumes the connection can be kept alive indefinitely. If Envoy removes this Keep-Alive response header, the client may attempt to reuse a closed connection, resulting in I/O errors.

This is due to incorrect implementation of connection reuse mechanism by the client, but in our scenario, such clients are present on numerous old devices and cannot be upgraded. Therefore, we hope that Envoy can support configuring not to remove this response header. This would help us migrate from a large number of Nginx gateways to Envoy gateways.

(The following content is added after modification.)

I realize that it is inaccurate to solve this issue by simply not clearing the hop-by-hop headers. In fact, we need to set the Connection and Keep-Alive headers for the envoy proxy layer in order to adapt to certain special clients, instead of passing through these headers from upstream. However, currently there is no distinction made when mutating response headers whether these headers were added through response_headers_to_add in route config or returned by upstream, which prevents us from setting them.

A reasonable solution that I have considered is to refer to the keepalive_timeout directive in Nginx. Set the idle_timeout to the Keep-Alive Header when handling HTTP/1, but do not set it for HTTP/2 or HTTP/3.

johnlanni avatar Apr 17 '24 06:04 johnlanni

cc @alyssawilk @RyanTheOptimist IIRC you worked on the previous hop-by-hop headers removal.

adisuissa avatar Apr 19 '24 18:04 adisuissa

When using a lower version (<5.2) of Java Apache HTTP client, the client will determine how long a connection can be reused based on the Keep-Alive header in the backend response.If the Keep-Alive header is not present in the response, HttpClient assumes the connection can be kept alive indefinitely. If Envoy removes this Keep-Alive response header, the client may attempt to reuse a closed connection, resulting in I/O errors.

I think there's a misunderstanding on how keep-alive works

If you have upstream ---- Envoy --- client the upstream keep-alive header tells Envoy how long the connection between the upstream and Envoy can be used. There's no correlation between the lifetime of that connection and the Envoy-client connection.

Imagine the client sending request 1 over connection 1. This causes Envoy to fetch connection A upstream. In the response for A, the upstream sends a keep-alive response. This does not apply for connection 1. If the client reuses connection 1 for request 2, it may use a completely different upstream connection. In this case, sending keep-alive is going to result in problematic behavior.

alyssawilk avatar Apr 30 '24 12:04 alyssawilk

@alyssawilk You are right, I realize that it is inaccurate to solve this issue by simply not clearing the hop-by-hop headers. Sorry for not updating my understanding of this solution in this issue (already updated now). In this PR, my implementation is to support setting the keep-alive state for the connection between Downstream and Envoy, instead of directly returning the Upstream's keep-alive header.

johnlanni avatar May 01 '24 03:05 johnlanni

https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout image

This nginx directive supports the following configuration: keepalive_timeout 90s 60s;

This configuration means that the idle timeout for connections between Nginx and Downstream is 90 seconds, and Nginx will return Connection: keep-alive and Keep-Alive: timeout=60 in the response of each HTTP/1 request.

Because Envoy sets idle_timeout in http_protocol_options, so here we add a setting for the timeout value of keep-alive header in HCM's configuration options.

johnlanni avatar May 01 '24 04:05 johnlanni

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] avatar May 31 '24 08:05 github-actions[bot]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

github-actions[bot] avatar Jun 07 '24 12:06 github-actions[bot]