Fix symbol-dict durability doc; add recovery test

The PersistedSymbolDict class doc claimed a host-crash-torn dictionary
is "caught at replay by the send loop's guard ... rather than corrupting
the target table." The guard is tail-only: it fires only when a frame's
delta start id exceeds the recovered dictionary size. It does NOT catch
an interior page lost out of order (reads back as zeroes that parse as
empty-string entries) or a failed best-effort truncate that leaves a
stale trailing entry -- either shifts the dense id->symbol mapping
without changing the entry count the guard checks, so a replay silently
misattributes symbols. Correct the class doc and the truncate comment to
describe the actual, tail-only protection and name the residual
host-crash exposure (within the already-documented "not host-crash
durable" boundary); note that a per-entry or running CRC would close it.

Add testRecoveryAfterFailedPublishReplaysGapFree: a failed publish
persists a frame's symbol without recording the frame, leaving the
persisted dictionary a strict superset of the recorded frames. The test
recovers the slot on a fresh sender against a dictionary-reconstructing
server and asserts the catch-up re-registers the whole superset (the
unrecorded symbol included) and the replay is gap-free -- the end-to-end
fail -> recover -> replay chain the existing no-duplicate test omitted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: glasstiger

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/PersistedSymbolDict.java

@@ -67,9 +67,18 @@
  * dictionary is a superset of every recoverable frame's references. It is NOT
  * sufficient for a <b>host/power crash</b>, where unflushed pages can be lost out
  * of order and the dictionary may end up torn relative to the frames it serves --
- * exactly as the segment frames themselves may be lost on a host crash. A torn
- * dictionary is caught at replay by the send loop's guard, which fails loudly
- * (the unreplayable data must be resent) rather than corrupting the target table.
+ * exactly as the segment frames themselves may be lost on a host crash. The send
+ * loop's replay guard catches the COMMON host-crash outcome -- a truncated tail,
+ * where a frame's delta start id exceeds the recovered dictionary size -- and
+ * fails loudly (the unreplayable data must be resent) rather than sending a
+ * gapped frame. It does NOT catch every host-crash tear: an interior page lost
+ * out of order reads back as zeroes that parse as empty-string entries, and a
+ * failed best-effort truncate (see {@link #open}) can leave a stale trailing
+ * entry -- either SHIFTS the dense id->symbol mapping without changing the entry
+ * count the guard checks, so a replay silently misattributes symbols. That
+ * residual exposure sits within the "not host-crash durable" boundary above; a
+ * per-entry or running CRC would be needed to close it (the 3 reserved header
+ * bytes leave room for a future integrity field).
  * <p>
  * A torn trailing entry from a crash mid-append is self-healing: {@link #open}
  * stops parsing at the first incomplete entry and the next append overwrites it.
@@ -367,8 +376,12 @@ private static PersistedSymbolDict openExisting(String filePath, long fileLen) {
             // shift every subsequent dense id. Without this, appendOffset already
             // lands just past the last complete entry, so the next append
             // overwrites FROM there, but only truncation guarantees nothing
-            // survives BEYOND it. Best-effort: a failed truncate just forgoes the
-            // hardening and falls back to the overwrite-from-appendOffset behaviour.
+            // survives BEYOND it. Best-effort: a failed truncate (rare -- an I/O
+            // error) leaves a latent silent-corruption path, not merely a lost
+            // optimization: if a later, shorter append then leaves stale bytes past
+            // its end, a subsequent recovery mis-parses them as a ghost symbol and
+            // shifts the dense ids, and the count-based replay guard does not catch
+            // it (see the class-level durability note).
             long validLen = HEADER_SIZE + entriesLen;
             if (validLen < len) {
                 Files.truncate(fd, validLen);

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

@@ -480,6 +480,100 @@ public void testFailedPublishDoesNotDuplicatePersistedSymbols() throws Exception
         });
     }
 
+    @Test
+    public void testRecoveryAfterFailedPublishReplaysGapFree() throws Exception {
+        // M3 end-to-end: chains a failed publish -> fresh-process recovery -> replay.
+        // A failed publish persists the frame's symbol (write-ahead) but does NOT
+        // record the frame, so the persisted dictionary becomes a strict SUPERSET of
+        // the recorded frames' references. A recovering sender must still replay
+        // gap-free: it re-registers the whole (superset) dictionary via the catch-up
+        // -- including the symbol whose frame never reached disk -- so the fresh
+        // server reconstructs a complete, gap-free dictionary. The sibling
+        // testFailedPublishDoesNotDuplicatePersistedSymbols proves the dict has no
+        // duplicate after the failed publish; this proves the resulting slot then
+        // recovers and replays end-to-end against a real server.
+        assertMemoryLeak(() -> {
+            // Phase 1: a silent server (no acks) + a small SF segment. Four small
+            // rows register sym-0..sym-3 and their frames are recorded; a fifth,
+            // oversized row registers (persists) sym-4 but its frame is too large for
+            // the segment, so appendBlocking throws and the frame is NOT recorded.
+            try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) {
+                int port = silent.getPort();
+                silent.start();
+                Assert.assertTrue(silent.awaitStart(5, TimeUnit.SECONDS));
+                String cfg = "ws::addr=localhost:" + port
+                        + ";sf_dir=" + sfDir
+                        + ";sf_max_bytes=4096"
+                        + ";close_flush_timeout_millis=0;";
+                Sender s1 = Sender.fromConfig(cfg);
+                try {
+                    for (int i = 0; i < 4; i++) {
+                        s1.table("m").symbol("s", "sym-" + i).longColumn("v", i).atNow();
+                        s1.flush();
+                    }
+                    // Oversized frame: sym-4 is persisted (write-ahead) before the
+                    // publish fails, so the dictionary runs one ahead of the frames.
+                    s1.table("m").symbol("s", "sym-4")
+                            .stringColumn("p", TestUtils.repeat("x", 8000))
+                            .longColumn("v", 4).atNow();
+                    try {
+                        s1.flush();
+                        Assert.fail("oversized frame must fail to publish");
+                    } catch (LineSenderException expected) {
+                        // PAYLOAD_TOO_LARGE -- frame not recorded, sym-4 stays persisted
+                    }
+                } finally {
+                    try {
+                        // Re-flushes the still-buffered oversized row and fails again
+                        // (expected); resources are still released, and the idempotent
+                        // write-ahead does not re-append sym-4.
+                        s1.close();
+                    } catch (LineSenderException ignored) {
+                    }
+                }
+            }
+
+            // The persisted dictionary must hold the superset: sym-0..sym-4 (5 ids),
+            // one more than the four recorded frames reference, with no duplicate.
+            PersistedSymbolDict pd = PersistedSymbolDict.open(Paths.get(sfDir, "default").toString());
+            Assert.assertNotNull(pd);
+            try {
+                Assert.assertEquals("failed publish must leave the dict a superset (sym-0..sym-4)",
+                        5, pd.size());
+            } finally {
+                pd.close();
+            }
+
+            // Phase 2: recover against a fresh server that reconstructs its
+            // dictionary from the wire. The recovering sender must re-register all 5
+            // symbols via a catch-up (sym-4 exists ONLY in the dictionary -- no frame
+            // carries it) and replay the 4 recorded frames, leaving a complete,
+            // gap-free server dictionary.
+            DictReconstructingHandler handler = new DictReconstructingHandler();
+            try (TestWebSocketServer good = new TestWebSocketServer(handler)) {
+                int port = good.getPort();
+                good.start();
+                Assert.assertTrue(good.awaitStart(5, TimeUnit.SECONDS));
+                String cfg = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";";
+                try (Sender ignored = Sender.fromConfig(cfg)) {
+                    long deadline = System.currentTimeMillis() + 5_000;
+                    while (System.currentTimeMillis() < deadline && handler.maxDictSize() < 5) {
+                        Thread.sleep(20);
+                    }
+                }
+                Assert.assertTrue("recovery must send a full-dictionary catch-up frame",
+                        handler.sawCatchUpFrame);
+                List<String> dict = handler.dictSnapshot();
+                Assert.assertEquals("recovered dictionary must include the failed-publish symbol",
+                        5, dict.size());
+                for (int i = 0; i < 5; i++) {
+                    Assert.assertEquals("dictionary id " + i + " must be gap-free",
+                            "sym-" + i, dict.get(i));
+                }
+            }
+        });
+    }
+
     private static void writeAckWatermark(java.nio.file.Path path, long fsn) throws IOException {
         byte[] buf = new byte[16];
         ByteBuffer bb = ByteBuffer.wrap(buf).order(ByteOrder.LITTLE_ENDIAN);