andreaslind01 commented on PR #6742: URL: https://github.com/apache/jmeter/pull/6742#issuecomment-5252916532
Thanks @milamberspace - fixed and covered by a regression test. **Fix:** `setValidateAfterInactivity` is now applied unconditionally (not under the `connectTimeout > 0` guard) to the `ConnectionConfig` of both `createHttp2Client()` and `createClient()`, exactly as suggested. The interval is configurable via a new `httpclient5.validate_after_inactivity` property (ms, default `2000`, `-1` disables), documented in `jmeter.properties`, `properties_reference.xml` and `changes.xml`. **One correction on the mechanics,** which changed how the test had to be shaped. Tracing httpclient5 5.6.4 / httpcore5 5.4.3: a *cleanly* closed connection isn't what fails. `LaxConnPool.getAvailableEntry` does hand out the dead entry, but `AsyncConnectExec` then checks `isEndpointConnected()` and transparently reconnects - and `AbstractH2StreamMultiplexer.isOpen()` is `connState == ACTIVE`, so once the reactor has processed the FIN (or a `GOAWAY`) the endpoint reports "not connected" and recovers on its own. Step 3 as suggested would therefore have passed with *and* without the fix. The failure needs the client to still *believe* the connection is usable when the request is submitted - that's the `H2Streams.shutdownAndReleaseAll` path in the trace. **What the test does instead,** keeping the idle-gap framing: A raw TCP relay sits in front of WireMock's h2/TLS port, so the test owns the client-facing socket. It can mark a connection doomed: it stays open, but is dropped as soon as the client writes to it again - what an idle server/LB timeout looks like to a client that hasn't noticed yet. 1. Request 1 → 200, connection pooled. 2. Sleep `validateAfterInactivity + 500ms` - the idle gap, not the request count, arms the check. 3. Doom the pooled connection. 4. Request 2 from the same sampler/thread, reusing the cached `HTTP_2_CLIENTS` entry and pool. 5. Assert 200 **and** that the relay accepted exactly 2 connections - proving the stale one was replaced, not that it passed for unrelated reasons. On asserting it fails without the fix: verified directly by forcing the default to `-1`, which reproduces the reported error verbatim (`Could not execute HTTP/2 request` / `Non HTTP response code: java.io.IOException`). Since that can't be asserted in one build, a companion test `doesNotRevalidatePooledHttp2ConnectionWithoutAnIdleGap` runs the same body with no idle gap, where the check is legitimately skipped and the sample fails - pinning the mechanism from the other side. `./gradlew classes style` clean, full `:src:protocol:http:test` (983 tests) green, HC5 tests run 3× without flakiness. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
