Skip to content

Commit f9bb589

Browse files
glasstigerclaude
andcommitted
Cover three untested-critical rows; explain the two that resist
C22. The full-dict-fallback discard is gated on BOTH recoveredMaxSymbolId >= size AND recoveredMaxSymbolDeltaStart == 0, and the second conjunct had no failing test. Every candidate passed either way: the torn-dict tests have frames covering from id 0, so a re-fold at baseline 0 succeeds anyway, and the covered-prefix test fails the size conjunct first. The new test builds the shape that discriminates -- a side-file holding [a,b] beside frames registering c@2 and d@3, so the frames out-reach the dictionary but are NOT self-sufficient. Delete the conjunct and recovery discards the only source of ids 0..1, re-folds at 0, finds deltaStart 2 above a coverage of 0 and quarantines a perfectly recoverable slot. C23. start()'s catch of CatchUpSendException had no test: no other case combines a seeded mirror with a failing client through start(). It matters because a recovered sender ships its catch-up on the CALLER's thread in SYNC/OFF startup, so without the catch a server that drops mid-catch-up turns a transient outage into a build() failure -- an Invariant B violation. Reverted, the new test reports CatchUpSend "transient wire failure during catch-up" straight out of start(). C25. Nothing exercised a split failing at frame k>1; every other failed-publish test fails at frame 1. The new test injects at the RING rather than the cap -- both frames clear the pre-flight, but the second does not fit a fresh sf_max_bytes segment -- so frame 1 publishes and frame 2 throws. It is labelled a CHARACTERISATION test, not a mutation guard, because it does not discriminate against reverting the pd.size() persist key: frame 1 published successfully, so advanceSentMaxSymbolId had already moved the baseline past the whole batch and the retry is an early return under either key. That is exactly why the k>1 path is safe, and it is what the test records. C21 and C24 are NOT covered, and both are downgraded. C21 -- the advanceSentMaxSymbolId/sealAndSwapBuffer ordering -- needs a publish that fails ONCE and then succeeds. Every failure mode reachable from the public API is permanent for a given batch: an oversized frame stays oversized, and the rows are retained deliberately, so the retry re-encodes the same bytes. Reaching it would need a fault-injection seam at appendBlocking, which is more production test-surface than the row justifies now that C25 covers the k>1 shape. C24 -- trySendOne's re-anchor catch and its cap-gap discrimination -- is unreachable, for the reason that retracted C13 in the previous commit: both connection setup sites call tryRetireOrphanTail before any send, so that arm is only reached once frames below the tail have been sent on this connection, and the catch sits inside the nextWireSeq == 0 branch that then cannot hold. Full client suite: 2701 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2ee6e8f commit f9bb589

3 files changed

Lines changed: 171 additions & 0 deletions

File tree

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,90 @@ public void testSplitBatchShipsDeltaOnFirstFrameOnly() throws Exception {
273273
});
274274
}
275275

276+
/**
277+
* A split that fails at frame k&gt;1 must leave the symbol dictionary consistent for
278+
* the retry.
279+
* <p>
280+
* flushPendingRowsSplit publishes frames one at a time and documents that a failure
281+
* partway through leaves frames 1..k-1 on the ring as deferred, with the next flush
282+
* re-emitting the whole batch -- at-least-once, absorbed by DEDUP or a durable-ack
283+
* await. Nothing exercised the k&gt;1 shape at all: every other failed-publish test
284+
* fails at frame 1.
285+
* <p>
286+
* This is a CHARACTERISATION test, not a mutation guard, and the reason is worth
287+
* recording. It confirms the javadoc's claim that "the re-sent frames carry empty
288+
* deltas and the write-ahead persist is a pd.size() no-op" -- but it does not
289+
* discriminate against reverting that pd.size() key, because frame 1 published
290+
* SUCCESSFULLY and advanceSentMaxSymbolId therefore already moved the baseline past
291+
* the whole batch. The retry is an early return under either key. That is precisely
292+
* why the k&gt;1 path is safe, and it is the fact the test pins.
293+
* <p>
294+
* The failure is injected at the RING, not the cap: both frames pass the pre-flight
295+
* (each is under the advertised cap) but "big"'s frame does not fit a fresh
296+
* sf_max_bytes segment, so it fails in sealAndSwapBuffer AFTER "a" was published.
297+
*/
298+
@Test(timeout = 60_000L)
299+
public void testMidSplitPublishFailureLeavesTheDictionaryIdempotent() throws Exception {
300+
Path sfDir = temporaryFolder.newFolder("qwp-sf-midsplit").toPath();
301+
assertMemoryLeak(() -> {
302+
CapturingHandler handler = new CapturingHandler();
303+
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
304+
server.setAdvertisedMaxBatchSize(3_400);
305+
int port = server.getPort();
306+
server.start();
307+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
308+
309+
String pad = new String(new char[100]).replace('\0', 'x');
310+
String cfg = "ws::addr=localhost:" + port
311+
+ ";sf_dir=" + sfDir
312+
+ ";sf_max_bytes=1024"
313+
+ ";auto_flush_bytes=off;auto_flush_rows=1000000;auto_flush_interval=60000"
314+
+ ";close_flush_timeout_millis=0;";
315+
Sender sender = Sender.fromConfig(cfg);
316+
try {
317+
// "a" first (small: fits a segment), "big" second (does not).
318+
sender.table("a").symbol("s", "alpha").longColumn("v", 1L).atNow();
319+
for (int i = 0; i < 28; i++) {
320+
sender.table("big").symbol("s", "bravo")
321+
.stringColumn("p", pad).longColumn("v", (long) i).atNow();
322+
}
323+
324+
ObjList<String> afterFirst = null;
325+
for (int attempt = 0; attempt < 2; attempt++) {
326+
try {
327+
sender.flush();
328+
Assert.fail("the second split frame cannot fit a fresh segment");
329+
} catch (LineSenderException expected) {
330+
// Ring-level failure, not the cap pre-flight.
331+
Assert.assertFalse("must fail at the ring, not the cap pre-flight: "
332+
+ expected.getMessage(),
333+
expected.getMessage().contains("too large for server batch cap"));
334+
}
335+
ObjList<String> persisted =
336+
((QwpWebSocketSender) sender).getPersistedSymbolsForTest();
337+
Assert.assertNotNull(persisted);
338+
if (attempt == 0) {
339+
afterFirst = persisted;
340+
} else {
341+
Assert.assertEquals("a mid-split retry must not re-append the "
342+
+ "symbols the failed attempt already persisted",
343+
afterFirst.size(), persisted.size());
344+
}
345+
}
346+
Assert.assertEquals("both symbols are persisted exactly once",
347+
2, afterFirst.size());
348+
} finally {
349+
try {
350+
// close() re-flushes the retained batch, which is permanently
351+
// unflushable at this sf_max_bytes -- not what we assert.
352+
sender.close();
353+
} catch (LineSenderException ignored) {
354+
}
355+
}
356+
}
357+
});
358+
}
359+
276360
@Test
277361
public void testOversizedTableSplitStrandsNothing() throws Exception {
278362
// Regression: flushPendingRowsSplit publishes each table's frame one at a

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,43 @@ private static void invokeConnectLoop(CursorWebSocketSendLoop loop) {
10011001
loop.connectLoopForTest(new LineSenderException("test reconnect"), "reconnect", 0L);
10021002
}
10031003

1004+
/**
1005+
* start()'s catch of CatchUpSendException must absorb a transient catch-up send
1006+
* failure, not let it out of Sender construction.
1007+
* <p>
1008+
* A recovered sender re-registers its dictionary with a catch-up on the very first
1009+
* connect, and in SYNC/OFF startup that runs on the CALLER's thread inside start().
1010+
* Without the catch, a server that drops during the catch-up turns a transient outage
1011+
* into a build() failure -- an Invariant B violation, and the one thing
1012+
* store-and-forward exists to prevent. Delete the block and everything else stays
1013+
* green: no other test combines a seeded mirror with a failing client through start().
1014+
* <p>
1015+
* The failure must also stay non-terminal: the client is dropped so the I/O thread
1016+
* reconnects and re-sends the catch-up off the caller's thread.
1017+
*/
1018+
@Test
1019+
public void testStartAbsorbsATransientCatchUpSendFailure() throws Exception {
1020+
TestUtils.assertMemoryLeak(() -> {
1021+
try (CursorSendEngine engine = newEngine()) {
1022+
CatchUpCapturingClient client = new CatchUpCapturingClient(0, true);
1023+
CursorWebSocketSendLoop loop = newForegroundLoop(engine, client);
1024+
try {
1025+
// One mirror entry is enough to make setWireBaselineWithCatchUp ship a
1026+
// catch-up, which the client then fails. loop.close() frees addr.
1027+
long addr = Unsafe.malloc(2, MemoryTag.NATIVE_DEFAULT);
1028+
long p = writeVarint(addr, 1);
1029+
Unsafe.getUnsafe().putByte(p, (byte) 'a');
1030+
loop.seedSentDictMirrorForTest(addr, 2, 1);
1031+
1032+
loop.start(); // must NOT throw: build() would fail on a transient
1033+
loop.checkError(); // and must not have latched a terminal
1034+
} finally {
1035+
loop.close();
1036+
}
1037+
}
1038+
});
1039+
}
1040+
10041041
private CursorWebSocketSendLoop newLoop(CursorSendEngine engine, WebSocketClient client) {
10051042
return newLoop(engine, client, 0L);
10061043
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,56 @@ public void testGapResetInsideTheDeferredTailFailsCleanInsteadOfMisattributing()
542542
});
543543
}
544544

545+
/**
546+
* A torn DELTA slot must KEEP its dictionary: the full-dict-fallback discard is
547+
* gated on {@code recoveredMaxSymbolDeltaStart == 0L} and that conjunct is
548+
* load-bearing in the negative direction.
549+
* <p>
550+
* Here the side-file holds [a,b] and the surviving frames register c@2 and d@3 --
551+
* so the frames out-reach the dictionary (recoveredMaxSymbolId 3 >= size 2, the
552+
* other conjunct) but are NOT self-sufficient. Drop the deltaStart conjunct and
553+
* recovery discards the only source of ids 0..1, re-folds at baseline 0, finds
554+
* deltaStart 2 above a coverage of 0, marks a gap and quarantines a slot that is
555+
* perfectly recoverable.
556+
* <p>
557+
* Every existing candidate passes either way: the torn-dict tests have frames
558+
* covering from id 0, and testRecoverySkipsEntriesAlreadyCoveredByPersistedPrefix
559+
* fails the size conjunct first, so neither one is load-bearing there.
560+
*/
561+
@Test
562+
public void testTornDeltaSlotKeepsItsDictionaryInsteadOfBeingDiscarded() throws Exception {
563+
TestUtils.assertMemoryLeak(() -> {
564+
try (CursorSendEngine engine = newEngine()) {
565+
appendDeltaSymbolFrame(engine, 2, 'c');
566+
appendDeltaSymbolFrame(engine, 3, 'd');
567+
}
568+
try (PersistedSymbolDict pd = PersistedSymbolDict.openClean(tmpDir)) {
569+
assertNotNull(pd);
570+
pd.appendSymbol("a");
571+
pd.appendSymbol("b");
572+
}
573+
574+
try (CursorSendEngine engine = newEngine()) {
575+
assertNotNull("a torn DELTA slot must keep its dictionary -- its frames "
576+
+ "cannot rebuild the ids below their own delta start",
577+
engine.getPersistedSymbolDict());
578+
assertEquals("scaffolding: the frames must out-reach the dictionary, so the "
579+
+ "OTHER discard conjunct is satisfied and this test really "
580+
+ "pins the deltaStart one",
581+
3L, engine.recoveredMaxSymbolId());
582+
assertEquals(2, engine.getPersistedSymbolDict().size());
583+
584+
GlobalSymbolDictionary recovered = new GlobalSymbolDictionary();
585+
assertEquals("recovery must cover ids 0..3, not fail clean",
586+
4L, engine.addRecoveredSymbolsTo(2, recovered));
587+
assertEquals("only the suffix above the persisted prefix is added",
588+
2, recovered.size());
589+
assertEquals("c", recovered.getSymbol(0));
590+
assertEquals("d", recovered.getSymbol(1));
591+
}
592+
});
593+
}
594+
545595
@Test
546596
public void testSelfSufficientFrameRepairsAckedRecoveryGap() throws Exception {
547597
// fsn 0 models the tail of an old delta epoch whose registering frames

0 commit comments

Comments
 (0)