Skip to content

Commit 8facb8d

Browse files
glasstigerclaude
andcommitted
Test the catch-up NACK guard at the ack boundary
The catch-up-NACK routing guard (fsn <= ackedFsn) had its equality case untested: the common single-frame catch-up maps its only catch-up frame to fsn == ackedFsn exactly, yet the existing test only exercised the strictly-below case (fsn = -2 < ackedFsn = -1). A "<=" -> "<" mutation therefore survived the suite while routing the single (or last) catch-up frame's NACK onto the poison-strike path and laundering the detector. Add testPostSendCatchUpNackAtAckedFsnBoundaryDoesNotStrike, which drives fsn == ackedFsn == -1 and asserts poisonStrikes stays 0. poisonStrikes, not poisonFsn, is the discriminator here: a wrongly-charged strike sets poisonFsn to the rejected fsn -1, which is the "no suspect" sentinel, so a poisonFsn check is blind at this boundary. The mutation charges the strike (0 -> 1) and the test fails; the production guard is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 084f93f commit 8facb8d

1 file changed

Lines changed: 42 additions & 0 deletions

File tree

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoopPoisonFrameTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,48 @@ public void testPostSendCatchUpNackDoesNotStrikeOrLaunderPoisonState() throws Ex
696696
});
697697
}
698698

699+
@Test
700+
public void testPostSendCatchUpNackAtAckedFsnBoundaryDoesNotStrike() throws Exception {
701+
// Boundary companion to testPostSendCatchUpNackDoesNotStrikeOrLaunderPoisonState:
702+
// the COMMON single-frame catch-up maps its only catch-up frame to
703+
// fsn == ackedFsn EXACTLY (the last catch-up frame lands on replayStart-1 ==
704+
// ackedFsn). The pre-send routing guard is "fsn <= ackedFsn", so this equality
705+
// case must also skip the poison strike. Without pinning it a "<=" -> "<"
706+
// mutation would slip the single/last catch-up frame's NACK onto the post-send
707+
// poison-strike path and launder the detector -- exactly the hazard the guard
708+
// closes -- yet the strictly-below test above would still pass.
709+
TestUtils.assertMemoryLeak(() -> {
710+
List<WebSocketClient> clients = new ArrayList<>();
711+
try (CursorSendEngine engine = newEngine()) {
712+
appendFrames(engine, 2);
713+
CursorWebSocketSendLoop loop = newDurableLoop(engine, clients);
714+
// 1 catch-up frame (wire seq 0) + 1 data frame (wire seq 1), nothing
715+
// acked (ackedFsn = -1), so fsnAtZero = replayStart(0) - catchUpFrames(1)
716+
// = -1. A NACK of catch-up wire seq 0 maps to fsn -1 == ackedFsn exactly.
717+
setPostCatchUpDataFrameBaseline(loop, -1L, 2L);
718+
assertEquals("precondition: no strikes yet",
719+
0L, getLongField(loop, "poisonStrikes"));
720+
721+
deliverRetriableNack(loop, 0, "disk full");
722+
723+
// poisonStrikes -- NOT poisonFsn -- is the discriminator at this exact
724+
// boundary: a wrongly-charged strike calls recordHeadRejectionStrike(-1),
725+
// which sets poisonFsn to the rejected fsn -1 -- the "no suspect"
726+
// sentinel -- so a poisonFsn check cannot see the laundering here. It
727+
// still bumps poisonStrikes 0 -> 1, so the correct pre-send routing (no
728+
// strike) shows as poisonStrikes staying 0. A "<=" -> "<" mutation of the
729+
// fsn-vs-ackedFsn guard charges the strike and fails this.
730+
assertEquals("catch-up NACK at fsn == ackedFsn must not charge a poison strike",
731+
0L, getLongField(loop, "poisonStrikes"));
732+
assertEquals("... and must leave the poison sentinel intact",
733+
-1L, getLongField(loop, "poisonFsn"));
734+
loop.checkError();
735+
} finally {
736+
closeAll(clients);
737+
}
738+
});
739+
}
740+
699741
@Test
700742
public void testPostSendNotWritableNackNeverEscalatesToPoisonTerminal() throws Exception {
701743
// RETRIABLE_OTHER (NOT_WRITABLE) is a node-state verdict, not a frame

0 commit comments

Comments
 (0)