milamberspace commented on PR #6742:
URL: https://github.com/apache/jmeter/pull/6742#issuecomment-5251907174

   Following up on the stale-connection bug above with a concrete repro I ran 
manually against a real server (own domain, browsing-style scenario: 5 threads, 
3 loops, ~800–2800ms think time between transactions on the same reused HTTP/2 
connection).
   
   Worth a regression test to prove `validateAfterInactivity` actually catches 
this rather than just proving it's configured. One nuance that matters for how 
the test is shaped: **it's not the request count that triggers this, it's the 
idle gap between two requests on the same pooled connection.** A tight 
rapid-fire loop with no pauses almost certainly won't reproduce it — my manual 
repro needed nothing more exotic than ~1–3s of think time between transactions, 
which was enough for the server side to close the idle connection before the 
next request picked it back up from the pool.
   
   Suggested shape for the test:
   
   1. Spin up an embedded HTTP/2-over-TLS server the test controls directly 
(not WireMock's black-box backend — this needs to forcibly close an 
*already-accepted* connection without stopping the whole server, which WireMock 
doesn't expose).
   2. Fire request #1 through `HTTPHC5Impl` with `HTTP/2` selected, get a 200.
   3. Forcibly close the underlying TCP connection from the server side right 
after responding (server keeps listening for new connections on the same port — 
just this one connection dies), simulating an idle server-side timeout.
   4. Sleep a little longer than whatever `validateAfterInactivity` ends up 
configured to (e.g. 1.1s if it's set to 1s).
   5. Fire request #2 from the **same `HTTPSamplerBase` instance / same 
thread**, so it goes through the cached `HTTP_2_CLIENTS` entry and reuses the 
pool.
   6. Assert request #2 succeeds (200, no `ConnectionClosedException`) — and 
ideally assert it *would* fail without the fix, e.g. by running the same test 
body against a build with `validateAfterInactivity` stripped out, or 
documenting in the test comment that it reproduces `apache/jmeter#<this-PR>`'s 
manual repro.
   
   Happy to be wrong about the exact mechanics of forcing step 3 cleanly with 
whatever test HTTP/2 server this project already has infrastructure for 
(`TestHTTPHC5Features` already stands up embedded h2/h2c servers per the PR 
description) — but the idle-gap-not-volume framing is the part I'd want the 
test to actually exercise, since that's what makes it a faithful regression 
test rather than one that happens to pass for unrelated reasons.
   


-- 
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]

Reply via email to