Regression: connection managers fail to take into account per route connection config when closing expired connections

This commit is contained in:
Oleg Kalnichevski 2022-11-06 11:01:10 +01:00
parent 233a5bdbb7
commit fe1e095ef9
4 changed files with 65 additions and 11 deletions

View File

@ -58,7 +58,6 @@ import org.apache.hc.core5.testing.classic.ClassicTestServer;
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
@ -276,7 +275,7 @@ public class TestConnectionManagement {
connManager.close();
}
@Test @Disabled
@Test
public void testCloseExpiredTTLConnections() throws Exception {
final ClassicTestServer server = startServer();
server.registerHandler("/random/*", new RandomHandler());
@ -294,6 +293,7 @@ public class TestConnectionManagement {
);
final PoolingHttpClientConnectionManager connManager = testResources.connManager();
connManager.setMaxTotal(1);
final HttpRoute route = new HttpRoute(target, null, false);
final HttpContext context = new BasicHttpContext();

View File

@ -107,7 +107,7 @@ public class ConnectionConfig implements Cloneable {
public String toString() {
final StringBuilder builder = new StringBuilder();
builder.append("[");
builder.append(", connectTimeout=").append(connectTimeout);
builder.append("connectTimeout=").append(connectTimeout);
builder.append(", socketTimeout=").append(socketTimeout);
builder.append(", validateAfterInactivity=").append(validateAfterInactivity);
builder.append(", timeToLive=").append(timeToLive);

View File

@ -188,19 +188,33 @@ public class PoolingHttpClientConnectionManager
this.connectionOperator = Args.notNull(httpClientConnectionOperator, "Connection operator");
switch (poolConcurrencyPolicy != null ? poolConcurrencyPolicy : PoolConcurrencyPolicy.STRICT) {
case STRICT:
this.pool = new StrictConnPool<>(
this.pool = new StrictConnPool<HttpRoute, ManagedHttpClientConnection>(
DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
DEFAULT_MAX_TOTAL_CONNECTIONS,
timeToLive,
poolReusePolicy,
null);
null) {
@Override
public void closeExpired() {
enumAvailable(e -> closeIfExpired(e));
}
};
break;
case LAX:
this.pool = new LaxConnPool<>(
this.pool = new LaxConnPool<HttpRoute, ManagedHttpClientConnection>(
DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
timeToLive,
poolReusePolicy,
null);
null) {
@Override
public void closeExpired() {
enumAvailable(e -> closeIfExpired(e));
}
};
break;
default:
throw new IllegalArgumentException("Unexpected PoolConcurrencyPolicy value: " + poolConcurrencyPolicy);
@ -570,6 +584,19 @@ public class PoolingHttpClientConnectionManager
this.tlsConfigResolver = tlsConfigResolver;
}
void closeIfExpired(final PoolEntry<HttpRoute, ManagedHttpClientConnection> entry) {
final long now = System.currentTimeMillis();
if (entry.getExpiryDeadline().isBefore(now)) {
entry.discardConnection(CloseMode.GRACEFUL);
} else {
final ConnectionConfig connectionConfig = resolveConnectionConfig(entry.getRoute());
final TimeValue timeToLive = connectionConfig.getTimeToLive();
if (timeToLive != null && Deadline.calculate(entry.getCreated(), timeToLive).isBefore(now)) {
entry.discardConnection(CloseMode.GRACEFUL);
}
}
}
/**
* @deprecated Use custom {@link #setConnectionConfigResolver(Resolver)}
*/

View File

@ -172,19 +172,33 @@ public class PoolingAsyncClientConnectionManager implements AsyncClientConnectio
this.connectionOperator = Args.notNull(connectionOperator, "Connection operator");
switch (poolConcurrencyPolicy != null ? poolConcurrencyPolicy : PoolConcurrencyPolicy.STRICT) {
case STRICT:
this.pool = new StrictConnPool<>(
this.pool = new StrictConnPool<HttpRoute, ManagedAsyncClientConnection>(
DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
DEFAULT_MAX_TOTAL_CONNECTIONS,
timeToLive,
poolReusePolicy,
null);
null) {
@Override
public void closeExpired() {
enumAvailable(e -> closeIfExpired(e));
}
};
break;
case LAX:
this.pool = new LaxConnPool<>(
this.pool = new LaxConnPool<HttpRoute, ManagedAsyncClientConnection>(
DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
timeToLive,
poolReusePolicy,
null);
null) {
@Override
public void closeExpired() {
enumAvailable(e -> closeIfExpired(e));
}
};
break;
default:
throw new IllegalArgumentException("Unexpected PoolConcurrencyPolicy value: " + poolConcurrencyPolicy);
@ -619,6 +633,19 @@ public class PoolingAsyncClientConnectionManager implements AsyncClientConnectio
this.tlsConfigResolver = tlsConfigResolver;
}
void closeIfExpired(final PoolEntry<HttpRoute, ManagedAsyncClientConnection > entry) {
final long now = System.currentTimeMillis();
if (entry.getExpiryDeadline().isBefore(now)) {
entry.discardConnection(CloseMode.GRACEFUL);
} else {
final ConnectionConfig connectionConfig = resolveConnectionConfig(entry.getRoute());
final TimeValue timeToLive = connectionConfig.getTimeToLive();
if (timeToLive != null && Deadline.calculate(entry.getCreated(), timeToLive).isBefore(now)) {
entry.discardConnection(CloseMode.GRACEFUL);
}
}
}
/**
* @deprecated Use custom {@link #setConnectionConfigResolver(Resolver)}
*/