Skip to content

Commit 35be10e

Browse files
arturobernalgarturobernalg
committed
make it optional for now
1 parent 050b7d9 commit 35be10e

File tree

3 files changed

+67
-31
lines changed

3 files changed

+67
-31
lines changed

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import org.apache.hc.core5.io.CloseMode;
7373
import org.apache.hc.core5.pool.ConnPoolControl;
7474
import org.apache.hc.core5.pool.DefaultDisposalCallback;
75+
import org.apache.hc.core5.pool.DisposalCallback;
7576
import org.apache.hc.core5.pool.LaxConnPool;
7677
import org.apache.hc.core5.pool.ManagedConnPool;
7778
import org.apache.hc.core5.pool.PoolConcurrencyPolicy;
@@ -111,7 +112,8 @@ public class PoolingHttpClientConnectionManager
111112

112113
private static final Logger LOG = LoggerFactory.getLogger(PoolingHttpClientConnectionManager.class);
113114

114-
private final OffLockDisposalCallback<ManagedHttpClientConnection> disposer; // off-lock draining
115+
private final DisposalCallback<ManagedHttpClientConnection> defaultDisposal;
116+
private final OffLockDisposalCallback<ManagedHttpClientConnection> offLockDisposer;
115117

116118
public static final int DEFAULT_MAX_TOTAL_CONNECTIONS = 25;
117119
public static final int DEFAULT_MAX_CONNECTIONS_PER_ROUTE = 5;
@@ -218,11 +220,25 @@ public PoolingHttpClientConnectionManager(
218220
final PoolReusePolicy poolReusePolicy,
219221
final TimeValue timeToLive,
220222
final HttpConnectionFactory<ManagedHttpClientConnection> connFactory) {
223+
this(httpClientConnectionOperator,poolConcurrencyPolicy,poolReusePolicy,timeToLive,connFactory,false);
224+
225+
}
226+
227+
@Internal
228+
public PoolingHttpClientConnectionManager(
229+
final HttpClientConnectionOperator httpClientConnectionOperator,
230+
final PoolConcurrencyPolicy poolConcurrencyPolicy,
231+
final PoolReusePolicy poolReusePolicy,
232+
final TimeValue timeToLive,
233+
final HttpConnectionFactory<ManagedHttpClientConnection> connFactory,
234+
final boolean offLockDisposalEnabled) {
221235
super();
222236
this.connectionOperator = Args.notNull(httpClientConnectionOperator, "Connection operator");
223237

224-
// off-lock disposer wraps the default closer; we explicitly drain after pool ops
225-
this.disposer = new OffLockDisposalCallback<>(new DefaultDisposalCallback<>());
238+
this.defaultDisposal = new DefaultDisposalCallback<>();
239+
this.offLockDisposer = offLockDisposalEnabled ? new OffLockDisposalCallback<>(this.defaultDisposal) : null;
240+
final DisposalCallback<ManagedHttpClientConnection> callbackForPool = offLockDisposalEnabled ? this.offLockDisposer : this.defaultDisposal;
241+
226242

227243
switch (poolConcurrencyPolicy != null ? poolConcurrencyPolicy : PoolConcurrencyPolicy.STRICT) {
228244
case STRICT:
@@ -231,7 +247,7 @@ public PoolingHttpClientConnectionManager(
231247
DEFAULT_MAX_TOTAL_CONNECTIONS,
232248
timeToLive,
233249
poolReusePolicy,
234-
disposer,
250+
callbackForPool,
235251
null) {
236252

237253
@Override
@@ -246,7 +262,7 @@ public void closeExpired() {
246262
DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
247263
timeToLive,
248264
poolReusePolicy,
249-
disposer,
265+
callbackForPool,
250266
null) {
251267

252268
@Override
@@ -273,7 +289,8 @@ protected PoolingHttpClientConnectionManager(
273289
this.pool = Args.notNull(pool, "Connection pool");
274290
this.connFactory = connFactory != null ? connFactory : ManagedHttpClientConnectionFactory.INSTANCE;
275291
this.closed = new AtomicBoolean(false);
276-
this.disposer = null;
292+
this.defaultDisposal = null;
293+
this.offLockDisposer = null;
277294
}
278295

279296
@Override
@@ -396,8 +413,7 @@ public ConnectionEndpoint get(
396413
}
397414
}
398415

399-
// Drain off-lock disposals here, before fetching the connection to use.
400-
// This is a lease-level lock, not a global pool lock.
416+
// Single drain point under the lease lock.
401417
drainDisposals();
402418

403419
final ManagedHttpClientConnection conn = poolEntry.getConnection();
@@ -423,8 +439,6 @@ public ConnectionEndpoint get(
423439
}
424440
} finally {
425441
lock.unlock();
426-
// Safety net: drain anything enqueued by the code above
427-
drainDisposals();
428442
}
429443
}
430444

@@ -853,8 +867,8 @@ public boolean isClosed() {
853867
}
854868

855869
private void drainDisposals() {
856-
if (disposer != null) {
857-
disposer.drain();
870+
if (offLockDisposer != null) {
871+
offLockDisposer.drain();
858872
}
859873
}
860874
}

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManagerBuilder.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
3939
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
4040
import org.apache.hc.client5.http.ssl.TlsSocketStrategy;
41+
import org.apache.hc.core5.annotation.Experimental;
4142
import org.apache.hc.core5.annotation.Internal;
4243
import org.apache.hc.core5.function.Resolver;
4344
import org.apache.hc.core5.http.HttpHost;
@@ -92,6 +93,8 @@ public class PoolingHttpClientConnectionManagerBuilder {
9293
private int maxConnTotal;
9394
private int maxConnPerRoute;
9495

96+
private boolean offLockDisposalEnabled;
97+
9598
public static PoolingHttpClientConnectionManagerBuilder create() {
9699
return new PoolingHttpClientConnectionManagerBuilder();
97100
}
@@ -304,6 +307,16 @@ public final PoolingHttpClientConnectionManagerBuilder useSystemProperties() {
304307
return this;
305308
}
306309

310+
/**
311+
* Enable/disable off-lock disposal.
312+
* @since 5.6
313+
*/
314+
@Experimental
315+
public final PoolingHttpClientConnectionManagerBuilder setOffLockDisposalEnabled(final boolean enabled) {
316+
this.offLockDisposalEnabled = enabled;
317+
return this;
318+
}
319+
307320
@Internal
308321
protected HttpClientConnectionOperator createConnectionOperator(
309322
final SchemePortResolver schemePortResolver,
@@ -332,7 +345,8 @@ public PoolingHttpClientConnectionManager build() {
332345
poolConcurrencyPolicy,
333346
poolReusePolicy,
334347
null,
335-
connectionFactory);
348+
connectionFactory,
349+
offLockDisposalEnabled);
336350
poolingmgr.setSocketConfigResolver(socketConfigResolver);
337351
poolingmgr.setConnectionConfigResolver(connectionConfigResolver);
338352
poolingmgr.setTlsConfigResolver(tlsConfigResolver);

‎httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManagerOffLockDisposal.java‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,15 @@
4848
import org.apache.hc.core5.http.URIScheme;
4949
import org.apache.hc.core5.http.io.HttpConnectionFactory;
5050
import org.apache.hc.core5.io.CloseMode;
51+
import org.apache.hc.core5.pool.PoolConcurrencyPolicy;
52+
import org.apache.hc.core5.pool.PoolReusePolicy;
5153
import org.apache.hc.core5.util.TimeValue;
5254
import org.apache.hc.core5.util.Timeout;
5355
import org.junit.jupiter.api.Test;
5456

5557
class TestPoolingHttpClientConnectionManagerOffLockDisposal {
5658

59+
// Simulates slow close only for GRACEFUL
5760
static final class SleeperConnection implements ManagedHttpClientConnection {
5861
private volatile boolean open = true;
5962
private volatile Timeout soTimeout = Timeout.DISABLED;
@@ -189,27 +192,38 @@ private static ConnectionEndpoint lease(final PoolingHttpClientConnectionManager
189192
return mgr.lease(id, route, Timeout.ofSeconds((int) sec), state).get(Timeout.ofSeconds((int) sec));
190193
}
191194

192-
// measure only the lease latency; release happens outside this timing window
195+
// Measure only the lease latency; release happens outside this window
193196
private static long leaseAndMeasure(final PoolingHttpClientConnectionManager mgr,
194197
final String id, final HttpRoute route, final Object state,
195198
final long sec) throws Exception {
196199
final long start = System.nanoTime();
197200
final ConnectionEndpoint ep = lease(mgr, id, route, state, sec);
198201
final long elapsed = (System.nanoTime() - start) / 1_000_000L;
199-
// keep-alive so it returns to AVAILABLE; drain happens on a later call
200-
mgr.release(ep, state, TimeValue.ofSeconds(30));
202+
mgr.release(ep, state, TimeValue.ofSeconds(30)); // keep-alive, goes back to AVAILABLE
201203
return elapsed;
202204
}
203205

206+
private static PoolingHttpClientConnectionManager newMgrStrict(final long sleeperMs) {
207+
return PoolingHttpClientConnectionManagerBuilder.create()
208+
.setOffLockDisposalEnabled(true)
209+
.setConnPoolPolicy(PoolReusePolicy.LIFO)
210+
.setPoolConcurrencyPolicy(PoolConcurrencyPolicy.STRICT)
211+
.setConnectionFactory(sleeperFactory(sleeperMs))
212+
.build();
213+
}
214+
215+
private static PoolingHttpClientConnectionManager newMgrLax(final long sleeperMs) {
216+
return PoolingHttpClientConnectionManagerBuilder.create()
217+
.setOffLockDisposalEnabled(true)
218+
.setConnPoolPolicy(PoolReusePolicy.LIFO)
219+
.setPoolConcurrencyPolicy(PoolConcurrencyPolicy.LAX)
220+
.setConnectionFactory(sleeperFactory(sleeperMs))
221+
.build();
222+
}
223+
204224
@Test
205225
void strictEviction_offLock_otherThreadLeasesFast() throws Exception {
206-
final PoolingHttpClientConnectionManager mgr =
207-
new PoolingHttpClientConnectionManager(
208-
new NoopOperator(),
209-
org.apache.hc.core5.pool.PoolConcurrencyPolicy.STRICT,
210-
org.apache.hc.core5.pool.PoolReusePolicy.LIFO,
211-
TimeValue.NEG_ONE_MILLISECOND,
212-
sleeperFactory(1200));
226+
final PoolingHttpClientConnectionManager mgr = newMgrStrict(1200);
213227

214228
final HttpRoute rA = new HttpRoute(new HttpHost(URIScheme.HTTP.id, "a.example", 80));
215229
final HttpRoute rB = new HttpRoute(new HttpHost(URIScheme.HTTP.id, "b.example", 80));
@@ -220,7 +234,6 @@ void strictEviction_offLock_otherThreadLeasesFast() throws Exception {
220234
mgr.setMaxPerRoute(rB, 1);
221235
mgr.setMaxPerRoute(rC, 1);
222236

223-
// Seed AVAILABLE A and B
224237
final ConnectionEndpoint epA0 = lease(mgr, "seedA", rA, null, 2);
225238
mgr.release(epA0, null, TimeValue.ofSeconds(30));
226239
final ConnectionEndpoint epB0 = lease(mgr, "seedB", rB, null, 2);
@@ -256,13 +269,7 @@ void strictEviction_offLock_otherThreadLeasesFast() throws Exception {
256269

257270
@Test
258271
void leaseNotBlocked_LAX_stateMismatchDiscard_offLockDisposal() throws Exception {
259-
final PoolingHttpClientConnectionManager mgr =
260-
new PoolingHttpClientConnectionManager(
261-
new NoopOperator(),
262-
org.apache.hc.core5.pool.PoolConcurrencyPolicy.LAX,
263-
org.apache.hc.core5.pool.PoolReusePolicy.LIFO,
264-
TimeValue.NEG_ONE_MILLISECOND,
265-
sleeperFactory(1200));
272+
final PoolingHttpClientConnectionManager mgr = newMgrLax(1200);
266273

267274
final HttpRoute route = new HttpRoute(new HttpHost(URIScheme.HTTP.id, "lax.example", 80));
268275
mgr.setMaxTotal(2);
@@ -281,6 +288,7 @@ void leaseNotBlocked_LAX_stateMismatchDiscard_offLockDisposal() throws Exception
281288
return (System.nanoTime() - start) / 1_000_000L;
282289
};
283290

291+
// T2: concurrent lease "B" should be fast
284292
final Callable<Long> t2Lease = () -> {
285293
Thread.sleep(50);
286294
return leaseAndMeasure(mgr, "t2", route, "B", 2);

0 commit comments

Comments
 (0)