From 361d39e70bd7ae5ab571d8b43d553dba50787fd5 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 20 Nov 2020 06:27:37 -0800 Subject: [PATCH 1/3] Allow configuring http connection pool size (#1474) See: https://github.com/docker-java/docker-java/issues/1466 --- .gitignore | 1 + .../httpclient5/ApacheDockerHttpClient.java | 13 +++-- .../ApacheDockerHttpClientImpl.java | 50 +++++++++++-------- .../httpclient5/ConnectionPoolConfig.java | 20 ++++++++ .../httpclient5/ZerodepDockerHttpClient.java | 17 +++++-- 5 files changed, 72 insertions(+), 29 deletions(-) create mode 100644 docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java diff --git a/.gitignore b/.gitignore index cc29f27cb..201acaa5f 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ .project .settings .classpath +.factorypath # Ignore all build/dist directories target diff --git a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java index cf2b7300d..10ba2cb09 100644 --- a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java +++ b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java @@ -13,6 +13,8 @@ public static final class Builder { private SSLConfig sslConfig = null; + private ConnectionPoolConfig connectionPoolConf = null; + public Builder dockerHost(URI value) { this.dockerHost = Objects.requireNonNull(value, "dockerHost"); return this; @@ -23,13 +25,18 @@ public Builder sslConfig(SSLConfig value) { return this; } + public Builder connectionPool(ConnectionPoolConfig conf) { + this.connectionPoolConf = conf; + return this; + } + public ApacheDockerHttpClient build() { Objects.requireNonNull(dockerHost, "dockerHost"); - return new ApacheDockerHttpClient(dockerHost, sslConfig); + return new ApacheDockerHttpClient(dockerHost, sslConfig, connectionPoolConf); } } - private ApacheDockerHttpClient(URI dockerHost, SSLConfig sslConfig) { - super(dockerHost, sslConfig); + private ApacheDockerHttpClient(URI dockerHost, SSLConfig sslConfig, ConnectionPoolConfig connectionPoolConfig) { + super(dockerHost, sslConfig, connectionPoolConfig); } } diff --git a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java index d06bd81ab..c17659d7a 100644 --- a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java +++ b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java @@ -41,12 +41,12 @@ class ApacheDockerHttpClientImpl implements DockerHttpClient { private final CloseableHttpClient httpClient; - private final HttpHost host; protected ApacheDockerHttpClientImpl( URI dockerHost, - SSLConfig sslConfig + SSLConfig sslConfig, + ConnectionPoolConfig connectionPoolConf ) { Registry socketFactoryRegistry = createConnectionSocketFactoryRegistry(sslConfig, dockerHost); @@ -66,27 +66,35 @@ protected ApacheDockerHttpClientImpl( host = HttpHost.create(dockerHost); } + PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager( + socketFactoryRegistry, + new ManagedHttpClientConnectionFactory( + null, + null, + null, + null, + message -> { + Header transferEncodingHeader = message.getFirstHeader(HttpHeaders.TRANSFER_ENCODING); + if (transferEncodingHeader != null) { + if ("identity".equalsIgnoreCase(transferEncodingHeader.getValue())) { + return ContentLengthStrategy.UNDEFINED; + } + } + return DefaultContentLengthStrategy.INSTANCE.determineLength(message); + }, + null + ) + ); + if (connectionPoolConf != null) { + Integer maxConnections = connectionPoolConf.getMaxConnections(); + if (maxConnections != null) { + connectionManager.setMaxTotal(maxConnections); + connectionManager.setDefaultMaxPerRoute(maxConnections); + } + } httpClient = HttpClients.custom() .setRequestExecutor(new HijackingHttpRequestExecutor(null)) - .setConnectionManager(new PoolingHttpClientConnectionManager( - socketFactoryRegistry, - new ManagedHttpClientConnectionFactory( - null, - null, - null, - null, - message -> { - Header transferEncodingHeader = message.getFirstHeader(HttpHeaders.TRANSFER_ENCODING); - if (transferEncodingHeader != null) { - if ("identity".equalsIgnoreCase(transferEncodingHeader.getValue())) { - return ContentLengthStrategy.UNDEFINED; - } - } - return DefaultContentLengthStrategy.INSTANCE.determineLength(message); - }, - null - ) - )) + .setConnectionManager(connectionManager) .build(); } diff --git a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java new file mode 100644 index 000000000..71d6921d9 --- /dev/null +++ b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java @@ -0,0 +1,20 @@ +package com.github.dockerjava.httpclient5; + +public class ConnectionPoolConfig { + + private Integer maxConnections; + + public Integer getMaxConnections() { + return maxConnections; + } + + public ConnectionPoolConfig setMaxConnections(Integer maxConnections) { + this.maxConnections = maxConnections; + return this; + } + + @Override + public String toString() { + return "ConnectionPoolConfig [maxConnections=" + maxConnections + "]"; + } +} diff --git a/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java b/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java index 2298da816..0dc33205f 100644 --- a/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java +++ b/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java @@ -1,10 +1,10 @@ package com.github.dockerjava.httpclient5; -import com.github.dockerjava.transport.SSLConfig; - import java.net.URI; import java.util.Objects; +import com.github.dockerjava.transport.SSLConfig; + @SuppressWarnings("unused") public final class ZerodepDockerHttpClient extends ApacheDockerHttpClientImpl { @@ -14,6 +14,8 @@ public static final class Builder { private SSLConfig sslConfig = null; + private ConnectionPoolConfig connectionPoolConfig = null; + public Builder dockerHost(URI value) { this.dockerHost = Objects.requireNonNull(value, "dockerHost"); return this; @@ -24,13 +26,18 @@ public Builder sslConfig(SSLConfig value) { return this; } + public Builder connectionPool(ConnectionPoolConfig conf) { + this.connectionPoolConfig = conf; + return this; + } + public ZerodepDockerHttpClient build() { Objects.requireNonNull(dockerHost, "dockerHost"); - return new ZerodepDockerHttpClient(dockerHost, sslConfig); + return new ZerodepDockerHttpClient(dockerHost, sslConfig, connectionPoolConfig); } } - protected ZerodepDockerHttpClient(URI dockerHost, SSLConfig sslConfig) { - super(dockerHost, sslConfig); + protected ZerodepDockerHttpClient(URI dockerHost, SSLConfig sslConfig, ConnectionPoolConfig connectionPoolConf) { + super(dockerHost, sslConfig, connectionPoolConf); } } From b8472d029b38bc05be311ea0f27e60a054269ab5 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Fri, 20 Nov 2020 17:55:01 +0100 Subject: [PATCH 2/3] Add a test, set default to max --- .../httpclient5/ApacheDockerHttpClient.java | 12 ++-- .../ApacheDockerHttpClientImpl.java | 11 +--- .../jaxrs/JerseyDockerHttpClient.java | 4 +- .../httpclient5/ZerodepDockerHttpClient.java | 16 ++--- .../dockerjava/cmd/LogContainerCmdIT.java | 65 ++++++++++++++++++- .../github/dockerjava/core/DockerRule.java | 15 +++-- 6 files changed, 92 insertions(+), 31 deletions(-) diff --git a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java index 10ba2cb09..2c1890f80 100644 --- a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java +++ b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClient.java @@ -13,7 +13,7 @@ public static final class Builder { private SSLConfig sslConfig = null; - private ConnectionPoolConfig connectionPoolConf = null; + private int maxConnections = Integer.MAX_VALUE; public Builder dockerHost(URI value) { this.dockerHost = Objects.requireNonNull(value, "dockerHost"); @@ -25,18 +25,18 @@ public Builder sslConfig(SSLConfig value) { return this; } - public Builder connectionPool(ConnectionPoolConfig conf) { - this.connectionPoolConf = conf; + public Builder maxConnections(int value) { + this.maxConnections = value; return this; } public ApacheDockerHttpClient build() { Objects.requireNonNull(dockerHost, "dockerHost"); - return new ApacheDockerHttpClient(dockerHost, sslConfig, connectionPoolConf); + return new ApacheDockerHttpClient(dockerHost, sslConfig, maxConnections); } } - private ApacheDockerHttpClient(URI dockerHost, SSLConfig sslConfig, ConnectionPoolConfig connectionPoolConfig) { - super(dockerHost, sslConfig, connectionPoolConfig); + private ApacheDockerHttpClient(URI dockerHost, SSLConfig sslConfig, int maxConnections) { + super(dockerHost, sslConfig, maxConnections); } } diff --git a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java index c17659d7a..40b13025f 100644 --- a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java +++ b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ApacheDockerHttpClientImpl.java @@ -46,7 +46,7 @@ class ApacheDockerHttpClientImpl implements DockerHttpClient { protected ApacheDockerHttpClientImpl( URI dockerHost, SSLConfig sslConfig, - ConnectionPoolConfig connectionPoolConf + int maxConnections ) { Registry socketFactoryRegistry = createConnectionSocketFactoryRegistry(sslConfig, dockerHost); @@ -85,13 +85,8 @@ protected ApacheDockerHttpClientImpl( null ) ); - if (connectionPoolConf != null) { - Integer maxConnections = connectionPoolConf.getMaxConnections(); - if (maxConnections != null) { - connectionManager.setMaxTotal(maxConnections); - connectionManager.setDefaultMaxPerRoute(maxConnections); - } - } + connectionManager.setMaxTotal(maxConnections); + connectionManager.setDefaultMaxPerRoute(maxConnections); httpClient = HttpClients.custom() .setRequestExecutor(new HijackingHttpRequestExecutor(null)) .setConnectionManager(connectionManager) diff --git a/docker-java-transport-jersey/src/main/java/com/github/dockerjava/jaxrs/JerseyDockerHttpClient.java b/docker-java-transport-jersey/src/main/java/com/github/dockerjava/jaxrs/JerseyDockerHttpClient.java index 8eb3c2c6a..74ef77e4b 100644 --- a/docker-java-transport-jersey/src/main/java/com/github/dockerjava/jaxrs/JerseyDockerHttpClient.java +++ b/docker-java-transport-jersey/src/main/java/com/github/dockerjava/jaxrs/JerseyDockerHttpClient.java @@ -54,9 +54,9 @@ public static final class Builder { private Integer connectTimeout = null; - private Integer maxTotalConnections = null; + private Integer maxTotalConnections = Integer.MAX_VALUE; - private Integer maxPerRouteConnections = null; + private Integer maxPerRouteConnections = Integer.MAX_VALUE; private Integer connectionRequestTimeout = null; diff --git a/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java b/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java index 0dc33205f..a0d2abaaf 100644 --- a/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java +++ b/docker-java-transport-zerodep/src/main/java/com/github/dockerjava/httpclient5/ZerodepDockerHttpClient.java @@ -1,10 +1,10 @@ package com.github.dockerjava.httpclient5; +import com.github.dockerjava.transport.SSLConfig; + import java.net.URI; import java.util.Objects; -import com.github.dockerjava.transport.SSLConfig; - @SuppressWarnings("unused") public final class ZerodepDockerHttpClient extends ApacheDockerHttpClientImpl { @@ -14,7 +14,7 @@ public static final class Builder { private SSLConfig sslConfig = null; - private ConnectionPoolConfig connectionPoolConfig = null; + private int maxConnections = Integer.MAX_VALUE; public Builder dockerHost(URI value) { this.dockerHost = Objects.requireNonNull(value, "dockerHost"); @@ -26,18 +26,18 @@ public Builder sslConfig(SSLConfig value) { return this; } - public Builder connectionPool(ConnectionPoolConfig conf) { - this.connectionPoolConfig = conf; + public Builder maxConnections(int value) { + this.maxConnections = value; return this; } public ZerodepDockerHttpClient build() { Objects.requireNonNull(dockerHost, "dockerHost"); - return new ZerodepDockerHttpClient(dockerHost, sslConfig, connectionPoolConfig); + return new ZerodepDockerHttpClient(dockerHost, sslConfig, maxConnections); } } - protected ZerodepDockerHttpClient(URI dockerHost, SSLConfig sslConfig, ConnectionPoolConfig connectionPoolConf) { - super(dockerHost, sslConfig, connectionPoolConf); + protected ZerodepDockerHttpClient(URI dockerHost, SSLConfig sslConfig, int maxConnections) { + super(dockerHost, sslConfig, maxConnections); } } diff --git a/docker-java/src/test/java/com/github/dockerjava/cmd/LogContainerCmdIT.java b/docker-java/src/test/java/com/github/dockerjava/cmd/LogContainerCmdIT.java index 37bf5f393..b0de380db 100644 --- a/docker-java/src/test/java/com/github/dockerjava/cmd/LogContainerCmdIT.java +++ b/docker-java/src/test/java/com/github/dockerjava/cmd/LogContainerCmdIT.java @@ -1,7 +1,10 @@ package com.github.dockerjava.cmd; +import com.github.dockerjava.api.DockerClient; +import com.github.dockerjava.api.async.ResultCallback; import com.github.dockerjava.api.command.CreateContainerResponse; import com.github.dockerjava.api.exception.NotFoundException; +import com.github.dockerjava.api.model.Frame; import com.github.dockerjava.api.model.StreamType; import com.github.dockerjava.utils.LogContainerTestCallback; import org.junit.Test; @@ -9,13 +12,25 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; +import java.util.stream.LongStream; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.CoreMatchers.everyItem; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.emptyString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -197,4 +212,52 @@ public void asyncLogContainerWithSince() throws Exception { assertThat(loggingCallback.toString(), containsString(snippet)); } + + @Test(timeout = 10_000) + public void simultaneousCommands() throws Exception { + // Create a new client to not affect other tests + DockerClient client = dockerRule.newClient(); + CreateContainerResponse container = client.createContainerCmd("busybox") + .withCmd("/bin/sh", "-c", "echo hello world; sleep infinity") + .exec(); + + client.startContainerCmd(container.getId()).exec(); + + // Simulate 100 simultaneous connections + int connections = 100; + + ExecutorService executor = Executors.newFixedThreadPool(connections); + try { + List firstFrames = new CopyOnWriteArrayList<>(); + executor.invokeAll( + LongStream.range(0, connections).>mapToObj(__ -> { + return () -> { + return client.logContainerCmd(container.getId()) + .withStdOut(true) + .withFollowStream(true) + .exec(new ResultCallback.Adapter() { + + final AtomicBoolean first = new AtomicBoolean(true); + + @Override + public void onNext(Frame object) { + if (first.compareAndSet(true, false)) { + firstFrames.add(object); + } + super.onNext(object); + } + }); + }; + }).collect(Collectors.toList()) + ); + + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> { + assertThat(firstFrames, hasSize(connections)); + }); + + assertThat(firstFrames, everyItem(hasToString("STDOUT: hello world"))); + } finally { + executor.shutdownNow(); + } + } } diff --git a/docker-java/src/test/java/com/github/dockerjava/core/DockerRule.java b/docker-java/src/test/java/com/github/dockerjava/core/DockerRule.java index c7ca2c0d9..e6e922bc6 100644 --- a/docker-java/src/test/java/com/github/dockerjava/core/DockerRule.java +++ b/docker-java/src/test/java/com/github/dockerjava/core/DockerRule.java @@ -46,11 +46,7 @@ public DockerRule(CmdIT cmdIT) { } - public DockerClient getClient() { - if (dockerClient != null) { - return dockerClient; - } - + public DockerClient newClient() { DockerClientImpl dockerClient = cmdIT.getFactoryType().createDockerClient(config()); DockerHttpClient dockerHttpClient = dockerClient.getHttpClient(); @@ -88,7 +84,7 @@ public CreateVolumeCmd.Exec createCreateVolumeCmdExec() { } ); - return this.dockerClient = new DockerClientDelegate(dockerClient) { + return new DockerClientDelegate(dockerClient) { @Override public DockerHttpClient getHttpClient() { return dockerHttpClient; @@ -96,6 +92,13 @@ public DockerHttpClient getHttpClient() { }; } + public DockerClient getClient() { + if (dockerClient != null) { + return dockerClient; + } + return this.dockerClient = newClient(); + } + @Override public Statement apply(Statement base, Description description) { return super.apply(base, description); From 09e48ea4998b81d15399bda1400062df97b01450 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Fri, 20 Nov 2020 17:56:16 +0100 Subject: [PATCH 3/3] remove unused class --- .../httpclient5/ConnectionPoolConfig.java | 20 ------------------- 1 file changed, 20 deletions(-) delete mode 100644 docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java diff --git a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java b/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java deleted file mode 100644 index 71d6921d9..000000000 --- a/docker-java-transport-httpclient5/src/main/java/com/github/dockerjava/httpclient5/ConnectionPoolConfig.java +++ /dev/null @@ -1,20 +0,0 @@ -package com.github.dockerjava.httpclient5; - -public class ConnectionPoolConfig { - - private Integer maxConnections; - - public Integer getMaxConnections() { - return maxConnections; - } - - public ConnectionPoolConfig setMaxConnections(Integer maxConnections) { - this.maxConnections = maxConnections; - return this; - } - - @Override - public String toString() { - return "ConnectionPoolConfig [maxConnections=" + maxConnections + "]"; - } -}