From d0f8564846863c8a88f2fb1da8391fc860517192 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 30 Mar 2023 22:26:54 +0200 Subject: [PATCH] [MRESOLVER-347] Better connection pool configuration (reuse, max TTL, maxPerRoute) (#274) Transport HTTP new config to not reuse connections (should not be used under any normal circumstance), new config to set HTTP connections max TTL (10 minutes by default). and new config to set max connections per route. --- https://issues.apache.org/jira/browse/MRESOLVER-347 https://issues.apache.org/jira/browse/MRESOLVER-348 --- .../aether/ConfigurationProperties.java | 45 +++++++++++++++ .../{SslConfig.java => ConnMgrConfig.java} | 28 +++++++-- .../aether/transport/http/GlobalState.java | 57 ++++++++++--------- .../transport/http/HttpTransporter.java | 26 ++++++++- .../aether/transport/http/LocalState.java | 6 +- .../transport/http/HttpTransporterTest.java | 17 ++++++ src/site/markdown/configuration.md | 3 + 7 files changed, 147 insertions(+), 35 deletions(-) rename maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/{SslConfig.java => ConnMgrConfig.java} (73%) diff --git a/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java b/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java index 0cfa9f4e4..63733b8f0 100644 --- a/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java +++ b/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java @@ -159,6 +159,51 @@ public final class ConfigurationProperties { */ public static final boolean DEFAULT_HTTP_PREEMPTIVE_AUTH = false; + /** + * Should HTTP client reuse connections (in other words, pool connections) or not? + * + * @see #DEFAULT_HTTP_REUSE_CONNECTIONS + * @since 1.9.8 + */ + public static final String HTTP_REUSE_CONNECTIONS = PREFIX_CONNECTOR + "http.reuseConnections"; + + /** + * The default value to use if {@link #HTTP_REUSE_CONNECTIONS} isn't set (true). + * + * @since 1.9.8 + */ + public static final boolean DEFAULT_HTTP_REUSE_CONNECTIONS = true; + + /** + * Time to live in seconds for an HTTP connection, after that time, the connection will be dropped. + * + * @see #DEFAULT_HTTP_CONNECTION_MAX_TTL + * @since 1.9.8 + */ + public static final String HTTP_CONNECTION_MAX_TTL = PREFIX_CONNECTOR + "http.connectionMaxTtl"; + + /** + * The default value to use if {@link #HTTP_CONNECTION_MAX_TTL} isn't set (600 seconds). + * + * @since 1.9.8 + */ + public static final int DEFAULT_HTTP_CONNECTION_MAX_TTL = 600; + + /** + * The maximum concurrent connections per route HTTP client is allowed to use. + * + * @see #DEFAULT_HTTP_MAX_CONNECTIONS_PER_ROUTE + * @since 1.9.8 + */ + public static final String HTTP_MAX_CONNECTIONS_PER_ROUTE = PREFIX_CONNECTOR + "http.maxConnectionsPerRoute"; + + /** + * The default value to use if {@link #HTTP_MAX_CONNECTIONS_PER_ROUTE} isn't set (50 connections). + * + * @since 1.9.8 + */ + public static final int DEFAULT_HTTP_MAX_CONNECTIONS_PER_ROUTE = 50; + /** * The mode that sets HTTPS transport "security mode": to ignore any SSL errors (certificate validity checks, * hostname verification). The default value is {@link #HTTPS_SECURITY_MODE_DEFAULT}. diff --git a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/SslConfig.java b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/ConnMgrConfig.java similarity index 73% rename from maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/SslConfig.java rename to maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/ConnMgrConfig.java index 33f04617f..a06d91df8 100644 --- a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/SslConfig.java +++ b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/ConnMgrConfig.java @@ -29,9 +29,10 @@ import org.eclipse.aether.util.ConfigUtils; /** - * SSL-related configuration and cache key for connection pools (whose scheme registries are derived from this config). + * Connection manager config: among other SSL-related configuration and cache key for connection pools (whose scheme + * registries are derived from this config). */ -final class SslConfig { +final class ConnMgrConfig { private static final String CIPHER_SUITES = "https.cipherSuites"; @@ -47,7 +48,16 @@ final class SslConfig { final String httpsSecurityMode; - SslConfig(RepositorySystemSession session, AuthenticationContext authContext, String httpsSecurityMode) { + final int connectionMaxTtlSeconds; + + final int maxConnectionsPerRoute; + + ConnMgrConfig( + RepositorySystemSession session, + AuthenticationContext authContext, + String httpsSecurityMode, + int connectionMaxTtlSeconds, + int maxConnectionsPerRoute) { context = (authContext != null) ? authContext.get(AuthenticationContext.SSL_CONTEXT, SSLContext.class) : null; verifier = (authContext != null) ? authContext.get(AuthenticationContext.SSL_HOSTNAME_VERIFIER, HostnameVerifier.class) @@ -56,6 +66,8 @@ final class SslConfig { cipherSuites = split(get(session, CIPHER_SUITES)); protocols = split(get(session, PROTOCOLS)); this.httpsSecurityMode = httpsSecurityMode; + this.connectionMaxTtlSeconds = connectionMaxTtlSeconds; + this.maxConnectionsPerRoute = maxConnectionsPerRoute; } private static String get(RepositorySystemSession session, String key) { @@ -81,11 +93,14 @@ public boolean equals(Object obj) { if (obj == null || !getClass().equals(obj.getClass())) { return false; } - SslConfig that = (SslConfig) obj; + ConnMgrConfig that = (ConnMgrConfig) obj; return Objects.equals(context, that.context) && Objects.equals(verifier, that.verifier) && Arrays.equals(cipherSuites, that.cipherSuites) - && Arrays.equals(protocols, that.protocols); + && Arrays.equals(protocols, that.protocols) + && Objects.equals(httpsSecurityMode, that.httpsSecurityMode) + && connectionMaxTtlSeconds == that.connectionMaxTtlSeconds + && maxConnectionsPerRoute == that.maxConnectionsPerRoute; } @Override @@ -95,6 +110,9 @@ public int hashCode() { hash = hash * 31 + hash(verifier); hash = hash * 31 + Arrays.hashCode(cipherSuites); hash = hash * 31 + Arrays.hashCode(protocols); + hash = hash * 31 + hash(httpsSecurityMode); + hash = hash * 31 + hash(connectionMaxTtlSeconds); + hash = hash * 31 + hash(maxConnectionsPerRoute); return hash; } diff --git a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/GlobalState.java b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/GlobalState.java index 1f6d7789d..2fdc80293 100644 --- a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/GlobalState.java +++ b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/GlobalState.java @@ -27,6 +27,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; import org.apache.http.HttpHost; import org.apache.http.config.RegistryBuilder; @@ -35,7 +36,11 @@ import org.apache.http.conn.socket.PlainConnectionSocketFactory; import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.impl.conn.DefaultHttpClientConnectionOperator; +import org.apache.http.impl.conn.DefaultSchemePortResolver; +import org.apache.http.impl.conn.ManagedHttpClientConnectionFactory; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.apache.http.impl.conn.SystemDefaultDnsResolver; import org.apache.http.ssl.SSLContextBuilder; import org.apache.http.ssl.SSLInitializationException; import org.eclipse.aether.ConfigurationProperties; @@ -86,7 +91,7 @@ public String toString() { private static final String CONFIG_PROP_CACHE_STATE = "aether.connector.http.cacheState"; - private final ConcurrentMap connectionManagers; + private final ConcurrentMap connectionManagers; private final ConcurrentMap userTokens; @@ -127,7 +132,7 @@ private GlobalState() { @Override public void close() { - for (Iterator> it = + for (Iterator> it = connectionManagers.entrySet().iterator(); it.hasNext(); ) { HttpClientConnectionManager connMgr = it.next().getValue(); @@ -136,39 +141,34 @@ public void close() { } } - public HttpClientConnectionManager getConnectionManager(SslConfig config) { - HttpClientConnectionManager manager = connectionManagers.get(config); - if (manager == null) { - HttpClientConnectionManager connMgr = newConnectionManager(config); - manager = connectionManagers.putIfAbsent(config, connMgr); - if (manager != null) { - connMgr.shutdown(); - } else { - manager = connMgr; - } - } - return manager; + public HttpClientConnectionManager getConnectionManager(ConnMgrConfig config) { + return connectionManagers.computeIfAbsent(config, GlobalState::newConnectionManager); } @SuppressWarnings("checkstyle:magicnumber") - public static HttpClientConnectionManager newConnectionManager(SslConfig sslConfig) { + public static HttpClientConnectionManager newConnectionManager(ConnMgrConfig connMgrConfig) { RegistryBuilder registryBuilder = RegistryBuilder.create() .register("http", PlainConnectionSocketFactory.getSocketFactory()); + int connectionMaxTtlSeconds = ConfigurationProperties.DEFAULT_HTTP_CONNECTION_MAX_TTL; + int maxConnectionsPerRoute = ConfigurationProperties.DEFAULT_HTTP_MAX_CONNECTIONS_PER_ROUTE; - if (sslConfig == null) { + if (connMgrConfig == null) { registryBuilder.register("https", SSLConnectionSocketFactory.getSystemSocketFactory()); } else { // config present: use provided, if any, or create (depending on httpsSecurityMode) - SSLSocketFactory sslSocketFactory = sslConfig.context != null ? sslConfig.context.getSocketFactory() : null; - HostnameVerifier hostnameVerifier = sslConfig.verifier; - if (ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT.equals(sslConfig.httpsSecurityMode)) { + connectionMaxTtlSeconds = connMgrConfig.connectionMaxTtlSeconds; + maxConnectionsPerRoute = connMgrConfig.maxConnectionsPerRoute; + SSLSocketFactory sslSocketFactory = + connMgrConfig.context != null ? connMgrConfig.context.getSocketFactory() : null; + HostnameVerifier hostnameVerifier = connMgrConfig.verifier; + if (ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT.equals(connMgrConfig.httpsSecurityMode)) { if (sslSocketFactory == null) { sslSocketFactory = (SSLSocketFactory) SSLSocketFactory.getDefault(); } if (hostnameVerifier == null) { hostnameVerifier = SSLConnectionSocketFactory.getDefaultHostnameVerifier(); } - } else if (ConfigurationProperties.HTTPS_SECURITY_MODE_INSECURE.equals(sslConfig.httpsSecurityMode)) { + } else if (ConfigurationProperties.HTTPS_SECURITY_MODE_INSECURE.equals(connMgrConfig.httpsSecurityMode)) { if (sslSocketFactory == null) { try { sslSocketFactory = new SSLContextBuilder() @@ -177,7 +177,7 @@ public static HttpClientConnectionManager newConnectionManager(SslConfig sslConf .getSocketFactory(); } catch (Exception e) { throw new SSLInitializationException( - "Could not configure '" + sslConfig.httpsSecurityMode + "' HTTPS security mode", e); + "Could not configure '" + connMgrConfig.httpsSecurityMode + "' HTTPS security mode", e); } } if (hostnameVerifier == null) { @@ -185,18 +185,23 @@ public static HttpClientConnectionManager newConnectionManager(SslConfig sslConf } } else { throw new IllegalArgumentException( - "Unsupported '" + sslConfig.httpsSecurityMode + "' HTTPS security mode."); + "Unsupported '" + connMgrConfig.httpsSecurityMode + "' HTTPS security mode."); } registryBuilder.register( "https", new SSLConnectionSocketFactory( - sslSocketFactory, sslConfig.protocols, sslConfig.cipherSuites, hostnameVerifier)); + sslSocketFactory, connMgrConfig.protocols, connMgrConfig.cipherSuites, hostnameVerifier)); } - PoolingHttpClientConnectionManager connMgr = new PoolingHttpClientConnectionManager(registryBuilder.build()); - connMgr.setMaxTotal(100); - connMgr.setDefaultMaxPerRoute(50); + PoolingHttpClientConnectionManager connMgr = new PoolingHttpClientConnectionManager( + new DefaultHttpClientConnectionOperator( + registryBuilder.build(), DefaultSchemePortResolver.INSTANCE, SystemDefaultDnsResolver.INSTANCE), + ManagedHttpClientConnectionFactory.INSTANCE, + connectionMaxTtlSeconds, + TimeUnit.SECONDS); + connMgr.setMaxTotal(maxConnectionsPerRoute * 2); + connMgr.setDefaultMaxPerRoute(maxConnectionsPerRoute); return connMgr; } diff --git a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java index b613c306e..471300a0d 100644 --- a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java +++ b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java @@ -61,6 +61,7 @@ import org.apache.http.config.SocketConfig; import org.apache.http.entity.AbstractHttpEntity; import org.apache.http.entity.ByteArrayEntity; +import org.apache.http.impl.NoConnectionReuseStrategy; import org.apache.http.impl.auth.BasicScheme; import org.apache.http.impl.auth.BasicSchemeFactory; import org.apache.http.impl.auth.DigestSchemeFactory; @@ -161,7 +162,21 @@ final class HttpTransporter extends AbstractTransporter { ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT, ConfigurationProperties.HTTPS_SECURITY_MODE + "." + repository.getId(), ConfigurationProperties.HTTPS_SECURITY_MODE); - this.state = new LocalState(session, repository, new SslConfig(session, repoAuthContext, httpsSecurityMode)); + final int connectionMaxTtlSeconds = ConfigUtils.getInteger( + session, + ConfigurationProperties.DEFAULT_HTTP_CONNECTION_MAX_TTL, + ConfigurationProperties.HTTP_CONNECTION_MAX_TTL + "." + repository.getId(), + ConfigurationProperties.HTTP_CONNECTION_MAX_TTL); + final int maxConnectionsPerRoute = ConfigUtils.getInteger( + session, + ConfigurationProperties.DEFAULT_HTTP_MAX_CONNECTIONS_PER_ROUTE, + ConfigurationProperties.HTTP_MAX_CONNECTIONS_PER_ROUTE + "." + repository.getId(), + ConfigurationProperties.HTTP_MAX_CONNECTIONS_PER_ROUTE); + this.state = new LocalState( + session, + repository, + new ConnMgrConfig( + session, repoAuthContext, httpsSecurityMode, connectionMaxTtlSeconds, maxConnectionsPerRoute)); this.headers = ConfigUtils.getMap( session, @@ -243,6 +258,15 @@ final class HttpTransporter extends AbstractTransporter { builder.useSystemProperties(); } + final boolean reuseConnections = ConfigUtils.getBoolean( + session, + ConfigurationProperties.DEFAULT_HTTP_REUSE_CONNECTIONS, + ConfigurationProperties.HTTP_REUSE_CONNECTIONS + "." + repository.getId(), + ConfigurationProperties.HTTP_REUSE_CONNECTIONS); + if (!reuseConnections) { + builder.setConnectionReuseStrategy(NoConnectionReuseStrategy.INSTANCE); + } + this.client = builder.build(); } diff --git a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/LocalState.java b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/LocalState.java index a4812361e..c7352bcd0 100644 --- a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/LocalState.java +++ b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/LocalState.java @@ -50,16 +50,16 @@ final class LocalState implements Closeable { private final ConcurrentMap authSchemePools; - LocalState(RepositorySystemSession session, RemoteRepository repo, SslConfig sslConfig) { + LocalState(RepositorySystemSession session, RemoteRepository repo, ConnMgrConfig connMgrConfig) { global = GlobalState.get(session); userToken = this; if (global == null) { - connMgr = GlobalState.newConnectionManager(sslConfig); + connMgr = GlobalState.newConnectionManager(connMgrConfig); userTokenKey = null; expectContinueKey = null; authSchemePools = new ConcurrentHashMap<>(); } else { - connMgr = global.getConnectionManager(sslConfig); + connMgr = global.getConnectionManager(connMgrConfig); userTokenKey = new CompoundKey(repo.getId(), repo.getUrl(), repo.getAuthentication(), repo.getProxy()); expectContinueKey = new CompoundKey(repo.getUrl(), repo.getProxy()); authSchemePools = global.getAuthSchemePools(); diff --git a/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java b/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java index 200f5b8d7..6341c1553 100644 --- a/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java +++ b/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java @@ -1165,6 +1165,23 @@ public void testConnectionReuse() throws Exception { assertEquals(stats.toString(), 1, stats.getAvailable()); } + @Test + public void testConnectionNoReuse() throws Exception { + httpServer.addSslConnector(); + session.setCache(new DefaultRepositoryCache()); + session.setConfigProperty(ConfigurationProperties.HTTP_REUSE_CONNECTIONS, false); + for (int i = 0; i < 3; i++) { + newTransporter(httpServer.getHttpsUrl()); + GetTask task = new GetTask(URI.create("repo/file.txt")); + transporter.get(task); + assertEquals("test", task.getDataString()); + } + PoolStats stats = ((ConnPoolControl) + ((HttpTransporter) transporter).getState().getConnectionManager()) + .getTotalStats(); + assertEquals(stats.toString(), 0, stats.getAvailable()); + } + @Test(expected = NoTransporterException.class) public void testInit_BadProtocol() throws Exception { newTransporter("bad:/void"); diff --git a/src/site/markdown/configuration.md b/src/site/markdown/configuration.md index 714661e48..322a0b283 100644 --- a/src/site/markdown/configuration.md +++ b/src/site/markdown/configuration.md @@ -35,11 +35,14 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix `aether.connector.classpath.loader` | ClassLoader | `ClassLoader` from which resources should be retrieved which start with the `classpath:` protocol. | `Thread.currentThread().getContextClassLoader()` | no `aether.connector.connectTimeout` | long | Connect timeout in milliseconds. | `10000` | yes `aether.connector.http.cacheState` | boolean | Flag indicating whether a memory-based cache is used for user tokens, connection managers, expect continue requests and authentication schemes. | `true` | no +`aether.connector.http.connectionMaxTtl` | int | Time to live in seconds for an HTTP connection, after that time, the connection will be dropped. | `600` | yes `aether.connector.http.credentialEncoding` | String | The encoding/charset to use when exchanging credentials with HTTP servers. | `"ISO-8859-1"` | yes `aether.connector.http.headers` | `Map` | The request headers to use for HTTP-based repository connectors. The headers are specified using a map of strings mapping a header name to its value. The repository-specific headers map is supposed to be complete, i.e. is not merged with the general headers map. | - | yes +`aether.connector.http.maxConnectionsPerRoute` | int | The maximum concurrent connections per route HTTP client is allowed to use. | `50` | yes `aether.connector.http.preemptiveAuth` | boolean | Should HTTP client use preemptive-authentication for all HTTP verbs (works only w/ BASIC). By default is disabled, as it is considered less secure. | `false` | yes `aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes `aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes +`aether.connector.http.reuseConnections` | boolean | Should HTTP client reuse connections (in other words, pool connections) or not? | `true` | yes `aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes `aether.connector.http.useSystemProperties` | boolean | If enabled, underlying Apache HttpClient will use system properties as well to configure itself (typically used to set up HTTP Proxy via Java system properties). See HttpClientBuilder for used properties. This mode is **not recommended**, better use documented ways of configuration instead. | `false` | yes `aether.connector.https.cipherSuites` | String | Comma-separated list of [Cipher Suites](https://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#ciphersuites) which are enabled for HTTPS connections. | - (no restriction) | no