h1client: a dial in progress cannot be cancelled, so a SYN retransmission at the end of a run stretches Run's elapsed time and deflates requests_per_sec #95
Description
Found in: review round 4 of #90 (protocol lens). This predates #90: main's reconnect dialed inline too, and also held hc.mu across the dial, which #90 removes. #90 makes it matter more, because in -close mode every request now dials.
What happens
The H1 dial takes no context, so nothing can interrupt it once it has started:
dialH1(h1client.go:212, at fix(h1client): never reuse a connection that is closing, so -close stops counting one error per request (#87) #90's head 99a0089) callsdialTCPRetry(addr, dialTimeout)ordialTLSRetry(addr, dialTimeout, cfg).- Those (
dial.go:110-151) usenet.DialTimeoutandtls.DialWithDialer(&net.Dialer{Timeout: timeout}, ...), and sleep between RST retries withtime.Sleep. redial(h1client.go:590) checksctx.Err()only before the dial.(*h1Client).Closecannot reach a connection that is still being dialed; the dial's result is closed afterwards, but only once the dial returns.
At the end of the measured window, Run cancels runCtx (bench.go:767), closes the client (:776), waits for the workers (:778), and only then takes elapsed := time.Since(start) (:781). RequestsPerSec is reqs / elapsed (:1092).
So a worker that started a dial just before the end holds b.wg.Wait() until the dial returns, for up to DialTimeout, which defaults to 10 s (bench.go:449). (A timeout is not retried; only a transient RST is, with short sleeps.)
When it bites
A dial blocks for seconds when the SUT's accept queue is full: Linux drops the SYN (tcp_conn_request, sk_acceptq_is_full), and the client resends it 1 s, 3 s and 7 s after the first SYN. That is exactly the state churn-close (probatorium, -close, PoolSize=1) drives a server into.
- A 30 s window followed by a 3 s stall reports saturation RPS about 10% low (
reqs / 33 sinstead ofreqs / 30 s). - In rated mode, the step's duration stretches the same way.
- The stall is not a request: the dial's request is dropped (
ctx.Err()) and is neither a success nor an error, so nothing in the Result shows it.
Keep-alive runs dial only after a failure, so they are exposed much less.
The H2 paths share dial.go (h2client.go:476, :487; h2c_upgrade.go:146).
Possible fixes
Either or both:
- Make the dial cancellable:
dialTCPRetryanddialTLSRetrytake a context and use(&net.Dialer{Timeout: timeout}).DialContext/tls.Dialer.DialContext, with a context-aware sleep between retries.redialpasses the request's context, which isrunCtx, cancelled atbench.go:767.newH1Client's up-front dials can keepcontext.Background(). - Take
elapsedwhen the window ends (whenrunningflips false), not afterwg.Wait(), so a straggler cannot stretch the denominator.
A test: a listener with a full backlog (or a dialer hook that blocks until its context is done), a run of 300 ms, and an assertion that Run returns within a small bound and that Duration is close to the configured window.
Activity
FumingPower3925 commentedon Sep 27, 2026
The HTTP/2 client has the same exposure, plus a lock that spreads it. This came from the round-3 review of #91 (protocol lens), and it predates #91.
reconnectSlotholds the slot's lock (slot.mu) across the dial and the whole handshake. That was so onmain(h2client.go:389-413ata89f02d, which also slept the backoff under it) and still is at fix(h2client): real status and body bytes, no lifetime deadline, redial dead connections (#85, #86, #88, #89) #91's head. fix(h2client): real status and body bytes, no lifetime deadline, redial dead connections (#85, #86, #88, #89) #91 moved the backoff sleep out of the lock, but not the dial.dialH2anddialH2CUpgradecalldialTCPRetry/dialTLSRetry, as above.completeH2Handshake,conn.SetReadDeadline(now + 10 s)).101(h2c_upgrade.go:164).slot.muwhile this runs.ctxcannot reach them:Lockdoes not watch a context.(*h2Client).Close, which takes each slot'sslot.mubefore it closes that slot's connection.Runcancels its context (bench.go:750ata89f02d; fix(h2client): real status and body bytes, no lifetime deadline, redial dead connections (#85, #86, #88, #89) #91 does not touchbench.go), callsClose(:759), which waits for those locks, and thenwg.Wait()s for the workers (:761); the rated warmup cancels and waits the same way (:852-853).Close, and with them the end of the run, for up to the dial timeout plus 10 s, and plus 10 s more for h2c.The same fix covers it: a context-aware dial, as proposed above. The handshake read must also end when the context does, for example with
context.AfterFunc(ctx, func() { _ = conn.SetDeadline(time.Now()) })aroundcompleteH2Handshakeand the upgrade's read. With that in place the lock is released as soon as the run's context ends, andClose, which runs after the cancel, no longer waits behind a dial.