Skip to content

Commit 3b73bc8

Browse files
glasstigerclaude
andcommitted
Pin the producer's recovery seed on the unopenable-dictionary path
The seed's ungating had no test: nothing wrote a NEW symbol after recovering a slot whose dictionary could not be opened, which is the only place the difference shows. It shows on the wire. Re-gate the seed and the resumed producer restarts its id space at 0, on top of ids the surviving frames already define -- the server's dictionary comes back as [after-recovery, sym-1, sym-2, sym-3, sym-4, sym-5] with sym-0 overwritten. Two symbols on one id, and the send loop's mirror still reads that id as sym-0, so mirror and producer disagree about what it MEANS. That is the misattribution the whole dictionary machinery exists to prevent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ff53e1f commit 3b73bc8

1 file changed

Lines changed: 51 additions & 0 deletions

File tree

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,57 @@ private static int countSegmentFiles(java.nio.file.Path dir) {
287287
return n;
288288
}
289289

290+
@Test
291+
public void testUnopenableDictSeedsTheProducerAboveTheRecoveredIds() throws Exception {
292+
// The producer must resume ABOVE the ids the recovered frames already define, even when
293+
// the dictionary could not be opened.
294+
//
295+
// seedGlobalDictionaryFromPersisted used to be gated on deltaDictEnabled, which is false
296+
// exactly here -- so the producer restarted its id space at 0, on top of ids the
297+
// surviving frames define. The send loop's mirror (rebuilt from those frames) still read
298+
// id 0 as sym-0 while the producer meant something else by it: the two disagree about
299+
// what an id MEANS, which is the whole failure mode the dictionary machinery exists to
300+
// prevent.
301+
//
302+
// Visible on the wire: the new symbol must land ABOVE the recovered ones, not on top of
303+
// sym-0.
304+
assertMemoryLeak(() -> {
305+
recordSixDeltaFrames();
306+
java.nio.file.Path slot = Paths.get(sfDir, "default");
307+
java.nio.file.Path dict = slot.resolve(".symbol-dict");
308+
java.nio.file.Files.delete(dict);
309+
java.nio.file.Files.createDirectory(dict);
310+
writeAckWatermark(slot.resolve(".ack-watermark"), 2);
311+
312+
DictReconstructingHandler handler = new DictReconstructingHandler();
313+
try (TestWebSocketServer good = new TestWebSocketServer(handler)) {
314+
int port = good.getPort();
315+
good.start();
316+
Assert.assertTrue(good.awaitStart(5, TimeUnit.SECONDS));
317+
String cfg = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";";
318+
try (Sender s2 = Sender.fromConfig(cfg)) {
319+
long deadline = System.currentTimeMillis() + 10_000;
320+
while (System.currentTimeMillis() < deadline && handler.maxDictSize() < 6) {
321+
Thread.sleep(20);
322+
}
323+
// ...and now the resumed producer introduces a symbol of its own.
324+
s2.table("m").symbol("s", "after-recovery").longColumn("v", 99).atNow();
325+
s2.flush();
326+
deadline = System.currentTimeMillis() + 10_000;
327+
while (System.currentTimeMillis() < deadline && handler.maxDictSize() < 7) {
328+
Thread.sleep(20);
329+
}
330+
}
331+
Assert.assertEquals(
332+
"the resumed producer must take the NEXT id, not reuse id 0 -- reusing it "
333+
+ "puts two symbols on one id and silently misattributes values",
334+
Arrays.asList("sym-0", "sym-1", "sym-2", "sym-3", "sym-4", "sym-5",
335+
"after-recovery"),
336+
handler.dictSnapshot());
337+
}
338+
});
339+
}
340+
290341
@Test
291342
public void testRecoveredSenderContinuesIngestingNewSymbols() throws Exception {
292343
// M2 regression: seedGlobalDictionaryFromPersisted resumes the producer's

0 commit comments

Comments
 (0)