Skip to content

test(qwp): close TestWebSocketServer connections gracefully so scripted drops deliver their acks - #95

Open
jovfer wants to merge 1 commit into
mainfrom
sm_ws_test_server_graceful_close
Open

test(qwp): close TestWebSocketServer connections gracefully so scripted drops deliver their acks#95
jovfer wants to merge 1 commit into
mainfrom
sm_ws_test_server_graceful_close

Conversation

@jovfer

@jovfer jovfer commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Failure

BackgroundDrainerMidDrainCapabilityGapTest#testDeliveringBetweenTwoGapWindowsGrantsAFreshSettleBudget failed on the "Build, test & javadoc (JDK 8)" job of #91 (run https://github.com/questdb/java-questdb-client/actions/runs/34368418967):

delivering between the windows ends the episode, so neither window reaches the threshold
and the slot must still drain [attempts=20] expected:<SUCCESS> but was:<FAILED>

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:

  • Red run, the delivering session between the two gap windows: peer disconnect [104] with the pending range still [1,4]. No STATUS_DURABLE_ACK advanced the watermark, so noteAckProgress never reset the episode and the second window ran the settle counter on to 16.
  • Green runs, same session: 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.
  • Main's run shows the same [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

GapScenarioHandler acks a batch with two back-to-back sendBinary writes (OK frame, then STATUS_DURABLE_ACK) and drops the wire with client.close() at the next sequence number. TestWebSocketServer set no TCP_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 at socket.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 did readThread.join(5000) on the read thread itself when called from a handler callback. That self-join parked the synchronized handler 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() calls shutdownOutput() 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 before socket.close(), so no unread data remains to turn the close into an RST on any platform.
  • The read-thread join is skipped when 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:

Run Tests Failures Errors Skipped Wall time
main 981bdb0 (baseline) 3456 0 0 5 6 min 35 s
this branch 3456 0 0 5 3 min 32 s

BackgroundDrainerMidDrainCapabilityGapTest run 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

…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant