test(qwp): close TestWebSocketServer connections gracefully so scripted drops deliver their acks - #95
Open
jovfer wants to merge 1 commit into
Open
test(qwp): close TestWebSocketServer connections gracefully so scripted drops deliver their acks#95jovfer wants to merge 1 commit into
jovfer wants to merge 1 commit into
Conversation
…ed drops deliver their acks ClientHandler.close() closed the socket outright. With the peer's trailing frames still unread, that close is a TCP RST, and the Nagle-held second ack write (STATUS_DURABLE_ACK) of the last acked batch was purged with it. The drainer then saw a reset with no ack progress, and BackgroundDrainerMidDrainCapabilityGapTest#testDeliveringBetweenTwoGapWindowsGrantsAFreshSettleBudget failed on the JDK 8 CI job (java-questdb-client #91, run 34368418967). Now: TCP_NODELAY on accepted sockets, shutdownOutput() before close() so the written frames are followed by FIN, a bounded drain of unread input when the close runs on the read thread (the scripted-drop path), and no self-join of the read thread -- that join(5000) inside the synchronized handler callback serialized every scripted drop at 5 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017S9gyXv4MwoDHEohA9At5d
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Failure
BackgroundDrainerMidDrainCapabilityGapTest#testDeliveringBetweenTwoGapWindowsGrantsAFreshSettleBudgetfailed on the "Build, test & javadoc (JDK 8)" job of #91 (run https://github.com/questdb/java-questdb-client/actions/runs/34368418967):The two pushes of #91 earlier that day and main's own JDK 8 run (981bdb0, 2026-09-04) were green; the only delta between the green and red heads was two unrelated test files. The test and the code it exercises are main code, not part of #91.
Evidence
Comparing the drainer logs of the red run, the green run and main's run for this test class:
peer disconnect [104]with the pending range still[1,4]. NoSTATUS_DURABLE_ACKadvanced the watermark, sonoteAckProgressnever reset the episode and the second window ran the settle counter on to 16.peer disconnect [11](FIN) with the range advanced to[3,4], then the counter restarted at 1 for the second window and the slot drained.[104]/ no-advance pairing on the first connection of the same test, where it happens not to matter. The pairing "errno 104, watermark unchanged" appears in every run; the test fails only when it lands on the delivering session.Mechanism
GapScenarioHandleracks a batch with two back-to-backsendBinarywrites (OK frame, thenSTATUS_DURABLE_ACK) and drops the wire withclient.close()at the next sequence number.TestWebSocketServerset noTCP_NODELAY, so the second small write was held by Nagle; the client does set it, so its remaining frames arrive as separate segments. When one of them is still unread atsocket.close(), the kernel sends RST instead of FIN and purges the held durable-ack write. Which connection hits that depends on scheduling, hence the flake.The old
ClientHandler.close()also didreadThread.join(5000)on the read thread itself when called from a handler callback. That self-join parked thesynchronizedhandler for the full 5 s on every scripted drop, which is where the 5 s cadence between sessions in the CI logs came from, and it kept the client's trailing frames unread in the kernel buffer for the whole window.Fix
In
TestWebSocketServer:setTcpNoDelay(true)on every accepted socket, so both ack writes leave immediately.ClientHandler.close()callsshutdownOutput()first, so already-written frames are followed by FIN rather than an RST; when the close runs on the read thread (the scripted-drop path) it drains unread input with a 20 ms socket timeout and a 200 ms cap beforesocket.close(), so no unread data remains to turn the close into an RST on any platform.close()runs on that thread.Cross-thread closes (server teardown) keep today's fast path: FIN, then
close(), then the join.Verification
Local, arm64 JDK 25,
mvn -pl core surefire:test:BackgroundDrainerMidDrainCapabilityGapTestrun five times in a row: 6/6 green each time, 0.99 s to 1.16 s per run, down from 40.15 s on the baseline. The suite speed-up is the removed 5 s self-join stall per scripted drop across the drainer tests.The flake itself is a Linux kernel RST-vs-FIN race that does not reproduce on macOS; the CI JDK 8 job is the check that exercises it.
🤖 Generated with Claude Code
https://claude.ai/code/session_017S9gyXv4MwoDHEohA9At5d