Skip to content

Commit ff53e1f

Browse files
glasstigerclaude
andcommitted
Rebuild the dictionary from the frames still on disk
A slot whose symbol dictionary was damaged was being condemned as unreplayable while every symbol it needed was sitting on disk. The sender told the user to resend data that had never been lost, and after the previous commit it set the slot aside instead. Three things kept the rebuild from ever running. setCursorEngine gated the producer's seed on deltaDictEnabled. That flag is false exactly when the slot's dictionary failed to open -- fd exhaustion, a read-only remount, ENOSPC -- which is precisely when the frames on disk are the only surviving copy of the symbols. The rebuild was dead code for the very case it was written for. collectReplaySymbolsAbove walked from ackedFsn+1, so it skipped the acked frames. But an acked frame is not trimmed the instant it is acked: trim unlinks whole SEALED segments. The frames that registered the early ids are almost always still there, and they are exactly the ids the replay set's deltas start above. It walks every frame on disk now, and a slot whose registering frames really HAVE been trimmed still reports the gap, because then nothing holds them. And the send loop's mirror seeded only from the dictionary file, never from the frames -- so even with the other two fixed, the loop would still condemn (deltaStart > sentDictCount) a slot the producer had just seeded successfully. The mirror is the loop's model of what the SERVER holds and it is what the catch-up ships, so it now seeds from the same two sources in the same order, and the two land on the same number by construction. Only when a surviving frame actually depends on ids it does not carry: at maxSymbolDeltaStart == 0 every frame re-registers from id 0 as it replays, so a full-dict slot stays catch-up-free. The recovery tests now assert the strong form -- the fresh server's reconstructed dictionary must come back COMPLETE and in order, which can only happen if the catch-up carried the ids the acked frames still hold. A null-padded or shifted dictionary fails there, which is the corruption the old guard condemned these slots to avoid. It never had to. The quarantine now covers only what is genuinely unrecoverable: the registering frames trimmed away and the dictionary torn, so nothing anywhere holds those ids. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent aeb92b5 commit ff53e1f

4 files changed

Lines changed: 216 additions & 244 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2258,7 +2258,11 @@ public void setCursorEngine(CursorSendEngine engine, boolean takeOwnership) {
22582258
// persisted dictionary so newly ingested symbols continue from the
22592259
// recovered ids (rather than colliding with them at 0), and the delta
22602260
// baseline resumes where the crashed session left off.
2261-
if (deltaDictEnabled && engine.wasRecoveredFromDisk()) {
2261+
// NOT gated on deltaDictEnabled. That flag is false exactly when the slot's dictionary
2262+
// failed to open -- which is precisely when the frames on disk are the only surviving
2263+
// copy of the symbols and the rebuild matters most. Gating the seed on it made the
2264+
// rebuild dead code for the very case it was written for.
2265+
if (engine != null && engine.wasRecoveredFromDisk()) {
22622266
seedGlobalDictionaryFromPersisted(engine.getPersistedSymbolDict());
22632267
}
22642268
}

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

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,14 @@ public final class CursorSendEngine implements QuietCloseable {
108108
// against the recovered dictionary size to fail clean instead. Computed once
109109
// in the constructor's recovery branch; -1 elsewhere.
110110
private long recoveredMaxSymbolId = -1L;
111+
// Highest deltaStart across the recovered COMMITTED frames; 0 when none carries a symbol
112+
// dictionary. ZERO means every surviving frame is SELF-SUFFICIENT -- it re-registers its
113+
// dictionary from id 0 -- so the slot replays with no dictionary at all and the send loop
114+
// needs no catch-up. ABOVE zero means at least one frame is a true delta whose ids depend
115+
// on registrations it does not itself carry, so the loop must seed its mirror (and ship a
116+
// catch-up) before replaying. Both the full-dict-fallback discard below and the send
117+
// loop's mirror seeding key off this.
118+
private long recoveredMaxSymbolDeltaStart;
111119
// FSN of the last frame of a recovered orphaned deferred tail, or -1 when
112120
// the recovered ring has no such tail. When >= 0, frames
113121
// [recoveredCommitBoundaryFsn + 1 .. recoveredOrphanTipFsn] all carry
@@ -380,14 +388,15 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man
380388
// dictionary. The recoveredMaxSymbolId >= size guard means this never
381389
// fires for a slot whose dictionary is intact, nor for an empty slot
382390
// (recoveredMaxSymbolId == -1). Single-threaded; before the I/O loop.
383-
if (persistedDictInProgress != null
384-
&& recoveredMaxSymbolId >= persistedDictInProgress.size()
385-
&& recovered.maxSymbolDeltaStart(
391+
this.recoveredMaxSymbolDeltaStart = recovered.maxSymbolDeltaStart(
386392
QwpConstants.MAGIC_MESSAGE,
387393
QwpConstants.HEADER_OFFSET_FLAGS,
388394
QwpConstants.FLAG_DELTA_SYMBOL_DICT,
389395
QwpConstants.HEADER_SIZE,
390-
recoveredCommitBoundaryFsn) == 0L) {
396+
recoveredCommitBoundaryFsn);
397+
if (persistedDictInProgress != null
398+
&& recoveredMaxSymbolId >= persistedDictInProgress.size()
399+
&& recoveredMaxSymbolDeltaStart == 0L) {
391400
persistedDictInProgress.close();
392401
persistedDictInProgress = null;
393402
}
@@ -715,7 +724,14 @@ public long collectReplaySymbolsAbove(int baseline, ObjList<String> out) {
715724
QwpConstants.HEADER_OFFSET_FLAGS,
716725
QwpConstants.FLAG_DELTA_SYMBOL_DICT,
717726
QwpConstants.HEADER_SIZE,
718-
ackedFsn() + 1L,
727+
// Walk from the LOWEST frame still on disk, not from ackedFsn+1. An acked frame
728+
// is not trimmed the instant it is acked -- trim drops whole SEALED segments --
729+
// so the symbols it registered are usually still sitting right there, and they
730+
// are exactly the ids the replay set's deltas start above. Skipping them threw
731+
// away the only surviving copy of those symbols and condemned a slot that had
732+
// everything it needed. A slot whose registering frames really HAVE been
733+
// trimmed still reports the gap, because then no frame on disk holds them.
734+
0L,
719735
recoveredCommitBoundaryFsn,
720736
baseline,
721737
out
@@ -842,6 +858,20 @@ public long recoveredMaxSymbolId() {
842858
return recoveredMaxSymbolId;
843859
}
844860

861+
/**
862+
* Highest {@code deltaStart} across the recovered committed frames; {@code 0} when every
863+
* surviving frame is self-sufficient (or none carries a dictionary at all).
864+
* <p>
865+
* The send loop uses this to decide whether it needs a catch-up: at zero, every frame
866+
* re-registers its dictionary from id 0 as it replays, so seeding the mirror -- and
867+
* shipping a catch-up frame off it -- would be pure redundancy. Above zero, at least one
868+
* frame's delta starts above ids it does not itself carry, so the mirror must hold those
869+
* ids before the replay begins or the server null-pads the hole.
870+
*/
871+
public long recoveredMaxSymbolDeltaStart() {
872+
return recoveredMaxSymbolDeltaStart;
873+
}
874+
845875
/**
846876
* FSN of the last frame of a recovered orphaned deferred tail, or
847877
* {@code -1} when none. See {@link #recoveredCommitBoundaryFsn()}: the

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
import io.questdb.client.std.CharSequenceLongHashMap;
4444
import io.questdb.client.std.MemoryTag;
4545
import io.questdb.client.std.QuietCloseable;
46+
import io.questdb.client.std.str.Utf8s;
47+
import io.questdb.client.std.ObjList;
4648
import io.questdb.client.std.Unsafe;
4749
import org.jetbrains.annotations.TestOnly;
4850
import org.slf4j.Logger;
@@ -699,6 +701,25 @@ public CursorWebSocketSendLoop(WebSocketClient client, CursorSendEngine engine,
699701
sentDictCount = pd.size();
700702
}
701703
}
704+
// ...and then from the surviving frames' own delta sections, exactly as the producer
705+
// seeds its dictionary. The mirror is the loop's model of what the SERVER holds and it
706+
// is what the catch-up frame ships, so when it stops at the persisted prefix while the
707+
// producer's baseline runs past it, the loop condemns (deltaStart > sentDictCount) a
708+
// slot the producer just seeded successfully. Same two sources, same order, so the two
709+
// land on the same number by construction.
710+
// ...but ONLY when a surviving frame actually depends on ids it does not carry
711+
// (recoveredMaxSymbolDeltaStart > 0). At zero every frame is self-sufficient and
712+
// re-registers its dictionary from id 0 as it replays, so seeding the mirror would buy
713+
// nothing and cost a catch-up frame on every connection -- full-dict slots must stay
714+
// catch-up-free.
715+
if (engine.recoveredMaxSymbolDeltaStart() > 0L) {
716+
ObjList<String> fromFrames = new ObjList<>();
717+
if (engine.collectReplaySymbolsAbove(sentDictCount, fromFrames) >= 0) {
718+
for (int i = 0, n = fromFrames.size(); i < n; i++) {
719+
appendSymbolToMirror(fromFrames.getQuick(i));
720+
}
721+
}
722+
}
702723
this.fsnAtZero = fsnAtZero;
703724
this.parkNanos = parkNanos;
704725
this.reconnectFactory = reconnectFactory;
@@ -2268,6 +2289,23 @@ private void accumulateSentDict(long payloadAddr, int payloadLen, int deltaStart
22682289
sentDictCount += (int) newCount;
22692290
}
22702291

2292+
/**
2293+
* Appends one symbol to the sent-dictionary mirror in wire form
2294+
* ({@code [len varint][utf8]}), advancing {@code sentDictCount}. Seeds the mirror at
2295+
* construction; {@link #accumulateSentDict} extends it from live frames thereafter.
2296+
*/
2297+
private void appendSymbolToMirror(CharSequence symbol) {
2298+
int utf8Len = Utf8s.utf8Bytes(symbol);
2299+
int wireLen = NativeBufferWriter.varintSize(utf8Len) + utf8Len;
2300+
ensureSentDictCapacity((long) sentDictBytesLen + wireLen);
2301+
long p = NativeBufferWriter.writeVarint(sentDictBytesAddr + sentDictBytesLen, utf8Len);
2302+
if (utf8Len > 0) {
2303+
Utf8s.strCpyUtf8(symbol, p, utf8Len);
2304+
}
2305+
sentDictBytesLen += wireLen;
2306+
sentDictCount++;
2307+
}
2308+
22712309
private void ensureSentDictCapacity(long required) {
22722310
if (sentDictBytesCapacity >= required) {
22732311
return;

0 commit comments

Comments
 (0)