Skip to content

Commit 7ea0748

Browse files
committed
test(qwp): make Invariant-B tests detect a silent retry-loop give-up
The "retries forever" tests asserted only no-terminal-in-window, but a regressed I/O thread that gives up WITHOUT latching a terminal also delivers nothing: flush() publishes to the cursor engine on the user thread regardless of wire liveness, so both tests passed green with a dead retry loop. Back the no-terminal assertions with two discriminators: - LIVENESS: snapshot getTotalReconnectAttempts() only after the (ignored) reconnect budget has elapsed and require it to advance -- connectLoop bumps the counter on every attempt in both the async-initial-connect and mid-stream reconnect phases, so a frozen counter means the loop exited silently. A plain attempts >= 1 check would not discriminate: pre-budget attempts would satisfy it. Applied to testAsyncNoServerRetriesForeverNoTerminal, testConnectionLostRetriesForeverNoTerminal, and testReconnectNeverGivesUpInvariantB. - RECOVERY: in testReconnectNeverGivesUpInvariantB, revive a server on the SAME port (the requestedPort constructor exists for down-then-up outage realism) and require a replayed frame to arrive -- end-to-end proof that "retry forever" converts into delivery once the server returns. The revival handler is receipt-only: faking ACKs would couple the test to the exact replay FSN sequence and a wrong-seq ACK risks the invalid-ACK terminal. Also fix the test comment claiming flush publishes to on-disk SF: no sf_dir is configured there, so rows buffer in the in-RAM cursor ring.
1 parent 53bab20 commit 7ea0748

2 files changed

Lines changed: 100 additions & 4 deletions

File tree

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/InitialConnectAsyncTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,14 @@ public void testAsyncNoServerRetriesForeverNoTerminal() throws Exception {
139139
Assert.assertFalse(
140140
"wasEverConnected() stays false while no server is reachable",
141141
((QwpWebSocketSender) sender).wasEverConnected());
142+
// LIVENESS: no-terminal-in-window alone cannot distinguish
143+
// "retries forever" from "gave up silently" -- an I/O thread
144+
// that exits without latching a terminal also delivers nothing,
145+
// and the flush above lands in SF regardless of wire liveness.
146+
// The retry loop bumps getTotalReconnectAttempts() on every
147+
// iteration, so the counter advancing NOW -- long past the
148+
// (ignored) 200ms budget -- proves the loop is still running.
149+
awaitReconnectAttemptsAdvance((QwpWebSocketSender) sender);
142150
} finally {
143151
sender.close();
144152
}
@@ -269,6 +277,11 @@ public void testConnectionLostRetriesForeverNoTerminal() throws Exception {
269277
Assert.assertTrue(
270278
"wasEverConnected() must remain true after the outage",
271279
((QwpWebSocketSender) sender).wasEverConnected());
280+
// LIVENESS: same discriminator as the async-init test above.
281+
// The mid-stream reconnect loop must still be making connect
282+
// attempts long past the (ignored) 200ms budget -- not merely
283+
// failing to report that it stopped.
284+
awaitReconnectAttemptsAdvance((QwpWebSocketSender) sender);
272285
} finally {
273286
// closeQuietly (not a bare close()) so a close-path exception
274287
// cannot replace a pending AssertionError from the contract
@@ -321,6 +334,31 @@ private static void awaitAtLeastOneConnectAttempt(QwpWebSocketSender wss) {
321334
}
322335
}
323336

337+
/**
338+
* Proves the I/O thread's retry loop is still ALIVE rather than merely
339+
* quiet: snapshots {@link QwpWebSocketSender#getTotalReconnectAttempts()}
340+
* and spins until it advances past the snapshot. Callers invoke this
341+
* after the (ignored) reconnect budget has already elapsed, so a stuck
342+
* counter means the loop gave up silently -- exactly the Invariant B
343+
* regression a no-terminal-in-window assertion cannot see. A plain
344+
* {@code attempts >= 1} check would NOT discriminate: attempts made
345+
* before the budget expired would satisfy it even if the loop then
346+
* exited.
347+
*/
348+
private static void awaitReconnectAttemptsAdvance(QwpWebSocketSender wss) {
349+
long snapshot = wss.getTotalReconnectAttempts();
350+
long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
351+
while (wss.getTotalReconnectAttempts() <= snapshot) {
352+
if (System.nanoTime() > deadlineNanos) {
353+
throw new AssertionError(
354+
"Invariant B violation: reconnect attempts stuck at "
355+
+ snapshot + " for 5s past the budget -- the I/O "
356+
+ "thread stopped retrying without surfacing a terminal");
357+
}
358+
io.questdb.client.std.Compat.onSpinWait();
359+
}
360+
}
361+
324362
/**
325363
* Closes the sender, tolerating close-path teardown noise only. Used
326364
* instead of a broad {@code catch (Exception ignored)} around a whole

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/ReconnectTest.java

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,16 @@ public void testReconnectAfterServerInducedDisconnect() throws Exception {
104104
public void testReconnectNeverGivesUpInvariantB() throws Exception {
105105
// INVARIANT B: server is up at first (initial connect + ACK), then torn
106106
// down. The I/O loop enters reconnect and must retry FOREVER -- flush()
107-
// must keep succeeding (publishing to on-disk SF), never surface a
108-
// give-up / budget terminal. The rows are safe in SF and the server may
109-
// return, so reconnect_max_duration_millis is ignored as a give-up
110-
// deadline.
107+
// must keep succeeding (publishing to the in-RAM cursor ring; no sf_dir
108+
// is configured here), never surface a give-up / budget terminal. The
109+
// rows are buffered and the server may return, so
110+
// reconnect_max_duration_millis is ignored as a give-up deadline.
111+
// Because flush() succeeds on the user thread regardless of I/O-thread
112+
// liveness, the no-throw loop below is backed by two discriminators:
113+
// the reconnect-attempt counter must keep advancing past the budget
114+
// (loop is alive, not silently exited), and a server revived on the
115+
// SAME port must receive the replayed frames (retrying converts into
116+
// delivery).
111117
try (TestWebSocketServer server = new TestWebSocketServer(new AckHandler())) {
112118
int port = server.getPort();
113119
server.start();
@@ -144,6 +150,42 @@ public void testReconnectNeverGivesUpInvariantB() throws Exception {
144150
}
145151
Thread.sleep(50);
146152
}
153+
154+
if (observed == null) {
155+
// LIVENESS: snapshot the attempt counter well past the
156+
// (ignored) 300ms budget and require it to advance. A
157+
// silently-exited I/O thread leaves it frozen -- the one
158+
// regression the flush loop above cannot see.
159+
io.questdb.client.cutlass.qwp.client.QwpWebSocketSender wss =
160+
(io.questdb.client.cutlass.qwp.client.QwpWebSocketSender) sender;
161+
long snapshot = wss.getTotalReconnectAttempts();
162+
long spinDeadline = System.currentTimeMillis() + 5_000;
163+
while (wss.getTotalReconnectAttempts() <= snapshot
164+
&& System.currentTimeMillis() < spinDeadline) {
165+
Thread.sleep(20);
166+
}
167+
Assert.assertTrue(
168+
"Invariant B violation: reconnect attempts stuck at "
169+
+ snapshot + " for 5s past the (ignored) 300ms "
170+
+ "budget -- the I/O loop stopped retrying "
171+
+ "without surfacing a terminal",
172+
wss.getTotalReconnectAttempts() > snapshot);
173+
174+
// RECOVERY: bring a server back on the SAME port (the
175+
// requestedPort constructor exists for down-then-up
176+
// outage realism). The still-retrying loop must reconnect
177+
// and replay the frames that accumulated unacked during
178+
// the outage -- a binary frame arriving on the revived
179+
// endpoint is end-to-end proof that "retry forever"
180+
// converts into delivery once the server returns.
181+
ReceiveOnlyHandler revived = new ReceiveOnlyHandler();
182+
try (TestWebSocketServer server2 =
183+
new TestWebSocketServer(revived, false, null, port)) {
184+
server2.start();
185+
Assert.assertTrue(server2.awaitStart(5, TimeUnit.SECONDS));
186+
waitFor(() -> revived.totalBinaryReceived.get() >= 1, 5_000);
187+
}
188+
}
147189
} finally {
148190
try {
149191
sender.close();
@@ -529,6 +571,22 @@ public void close() {
529571
}
530572
}
531573

574+
/**
575+
* Counts binary frames without ACKing. Receipt-only revival probe for
576+
* {@link #testReconnectNeverGivesUpInvariantB}: observing a replayed
577+
* frame proves reconnect + replay without coupling the test to the
578+
* exact FSN sequence a faked ACK would have to carry (a wrong-seq ACK
579+
* risks tripping the invalid-ACK terminal and polluting the result).
580+
*/
581+
private static class ReceiveOnlyHandler implements TestWebSocketServer.WebSocketServerHandler {
582+
final AtomicLong totalBinaryReceived = new AtomicLong();
583+
584+
@Override
585+
public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) {
586+
totalBinaryReceived.incrementAndGet();
587+
}
588+
}
589+
532590
/** Acks every binary frame so the sender doesn't hang. */
533591
private static class AckHandler implements TestWebSocketServer.WebSocketServerHandler {
534592
private final AtomicLong nextSeq = new AtomicLong(0);

0 commit comments

Comments
 (0)