Skip to content

Commit 2ad8f50

Browse files
glasstigerclaude
andcommitted
Fail clean on a total symbol-dict tear on resume
seedGlobalDictionaryFromPersisted returned early when the persisted .symbol-dict recovered empty (size 0), skipping the torn-dictionary guard on the line just below. A host crash -- or a prior full-dict-mode session whose openClean failed and never wrote the side-file -- can leave a size-0 dictionary while the segment frames that introduced its ids survive. Resuming then seeds the producer with an empty dictionary, so the next new symbol reuses id 0; once that frame is trimmed and the connection recycles, the catch-up re-registers the recovered ids and the producer's rows are silently misattributed. The send loop's replay guard cannot catch this: a deltaStart=0 frame self-heals the catch-up mirror rather than tripping the gap check, so the seed-time guard is the only defense. Run it before the size-0 short-circuit. A genuinely empty slot (no symbol-bearing frames) still has recoveredMaxSymbolId == -1, so -1 >= 0 is false and the empty case returns without throwing. Add testTornDictTotalLossFailsCleanOnResume covering the size-0 total tear (frames replay from deltaStart=0, so only the seed-time guard can reject it). Repoint two tests that had reached the I/O-thread guard through the bypassed early return: testTornDictionaryFailsCleanly... now expects the build-time guard, and testTornDictionaryOneIdGap... reaches the I/O guard through the unopenable-dictionary path instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7880b7a commit 2ad8f50

2 files changed

Lines changed: 116 additions & 44 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3882,9 +3882,18 @@ private void resetSymbolDictStateForNewConnection() {
38823882
* write-ahead ordering keeps the dictionary a superset of the frames.
38833883
*/
38843884
private void seedGlobalDictionaryFromPersisted(PersistedSymbolDict pd) {
3885-
if (pd == null || pd.size() == 0) {
3885+
if (pd == null) {
38863886
return;
38873887
}
3888+
// Run the torn-dictionary guard BEFORE the empty-dictionary short-circuit
3889+
// below: a TOTAL tear (pd.size() == 0) with surviving symbol-bearing frames
3890+
// must fail clean too, not slip through. Such a frame starts at deltaStart=0
3891+
// and self-heals the I/O-thread catch-up mirror, so the send loop's replay
3892+
// guard (deltaStart > sentDictCount) never fires -- this seed-time guard is
3893+
// then the only defense against the producer resuming unseeded and silently
3894+
// reusing ids the frames already define. A genuinely empty slot (no
3895+
// symbol-bearing frames) has recoveredMaxSymbolId() == -1, so -1 >= 0 is false
3896+
// and the pd.size() == 0 return below still fires.
38883897
if (cursorEngine != null && cursorEngine.recoveredMaxSymbolId() >= pd.size()) {
38893898
throw new LineSenderException(
38903899
"recovered store-and-forward symbol dictionary is a subset of the surviving frames "
@@ -3893,6 +3902,9 @@ private void seedGlobalDictionaryFromPersisted(PersistedSymbolDict pd) {
38933902
+ pd.size() + " id(s); resuming would reuse ids the frames already define -- "
38943903
+ "resend the affected data");
38953904
}
3905+
if (pd.size() == 0) {
3906+
return;
3907+
}
38963908
ObjList<String> symbols = pd.readLoadedSymbols();
38973909
for (int i = 0, n = symbols.size(); i < n; i++) {
38983910
globalSymbolDictionary.addRecoveredSymbol(symbols.getQuick(i));

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

Lines changed: 103 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -171,19 +171,21 @@ public void testTornDictionaryFailsCleanlyInsteadOfCorrupting() throws Exception
171171
}
172172
}
173173

174-
// Simulate a host/power crash: the segment frames survive but the persisted
175-
// dictionary is lost, and the ack watermark was left mid-stream. Truncate
176-
// .symbol-dict to its 8-byte header (0 symbols) and stamp the watermark at
177-
// FSN 2, so recovery replays from FSN 3 -- a frame with deltaStart=3.
174+
// Simulate a host/power crash: the segment frames survive but the WHOLE
175+
// persisted dictionary is lost (truncate .symbol-dict to its 8-byte header,
176+
// 0 symbols), and the ack watermark was left mid-stream (FSN 2). The
177+
// surviving frames still reference the lost ids.
178178
java.nio.file.Path slot = Paths.get(sfDir, "default");
179179
java.nio.file.Path dict = slot.resolve(".symbol-dict");
180180
byte[] header = Arrays.copyOf(java.nio.file.Files.readAllBytes(dict), 8);
181181
java.nio.file.Files.write(dict, header);
182182
writeAckWatermark(slot.resolve(".ack-watermark"), 2);
183183

184-
// Phase 2: recover against a fresh counting server. The replay guard must
185-
// fire (frame deltaStart 3 > recovered dictionary size 0) and fail terminally
186-
// rather than send a gapped frame that would corrupt the table.
184+
// Phase 2: recover against a fresh counting server. With the whole
185+
// dictionary lost (size 0) while the surviving frames still reference its
186+
// ids, seedGlobalDictionaryFromPersisted must refuse the resume at BUILD --
187+
// its size==0 short-circuit must NOT skip the torn-dict guard -- rather than
188+
// let the producer resume unseeded and silently misattribute reused ids.
187189
CountingHandler handler = new CountingHandler();
188190
try (TestWebSocketServer good = new TestWebSocketServer(handler)) {
189191
int port = good.getPort();
@@ -192,47 +194,34 @@ public void testTornDictionaryFailsCleanlyInsteadOfCorrupting() throws Exception
192194

193195
String cfg = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";";
194196
LineSenderException terminal = null;
195-
Sender s2 = Sender.fromConfig(cfg);
196197
try {
197-
// Poll for the replay guard to fire (it recordFatal's on the I/O
198-
// thread); flush() surfaces the latched terminal to the producer.
199-
// A bounded poll replaces a fixed sleep and captures it as soon as
200-
// it fires; close() below is the fallback if it surfaces only there.
201-
long deadline = System.currentTimeMillis() + 10_000;
202-
while (System.currentTimeMillis() < deadline && terminal == null) {
203-
try {
204-
s2.flush();
205-
Thread.sleep(20);
206-
} catch (LineSenderException e) {
207-
terminal = e;
208-
}
209-
}
210-
} finally {
211-
try {
212-
s2.close();
213-
} catch (LineSenderException e) {
214-
if (terminal == null) {
215-
terminal = e;
216-
}
217-
}
198+
Sender s2 = Sender.fromConfig(cfg);
199+
s2.close();
200+
} catch (LineSenderException e) {
201+
terminal = e;
218202
}
219203
Assert.assertEquals("no frame may be replayed to a fresh server with a torn dictionary",
220204
0, handler.frames.get());
221205
Assert.assertNotNull("a torn dictionary must surface a terminal error", terminal);
222206
Assert.assertTrue(terminal.getMessage(),
223-
terminal.getMessage().contains("symbol dictionary is incomplete"));
207+
terminal.getMessage().contains("subset of the surviving frames")
208+
&& terminal.getMessage().contains("resend"));
224209
}
225210
});
226211
}
227212

228213
@Test
229214
public void testTornDictionaryOneIdGapFailsCleanly() throws Exception {
230-
// Boundary variant of testTornDictionaryFailsCleanlyInsteadOfCorrupting: the
231-
// recovered dictionary is short by EXACTLY ONE id -- the tightest gap the
232-
// guard must still reject (deltaStart == recoveredSize + 1). A one-entry-short
233-
// tail is the common host-crash outcome, so this pins the guard's
234-
// false-negative edge: a "deltaStart > size + 1" mutation would let this
235-
// single-id gap through and null-pad the missing symbol on the server.
215+
// One-id-gap boundary variant of
216+
// testUnopenablePersistedDictStillGuardsAgainstReplayingDeltaFrames: the first
217+
// replayed frame starts EXACTLY ONE id past the empty mirror -- the tightest
218+
// gap the I/O-thread replay guard must still reject (deltaStart ==
219+
// sentDictCount + 1). It reaches that guard via the unopenable-dictionary path
220+
// (deltaDictEnabled=false), NOT a size-0 openable dict, which now fails clean
221+
// earlier in seedGlobalDictionaryFromPersisted. A one-entry-short tail is the
222+
// common host-crash outcome, so this pins the guard's false-negative edge: a
223+
// "deltaStart > size + 1" mutation would let this single-id gap through and
224+
// null-pad the missing symbol on the server.
236225
assertMemoryLeak(() -> {
237226
// Phase 1: each row introduces a new symbol (frame i carries deltaStart=i).
238227
try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) {
@@ -249,15 +238,17 @@ public void testTornDictionaryOneIdGapFailsCleanly() throws Exception {
249238
}
250239
}
251240

252-
// Lose the whole dictionary (truncate to the 8-byte header, size 0) but
253-
// stamp the watermark at FSN 0, so recovery replays from FSN 1 -- a frame
254-
// with deltaStart=1. The dictionary covers no ids, so id 0 is the single
255-
// missing entry: deltaStart(1) == recoveredSize(0) + 1. Because size is 0
256-
// no catch-up is sent, so any counted frame would be the gapped data frame.
241+
// Make the persisted dictionary UNOPENABLE (replace .symbol-dict with a
242+
// directory, so both openRW and openCleanRW fail and the engine reports
243+
// deltaDictEnabled=false), then stamp the watermark at FSN 0 so recovery
244+
// replays from FSN 1 -- a frame with deltaStart=1. The mirror is unseeded
245+
// (size 0), so id 0 is the single missing entry: deltaStart(1) ==
246+
// sentDictCount(0) + 1. No catch-up is sent, so any counted frame would be
247+
// the gapped data frame.
257248
java.nio.file.Path slot = Paths.get(sfDir, "default");
258249
java.nio.file.Path dict = slot.resolve(".symbol-dict");
259-
byte[] header = Arrays.copyOf(java.nio.file.Files.readAllBytes(dict), 8);
260-
java.nio.file.Files.write(dict, header);
250+
java.nio.file.Files.delete(dict);
251+
java.nio.file.Files.createDirectory(dict);
261252
writeAckWatermark(slot.resolve(".ack-watermark"), 0);
262253

263254
// Phase 2: recover against a fresh counting server. The guard must fire on
@@ -837,6 +828,75 @@ public void testTornDictSubsetFailsCleanOnResume() throws Exception {
837828
});
838829
}
839830

831+
@Test
832+
public void testTornDictTotalLossFailsCleanOnResume() throws Exception {
833+
// C1 regression (TOTAL-loss variant of testTornDictSubsetFailsCleanOnResume):
834+
// a host crash can lose the ENTIRE persisted .symbol-dict (size 0) while the
835+
// frames that introduced its ids survive; equivalently, a prior session whose
836+
// openClean failed ran full-dict mode and never wrote the side-file, so this
837+
// session recovers those frames against a FRESH EMPTY dictionary. The recovered
838+
// frames start at deltaStart=0 and self-heal the I/O-thread catch-up mirror, so
839+
// the send loop's replay guard (deltaStart > mirror) never fires -- only
840+
// seedGlobalDictionaryFromPersisted can catch it. Its pd.size()==0 early return
841+
// must NOT skip the torn-dict guard: the surviving frames reference id 2 while
842+
// the recovered dictionary holds 0 id(s), so resuming unseeded would reuse those
843+
// ids and silently misattribute symbol values on the wire. Without the fix the
844+
// build below does NOT throw (the guard is bypassed) and the test's fail() fires.
845+
assertMemoryLeak(() -> {
846+
// Phase 1: record three delta frames (a@0, b@1, c@2) with nothing acked, so
847+
// all three survive and replay -- from FSN 0 (deltaStart=0), the case the
848+
// I/O-thread guard self-heals rather than rejects.
849+
try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) {
850+
int port = silent.getPort();
851+
silent.start();
852+
Assert.assertTrue(silent.awaitStart(5, TimeUnit.SECONDS));
853+
String cfg = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir
854+
+ ";close_flush_timeout_millis=0;";
855+
Sender s1 = Sender.fromConfig(cfg);
856+
try {
857+
s1.table("m").symbol("s", "a").longColumn("v", 0).atNow();
858+
s1.flush();
859+
s1.table("m").symbol("s", "b").longColumn("v", 1).atNow();
860+
s1.flush();
861+
s1.table("m").symbol("s", "c").longColumn("v", 2).atNow();
862+
s1.flush();
863+
} finally {
864+
s1.close();
865+
}
866+
}
867+
868+
// Total dictionary loss: openClean truncates .symbol-dict to its bare header
869+
// (size 0) with zero appends, while every frame -- including the ones
870+
// referencing a@0,b@1,c@2 -- stays on disk. This is the fresh-empty-dict
871+
// state a full-dict-mode prior session or a total host-crash tear leaves.
872+
String slotDir = Paths.get(sfDir, "default").toString();
873+
try (PersistedSymbolDict torn = PersistedSymbolDict.openClean(slotDir)) {
874+
Assert.assertNotNull(torn);
875+
Assert.assertEquals(0, torn.size());
876+
}
877+
878+
// Phase 2: a resuming sender must refuse at build -- the surviving frames
879+
// reference id 2 but the recovered dictionary holds 0 id(s). The I/O-thread
880+
// guard cannot help (deltaStart 0 self-heals the mirror), so the seed-time
881+
// guard is the sole defense.
882+
try (TestWebSocketServer good = new TestWebSocketServer(new DictReconstructingHandler())) {
883+
int port = good.getPort();
884+
good.start();
885+
Assert.assertTrue(good.awaitStart(5, TimeUnit.SECONDS));
886+
String cfg = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";";
887+
try {
888+
Sender resumed = Sender.fromConfig(cfg);
889+
resumed.close();
890+
Assert.fail("resume on a totally lost (size 0) dictionary must fail clean");
891+
} catch (LineSenderException expected) {
892+
Assert.assertTrue("message must name the torn-dict/resend failure: " + expected.getMessage(),
893+
expected.getMessage().contains("subset of the surviving frames")
894+
&& expected.getMessage().contains("resend"));
895+
}
896+
}
897+
});
898+
}
899+
840900
private static int globalDictSize(Sender sender) throws Exception {
841901
Field f = sender.getClass().getDeclaredField("globalSymbolDictionary");
842902
f.setAccessible(true);

0 commit comments

Comments
 (0)