Skip to content

Commit 7299925

Browse files
glasstigerclaude
andcommitted
Correct stale and stranded symbol-dict comments
Comment-only follow-ups from the delta symbol-dict review; no behaviour change. - quarantineTornSlot marks the set-aside slot .failed unconditionally, so the orphan drainer never re-examines it. Fix the two comments that claimed the drainer could still deliver a quarantined slot; state that it is a human-in-the-loop copy whose bytes are kept for a manual resend. - sendCommitMessage's full-dict-mode comment claimed the commit frame stays self-sufficient, but the prior flush reset currentBatchMaxSymbolId to -1, so the frame carries an empty delta. Say so -- it is harmless, the preceding data frames already registered the dictionary. - Move three javadoc blocks stranded above the wrong method back onto their own: MmapSegment.maxSymbolDeltaEnd, SegmentRing.maxSymbolDeltaEnd and Sender.closeFlushTimeoutMillis were left undocumented, and the stranded @param names no longer matched the method beneath them. - Drop a double blank line in CursorSendEngine. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 63dc52a commit 7299925

5 files changed

Lines changed: 73 additions & 67 deletions

File tree

core/src/main/java/io/questdb/client/Sender.java

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,8 +1013,9 @@ final class LineSenderBuilder {
10131013
private static final int PROTOCOL_UDP = 3;
10141014
private static final int PROTOCOL_WEBSOCKET = 2;
10151015
// Suffix for a slot set aside by quarantineTornSlot. Deliberately NOT the
1016-
// sender's own slot name, so OrphanScanner sees the quarantined copy and the
1017-
// orphan drainer can still deliver it if its frames turn out to be replayable.
1016+
// sender's own slot name, so a restarted sender does not re-adopt it as its own;
1017+
// quarantineTornSlot then marks it .failed, so the orphan drainer skips it too and
1018+
// the bytes stay put for a human to inspect and resend.
10181019
private static final String QUARANTINE_SLOT_SUFFIX = ".unreplayable-";
10191020
private final ObjList<String> hosts = new ObjList<>();
10201021
private final IntList ports = new IntList();
@@ -1757,23 +1758,6 @@ public Sender build() {
17571758
return sender;
17581759
}
17591760

1760-
/**
1761-
* close() drain timeout in milliseconds. The sender's {@code close()}
1762-
* method blocks up to this many millis waiting for the server to ACK
1763-
* every batch already published into the engine before shutting down
1764-
* the I/O loop. Default {@code 60000} (60 s) -- generous enough to
1765-
* survive real-workload backlogs (slow consumers, catch-up replicas,
1766-
* chunky payloads on small server send buffers) without silently
1767-
* dropping unacked rows; callers that need a longer pre-close wait
1768-
* for a specific submission can call
1769-
* {@link Sender#drain(long)} explicitly before close().
1770-
* <p>
1771-
* Set to {@code 0} or {@code -1} to opt out — close() will not wait
1772-
* at all (fast close). Pending data is then lost in memory mode and
1773-
* recovered by the next sender in SF mode.
1774-
* <p>
1775-
* WebSocket transport only.
1776-
*/
17771761
/**
17781762
* Minimum wall-clock time (millis) a symbol-dictionary catch-up CAP GAP must
17791763
* persist before the sender gives up on it and fails.
@@ -1805,6 +1789,23 @@ public LineSenderBuilder catchUpCapGapMinEscalationWindowMillis(long millis) {
18051789
return this;
18061790
}
18071791

1792+
/**
1793+
* close() drain timeout in milliseconds. The sender's {@code close()}
1794+
* method blocks up to this many millis waiting for the server to ACK
1795+
* every batch already published into the engine before shutting down
1796+
* the I/O loop. Default {@code 60000} (60 s) -- generous enough to
1797+
* survive real-workload backlogs (slow consumers, catch-up replicas,
1798+
* chunky payloads on small server send buffers) without silently
1799+
* dropping unacked rows; callers that need a longer pre-close wait
1800+
* for a specific submission can call
1801+
* {@link Sender#drain(long)} explicitly before close().
1802+
* <p>
1803+
* Set to {@code 0} or {@code -1} to opt out — close() will not wait
1804+
* at all (fast close). Pending data is then lost in memory mode and
1805+
* recovered by the next sender in SF mode.
1806+
* <p>
1807+
* WebSocket transport only.
1808+
*/
18081809
public LineSenderBuilder closeFlushTimeoutMillis(long timeoutMillis) {
18091810
if (protocol != PARAMETER_NOT_SET_EXPLICITLY && protocol != PROTOCOL_WEBSOCKET) {
18101811
throw new LineSenderException("close_flush_timeout_millis is only supported for WebSocket transport");
@@ -3019,13 +3020,17 @@ private static long parseSizeValue(@NotNull StringSink value, @NotNull String na
30193020
* already-lost batch for an unbounded outage of everything after it, which inverts
30203021
* the one guarantee store-and-forward exists to give.
30213022
* <p>
3022-
* So: rename the slot aside instead. The bytes are preserved for forensics and for
3023-
* the orphan drainer, which reaches the same verdict independently (its send loop's
3024-
* replay guard fires and it marks the slot {@code .failed}) -- and which, on a slot
3025-
* that turns out to be drainable after all (frames written in full-dictionary
3026-
* fallback mode are self-sufficient), simply drains it. The new name is NOT the
3027-
* sender's own slot name, so {@code OrphanScanner} will consider it. The producer,
3028-
* meanwhile, starts on a clean empty slot and never notices.
3023+
* So: rename the slot aside instead, and mark it {@code .failed}. The verdict is
3024+
* authoritative -- the recovery seed already tried every source of truth (the
3025+
* persisted prefix AND the surviving frames' own deltas), and the orphan drainer's
3026+
* own replay guard uses that same walk, so there is nothing a drainer could rebuild
3027+
* that the seed did not. {@code markFailed} (below) therefore quarantines the copy
3028+
* for a human rather than leaving a drainer to retry an unreplayable slot forever;
3029+
* a full-dictionary-fallback slot never reaches here, because its dictionary is
3030+
* discarded at recovery and it never throws. The bytes are preserved on disk for
3031+
* forensics and a manual resend, and the new name -- NOT the sender's own slot name
3032+
* -- keeps a restarted sender from re-adopting it. The producer, meanwhile, starts
3033+
* on a clean empty slot and never notices.
30293034
* <p>
30303035
* If the rename fails (a Windows share lock, a read-only mount) there is no way to
30313036
* free the slot name without destroying data, so fall back to the old behaviour and

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3751,7 +3751,9 @@ private void sendCommitMessage() {
37513751
// what has already been sent -- and therefore already persisted. In delta
37523752
// mode pass sentMaxSymbolId, yielding an empty delta
37533753
// [sentMaxSymbolId+1 .. sentMaxSymbolId]; in full-dict mode keep
3754-
// currentBatchMaxSymbolId so the frame stays self-sufficient. Any symbol a
3754+
// currentBatchMaxSymbolId (reset to -1 by the prior flush, so the commit frame
3755+
// carries an empty delta too -- harmless, since the preceding full-dict data
3756+
// frames already registered the dictionary on this connection). Any symbol a
37553757
// cancelled row leaked is picked up (and persisted) by the next real flush,
37563758
// whose persistNewSymbolsBeforePublish resumes from pd.size().
37573759
int commitBatchMaxId = deltaDictEnabled ? sentMaxSymbolId : currentBatchMaxSymbolId;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,6 @@ public boolean isDeltaDictEnabled() {
783783
return sfDir == null || persistedSymbolDict != null;
784784
}
785785

786-
787786
/**
788787
* Pass-through to {@link SegmentRing#nextSealedAfter(MmapSegment)}.
789788
*/

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

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -524,37 +524,6 @@ public long findLastFrameFsnWithoutPayloadFlag(int flagsOffset, int flagMask, in
524524
return best;
525525
}
526526

527-
/**
528-
* Highest {@code deltaStart + deltaCount} (one past the highest symbol id) any
529-
* delta-flagged frame in this segment carries, or {@code 0} only when NO
530-
* delta-flagged frame is present. A frame with {@code deltaCount == 0} (a commit
531-
* or a symbol-reusing frame -- the encoder always sets the delta flag) still
532-
* contributes its {@code deltaStart}, i.e. the producer's baseline at encode
533-
* time, NOT 0. That is deliberate: it anchors the torn-dict guard at that
534-
* baseline so a dictionary torn below it is detected -- a conservative
535-
* over-strand (it may over-reject a frame whose rows reference only surviving
536-
* ids), never an under-strand that could silently shift the dense id map.
537-
* Read-only walk over the recovered frames, used
538-
* once at recovery to detect a persisted {@code .symbol-dict} torn (host crash,
539-
* out-of-order page loss) below the ids the surviving frames still reference: if
540-
* the max here reaches at or beyond the recovered dictionary size, a resuming
541-
* producer -- seeded from the shorter dictionary -- would re-use ids the frames
542-
* already define. Only frames at or below {@code maxFsnInclusive} count: frames
543-
* above it are the aborted orphan-deferred tail, which {@code trySendOne} retires
544-
* without ever transmitting, so their ids must not inflate the result (a resuming
545-
* producer never reuses them on the wire). The frame layout mirrors
546-
* {@link #findLastFrameFsnWithoutPayloadFlag}: the QWP message header
547-
* ({@code qwpHeaderSize} bytes) is followed by the delta section
548-
* {@code [deltaStart varint][deltaCount varint]...}.
549-
*
550-
* @param headerMagic the QWP message magic identifying a well-formed frame
551-
* @param flagsOffset byte offset of the flags field within the QWP header
552-
* @param flagDeltaMask the FLAG_DELTA_SYMBOL_DICT bit
553-
* @param qwpHeaderSize the QWP message header size (delta section starts past it)
554-
* @param maxFsnInclusive highest frame FSN to consider; frames above it are the
555-
* retired orphan-deferred tail -- pass
556-
* {@code recoveredCommitBoundaryFsn}
557-
*/
558527
/**
559528
* Rebuilds, from the frames' own delta sections, the symbols a replay of
560529
* {@code [minFsnInclusive .. maxFsnInclusive]} would register ABOVE {@code coverage},
@@ -670,6 +639,37 @@ public long collectReplaySymbolsAbove(
670639
return coverage;
671640
}
672641

642+
/**
643+
* Highest {@code deltaStart + deltaCount} (one past the highest symbol id) any
644+
* delta-flagged frame in this segment carries, or {@code 0} only when NO
645+
* delta-flagged frame is present. A frame with {@code deltaCount == 0} (a commit
646+
* or a symbol-reusing frame -- the encoder always sets the delta flag) still
647+
* contributes its {@code deltaStart}, i.e. the producer's baseline at encode
648+
* time, NOT 0. That is deliberate: it anchors the torn-dict guard at that
649+
* baseline so a dictionary torn below it is detected -- a conservative
650+
* over-strand (it may over-reject a frame whose rows reference only surviving
651+
* ids), never an under-strand that could silently shift the dense id map.
652+
* Read-only walk over the recovered frames, used
653+
* once at recovery to detect a persisted {@code .symbol-dict} torn (host crash,
654+
* out-of-order page loss) below the ids the surviving frames still reference: if
655+
* the max here reaches at or beyond the recovered dictionary size, a resuming
656+
* producer -- seeded from the shorter dictionary -- would re-use ids the frames
657+
* already define. Only frames at or below {@code maxFsnInclusive} count: frames
658+
* above it are the aborted orphan-deferred tail, which {@code trySendOne} retires
659+
* without ever transmitting, so their ids must not inflate the result (a resuming
660+
* producer never reuses them on the wire). The frame layout mirrors
661+
* {@link #findLastFrameFsnWithoutPayloadFlag}: the QWP message header
662+
* ({@code qwpHeaderSize} bytes) is followed by the delta section
663+
* {@code [deltaStart varint][deltaCount varint]...}.
664+
*
665+
* @param headerMagic the QWP message magic identifying a well-formed frame
666+
* @param flagsOffset byte offset of the flags field within the QWP header
667+
* @param flagDeltaMask the FLAG_DELTA_SYMBOL_DICT bit
668+
* @param qwpHeaderSize the QWP message header size (delta section starts past it)
669+
* @param maxFsnInclusive highest frame FSN to consider; frames above it are the
670+
* retired orphan-deferred tail -- pass
671+
* {@code recoveredCommitBoundaryFsn}
672+
*/
673673
public long maxSymbolDeltaEnd(int headerMagic, int flagsOffset, int flagDeltaMask, int qwpHeaderSize, long maxFsnInclusive) {
674674
long maxEnd = 0L;
675675
long off = HEADER_SIZE;

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -546,14 +546,6 @@ 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 at or below
551-
* {@code maxFsnInclusive} references (0 when none). See
552-
* {@link MmapSegment#maxSymbolDeltaEnd}; used once at recovery to detect a
553-
* persisted dictionary torn below the ids the surviving committed frames
554-
* reference. Frames above {@code maxFsnInclusive} are the retired orphan-deferred
555-
* tail and are excluded.
556-
*/
557549
/**
558550
* Rebuilds the symbols a replay of {@code [minFsnInclusive .. maxFsnInclusive]} would
559551
* register ABOVE {@code baseline}, appending them to {@code out} in ascending id order.
@@ -589,6 +581,14 @@ public synchronized long collectReplaySymbolsAbove(
589581
minFsnInclusive, maxFsnInclusive, coverage, out);
590582
}
591583

584+
/**
585+
* Highest {@code deltaStart + deltaCount} any symbol-dict delta frame at or below
586+
* {@code maxFsnInclusive} references (0 when none). See
587+
* {@link MmapSegment#maxSymbolDeltaEnd}; used once at recovery to detect a
588+
* persisted dictionary torn below the ids the surviving committed frames
589+
* reference. Frames above {@code maxFsnInclusive} are the retired orphan-deferred
590+
* tail and are excluded.
591+
*/
592592
public synchronized long maxSymbolDeltaEnd(int headerMagic, int flagsOffset, int flagDeltaMask, int qwpHeaderSize, long maxFsnInclusive) {
593593
long maxEnd = 0L;
594594
for (int i = 0, n = sealedSegments.size(); i < n; i++) {

0 commit comments

Comments
 (0)