Skip to content

Commit 66bdc0f

Browse files
glasstigerclaude
andcommitted
Pin the C3 and C4 fixes with regression tests
Each test was verified to fail with its production fix reverted, and the revert was applied in place rather than via git checkout so no other uncommitted work could be discarded. testPersistFailureDegradesToFullDictInsteadOfKillingFlushForever arms the full-disk facade before the first flush, so the very first ensureAppendMap is refused -- a later append would sit inside the window already mapped and never call allocate at all. The first flush must still throw, because beginMessage has already baked a delta deltaStart into the staged frame, but the second must succeed with deltaStart 0. Reverted, the retry fails with "failed to persist symbol dictionary before publish". testRecoveryBaselineMismatchIsQuarantinableNotAPermanentBrick recovers a slot and asks addRecoveredSymbolsTo for a baseline the fold cannot match. Reverted, it surfaces IllegalState "recovery symbol baseline mismatch [expected=0, actual=9999]", which Sender.build() does not route to its quarantine handler. testUndecodableLoadedRegionIsQuarantinableNotAPermanentBrick hand-writes a CRC-valid chunk declaring 3 entries inside a 2-byte region. Reverted, it surfaces IllegalState "truncated loaded symbol dictionary entry". The fixture also documents a gap worth closing separately: scanRecoveredChunks verifies the chunk CRC and rejects entryCount <= 0, but never checks that entryBytes actually holds entryCount well-formed entries, so it accepts this chunk and only the decoder notices. Validating the triple during the scan would end the trusted prefix there, as a CRC failure already does. Full client suite: 2690 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0fe4873 commit 66bdc0f

3 files changed

Lines changed: 182 additions & 0 deletions

File tree

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1761,6 +1761,80 @@ public void testRecoveryHealsThePersistedDictionaryBeforeAnyNewFrame() throws Ex
17611761
});
17621762
}
17631763

1764+
/**
1765+
* A dictionary that becomes unwritable mid-run must cost bandwidth, not the sender.
1766+
* <p>
1767+
* deltaDictEnabled was written once at setCursorEngine with no runtime disable, so
1768+
* the first persist failure killed flush() permanently -- every later flush re-threw.
1769+
* A full disk reaches exactly that state and is survivable: SF's segments are
1770+
* pre-allocated mmap files and stay writable while the growing .symbol-dict does not,
1771+
* and full self-sufficient frames need no side file at all (each carries the whole
1772+
* dictionary from id 0, which is what recovery replays anyway).
1773+
* <p>
1774+
* So the FIRST flush must still fail -- beginMessage has already baked a delta
1775+
* deltaStart into the staged frame, and publishing it would put ids on the ring the
1776+
* side-file cannot describe -- but the SECOND must succeed, in full-dict mode. Remove
1777+
* the disableDeltaDict call and the second flush throws exactly like the first.
1778+
*/
1779+
@Test(timeout = 60_000L)
1780+
public void testPersistFailureDegradesToFullDictInsteadOfKillingFlushForever() throws Exception {
1781+
assertMemoryLeak(() -> {
1782+
DictReconstructingHandler handler = new DictReconstructingHandler();
1783+
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
1784+
int port = server.getPort();
1785+
server.start();
1786+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
1787+
1788+
String slot = Paths.get(sfDir, "default").toString();
1789+
Assert.assertEquals(0, io.questdb.client.std.Files.mkdir(sfDir,
1790+
io.questdb.client.std.Files.DIR_MODE_DEFAULT));
1791+
FullDiskDictFacade ff = new FullDiskDictFacade();
1792+
CursorSendEngine engine = new CursorSendEngine(
1793+
slot, 4L * 1024 * 1024, CursorSendEngine.DEFAULT_APPEND_DEADLINE_NANOS,
1794+
CursorSendEngine.DEFAULT_APPEND_DEADLINE_NANOS, ff);
1795+
Sender sender = QwpWebSocketSender.connect(
1796+
"localhost", port, null, 0, 0, 0L, null, false, engine);
1797+
try {
1798+
// Armed from the start, so the very first ensureAppendMap is refused --
1799+
// a later append would sit inside the window already mapped and never
1800+
// call allocate at all.
1801+
ff.armed = true;
1802+
sender.table("m").symbol("s", "a").longColumn("v", 1L).atNow();
1803+
try {
1804+
sender.flush();
1805+
Assert.fail("the first flush must fail: its staged frame carries a delta "
1806+
+ "deltaStart the side-file cannot describe");
1807+
} catch (LineSenderException expected) {
1808+
Assert.assertTrue("expected the persist-failure message, got: "
1809+
+ expected.getMessage(),
1810+
expected.getMessage().contains("failed to persist symbol dictionary"));
1811+
}
1812+
1813+
// The rows are still buffered (the throw precedes every publish), so the
1814+
// retry re-encodes them from id 0. Pre-fix this threw forever.
1815+
sender.flush();
1816+
1817+
long deadline = System.currentTimeMillis() + 5_000;
1818+
while (System.currentTimeMillis() < deadline && handler.dataFrameCount() < 1) {
1819+
Thread.sleep(20);
1820+
}
1821+
Assert.assertTrue("the degraded flush must actually reach the server",
1822+
handler.dataFrameCount() >= 1);
1823+
Assert.assertEquals("a degraded frame must be self-sufficient (deltaStart 0)",
1824+
0, handler.lastDataDeltaStart());
1825+
Assert.assertEquals("the full dictionary must ride along",
1826+
java.util.Collections.singletonList("a"), handler.dictSnapshot());
1827+
} finally {
1828+
try {
1829+
sender.close();
1830+
} catch (LineSenderException ignored) {
1831+
// not what we assert
1832+
}
1833+
}
1834+
}
1835+
});
1836+
}
1837+
17641838
private static boolean hasSegmentFile(java.nio.file.Path dir) {
17651839
java.io.File[] files = dir.toFile().listFiles();
17661840
if (files != null) {

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import io.questdb.client.cutlass.line.LineSenderException;
2828
import io.questdb.client.cutlass.qwp.client.sf.cursor.AckWatermark;
2929
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
30+
import io.questdb.client.cutlass.qwp.client.sf.cursor.UnreplayableSlotException;
31+
import io.questdb.client.cutlass.qwp.client.GlobalSymbolDictionary;
3032
import io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment;
3133
import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager;
3234
import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock;
@@ -57,6 +59,44 @@
5759

5860
public class CursorSendEngineTest {
5961

62+
/**
63+
* A recovery-seed baseline mismatch must be quarantinable, not a permanent brick.
64+
* <p>
65+
* checkedRecoveryAnalysis threw a raw IllegalStateException, and Sender.build() routes
66+
* on the TYPE -- only UnreplayableSlotException reaches its quarantine handler. With a
67+
* stable senderId and a not-fully-drained slot retained on close, every restart
68+
* re-recovered the same slot and rethrew: the application could never construct a
69+
* Sender, so it could not even BUFFER new rows. Revert the type and this goes red.
70+
*/
71+
@Test(timeout = 10_000L)
72+
public void testRecoveryBaselineMismatchIsQuarantinableNotAPermanentBrick() throws Exception {
73+
TestUtils.assertMemoryLeak(() -> {
74+
long buf = Unsafe.malloc(64, MemoryTag.NATIVE_DEFAULT);
75+
try {
76+
try (CursorSendEngine engine = new CursorSendEngine(tmpDir, 4096)) {
77+
engine.appendBlocking(buf, 64);
78+
}
79+
} finally {
80+
Unsafe.free(buf, 64, MemoryTag.NATIVE_DEFAULT);
81+
}
82+
try (CursorSendEngine reopened = new CursorSendEngine(tmpDir, 4096)) {
83+
assertTrue("scaffolding: the slot must have been recovered",
84+
reopened.wasRecoveredFromDisk());
85+
try {
86+
// The fold was keyed to the persisted prefix size; ask for a baseline
87+
// that cannot match it.
88+
reopened.addRecoveredSymbolsTo(9_999, new GlobalSymbolDictionary());
89+
fail("a baseline mismatch must be reported");
90+
} catch (UnreplayableSlotException expected) {
91+
assertTrue("expected the baseline-mismatch message, got: "
92+
+ expected.getMessage(),
93+
expected.getMessage().contains("recovery symbol baseline mismatch"));
94+
}
95+
}
96+
});
97+
}
98+
99+
60100
private String tmpDir;
61101

62102
@Before

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,12 @@
2626

2727
import io.questdb.client.cutlass.qwp.client.GlobalSymbolDictionary;
2828
import io.questdb.client.cutlass.qwp.client.sf.cursor.PersistedSymbolDict;
29+
import io.questdb.client.cutlass.qwp.client.sf.cursor.UnreplayableSlotException;
2930
import io.questdb.client.std.FilesFacade;
31+
import io.questdb.client.std.MemoryTag;
32+
import io.questdb.client.std.Crc32c;
3033
import io.questdb.client.std.ObjList;
34+
import io.questdb.client.std.Unsafe;
3135
import io.questdb.client.test.tools.DelegatingFilesFacade;
3236
import io.questdb.client.test.tools.TestUtils;
3337
import org.junit.Assert;
@@ -500,6 +504,70 @@ public void testMappedAppendSpansMultipleWindowsAndRecoversDenseIds() throws Exc
500504
});
501505
}
502506

507+
/**
508+
* A loaded region that cannot be decoded must surface as an UnreplayableSlotException,
509+
* not a raw IllegalStateException.
510+
* <p>
511+
* Sender.build() routes on the TYPE -- only UnreplayableSlotException reaches its
512+
* quarantine handler. A raw IllegalStateException escapes build() instead, and with a
513+
* stable senderId and a retained slot every restart re-recovers and rethrows: the
514+
* application can never construct a Sender, so it cannot even BUFFER new rows.
515+
* <p>
516+
* The fixture also documents a real gap: scanRecoveredChunks validates the chunk CRC
517+
* and rejects entryCount &lt;= 0, but never checks that entryBytes actually holds
518+
* entryCount well-formed entries. A chunk claiming 3 entries inside a 2-byte region is
519+
* therefore ACCEPTED here, and only the decoder notices. Validating the triple during
520+
* the scan would end the trusted prefix at that chunk instead -- the same treatment a
521+
* CRC failure gets.
522+
*/
523+
@Test
524+
public void testUndecodableLoadedRegionIsQuarantinableNotAPermanentBrick() throws Exception {
525+
assertMemoryLeak(() -> {
526+
Path dir = newFolder("qwp-symdict");
527+
Path f = dir.resolve(".symbol-dict");
528+
529+
// [entryCount=3][entryBytes=2][len=1]['a'] -- 3 entries claimed, 1 supplied.
530+
byte[] body = new byte[]{0x03, 0x02, 0x01, (byte) 'a'};
531+
int crc;
532+
long scratch = Unsafe.malloc(body.length, MemoryTag.NATIVE_DEFAULT);
533+
try {
534+
for (int i = 0; i < body.length; i++) {
535+
Unsafe.getUnsafe().putByte(scratch + i, body[i]);
536+
}
537+
crc = Crc32c.update(Crc32c.INIT, scratch, body.length);
538+
} finally {
539+
Unsafe.free(scratch, body.length, MemoryTag.NATIVE_DEFAULT);
540+
}
541+
542+
byte[] file = new byte[8 + body.length + 4];
543+
file[0] = 'S';
544+
file[1] = 'Y';
545+
file[2] = 'D';
546+
file[3] = '1';
547+
file[4] = 3; // VERSION
548+
System.arraycopy(body, 0, file, 8, body.length);
549+
int crcAt = 8 + body.length;
550+
file[crcAt] = (byte) crc;
551+
file[crcAt + 1] = (byte) (crc >>> 8);
552+
file[crcAt + 2] = (byte) (crc >>> 16);
553+
file[crcAt + 3] = (byte) (crc >>> 24);
554+
Files.write(f, file);
555+
556+
try (PersistedSymbolDict dict = PersistedSymbolDict.open(dir.toString())) {
557+
Assert.assertNotNull("the CRC is valid, so the chunk is accepted by the scan", dict);
558+
Assert.assertEquals("the scan trusts the declared entryCount", 3, dict.size());
559+
try {
560+
dict.addLoadedSymbolsTo(new GlobalSymbolDictionary());
561+
Assert.fail("decoding 3 entries out of a 2-byte region must fail");
562+
} catch (UnreplayableSlotException expected) {
563+
Assert.assertTrue("expected the truncated-entry message, got: "
564+
+ expected.getMessage(),
565+
expected.getMessage().contains("truncated loaded symbol dictionary entry"));
566+
}
567+
}
568+
});
569+
}
570+
503571
private static int varintSize(int v) {
504572
int n = 1;
505573
while ((v >>>= 7) != 0) {

0 commit comments

Comments
 (0)