Skip to content

Expire idle pooled connections when the peer sends no Keep-Alive #2683

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

Open
DavidTavoularis wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Expire idle pooled connections when the peer sends no Keep-Alive #2683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions ...tpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@

class ApacheDockerHttpClientImpl implements DockerHttpClient {

/**
* Idle lifetime of a pooled connection when the peer advertises no {@code Keep-Alive} timeout.
* Visible for the test that brackets it.
*/
static final TimeValue CONNECTION_KEEP_ALIVE = TimeValue.ofSeconds(2);

private final CloseableHttpClient httpClient;
private final HttpHost host;
private final String pathPrefix;
Expand Down Expand Up @@ -132,6 +138,24 @@ protected ApacheDockerHttpClientImpl(
.setConnectionManager(connectionManager)
.setDefaultRequestConfig(RequestConfig.custom()
.setResponseTimeout(responseTimeout != null ? Timeout.of(responseTimeout.toNanos(), TimeUnit.NANOSECONDS) : null)
// How long a pooled connection may sit idle when the peer does not say. This is a
// fallback, not a ceiling: DefaultConnectionKeepAliveStrategy returns a response's
// Keep-Alive timeout if there is one and only consults this otherwise -- which is
// what we want, since a peer advertising its own window is telling the truth about
// its own connection.
//
// Both daemons that matter advertise nothing. Docker then never closes the
// connection, so any value is safe. Podman closes it once idle for twice its
// service_timeout -- that setting defaults to 5s, so the window is 10s -- and
// without this the strategy falls back to HttpClient5's 3-minute default and hands
// out connections podman dropped long ago. That is
// testcontainers-java#7310 / #7593.
//
// Kept well under 10s because podman's window is configurable downwards. The cost
// is a reconnect per gap longer than this, which is cheap over a socket or pipe but
// is a full TLS handshake for tcp:// with DOCKER_TLS_VERIFY. IdleConnectionReuseTest
// pins its timings either side of this value, so changing it means changing them.
.setConnectionKeepAlive(CONNECTION_KEEP_ALIVE)
.build())
.disableConnectionState()
.build();
Expand Down
Loading