Skip to content

Commit

Permalink
[MRESOLVER-347] Better connection pool configuration (reuse, max TTL,…
Browse files Browse the repository at this point in the history
… maxPerRoute) (apache#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
  • Loading branch information
cstamas authored Mar 30, 2023
1 parent 267d288 commit d0f8564
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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)
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -86,7 +91,7 @@ public String toString() {

private static final String CONFIG_PROP_CACHE_STATE = "aether.connector.http.cacheState";

private final ConcurrentMap<SslConfig, HttpClientConnectionManager> connectionManagers;
private final ConcurrentMap<ConnMgrConfig, HttpClientConnectionManager> connectionManagers;

private final ConcurrentMap<CompoundKey, Object> userTokens;

Expand Down Expand Up @@ -127,7 +132,7 @@ private GlobalState() {

@Override
public void close() {
for (Iterator<Map.Entry<SslConfig, HttpClientConnectionManager>> it =
for (Iterator<Map.Entry<ConnMgrConfig, HttpClientConnectionManager>> it =
connectionManagers.entrySet().iterator();
it.hasNext(); ) {
HttpClientConnectionManager connMgr = it.next().getValue();
Expand All @@ -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<ConnectionSocketFactory> registryBuilder = RegistryBuilder.<ConnectionSocketFactory>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()
Expand All @@ -177,26 +177,31 @@ 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) {
hostnameVerifier = NoopHostnameVerifier.INSTANCE;
}
} 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,16 @@ final class LocalState implements Closeable {

private final ConcurrentMap<HttpHost, AuthSchemePool> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
3 changes: 3 additions & 0 deletions src/site/markdown/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String>` | 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 <a href="https://hc.apache.org/httpcomponents-client-4.5.x/current/httpclient/apidocs/org/apache/http/impl/client/HttpClientBuilder.html">HttpClientBuilder</a> 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
Expand Down

0 comments on commit d0f8564

Please sign in to comment.