You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "ok2c (via GitHub)" <gi...@apache.org> on 2026/09/30 17:01:59 UTC
[PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
ok2c opened a new pull request, #715:
URL: https://github.com/apache/httpcomponents-core/pull/715
@arturobernalg you have been working a lot with the connection pools. Could you please double-check my changes to make sure we are on the same page as far as idle connection handling is concerned?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715#discussion_r4183891661
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java:
##########
@@ -155,6 +170,15 @@ protected void validateSession(
}
}
+ @Override
+ protected boolean isIdle(final IOSession session) {
+ final IOEventHandler handler = session.getHandler();
+ if (handler instanceof HttpConnection) {
+ return ((HttpConnection) handler).isIdle();
Review Comment:
@arturobernalg Good catch. Corrected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715#issuecomment-5927016357
@arturobernalg I suspected something was off. I misunderstood how `System.nanoTime()` worked. Now I think we have a problem. Why did we need to convert the i/o reactor timestamps to using `System.nanoTime()`? What was the point? I will start a discussion on the dev list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715#discussion_r4183928875
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java:
##########
@@ -65,16 +67,27 @@ public final class H2ConnPool extends AbstractIOSessionPool<HttpHost> {
private volatile TimeValue validateAfterInactivity = TimeValue.NEG_ONE_MILLISECOND;
+ /**
+ * @since 5.5
+ */
public H2ConnPool(
+ final Clock clock,
final ConnectionInitiator connectionInitiator,
final Resolver<HttpHost, InetSocketAddress> addressResolver,
final TlsStrategy tlsStrategy) {
- super();
+ super(clock);
this.connectionInitiator = Args.notNull(connectionInitiator, "Connection initiator");
- this.addressResolver = addressResolver != null ? addressResolver : DefaultAddressResolver.INSTANCE;
+ this.addressResolver = addressResolver;
Review Comment:
@arturobernalg It was. H2ConnPool violates our package layering policy by importing an impl class into a non-impl one. However you likely have more violations like that. They all should be addressed consistency across teh entire code base.
I reverted my changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715#discussion_r4183425457
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java:
##########
@@ -155,6 +170,15 @@ protected void validateSession(
}
}
+ @Override
+ protected boolean isIdle(final IOSession session) {
+ final IOEventHandler handler = session.getHandler();
+ if (handler instanceof HttpConnection) {
+ return ((HttpConnection) handler).isIdle();
Review Comment:
Could this condition be inverted? Shouldn’t `isIdle`() return true when `streamCount() == 0` rather than > 0?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715#discussion_r4183488500
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java:
##########
@@ -65,16 +67,27 @@ public final class H2ConnPool extends AbstractIOSessionPool<HttpHost> {
private volatile TimeValue validateAfterInactivity = TimeValue.NEG_ONE_MILLISECOND;
+ /**
+ * @since 5.5
+ */
public H2ConnPool(
+ final Clock clock,
final ConnectionInitiator connectionInitiator,
final Resolver<HttpHost, InetSocketAddress> addressResolver,
final TlsStrategy tlsStrategy) {
- super();
+ super(clock);
this.connectionInitiator = Args.notNull(connectionInitiator, "Connection initiator");
- this.addressResolver = addressResolver != null ? addressResolver : DefaultAddressResolver.INSTANCE;
+ this.addressResolver = addressResolver;
Review Comment:
Was the change in the `addressResolver` fallback intentional? With a null resolver this no longer seems equivalent to `DefaultAddressResolver`, in particular for default ports and an explicitly provided `HttpHost` address.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715#issuecomment-5992500622
@arturobernalg Please do another pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715#discussion_r4184960097
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java:
##########
@@ -65,16 +67,27 @@ public final class H2ConnPool extends AbstractIOSessionPool<HttpHost> {
private volatile TimeValue validateAfterInactivity = TimeValue.NEG_ONE_MILLISECOND;
+ /**
+ * @since 5.5
+ */
public H2ConnPool(
+ final Clock clock,
final ConnectionInitiator connectionInitiator,
final Resolver<HttpHost, InetSocketAddress> addressResolver,
final TlsStrategy tlsStrategy) {
- super();
+ super(clock);
this.connectionInitiator = Args.notNull(connectionInitiator, "Connection initiator");
- this.addressResolver = addressResolver != null ? addressResolver : DefaultAddressResolver.INSTANCE;
+ this.addressResolver = addressResolver;
Review Comment:
@ok2c your right here about the package layering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Improved handling of idle sessions by AbstractIOSessionPool / H2ConnPool [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c merged PR #715:
URL: https://github.com/apache/httpcomponents-core/pull/715
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org