Skip to content

Commit ff2e841

Browse files
glasstigerclaude
andcommitted
Guard torn-dict resume and skip split re-encode
Two hardening fixes to the delta symbol-dictionary path. Torn-dictionary resume (data integrity): the persisted .symbol-dict is not fsync'd, so a host crash can lose its newest entries while the segment frames that introduced those ids survive and replay. A resumed producer, seeded from the shorter dictionary, would reuse ids the surviving frames already define and silently misattribute symbols. The send loop's replay guard only catches a gap (deltaStart > mirror); the contiguous tear self-heals the mirror and leaves the producer diverged. CursorSendEngine now records the highest symbol id the recovered frames reference via a read-only MmapSegment/SegmentRing walk, and seedGlobalDictionaryFromPersisted fails clean when it reaches the recovered dictionary size -- the affected data must be resent. Only the resuming producer refuses; the orphan drainer still self-heals and drains the slot. testTornDictSubsetFailsCleanOnResume covers it. Split-flush sizing (performance): flushPendingRowsSplit re-encoded the whole batch a second time just to size each split frame. It now derives each frame size arithmetically from the combined encode flushPendingRows already performed, capturing each table's body bytes and the delta-entry length. Body encoding is context-free, so the sizes are exact; the publish loop's assert cross-checks them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6c6dc40 commit ff2e841

5 files changed

Lines changed: 253 additions & 12 deletions

File tree

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

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import io.questdb.client.std.Decimal128;
5858
import io.questdb.client.std.Decimal256;
5959
import io.questdb.client.std.Decimal64;
60+
import io.questdb.client.std.IntList;
6061
import io.questdb.client.std.Misc;
6162
import io.questdb.client.std.Numbers;
6263
import io.questdb.client.std.NumericException;
@@ -169,6 +170,11 @@ public class QwpWebSocketSender implements Sender {
169170
// behind a drainer's endpoint walk.
170171
private final ReentrantLock connectWalkLock = new ReentrantLock();
171172
private final QwpHostHealthTracker hostTracker;
173+
// Per-table encoded body byte counts captured during flushPendingRows' combined
174+
// encode, reused by flushPendingRowsSplit to size each split frame arithmetically
175+
// instead of re-encoding the batch a second time. Cleared and repopulated on every
176+
// flush; only consumed on the (exceptional) split path. Reused to stay zero-GC.
177+
private final IntList splitFrameBodyBytes = new IntList();
172178
private final CharSequenceObjHashMap<QwpTableBuffer> tableBuffers;
173179
// null means plain text (no TLS)
174180
private final ClientTlsConfiguration tlsConfig;
@@ -3474,6 +3480,15 @@ private void flushPendingRows(boolean deferCommit) {
34743480
encoder.setDeferCommit(deferCommit);
34753481
encoder.beginMessage(tableCount, globalSymbolDictionary,
34763482
symbolDeltaBaseline(), currentBatchMaxSymbolId);
3483+
// Record each table's encoded body size (position delta across addTable) so
3484+
// the split path can size its per-table frames arithmetically rather than
3485+
// re-encoding the whole batch. Body encoding is context-free (a table's bytes
3486+
// don't depend on its siblings or the delta section), so the size a table
3487+
// takes here equals the size it takes in its own split frame. Only consumed
3488+
// when the batch overflows the cap; the capture is a couple of int ops per
3489+
// table on the common path.
3490+
splitFrameBodyBytes.clear();
3491+
int bodyStart = encoder.getBuffer().getPosition();
34773492
for (int i = 0, n = keys.size(); i < n; i++) {
34783493
CharSequence tableName = keys.getQuick(i);
34793494
if (tableName == null) {
@@ -3490,12 +3505,18 @@ private void flushPendingRows(boolean deferCommit) {
34903505
}
34913506

34923507
encoder.addTable(tableBuffer);
3508+
int bodyEnd = encoder.getBuffer().getPosition();
3509+
splitFrameBodyBytes.add(bodyEnd - bodyStart);
3510+
bodyStart = bodyEnd;
34933511
}
34943512
int messageSize = encoder.finishMessage();
34953513
QwpBufferWriter buffer = encoder.getBuffer();
34963514

34973515
if (serverMaxBatchSize > 0 && messageSize > serverMaxBatchSize) {
3498-
flushPendingRowsSplit(keys, deferCommit);
3516+
// The combined frame's delta-entry bytes are byte-identical to the first
3517+
// split frame's (same baseline + batch max), so capture the length now
3518+
// for the arithmetic frame-sizing in flushPendingRowsSplit.
3519+
flushPendingRowsSplit(keys, deferCommit, encoder.getDeltaEntriesLen());
34993520
return;
35003521
}
35013522

@@ -3547,7 +3568,7 @@ private void flushPendingRows(boolean deferCommit) {
35473568
* carry FLAG_DEFER_COMMIT. When false, only the
35483569
* last message omits the flag.
35493570
*/
3550-
private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferCommit) {
3571+
private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferCommit, int combinedDeltaEntriesLen) {
35513572
if (LOG.isDebugEnabled()) {
35523573
LOG.debug("Splitting flush across multiple messages [serverMaxBatchSize={}, defer={}]", serverMaxBatchSize, deferCommit);
35533574
}
@@ -3561,15 +3582,19 @@ private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferComm
35613582
// resetTableBuffersAfterFlush would discard every source row -- a partial
35623583
// commit the caller was told (by the throw) had failed. Checking all sizes up
35633584
// front makes the split all-or-nothing: either every frame fits and all
3564-
// publish, or none publish and we throw with nothing stranded. The cost is a
3565-
// second encode pass over the split batch, which is already the exceptional
3566-
// large-batch path. encode is read-only on the table buffer, and simBaseline
3567-
// mirrors the publish loop's baseline advance (advanceSentMaxSymbolId), so
3568-
// each measured size equals the frame the publish loop will build; this pass
3569-
// mutates no delta/persist state (the defer-commit flag is a header bit that
3570-
// does not change frame size).
3585+
// publish, or none publish and we throw with nothing stranded.
3586+
//
3587+
// Each split frame's size is derived ARITHMETICALLY from the combined encode
3588+
// flushPendingRows already performed -- header + the two delta-section varints
3589+
// + the delta entries (byte-identical to the combined frame's when this frame
3590+
// carries them) + the table's own body bytes (captured in splitFrameBodyBytes,
3591+
// context-free so identical here and in its solo frame) -- rather than
3592+
// re-encoding every table a second time. simBaseline mirrors the publish loop's
3593+
// baseline advance (advanceSentMaxSymbolId), so each size equals the frame the
3594+
// publish loop will build; this pass mutates no delta/persist state.
35713595
int nonEmptyCount = 0;
35723596
int simBaseline = symbolDeltaBaseline();
3597+
int bodyIdx = 0;
35733598
for (int i = 0, n = keys.size(); i < n; i++) {
35743599
CharSequence tableName = keys.getQuick(i);
35753600
if (tableName == null) {
@@ -3580,9 +3605,14 @@ private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferComm
35803605
continue;
35813606
}
35823607
nonEmptyCount++;
3583-
encoder.beginMessage(1, globalSymbolDictionary, simBaseline, currentBatchMaxSymbolId);
3584-
encoder.addTable(tableBuffer);
3585-
int messageSize = encoder.finishMessage();
3608+
int deltaStart = simBaseline + 1;
3609+
int deltaCount = Math.max(0, currentBatchMaxSymbolId - simBaseline);
3610+
int messageSize = QwpConstants.HEADER_SIZE
3611+
+ NativeBufferWriter.varintSize(deltaStart)
3612+
+ NativeBufferWriter.varintSize(deltaCount)
3613+
+ (deltaCount > 0 ? combinedDeltaEntriesLen : 0)
3614+
+ splitFrameBodyBytes.getQuick(bodyIdx);
3615+
bodyIdx++;
35863616
if (messageSize > serverMaxBatchSize) {
35873617
resetTableBuffersAfterFlush(keys);
35883618
throw new LineSenderException("single table batch too large for server batch cap")
@@ -3821,11 +3851,38 @@ private void resetSymbolDictStateForNewConnection() {
38213851
* dictionary shorter than {@code pd.size()} and desyncing
38223852
* {@code sentMaxSymbolId} from the mirror's {@code sentDictCount = pd.size()},
38233853
* which silently misattributes later symbols after a reconnect.
3854+
* <p>
3855+
* <b>Host-crash tear guard.</b> The persisted dictionary is NOT fsync'd (see
3856+
* {@code PersistedSymbolDict}), so a host/power crash can lose its
3857+
* most-recently-written (highest-id) entries while the segment frames that
3858+
* introduced those ids survive -- and those newest frames, being the least
3859+
* likely to be acked, replay on recovery. The send loop's catch-up mirror then
3860+
* rebuilds the missing ids from those frames' own delta bytes, but THIS producer
3861+
* -- seeded only from the shorter dictionary -- would assign its next new symbol
3862+
* an id the surviving frames already define, putting two symbols on one id and
3863+
* silently misattributing values. The send loop's replay guard only catches a
3864+
* GAP ({@code deltaStart > sentDictCount}); a frame that introduces exactly the
3865+
* torn-off id ({@code deltaStart == pd.size()}) slips through and self-heals the
3866+
* mirror, leaving only this producer diverged. Detect it here -- the surviving
3867+
* frames reference an id at or beyond the recovered dictionary size -- and fail
3868+
* clean: the affected data must be resent, matching the design's torn-dict
3869+
* "resend required" contract. The recovered data is not lost; the background
3870+
* drainer still drains this slot (its mirror self-heals from the frames). Only a
3871+
* host crash reaches this -- a process crash keeps the page cache, so the
3872+
* write-ahead ordering keeps the dictionary a superset of the frames.
38243873
*/
38253874
private void seedGlobalDictionaryFromPersisted(PersistedSymbolDict pd) {
38263875
if (pd == null || pd.size() == 0) {
38273876
return;
38283877
}
3878+
if (cursorEngine != null && cursorEngine.recoveredMaxSymbolId() >= pd.size()) {
3879+
throw new LineSenderException(
3880+
"recovered store-and-forward symbol dictionary is a subset of the surviving frames "
3881+
+ "(likely a host crash tore its unsynced tail): frames reference symbol id "
3882+
+ cursorEngine.recoveredMaxSymbolId() + " but the recovered dictionary holds only "
3883+
+ pd.size() + " id(s); resuming would reuse ids the frames already define -- "
3884+
+ "resend the affected data");
3885+
}
38293886
ObjList<String> symbols = pd.readLoadedSymbols();
38303887
for (int i = 0, n = symbols.size(); i < n; i++) {
38313888
globalSymbolDictionary.addRecoveredSymbol(symbols.getQuick(i));

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,15 @@ public final class CursorSendEngine implements QuietCloseable {
9696
// covers them. Read by the sender's close-time drain to avoid waiting on
9797
// acks that cannot arrive.
9898
private long recoveredCommitBoundaryFsn = -1L;
99+
// Highest symbol id any recovered delta frame references, or -1 for
100+
// fresh/memory rings (and recovered rings with no symbol-bearing frame). A
101+
// resuming producer seeds its dictionary baseline from the persisted
102+
// .symbol-dict; if that dictionary was torn below this id by a host crash
103+
// (the side-file is not fsync'd), the producer would re-use ids the surviving
104+
// frames already define. seedGlobalDictionaryFromPersisted compares this
105+
// against the recovered dictionary size to fail clean instead. Computed once
106+
// in the constructor's recovery branch; -1 elsewhere.
107+
private long recoveredMaxSymbolId = -1L;
99108
// FSN of the last frame of a recovered orphaned deferred tail, or -1 when
100109
// the recovered ring has no such tail. When >= 0, frames
101110
// [recoveredCommitBoundaryFsn + 1 .. recoveredOrphanTipFsn] all carry
@@ -312,6 +321,18 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man
312321
publishedFsn - Math.max(recoveredCommitBoundaryFsn, -1L),
313322
recoveredCommitBoundaryFsn, publishedFsn);
314323
}
324+
// Highest symbol id the surviving frames reference. A resuming
325+
// producer compares this against its recovered dictionary size
326+
// (seedGlobalDictionaryFromPersisted) to detect a host-crash tear:
327+
// if a frame references an id the (unsynced, torn) .symbol-dict no
328+
// longer holds, resuming would re-use it. maxSymbolDeltaEnd returns
329+
// 0 when no frame carries a symbol, yielding -1 here. Computed
330+
// before the I/O loop or producer append; single-threaded here.
331+
this.recoveredMaxSymbolId = recovered.maxSymbolDeltaEnd(
332+
io.questdb.client.cutlass.qwp.protocol.QwpConstants.MAGIC_MESSAGE,
333+
io.questdb.client.cutlass.qwp.protocol.QwpConstants.HEADER_OFFSET_FLAGS,
334+
io.questdb.client.cutlass.qwp.protocol.QwpConstants.FLAG_DELTA_SYMBOL_DICT,
335+
io.questdb.client.cutlass.qwp.protocol.QwpConstants.HEADER_SIZE) - 1L;
315336
} else {
316337
// Fresh start with no recovered segments. Any stale
317338
// watermark from a prior fully-drained session refers
@@ -714,6 +735,16 @@ public long recoveredCommitBoundaryFsn() {
714735
return recoveredCommitBoundaryFsn;
715736
}
716737

738+
/**
739+
* Highest symbol id any recovered delta frame references, or {@code -1} for
740+
* fresh/memory rings (and recovered rings with no symbol-bearing frame). A
741+
* resuming producer compares this against its recovered dictionary size to
742+
* detect a host-crash tear of the persisted {@code .symbol-dict}.
743+
*/
744+
public long recoveredMaxSymbolId() {
745+
return recoveredMaxSymbolId;
746+
}
747+
717748
/**
718749
* FSN of the last frame of a recovered orphaned deferred tail, or
719750
* {@code -1} when none. See {@link #recoveredCommitBoundaryFsn()}: the

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,72 @@ public long findLastFrameFsnWithoutPayloadFlag(int flagsOffset, int flagMask, in
522522
return best;
523523
}
524524

525+
/**
526+
* Highest {@code deltaStart + deltaCount} (one past the highest symbol id) that
527+
* any symbol-dict delta frame in this segment references, or {@code 0} when no
528+
* such frame carries a symbol. Read-only walk over the recovered frames, used
529+
* once at recovery to detect a persisted {@code .symbol-dict} torn (host crash,
530+
* out-of-order page loss) below the ids the surviving frames still reference: if
531+
* the max here reaches at or beyond the recovered dictionary size, a resuming
532+
* producer -- seeded from the shorter dictionary -- would re-use ids the frames
533+
* already define. The frame layout mirrors {@link #findLastFrameFsnWithoutPayloadFlag}:
534+
* the QWP message header ({@code qwpHeaderSize} bytes) is followed by the delta
535+
* section {@code [deltaStart varint][deltaCount varint]...}.
536+
*
537+
* @param headerMagic the QWP message magic identifying a well-formed frame
538+
* @param flagsOffset byte offset of the flags field within the QWP header
539+
* @param flagDeltaMask the FLAG_DELTA_SYMBOL_DICT bit
540+
* @param qwpHeaderSize the QWP message header size (delta section starts past it)
541+
*/
542+
public long maxSymbolDeltaEnd(int headerMagic, int flagsOffset, int flagDeltaMask, int qwpHeaderSize) {
543+
long maxEnd = 0L;
544+
long off = HEADER_SIZE;
545+
long frames = frameCount;
546+
for (long i = 0; i < frames; i++) {
547+
int payloadLen = Unsafe.getUnsafe().getInt(mmapAddress + off + 4);
548+
long payload = mmapAddress + off + FRAME_HEADER_SIZE;
549+
if (payloadLen >= qwpHeaderSize
550+
&& payloadLen > flagsOffset
551+
&& Unsafe.getUnsafe().getInt(payload) == headerMagic
552+
&& (Unsafe.getUnsafe().getByte(payload + flagsOffset) & flagDeltaMask) != 0) {
553+
long p = payload + qwpHeaderSize;
554+
long limit = payload + payloadLen;
555+
long deltaStart = 0L;
556+
int shift = 0;
557+
while (p < limit) {
558+
byte b = Unsafe.getUnsafe().getByte(p++);
559+
deltaStart |= (long) (b & 0x7F) << shift;
560+
if ((b & 0x80) == 0) {
561+
break;
562+
}
563+
shift += 7;
564+
if (shift > 35) {
565+
break; // corrupt run; recovered frames are CRC-valid, so defensive only
566+
}
567+
}
568+
long deltaCount = 0L;
569+
shift = 0;
570+
while (p < limit) {
571+
byte b = Unsafe.getUnsafe().getByte(p++);
572+
deltaCount |= (long) (b & 0x7F) << shift;
573+
if ((b & 0x80) == 0) {
574+
break;
575+
}
576+
shift += 7;
577+
if (shift > 35) {
578+
break;
579+
}
580+
}
581+
long end = deltaStart + deltaCount;
582+
if (end > maxEnd) {
583+
maxEnd = end;
584+
}
585+
}
586+
off += FRAME_HEADER_SIZE + payloadLen;
587+
}
588+
return maxEnd;
589+
}
590+
525591
/**
526592
* Number of frames written since {@link #create} (or recovered by
527593
* {@link #openExisting}). Used by {@code SegmentRing} to compute

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,24 @@ public synchronized long findLastFsnWithoutPayloadFlag(int flagsOffset, int flag
546546
return Math.max(best, fsn);
547547
}
548548

549+
/**
550+
* Highest {@code deltaStart + deltaCount} any symbol-dict delta frame in the
551+
* ring references (0 when none). See {@link MmapSegment#maxSymbolDeltaEnd};
552+
* used once at recovery to detect a persisted dictionary torn below the ids
553+
* the surviving frames reference.
554+
*/
555+
public synchronized long maxSymbolDeltaEnd(int headerMagic, int flagsOffset, int flagDeltaMask, int qwpHeaderSize) {
556+
long maxEnd = 0L;
557+
for (int i = 0, n = sealedSegments.size(); i < n; i++) {
558+
long end = sealedSegments.get(i).maxSymbolDeltaEnd(headerMagic, flagsOffset, flagDeltaMask, qwpHeaderSize);
559+
if (end > maxEnd) {
560+
maxEnd = end;
561+
}
562+
}
563+
long end = active.maxSymbolDeltaEnd(headerMagic, flagsOffset, flagDeltaMask, qwpHeaderSize);
564+
return Math.max(maxEnd, end);
565+
}
566+
549567
public MmapSegment getActive() {
550568
return active;
551569
}

0 commit comments

Comments
 (0)