Evidently my JEP380 SocketChannel->Socket adapter is broken on Windows. If I revert 97703d901 the tests run fine on my Windows machine across all four Java versions. If you're seeing failures on master unrelated to 97703d901, something else is up. (Firewall settings?)
On Thu, Jul 3, 2025 at 3:07 AM Oleg Kalnichevski <ol...@apache.org> wrote: > > On 2025-07-02 20:55, Ryan Schmitt wrote: > > > "Stale" means that the remote peer closed the connection but we don't > > know it until we try to reuse the connection, right? > > Correct. However, this can happen to blocking connections only because > due to the classic i/o inherent design limitations they cannot react to > i/o events when not blocked in an i/o operation (for instance when they > are sitting idly in the connection pool). Non-blocking connections > _always_ react to i/o events even when sitting in the connection pool. > The opposite endpoints can even push messages on those connections if > the chosen protocol supports request promises. > > > Because this is > > definitely a problem with the async client. Users are seeing > > `ConnectionClosedException` immediately thrown from the `execute` > > method, and they're reaching for `setValidateAfterInactivity` to fix > > it, which is unexpectedly closing perfectly good connections. > > Non-blocking connections can still become invalid if the opposite > endpoint dies silently without sending a RST packet to the opposite > endpoint. > > I agree this setting should not apply to non-blocking HTTP/1.1 connections. > > > I don't have any further work planned for Unix domain socket support, > > so my next task is to audit this. > > I just discovered that integration tests in master are helplessly broken > on Windows. What is worse some of those tests pass with the 5.5.x > branch. I am still trying to understand what is going on. I will make a > separate post on the matter but it appears there is still plenty of work > to be done to stabilize the master. > > Oleg > > > I'll come back with more information > > on the stale connection handling of the sync and async clients on all > > the LTS versions of Java. (One reason I also want to cover the sync > > client is that JEP 353 might have subtly changed some behaviors on > > Java 13+.) > > > > On Tue, Jul 1, 2025 at 2:48 PM Oleg Kalnichevski<ol...@apache.org> wrote: > >> On Tue, 2025-07-01 at 11:26 -0700, Ryan Schmitt wrote: > >>> What is the intended behavior of `setValidateAfterInactivity` on the > >>> async client with HTTP/1.1 connections? The current implementation > >>> is: > >>> > >>> if (poolEntry.hasConnection()) { > >>> final ManagedAsyncClientConnection connection = > >>> poolEntry.getConnection(); > >>> final TimeValue timeValue = > >>> connectionConfig.getValidateAfterInactivity(); > >>> if (connection.isOpen() && TimeValue.isNonNegative(timeValue)) { > >>> if (timeValue.getDuration() == 0 > >>> || Deadline.calculate(poolEntry.getUpdated(), > >>> timeValue).isExpired()) { > >>> final ProtocolVersion protocolVersion = > >>> connection.getProtocolVersion(); > >>> if (protocolVersion != null && > >>> protocolVersion.greaterEquals(HttpVersion.HTTP_2_0)) { > >>> connection.submitCommand(new PingCommand(/* ... */, > >>> Command.Priority.IMMEDIATE); > >>> return; > >>> } > >>> if (LOG.isDebugEnabled()) { > >>> LOG.debug("{} connection {} is closed", id, > >>> ConnPoolSupport.getId(connection)); > >>> } > >>> poolEntry.discardConnection(CloseMode.IMMEDIATE); > >>> } > >>> } > >>> } > >>> > >>> In other words, the current implementation is actually just a > >>> connection TTL. I got a report from someone who was seeing zero > >>> connection reuse with their client after upgrading to 5.4, and it > >>> turns out that it's because they were calling > >>> `setValidateAfterInactivity(0)`. The intention was to _always_ check > >>> for a stale connection before reuse, but it actually resulted in all > >>> pooled connections being immediately closed, since 5.4 fixes the > >>> handling of validateAfterInactivity/connectionTimeToLive being set to > >>> `0`. This seems like a bug, but does Java provide us any way to fix > >>> it? > >> I do not think so. I do not know of a way to reliably perform a > >> validity check on non-blocking connections other than sending a ping > >> message. This obviously can only work with HTTP/2 only, so probably > >> `validateAfterInactivity` should not apply to HTTP/1.1 connections at > >> all. Generally non-blocking connections should never get stale like > >> their blocking counterparts. > >> > >> Oleg > >> > >>> I thought the usual trick was to perform a blocking read with a > >>> 1ms timeout, which will surface the fact that the remote peer has > >>> closed the connection. > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail:dev-unsubscr...@hc.apache.org > >> For additional commands, e-mail:dev-h...@hc.apache.org > >> > > --------------------------------------------------------------------- > > To unsubscribe, e-mail:dev-unsubscr...@hc.apache.org > > For additional commands, e-mail:dev-h...@hc.apache.org > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org > For additional commands, e-mail: dev-h...@hc.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org