feat(qwp): stop resending the full symbol dictionary on every message by glasstiger · Pull Request #66 · questdb/java-questdb-client

glasstiger · 2026-07-09T01:46:52Z

Summary

Every QWP ingress message used to carry the entire symbol dictionary, so a connection that ingests many distinct symbols re-transmitted the whole dictionary on every message. This change makes the client register each symbol id with the server only once per connection and send only new ids (a delta) thereafter, re-registering the full dictionary when a connection is replaced.

The bandwidth saving grows with symbol cardinality and message count; for low-cardinality or short-lived connections it is negligible, and the change adds the costs described under Tradeoffs.

What changed

Memory mode

The producer keeps a monotonic "sent" watermark; each frame's dictionary section carries only the ids above it instead of the full dictionary from id 0.
On reconnect or failover the fresh server starts with an empty dictionary, so the I/O thread replays the whole dictionary as a catch-up frame before any post-reconnect traffic. The producer's monotonic baseline is deliberately preserved across the wire boundary rather than reset.

Store-and-forward (file mode)

Each slot persists its dictionary to a dot-prefixed side-file (PersistedSymbolDict) so a recovered or orphan-drained slot on a fresh process -- which has no in-memory dictionary -- can rebuild what its (non-self-sufficient) delta frames reference.
Write-ahead ordering: new symbols are appended to the side-file before the frame that references them is published to the ring.

Catch-up split

The reconnect/recovery catch-up splits across as many frames as the server's advertised batch cap requires, so a dictionary larger than the cap is re-registered without any single frame exceeding it. The frames carry contiguous id ranges and reassemble on the server exactly as the original per-frame deltas would. When the server advertises no cap, or the whole dictionary fits, the behaviour is unchanged (a single frame).

Durability

The persisted dictionary intentionally does not fsync, matching the rest of store-and-forward: it is process-crash durable (the OS page cache survives a JVM crash) but not host-crash durable. Rather than fsync only the dictionary -- which would not make the frame data itself host-crash durable -- a host crash that tears the dictionary is caught at replay: the send loop detects a delta frame whose start id exceeds the recovered dictionary and fails cleanly with a "resend required" error instead of transmitting a gapped frame that would corrupt the table.

Tradeoffs

Each reconnect/failover now replays the full dictionary as a catch-up frame, so a reconnect on a very high-cardinality connection ships the whole dictionary once (previously every message did).
File mode writes a per-slot dictionary side-file (extra disk I/O and one small file per slot).
Without fsync, a host/power crash can still lose recently persisted symbols. The common outcome -- a truncated dictionary tail -- is caught at replay and fails clean ("resend required"), and the affected data must be re-sent. The send loop's guard is tail-only, though: an interior page lost out of order (or a failed best-effort truncate) can shift the dense id->symbol mapping without changing the entry count the guard checks, silently misattributing symbols on replay. That residual exposure is within the "not host-crash durable" boundary; a per-entry or running CRC in the side-file would close it.
On failover to a node advertising a smaller batch cap, a symbol accepted under a larger/absent cap can exceed the new cap during the catch-up and latch a terminal error that does not self-recover if a later node re-advertises a larger cap (the producer must be resumed after the data is re-sent). This cannot happen on a homogeneous cluster -- a symbol that fit inside a data frame under a given cap always fits the smaller catch-up frame under the same cap -- so it only affects heterogeneous/rolling-cap clusters or an operator lowering the cap below existing data.

Test plan

DeltaDictCatchUpTest -- reconnect catch-up rebuilds the dictionary (memory mode); a large dictionary splits across multiple catch-up frames under a small advertised batch cap and reassembles gap-free.
DeltaDictRecoveryTest -- a recovered file-mode slot replays its delta frames against a fresh server; a torn (host-crash) dictionary fails cleanly instead of corrupting.
PersistedSymbolDictTest -- side-file append/read/orphan-removal round trips.
SelfSufficientFramesTest, ReconnectTest -- full-dict fallback and reconnect replay still hold.
The enterprise SqlFailoverQwpClientLosslessTest (file-mode failover) passes end-to-end against a real server.

🤖 Generated with Claude Code

Previously every QWP ingress message re-sent the entire symbol dictionary, so a connection with many distinct symbols paid to retransmit the whole dictionary on every message. The client now sends each symbol id to the server only once per connection. Memory mode: - The producer keeps a monotonic "sent" watermark and each frame carries only the ids above it (a delta section), instead of the full dictionary from id 0. - On reconnect or failover the fresh server has an empty dictionary, so the I/O thread replays the whole dictionary as a catch-up frame before any post-reconnect traffic, keeping the producer's monotonic baseline valid across the wire boundary. Store-and-forward (file mode): - Each slot persists its dictionary to a dot-prefixed side-file (PersistedSymbolDict) using write-ahead ordering: new symbols are appended before the referencing frame is published, so a recovered or orphan-drained slot on a fresh process can always rebuild the dictionary that a delta frame references. - The persistence does not fsync, matching the rest of store-and-forward, which is process-crash durable (the page cache survives) but not host-crash durable. A host crash that tears the dictionary is caught at replay by a guard that fails the send cleanly ("resend required") instead of transmitting a gapped frame that would corrupt the table. Catch-up split: - The reconnect/recovery catch-up splits across as many frames as the server's advertised batch cap requires, so a dictionary larger than the cap is re-registered without any single frame exceeding it. The frames carry contiguous id ranges and reassemble on the server exactly as the original per-frame deltas would. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Update the java-questdb-client submodule to de86197, which makes the QWP client register each symbol id with the server only once per connection (delta symbol dictionary) instead of re-sending the whole dictionary on every ingress message. The OSS server already parses delta symbol-dictionary frames, so this is the OSS half of a tandem pair with the client PR questdb/java-questdb-client#66 and needs no server change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

glasstiger · 2026-07-09T02:00:54Z

Tandem OSS PR (submodule bump): questdb/questdb#7374 — merge together.

The symbol-dictionary catch-up called fail() on a send error, but the catch-up runs inside connectLoop (via swapClient) and, on the initial connect, on the caller thread (via start() -> positionCursorForStart). Calling fail() there re-entered connectLoop. On a reconnect this corrupted the wire mapping: the outer setWireBaselineWithCatchUp overwrote fsnAtZero while nextWireSeq kept the nested attempt's value, so a later ACK translated through engine.acknowledge(fsnAtZero + wireSeq) and trimmed un-acked frames from the store-and-forward log -- silent data loss. A flapping connection recursed connectLoop until the stack overflowed into a terminal, turning a transient outage into a hard failure (breaking Invariant B). On the initial connect the same fail() ran connectLoop on the caller thread and blocked Sender construction forever. sendDictCatchUp and sendCatchUpChunk now throw CatchUpSendException instead of calling fail(). connectLoop's own retry catch handles the swapClient path (one non-re-entrant reconnect with backoff); trySendOne's orphan-retire re-anchor turns it into a fresh fail() from the I/O loop body; start() drops the dead client so the I/O thread reconnects and re-sends the catch-up off the caller thread. A single dictionary entry too large for the server batch cap is non-retriable, so it latches a terminal (recordFatal) rather than looping -- also removing the oversized-entry reconnect livelock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

persistNewSymbolsBeforePublish keyed the append range off sentMaxSymbolId+1. That watermark only advances after the whole frame is published, whereas PersistedSymbolDict.size() advances per persisted entry. If a mid-batch appendSymbol threw (a short write on a full disk), the symbols before the failing one were already durable but the frame was not published, so sentMaxSymbolId stayed put. A retry then re-keyed from sentMaxSymbolId+1 and re-appended that already-persisted prefix, duplicating entries and breaking the dense id->symbol mapping recovery relies on (entry i must be symbol id i) -- a torn dictionary that re-registers the wrong symbols on the fresh server, or diverges the producer's watermark from the I/O thread's mirror. Resume from pd.size() instead: it is exactly the count already durable, so the retry continues past the persisted prefix (the next append overwrites any torn trailing bytes) without duplicating. In the happy path pd.size() equals sentMaxSymbolId+1, so behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Add a regression test: a dictionary entry larger than the reconnect server's per-chunk catch-up budget must latch a clean terminal, not reconnect-loop. Connection 1 advertises no cap so a ~200-byte symbol registers into the sent-dictionary mirror; the handler then shrinks the advertised cap and drops the socket, so the reconnect's catch-up cannot re-ship the entry. The test asserts the surfaced terminal names the catch-up path ("... during catch-up"). Reverting the fix (entry-too-large calling fail() again) fails this test with a StackOverflowError on the I/O thread -- the catch-up re-entering connectLoop -- confirming the guard bites both ways. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

persistNewSymbolsBeforePublish appended each new symbol with its own PersistedSymbolDict.appendSymbol call, and each appendSymbol issues one positioned write. A high-cardinality batch -- one new symbol per row, which is exactly the store-and-forward workload delta encoding targets -- therefore stalled the producer thread with up to one pwrite syscall per row per flush. Add PersistedSymbolDict.appendSymbols(dict, from, to): it encodes the whole [from..to] entry region into scratch once and issues a single positioned write, so a flush that introduces N symbols costs one syscall instead of N. It keeps appendSymbol's durability and idempotency contract -- no fsync, and a short write throws without advancing size, so a retry keyed off size() re-encodes and overwrites at the same offset. PersistedSymbolDictTest.testAppendSymbolsBatchWritesDenseRange checks the batched write produces the same dense, id-ordered file (including an empty symbol mid-range), that an empty range is a no-op, and that a follow-on batch keyed off the recovered size continues without a gap or duplicate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

On recovery / orphan-drain the CursorWebSocketSendLoop constructor seeds a native mirror (sentDictBytesAddr) from the slot's persisted dictionary so the first connection can re-register it. That mirror is freed only on ioLoop's exit path, so a loop that is constructed but never runs -- start() never called, or Thread.start() failing before the loop runs, or a close() racing an unstarted loop -- leaked it. close() already safety-nets the client for that same "loop never started" case; the mirror was missed. close() now frees the mirror when the loop never ran (ioThread was null on entry). It does NOT free it when the loop ran: ioLoop's exit owns the free there, and on the failed-stop path the thread may still be mid-send, so touching the mirror would race; a duplicate close observes a zero address and skips. CursorWebSocketSendLoopMirrorLeakTest populates a recoverable slot, then leak-checks constructing an engine + loop over it and closing WITHOUT start(). Reverting the free fails it with a 4096-byte NATIVE_DEFAULT leak. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

testRecoveredSlotReplaysDeltaFramesAgainstFreshServer never acked in phase 1, so recovery replayed from the very first frame -- whose delta already starts at id 0. The replayed frames were thus self-sufficient from 0, and the reconstructed-dictionary assertions passed whether or not the seeded catch-up carried the right symbols (or any at all). Only the sawCatchUpFrame existence check was load-bearing. Stamp the ack watermark at FSN DISTINCT_SYMBOLS-1 between the phases so recovery replays from the first frame past the symbol-introducing cycle: a frame with deltaStart=DISTINCT_SYMBOLS carrying no new symbols. The early ids it references now exist only in the persisted dictionary, so the reconstructed dictionary is complete solely because the catch-up re-registered them. Verified both ways: with a catch-up that sends a table-less frame but no symbols, the pre-change test still passes (the head frames carry the dictionary) while the stamped test fails at "dictionary id 0 expected sym-0 but was null". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

When a disk-mode slot's .symbol-dict cannot be opened, the engine reports delta encoding as unavailable and the sender must fall back to self-sufficient frames -- every batch re-ships the whole dictionary from id 0 -- because a recovered slot would have no dictionary to rebuild non-self-sufficient deltas from. Nothing exercised that path. Add a test that plants a directory where the dictionary file belongs, so openRW / openCleanRW fail and open() returns null. It then asserts both batches ship deltaStart=0 and that batch 2 re-ships the whole dictionary (deltaCount=2), rather than the monotonic delta (deltaStart=1, deltaCount=1) the enabled path emits. Verified it bites: forcing isDeltaDictEnabled() to stay true regresses batch 2 to deltaStart=1 and the test fails. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

openExisting parsed complete entries and set appendOffset past the last one, but left the file at its full length. A crash mid-append leaves a torn trailing record; if the next append after recovery is SHORTER than that torn tail, it overwrites only the tail's prefix and leaves residue beyond its own end. A later recovery can then mis-parse that residue as a ghost symbol, shifting every subsequent dense id -- so the "self-healing tail" guarantee was not actually airtight. open() now truncates the file to the end of the last complete entry (ftruncate) so nothing survives past appendOffset. Best-effort: a failed truncate falls back to the prior overwrite-from-appendOffset behaviour. testTornTrailingEntrySelfHeals now asserts the file returns to its clean length after the reopen; reverting the truncate fails it (19 vs 16 bytes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The I/O thread's lifetime-monotonic symbol-dictionary mirror is sized with int math: accumulateSentDict passed sentDictBytesLen + regionBytes (an int sum) to ensureSentDictCapacity, and the grow step doubled capacity*2, also int. On a pathological, very-high-cardinality connection the sum overflows negative -- so the capacity check passes and copyMemory scribbles past the buffer (silent heap corruption) -- and capacity*2 overflows negative near 1 GB, degrading the doubling to exact-fit reallocs. Reaching this needs ~200M+ distinct symbols on one connection, far past any real workload, but the failure mode is silent corruption. ensureSentDictCapacity now takes a long, the caller passes a long sum, and the method throws a LineSenderException above an int-addressable ceiling (Integer.MAX_VALUE - 8) instead of overflowing, growing in long math clamped to that ceiling. Defensive only -- not reachable at realistic symbol cardinality, so there is no scale test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

bluestreak01

@glasstiger

Review of PR #66 — `feat(qwp): stop resending the full symbol dictionary on every message`

Reviewing at level 3 (full mission-critical pass: all steps, all reviewer dimensions, per-finding source verification). Note: the subagent tool is unavailable in this environment, so the parallel-reviewer passes and per-finding verification were run inline by the parent session using read/bash against the source and a local build+test run — not delegated. Every finding below was verified against the cited source lines; false positives are listed in Downgraded.

Build/test evidence: mvn -pl core compile clean on JDK 25; DeltaDictCatchUpTest, DeltaDictRecoveryTest, PersistedSymbolDictTest, SelfSufficientFramesTest, ReconnectTest → 15 tests, 0 failures.

Committed-binary gate: PASS — git diff --numstat shows no binary files; all 10 changed files are .java with numeric line counts.

Critical

C1 — Persisted `.symbol-dict` accumulates duplicate entries when `appendBlocking` fails and a later flush succeeds → silent symbol corruption on recovery (file mode, delta enabled). [in-diff]

File: core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java:3660-3676 (persistNewSymbolsBeforePublish), triggered via flushPendingRows (3491/3498) and flushPendingRowsSplit (3574/3582).

Code-path trace (verified):

flushPendingRows runs, in order:

persistNewSymbolsBeforePublish();   // 3491 — appends [sentMaxSymbolId+1 .. currentBatchMaxSymbolId] to .symbol-dict (Files.write, no fsync)
activeBuffer.write(...);            // 3494
sealAndSwapBuffer();               // 3495 — calls cursorEngine.appendBlocking(); CAN THROW
advanceSentMaxSymbolId();          // 3498 — SKIPPED on throw
...
resetTableBuffersAfterFlush(keys); // SKIPPED on throw → rows + currentBatchMaxSymbolId preserved

sealAndSwapBuffer → appendBlocking throws LineSenderException("cursor SF append failed", …) on the two documented conditions (QwpWebSocketSender.java:3768,3783-3785): backpressure deadline (the SF ring hit sf_max_total_bytes and did not drain — i.e. exactly the store-and-forward stress scenario, server slow/down) and PAYLOAD_TOO_LARGE. The I/O loop is not failed, so cursorSendLoop.checkError() passes and the sender stays open and usable.

On the throw: the frame's new symbols are already durably on disk (persist ran before sealAndSwapBuffer), but sentMaxSymbolId was not advanced (advanceSentMaxSymbolId at 3498 skipped) and the table buffers/currentBatchMaxSymbolId are not reset (resetTableBuffersAfterFlush skipped — verified: currentBatchMaxSymbolId is reset only at 3607, 3686, and inside resetTableBuffersAfterFlush, none of which run on this path).

The next successful flush() (a transient backpressure clears the moment the server catches up) re-enters persistNewSymbolsBeforePublish with the same from = sentMaxSymbolId + 1 (3668) and to = currentBatchMaxSymbolId (3669) — because pd.appendSymbol has no dedup (PersistedSymbolDict.java:appendSymbol) and nothing rolled back the earlier append, the failed frame's symbols are written to the file a second time. The file's positional invariant ("symbol id i is the i-th entry", PersistedSymbolDict.java class doc) is now broken.

Impact on recovery/orphan-drain (a fresh process reads the file):

seedGlobalDictionaryFromPersisted (2243/3695) calls getOrAddSymbol, which de-dupes → producer globalSymbolDictionary.size() and sentMaxSymbolId are below the file's entry count.
The send loop's constructor seeds the mirror directly from the raw file bytes with sentDictCount = pd.size() (CursorWebSocketSendLoop.java:515-522), i.e. including the duplicate.
sendDictCatchUp re-registers the duplicated mirror on the fresh server, so every global id above the duplicate is shifted by +1.
Symbol column cells are encoded as absolute global ids (QwpColumnWriter.writeSymbolColumnWithGlobalIds, line 277 buffer.putVarint(globalId)). The replayed frames carry the original ids, which now resolve against the shifted server dictionary → rows get the wrong symbol values, silently. The torn-dictionary guard does not catch this (deltaStart never exceeds the now-larger sentDictCount, so trySendOne at 2223-2238 passes).

This is a store-and-forward data-integrity violation triggered by an ordinary transient outage — the exact failure class SF exists to survive.

Suggested fix: base the append range on the true persist watermark, not the wire baseline. pd.size() already tracks how many symbols are durably persisted at contiguous ids 0..size-1:

int from = pd.size();          // instead of sentMaxSymbolId + 1
int to = currentBatchMaxSymbolId;
if (to < from) return;
for (int id = from; id <= to; id++) pd.appendSymbol(globalSymbolDictionary.getSymbol(id));

In the happy path pd.size() == sentMaxSymbolId + 1, so behavior is identical; after a failed append it skips the already-persisted ids, making the operation idempotent across retries. Add a regression test: file mode + delta, force an appendBlocking failure (small sf_max_bytes + silent server), then a successful flush, then assert .symbol-dict has no duplicate and a fresh-process recovery reconstructs the dictionary gap-free.

C2 — Required Enterprise failover tandem is missing/unlinked; the HA path this feature targets is UNTESTED in CI (Step 2.7 gate). [tandem]

Verification (commands recorded):

OSS tandem: gh pr list --repo questdb/questdb --head qwp-delta-symbol-dict → #7374 present, matching branch, bidirectionally linked (body: "Tandem OSS half of #66"; a PR comment links back). It is a submodule bump only — "The OSS server already parses delta symbol-dictionary frames, so no server change is required." Its CI covers single-node QWP e2e.
Enterprise tandem: gh pr list --repo questdb/questdb-enterprise --head qwp-delta-symbol-dict → empty. gh can reach the private enterprise repo (confirmed), and a scan of the 60 most-recent enterprise PRs shows no client-bump/qwp-symbol-dict PR. SqlFailoverQwpClientLosslessTest exists in enterprise (questdb-ent/src/test/java/com/questdb/lifecycle/), and the PR body claims it "passes end-to-end against a real server" — but with no enterprise PR bumping the client submodule, that test runs against the old client in enterprise CI, not this change.

Why this trips the gate: the change is squarely HA-facing — it rewrites the SF drainer's on-the-wire framing, adds reconnect/failover dictionary catch-up (swapClient → setWireBaselineWithCatchUp → sendDictCatchUp), and adds recovery/orphan-drain dictionary rebuild. The headline benefit (dictionary survives a reconnect/failover) is only proven end-to-end by the enterprise failover suite the PR itself names. Per Step 2.7, a required-but-missing tandem is Critical and every behavior it would cover is treated as UNTESTED. The client-local loopback tests (C-tier coverage below) are strong, but they cannot prove (a) a real server accepts and correctly registers a 0-table catch-up frame mid-stream, or (b) primary→replica failover preserves the dictionary.

Required action: open (or link) the enterprise tandem that bumps the client submodule to this SHA and runs SqlFailoverQwpClientLosslessTest (and, ideally, a kill-9 recovery variant in the enterprise e2e-python suite for the file-mode host-crash/torn-dict path, which the unit test only simulates by truncating the file). Also confirm OSS #7374's e2e actually drives a reconnect (so the catch-up frame is exercised against a real server), not just a single connected ingest.

Moderate

M1 — One `Files.write` syscall per new symbol on the producer thread. [in-diff]

persistNewSymbolsBeforePublish (3660-3676) loops pd.appendSymbol(...), and each appendSymbol (PersistedSymbolDict.java) issues its own Files.write(fd, …) (one pwrite). A frame that introduces K new symbols does K syscalls on the user/producer thread. This is per-new-symbol (not per-row), so it's bounded by dictionary growth, but a high-cardinality first batch will burst syscalls synchronously in the flush path. Batch the frame's whole new-symbol range into a single scratch buffer and one Files.write. Not zero-GC-blocking (no allocation), but avoidable syscall amplification on the ingestion path.

M2 — `accumulateSentDict` silently drops symbols on a partial-overlap delta. [in-diff]

CursorWebSocketSendLoop.java:1946-1960: the guard is if (deltaCount <= 0 || deltaStart != sentDictCount) return;. A delta with deltaStart < sentDictCount and deltaStart + deltaCount > sentDictCount (overlaps the tip and extends past it) is dropped entirely — the new tail symbols never enter the mirror, so a later catch-up would be incomplete (→ the same shifted-id corruption as C1). I verified this is currently unreachable: the producer emits strictly contiguous, non-overlapping deltas (beginMessage computes deltaStart = confirmedMaxId+1; advanceSentMaxSymbolId moves the baseline to exactly currentBatchMaxSymbolId), and recovery seeds sentDictCount from a superset, so deltaStart < sentDictCount ⇒ deltaStart+deltaCount ≤ sentDictCount. But it is load-bearing correctness resting on an invariant enforced elsewhere. Harden it: handle the partial overlap (accumulate only the [sentDictCount .. deltaStart+deltaCount) tail) or assert deltaStart + deltaCount <= sentDictCount so a future producer change fails loudly instead of silently corrupting the mirror.

Minor

m1 — Stale "self-sufficient / delta from id 0" comments now contradict delta mode.

QwpWebSocketSender.java:3392, 3398-3399, and 3777 still say cursor frames are "self-sufficient (every frame carries … a symbol-dict delta from id 0)". In delta mode frames are explicitly not self-sufficient (the whole point of the PR), and the 3777 comment ("next batch re-emits … symbol-dict delta from id 0") describes behavior that no longer happens. Update to match the new baseline semantics to avoid misleading a future reader on the recovery/retry path (which is exactly where C1 lives).

m2 — Memory-mode mirror double-stores the dictionary.

The I/O-thread mirror (sentDictBytes*) holds every symbol's UTF-8 bytes while globalSymbolDictionary already holds them as Java Strings. Bounded by distinct-symbol count (not per-row), so acceptable, but worth a comment that memory-mode steady-state native footprint is ~2× the dictionary size for the reconnect-catch-up capability.

Downgraded (false positives — verified against source)

Negative fsnAtZero on fresh recovery (replayStart=0 ⇒ fsnAtZero = -catchUpFrames) corrupts ack accounting — dismissed. SegmentRing.acknowledge clamps to publishedFsn and no-ops when seq ≤ ackedFsn (339-349); the catch-up frame maps to an already-acked/nonexistent low FSN and its ack is a harmless no-op. DeltaDictRecoveryTest exercises exactly this (silent server, nothing acked) and passes.
pd.size() read race in the send-loop constructor vs producer appendSymbol — dismissed. The loop is constructed during sender build/startCursorSendLoop (or on the drainer thread with no producer at all), which happens-before the first user send; no concurrent append occurs, so sentDictCount == loadedEntries count.
Catch-up frame double-advances the durable-ack watermark — dismissed. The catch-up frame's OK enqueues a tableCount=0 (trivially durable) pending entry mapping to an ≤ackedFsn FSN; drainPendingDurable acks a no-op. Cumulative ack semantics make a missing catch-up OK harmless too.
Catch-up (non-DEFER_COMMIT) frame prematurely commits deferred WAL on reconnect — dismissed. It is the first frame on a fresh server connection, which holds no pending WAL state; committing nothing is a no-op before the deferred replay frames arrive.
positionCursorForStart re-sends a catch-up when retiring an orphan tail — dismissed. That branch is guarded by nextWireSeq == 0 (trySendOne 2166-2175), which cannot hold after sendDictCatchUp incremented nextWireSeq; when sentDictCount==0 there is nothing to re-send.
A symbol larger than the batch cap breaks catch-up — dismissed. The original data frame carrying that symbol (plus row data) would already exceed the cap and fail; the catch-up (symbol only, less overhead) is strictly smaller, so sendDictCatchUp's entryBytes > budget terminal is consistent, not a new failure.
Java 8 floor violations in new code — dismissed. No var, text blocks, instanceof patterns, List.of, etc. in the changed main files; the one -> is a pre-existing lambda. Compiles clean on JDK 25.
PersistedSymbolDict uses slf4j instead of QuestDB Log — dismissed. Its sibling SF-cursor classes (AckWatermark, SegmentRing, CursorSendEngine, the send loop) all use slf4j; this is consistent.

Coverage map

#	Behavioral change	Test (local unless noted)	Failure link	Dimensions	Verdict
1	Memory-mode monotonic delta (`symbolDeltaBaseline` in `beginMessage`)	`SelfSufficientFramesTest.testMemoryModeShipsMonotonicDelta`	asserts batch-2 `deltaStart=1,deltaCount=1` — fails if baseline reverts to -1	happy ✓; NULL N-A; boundary (2 symbols) ✓; concurrency N-A	TESTED
2	File-mode delta + write-ahead persist	`SelfSufficientFramesTest.testFileModeShipsMonotonicDeltaAndPersistsDict`	asserts monotonic delta + `.symbol-dict` retains both symbols	happy ✓; resource (dict file) ✓	TESTED
3	Reconnect catch-up (memory)	`DeltaDictCatchUpTest.testReconnectCatchUpRebuildsDictionary`	reconstructs conn-2 dict from wire; fails on null gap	happy ✓; reconnect ✓ (loopback)	TESTED
4	Split catch-up under batch cap	`DeltaDictCatchUpTest.testReconnectCatchUpSplitsLargeDictionaryAcrossFrames`	asserts ≥2 zero-table frames + gap-free reassembly	boundary (cap) ✓	TESTED
5	File-mode recovery replay to fresh server	`DeltaDictRecoveryTest.testRecoveredSlotReplaysDeltaFramesAgainstFreshServer`	asserts catch-up frame seen + gap-free dict	recovery ✓ (loopback); memory-leak N-A	TESTED
6	Torn-dictionary guard (simulated host crash)	`DeltaDictRecoveryTest.testTornDictionaryFailsCleanlyInsteadOfCorrupting`	asserts 0 frames replayed + terminal "incomplete" error	error path ✓	TESTED
7	`PersistedSymbolDict` open/append/reopen/torn-tail/bad-magic/removeOrphan	`PersistedSymbolDictTest` (5 tests, `assertMemoryLeak`)	round-trip + self-heal asserts	happy/boundary/empty-symbol/resource ✓	TESTED
8	`appendBlocking` failure → persist-then-retry dict duplication (file mode)	none (recorded search: no test references `appendBlocking`/backpressure/dup + persisted dict)	—	error+retry ✗; recovery-after-retry ✗	UNTESTED → Critical (C1)
9	Real-server 0-table catch-up acceptance + primary→replica failover	OSS tandem #7374 (single-node only); Enterprise tandem missing	—	real-server/failover ✗	UNTESTED → Critical (C2)
10	`seedGlobalDictionaryFromPersisted` id/baseline resume on recovery	indirect via `DeltaDictRecoveryTest` #5	dict reconstructed gap-free implies correct seed	happy ✓; retry-dup interaction ✗ (see C1)	TESTED (partial)

Summary

Verdict: REQUEST CHANGES.

The design is careful and the write-ahead/torn-dictionary reasoning is largely sound, but two blocking issues stand:

C1 (data integrity): a transient appendBlocking backpressure failure followed by any successful flush duplicates the failed frame's symbols in the persisted .symbol-dict; a later recovery/orphan-drain then silently misattributes symbol values via shifted global ids. This is a store-and-forward correctness violation on the very outage class SF exists to survive, it has no regression test, and the fix is small (base the persist range on pd.size()).
C2 (test gate): the HA failover behavior the feature targets has no linked, CI-running enterprise tandem; the OSS tandem #7374 covers single-node only.

Test & tandem gate: FAILS — one UNTESTED-Critical bug-fix-worthy path (C1, no regression test) and a required-but-missing Enterprise tandem (C2). Cannot approve.
Zero-GC gate: PASSES — no steady-state per-row/per-producer-call allocation on the ingestion path; producer-side additions (symbolDeltaBaseline, advanceSentMaxSymbolId, persistNewSymbolsBeforePublish) allocate nothing (M1 is syscall amplification, not GC). Catch-up/mirror allocations are I/O-thread, reconnect-only.
Coverage map: 10 behavioral-change groups — 8 tested locally (loopback), 2 UNTESTED (dict-dup-on-retry; HA-failover tandem).
Tandem status: OSS e2e tandem linked (#7374, single-node); Enterprise failover tandem required and missing; enterprise e2e-python kill-recovery coverage for the host-crash/torn-dict path recommended.
Findings: 6 verified (2 Critical, 2 Moderate, 2 Minor); 8 draft findings dropped as false positives after source verification.
In-diff vs out-of-diff: 4 in-diff (C1, M1, M2, m1), 1 tandem/process (C2), 1 cross-cutting (m2). The C1 mechanism spans the new persistNewSymbolsBeforePublish (in-diff) and the pre-existing sealAndSwapBuffer/appendBlocking failure path (out-of-diff) it now interacts with — the classic "diff quietly changed a contract at an unchanged callsite" case.

trySendOne decoded a frame's delta header twice: the pre-send torn-dictionary guard called frameDeltaStart (magic/flags check + start-id varint), then post-send accumulateSentDict re-ran isDeltaFrame and re-read the start id before reading deltaCount. Both run on every delta frame on the I/O send path. Decode the start id once in the guard, hoist the frame address into a local, and pass the start id into accumulateSentDict, which now locates deltaCount just past the canonical start-id encoding (via NativeBufferWriter.varintSize) instead of re-parsing the header. The non-delta-frame case is carried by the same start id (-1), so the post- send mirror update runs exactly when it did before. Also move the accumulateSentDict javadoc onto accumulateSentDict: it had drifted above frameDeltaStart (which kept its own doc), leaving accumulateSentDict undocumented. The per-entry region walk (to size the mirror copy) remains; eliminating it needs a wire-level deltaBytes field, a server-side change out of scope for this client fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Several comments predated file-mode delta encoding and claimed every cursor frame is self-sufficient with a "symbol-dict delta from id 0". That is now only the fallback: in delta mode (memory mode, and file mode when the persisted dictionary opened) frames carry monotonic deltas that are NOT self-sufficient, and the fresh server's dictionary is re-established by an I/O-thread catch-up frame before replay. The worst offender was the deltaDictEnabled field doc ("Enabled only in memory-mode ... File-mode keeps full self-sufficient frames"), which directly contradicted the feature. Corrected it plus the two ensureConnected call-site comments, the append-failed-path comment, and the wasRecoveredFromDisk field doc (schema stays self-sufficient per frame; the dictionary does not). No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Two robustness fixes to the delta symbol-dictionary tests. Deterministic synchronization (replaces fixed sleeps): - DeltaDictCatchUpTest waited a fixed 200 ms for the server to close connection 1 before sending batch 2. On a loaded machine that could under-wait and let batch 2 race into connection 1's pre-close window, changing which connection the catch-up lands on. The handler now sets a conn1Closed flag after it closes the socket, and the test waits on that. - DeltaDictRecoveryTest's torn-dictionary test slept a fixed 1 s to let the replay guard fire before close(). It now polls flush() for the latched terminal (close() remains the fallback), so it captures the terminal as soon as it fires -- the run dropped from ~1 s to ~0.3 s. Leak checks: the Sender-based tests allocate native memory (the send-loop mirror, persisted-dict buffers, segment mmaps) but were not wrapped in assertMemoryLeak, unlike the rest of the suite. Wrap all eight methods across the three classes; every one is balanced (they already cleaned up via try-with-resources -- the wrapper now guards against future leaks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

flushPendingRowsSplit fires when one flush's encoded size exceeds the server's batch cap: it emits one frame per table. The first frame must carry the whole batch's symbol-dict delta and advance the baseline, and the remaining frames must carry an empty delta that only references ids the first frame already registered -- otherwise a fresh server would see dangling symbol ids. No test drove that producer-side split. Add a test that buffers two padded tables into one flush under a small advertised cap, so the batch splits, and asserts the first frame ships deltaStart=0/deltaCount=2 while the second ships deltaStart=2/deltaCount=0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

accumulateSentDict dropped a frame entirely whenever deltaStart != sentDictCount. A delta that overlaps the mirror tip and extends past it (deltaStart < sentDictCount < deltaStart+deltaCount) was therefore discarded whole -- the new tail symbols never entered the mirror, which would leave a later reconnect catch-up incomplete and shift server-side ids. The producer only ever emits strictly contiguous, non-overlapping deltas, so this is currently unreachable, but it is load-bearing correctness resting on an invariant enforced elsewhere. Handle the overlap: skip the already-held prefix [deltaStart, sentDictCount) and copy only the new tail [sentDictCount, deltaStart+deltaCount). The steady-state case (deltaStart == sentDictCount) has skip == 0, so it is unchanged and free. A gap (deltaStart > sentDictCount, which the torn-dictionary guard rejects before send) now bails explicitly rather than implicitly. Also document that the I/O-thread mirror is a second, native copy of the dictionary (the producer's GlobalSymbolDictionary already holds the same symbols as Java Strings) -- so a memory-mode connection's steady-state dictionary footprint is ~2x the symbol set, an intentional cost of the reconnect-catch-up capability. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Regression test for the write-ahead persist path: persistNewSymbolsBefore- Publish runs before the frame is published (sealAndSwapBuffer -> appendBlocking). If publish fails after the persist -- here PAYLOAD_TOO_LARGE (a frame bigger than the SF segment), a backpressure deadline in production -- the symbols are already on disk but sentMaxSymbolId is not advanced and the rows stay buffered, so a retry re-runs the persist. The fix keys the persist range off pd.size() (idempotent); this pins it. The test drives one new-symbol row whose padded frame exceeds a 1 KB segment, flushes it twice (both fail to publish), then asserts the persisted .symbol-dict holds the symbol exactly once. Reverting the fix to sentMaxSymbolId+1 fails it with size 2 -- the duplicate that shifts every later global id and silently misattributes symbol values on recovery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…va-questdb-client into qwp-delta-symbol-dict

setWireBaselineWithCatchUp anchors fsnAtZero = replayStart - catchUpFrames so every catch-up frame maps to an already-acked FSN. Dropping the - catchUpFrames term is silent data loss: a server ACK for a catch-up frame then translates to an FSN at or above replayStart and trims a not-yet-delivered data frame from the store-and-forward log. The existing catch-up tests reconstruct the dictionary from wire bytes and never assert ACK/trim accounting, so they were blind to this line; the enterprise SqlFailoverQwpClientLosslessTest ingests no symbols and never enters the catch-up path at all. CursorWebSocketSendLoopCatchUpAlignmentTest drives the catch-up against a stub client and asserts the catch-up frame's OK leaves the real engine's ackedFsn untouched, for both a single catch-up frame and a split (multi-frame) catch-up. Reverting the - catchUpFrames term fails both. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

sendCatchUpChunk throws CatchUpSendException on a transient wire failure instead of calling fail(). From inside the catch-up fail() re-enters connectLoop -- desyncing the fsnAtZero/nextWireSeq wire mapping (a later ACK then trims un-acked store-and-forward frames), or overflowing the stack on a flapping connection -- turning a transient outage into a hard failure. Only the oversized-entry (non-retriable) terminal was covered; the retriable path had no test. testTransientCatchUpSendFailureIsRetriableNotTerminal drives the catch-up against a stub whose sendBinary throws, and asserts the failure surfaces as a retriable CatchUpSendException and leaves the producer-facing error latch clear. Reverting the throw to fail() fails it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Four minor cleanups on the delta symbol-dictionary catch-up, all behaviour-preserving on every reachable path: - The sentDict* field comment said the catch-up mirror is memory-mode only; it is also seeded and used in disk mode on a recovered / orphan-drained slot. Corrected. - positionCursorAt's javadoc said it runs after nextWireSeq was reset to 0, but the catch-up path leaves nextWireSeq past the frames it emitted. Corrected to describe setWireBaselineWithCatchUp anchoring the wire baseline; the method only moves the byte cursor. - The recovery-seed constructor set sentDictCount = pd.size() outside the loadedEntriesLen > 0 block. A recovered slot always has entries when size > 0, so the result is unchanged, but coupling the count to the mirror bytes stops sentDictCount ever claiming symbols the mirror does not hold. - sendDictCatchUp used Integer.MAX_VALUE as the no-cap per-frame budget, so sendCatchUpChunk's int frameLen could overflow on a multi-GB dictionary. Bound it by MAX_SENT_DICT_BYTES, the same ceiling ensureSentDictCapacity enforces. Unreachable at real cardinality (~200M+ symbols); defensive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Close three ways the delta symbol-dictionary feature could lose or corrupt data on the reconnect and store-and-forward recovery paths. Run the torn-dictionary guard unconditionally. trySendOne gated the guard on deltaDictEnabled, which CursorSendEngine reports false when a recovered disk slot cannot open its persisted dictionary (fd exhaustion, a read-only remount, ENOSPC). The recorded frames are still delta frames, so replaying them against a fresh empty-dictionary server null-padded the missing ids and silently corrupted the table. The guard now decodes the delta start for every frame and fails terminally on a gap regardless of the flag; only the sent-dictionary mirror stays gated. Stop treating a catch-up frame as the head data frame. sendCatchUpChunk advances nextWireSeq, but onClose's poison-strike gate and handleServerRejection's pre-send gate read nextWireSeq > 0 as "a data frame was sent". A transient non-orderly close or NACK after the catch-up but before the first replay frame then charged a poison strike on a frame that never left, and after a few flaps escalated a transient outage to a PROTOCOL_VIOLATION terminal that quarantines an orphan drainer. A new dataFrameSentThisConnection flag, set only after a real ring frame sends, now gates both decisions, so the drainer keeps retrying as Invariant B requires. Bound the commit message's dictionary delta to the sent watermark. sendCommitMessage skips the write-ahead persist yet encoded a delta up to currentBatchMaxSymbolId, so a symbol left in the batch by a cancelled row (cancelRow rolls back neither currentBatchMaxSymbolId nor the global registration) rode out on the commit frame without being persisted. A recovered slot then under-seeded the producer against the surviving frame and misattributed the reused id. The commit now caps the delta at sentMaxSymbolId in delta mode, giving an empty delta. Each fix carries a regression test proven to fail when the fix is reverted: a directory-shadowed .symbol-dict (guard), a close after only the catch-up (poison gate), and a cancelled-row symbol on a transactional commit (delta bound). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

On recovery the send loop copied the persisted dictionary's loaded-entries buffer into a fresh mirror allocation and left PersistedSymbolDict holding a second copy for the engine's lifetime -- roughly twice the dictionary size in native memory on a high-cardinality recovered slot, retained long after the one-time seed. The loop now adopts that buffer as its mirror backing via takeLoadedEntries(), which transfers ownership so the dictionary no longer retains or frees it. The producer's readLoadedSymbols() is the only other consumer and runs first (setCursorEngine seeds the producer before the loop is built; the drainer has no producer consumer), guarded by an assert. Add a recover-then-continue-ingest test. A file-mode sender writes symbols and crashes; a fresh sender recovers the slot and ingests a NEW symbol. It asserts the producer continues the dictionary from the recovered size instead of colliding at id 0, exercising seedGlobalDictionaryFromPersisted, which no prior test drove past recovery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Two fixes surfaced by review of the delta symbol-dictionary change. Recover oversized persisted symbols (C1). PersistedSymbolDict.openExisting rejected any entry larger than a fixed 1 MB ceiling as "oversized" and truncated the dictionary at that id, but the write path (appendSymbol/appendSymbols) caps nothing. A symbol value over 1 MB therefore persisted fine, yet a normal process-crash recovery dropped it and every higher id; the send loop's replay guard then fired a spurious "host crash / resend required" terminal -- a store-and-forward process-crash-durability violation on a reachable input (nothing caps symbol value length). Drop the fixed per-entry ceiling and bound entries only by the file length, which is the actual corruption guard and was already present. Keep the length in a long so a corrupt multi-gigabyte varint cannot wrap an int back under the check. The write and read paths now agree, so a legitimately persisted large symbol recovers. Guard the catch-up NACK pre-send gate (C2). handleServerRejection keys its pre-send branch off dataFrameSentThisConnection, because the symbol catch-up advances nextWireSeq without sending a data frame. That branch had no regression test: every existing NACK test sets both flags together, so reverting the gate to the old nextWireSeq-based predicate left the suite green. Add the server-NACK twin of testNonOrderlyCloseAfterOnlyCatchUpDoesNotStrike -- a WRITE_ERROR NACK of a catch-up frame must take the pre-send path (surface and recycle, no strike), not escalate a transient outage to a producer-fatal PROTOCOL_VIOLATION terminal. Both regression tests fail with their production line reverted and pass with it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

M1: persistNewSymbolsBeforePublish re-encoded each new symbol into a scratch buffer even though beginMessage had just written the identical [len][utf8] bytes into the frame's symbol-dict delta section. The encoder now records that entry region's offset and length; the producer writes those bytes straight to the slot .symbol-dict via a new PersistedSymbolDict.appendRawEntries, so a high-cardinality batch no longer re-encodes what the wire frame already carries. The common path (durable size == the frame's delta start id) takes the direct copy; a retry after a failed publish, where the durable size has run ahead of the wire baseline, falls back to re-encoding just the remaining suffix. The persisted bytes are byte-identical either way. M2: document, on the sendDictCatchUp oversized-entry terminal, that in a heterogeneous or rolling-cap cluster a symbol accepted under a larger cap can hit this hard stop on failover to a smaller-cap node and will not self-recover if a later node advertises a larger cap. Behavior is unchanged -- the terminal keeps the homogeneous common case livelock-free; this only records the tradeoff and the two ways to relax it (an ingest-side symbol-size bound, or a settle budget across reconnects before latching). Adds testAppendRawEntriesMatchesAppendSymbols; the file-mode persist, recovery, and failed-publish suites cover both the direct-copy and re-encode branches end to end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Two defensive spots in the delta symbol-dict path had no biting test, and one guard was incomplete (M3). Complete the catch-up frame-size guard (M3b). The int-overflow hardening covered ensureSentDictCapacity (the mirror growth) but not the single catch-up frame: with no server cap, frameLen = HEADER + varints + symbolsLen is an int and would wrap negative as symbolsLen approaches the mirror ceiling, feeding a bad Unsafe.malloc. sendDictCatchUp's budget already keeps each chunk under that bound, so this is unreachable at real cardinality -- but the guard must be local so a future caller cannot overflow it silently. Compute the size in long and fail loud (CatchUpSendException) before the malloc, matching the mirror-side guard. Cover accumulateSentDict's partial-overlap tail (M3a). A delta that straddles the mirror tip (deltaStart < sentDictCount < deltaEnd) must copy only the new tail, not drop the whole frame. The monotonic producer never emits a straddling delta in steady state, so reverting to the pre-fix drop-whole-frame guard passed every test; it is reachable on a torn-dict replay (mirror seeded smaller than a frame's coverage), where dropping the tail would leave the reconnect catch-up incomplete and shift server ids. Add a white-box test that drives the straddle directly. Both new tests fail with their production line reverted (the mirror stays at 1 id; the frame guard falls through to a negative malloc that throws IllegalArgumentException, not the clean terminal) and pass with it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The send loop seeded its reconnect catch-up mirror by calling PersistedSymbolDict.takeLoadedEntries(), a one-shot ownership transfer that zeroed the dictionary's loaded-entries buffer. The foreground sender builds a single loop, so that was safe. But BackgroundDrainer builds a fresh send loop per wire session against the SAME, persistent engine when a durable-ack capability gap forces a mid-drain recycle. The second loop then found an empty loaded-entries buffer, seeded an empty mirror (sentDictCount = 0), sent no reconnect catch-up, and the first replayed delta frame (deltaStart > 0) tripped the torn-dict guard -- falsely quarantining a healthy slot with a bogus "resend required" terminal and defeating the capability-gap settle-retry for delta slots. The send loop now COPIES the loaded entries into its own mirror and leaves the dictionary owning its buffer for the engine's lifetime, so every recycled loop re-seeds. PersistedSymbolDict.close() frees the dictionary's copy; each loop frees its own copy on exit. Peak footprint is unchanged (the drainer closes loop N before building loop N+1). Removes the now-unused takeLoadedEntries() and loadedEntriesTaken, and adds a regression test that builds two loops against one recovered engine and asserts the second re-seeds (pre-fix its mirror was empty). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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>

A fresh store-and-forward slot inherited a stale .symbol-dict when the best-effort removeOrphan failed to delete it (e.g. a Windows share lock) or a crash landed in the close window: open() parses and TRUSTS any survivor. Because the fresh-start producer is not seeded from the dictionary (that is gated on wasRecoveredFromDisk, while the send-loop mirror and the persist resume point key off pd.size()), the producer's ids then diverged from the dictionary the send loop replays, silently misattributing symbol values after a reconnect. The dictionary is load-bearing, unlike the max()-clamped ack watermark whose removeOrphan+open pattern it had copied. Enforce "fresh start -> empty dictionary" at the source: openClean() truncates any survivor via openCleanRW instead of trusting it, and if the clean open itself fails the sender falls back to full self-sufficient frames, which is also safe. The recovery path keeps open(); removeOrphan stays for the fully-drained close. Both regression tests fail without the fix (dict size 2, not 0): - PersistedSymbolDictTest.testOpenCleanDiscardsSurvivingDictionary - EmptyOrphanSlotChurnTest.testFreshStartDiscardsSurvivingStaleDictionary Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Four follow-ups from the delta symbol-dictionary review. Catch-up NACK no longer launders the poison detector. After a reconnect the loop ships the head data frame before tryReceiveAcks reads the server's NACK of a dictionary catch-up frame, so handleServerRejection saw dataFrameSentThisConnection=true and attributed the NACK a wire seq below the replay head -- fsn = fsnAtZero+cappedSeq, negative when replayStart < catchUpFrames. recordHeadRejectionStrike then set poisonFsn to that value (often -1, the "no suspect" sentinel), wiping a genuine in-progress poison run and reporting a bogus fsn. Route an fsn <= ackedFsn rejection to the pre-send path (surface + recycle, no strike, no watermark advance), symmetric with the success path's engine.acknowledge() no-op below ackedFsn. A real replayed data frame is at fsn > ackedFsn, so it is never caught. Regression test: testPostSendCatchUpNackDoesNotStrikeOrLaunderPoisonState. Persist short-write now surfaces as LineSenderException. A disk-full during the write-ahead persist threw a raw IllegalStateException that escaped flush() unwrapped, unlike every other flush-path failure (e.g. the cursor append in sealAndSwapBuffer). Wrap it so a caller catching LineSenderException around flush() sees a persist disk-full too; a JVM Error still propagates. PersistedSymbolDict close() and the append methods are synchronized. close() is callable from any thread (a shutdown hook); without mutual exclusion a close racing an in-flight append could free the scratch buffer or close the fd mid-write and let the write land on a descriptor the OS has reused for another file (silent cross-file corruption). The appendSymbols re-encode branch now has a regression test. After a failed publish leaves the durable dictionary size ahead of the wire baseline, a later flush introducing a new symbol re-encodes only the [pd.size() .. currentBatchMaxSymbolId] suffix; keying that off pd.size() (not sentMaxSymbolId+1) keeps it idempotent. Regression test: testFailedPublishThenNewSymbolPersistsSuffixWithoutDuplicating. Both new tests fail with their production hunk reverted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Minor follow-ups from the review. Remove the unused CursorSendEngine.isMemoryMode() -- it had no callers; isDeltaDictEnabled() tests sfDir == null directly. Sort the NativeBufferWriter import into its alphabetical place among the qwp.client imports in CursorWebSocketSendLoop. Harden PersistedSymbolDict.openExisting: hoist entriesAddr / entriesLen so the catch frees the loaded-entries buffer too. It is unreachable today (nothing between that malloc and the return throws, and on success the buffer is transferred to the returned dict), but the error path no longer leaks if a future edit adds a throwing step. Return a defensive copy from DeltaDictCatchUpTest's dictFor() instead of the live inner list: the caller iterates it unlocked while the server thread may still be appending, matching the sibling dictSnapshot(). Add testTornDictionaryOneIdGapFailsCleanly -- the tightest torn-dictionary boundary (deltaStart == recoveredSize + 1), the common one-entry-short host-crash tail. The existing torn test uses a 3-id gap, so it does not pin the guard's false-negative edge; a "deltaStart > size + 1" mutation passes it but ships the gapped frame here (the new test fails on it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The reconnect symbol-dictionary catch-up latched a terminal for any entry exceeding the packing budget (cap - HEADER_SIZE - 16). That reserve is larger than the minimal data-frame overhead, so a symbol the producer had already shipped under a given cap could exceed the budget and trip the terminal on reconnect -- permanently hard-failing a running producer on a homogeneous single-cap cluster, the exact failure the store-and-forward path exists to survive. sendDictCatchUp now tests the actual solo catch-up frame (header + the entry's delta varints + the entry bytes) against the cap. That frame is always smaller than the data frame that already carried the entry, so an accepted symbol always re-registers on the same cap; a genuinely oversized entry on a shrunk (heterogeneous) cap still terminates. The conservative packing budget stays, used only for multi-entry chunk splitting. Add a fixed-cap regression test: a 173-char symbol under a fixed cap of 200 is accepted, then re-registers gap-free across a reconnect. It fails before this change, where the reverted condition terminates a frame that fits the cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Two DISTINCT source symbols that collapse to the same UTF-8 bytes -- lone UTF-16 surrogates, which the encoder maps to '?' -- get distinct producer ids and persist as separate .symbol-dict entries. On recovery, seedGlobalDictionaryFromPersisted replayed them through getOrAddSymbol, which de-dups the decoded "?" strings, leaving the producer dictionary (and sentMaxSymbolId) one short of pd.size(). That desynced the delta baseline from the send-loop catch-up mirror (which uses pd.size()) and, after a reconnect, silently misattributed later well-formed symbols. Add GlobalSymbolDictionary.addRecoveredSymbol, which appends at the next id WITHOUT de-duplicating, and seed recovery through it so the producer id space always matches the persisted entry count. The reverse lookup keeps the highest id for a colliding string, which is harmless -- both ids encode to identical bytes. A GlobalSymbolDictionary unit test pins the no-dedup contract, and a new recovery test seeds a slot with two lone-surrogate symbols and asserts the producer dictionary and sentMaxSymbolId match the persisted entry count. It fails before this change (dictionary size 1 vs the file's 2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Three defensive fixes on currently-unreachable teardown paths, each guarding a future edit from a native-memory hazard: - PersistedSymbolDict.close() now nulls loadedEntriesAddr/Len after freeing them (like scratchAddr), so a post-close read of the non-closed-guarded getters cannot dereference freed memory. - CursorWebSocketSendLoop resets sentDictCount alongside the buffer in both mirror-free sites, keeping the mirror all-or-nothing. start() has no closed guard, so a hypothetical close()-then-start() would otherwise observe a non-zero count against a freed buffer and drive setWireBaselineWithCatchUp into a null-mirror catch-up. - PersistedSymbolDict.openFresh() now closes the fd on any throw (the header malloc moves inside the try, plus a catch), mirroring openExisting; previously a throw in the header-write body freed the scratch buffer but leaked the fd. Tests: testCloseNullsLoadedEntries asserts the getters read 0 after close; the mirror-leak test now also asserts sentDictCount is reset. Both fail before their fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Three delta symbol-dict cleanups (a fourth reported item, a dead CursorSendEngine.isMemoryMode(), was already removed earlier on this branch, so nothing to do there): - sendCatchUpChunk now sets FLAG_DEFER_COMMIT on the catch-up frame. It carries dictionary entries but no rows, so it must never trigger a server-side commit. Today it is always the first frame on a fresh (empty-transaction) connection, so committing nothing is a no-op -- but that invariant is load-bearing and was unasserted. Deferring the empty commit removes the dependency, so a future mid-stream catch-up cannot prematurely commit an in-flight deferred transaction. The dictionary delta still registers, as any deferred data frame's does. - flushPendingRows' comment claimed delta mode is "memory-mode only"; file-mode store-and-forward ships monotonic deltas too once the persisted dictionary opened. Corrected. - The deltaDictEnabled field comment implied delta mode only applies to recovered / orphan-drained disk slots; it applies to fresh disk slots too. Clarified that recovery additionally seeds the mirror. testReconnectCatchUpRebuildsDictionary now asserts the catch-up frame carries FLAG_DEFER_COMMIT; it fails without the flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Review cleanups: - ensureConnected's start()-failure catch now closes cursorSendLoop before nulling client. Previously it closed only the client; because the connected flag is set past the catch, a caller that retries -- re-entering ensureConnected and reassigning cursorSendLoop -- orphaned a recovered slot's ctor-seeded native mirror. That is a reachable leak on the recovered-slot + start()/dispatcher-OOM + retry path. close() is idempotent and frees the mirror via its loopNeverRan path, and also closes the shared client, so the following client.close() is a no-op. - Consolidated the triplicated raw-address varint writer into NativeBufferWriter.writeVarint (writeVarintDirect now delegates to it); PersistedSymbolDict and the catch-up frame builder in CursorWebSocketSendLoop use it, and PersistedSymbolDict.varintSize now reuses NativeBufferWriter.varintSize. - Alphabetized CursorSendEngine (getPersistedSymbolDict before getTotalBackpressureStalls before isDeltaDictEnabled) and moved PersistedSymbolDict.decodeVarint to the front of its private-static cluster. - readVarintAt gained the shift > 35 defensive bound that decodeVarint already has (unreachable today, all inputs are freshly-encoded or validated varints, but consistent). - CursorWebSocketSendLoopCatchUpAlignmentTest now uses the existing TestUtils.createTmpDir/removeTmpDir instead of reinventing them. Not changed: the reported dead CursorSendEngine.isMemoryMode() and a PersistedSymbolDict MAX_ENTRY_LEN constant do not exist at HEAD (the former removed earlier on this branch, the latter never present), and the NativeBufferWriter import is already correctly ordered. The buildAck/waitFor/rmDir/readVarint test helpers are duplicated across ~19 pre-existing test files, so consolidating them belongs in a dedicated repo-wide test-hygiene change rather than this feature PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The .symbol-dict side-file was page-cache durable but carried no integrity check, so a host-crash interior tear -- a lost page reading back as zeroes, or a stale entry left past the end by a failed truncate -- could shift the dense id->symbol map without changing the entry count the send-loop replay guard checks, silently misattributing symbol-column values on recovery. The SF segment frames are CRC-32C protected; the dictionary they depend on was not. Each entry now carries a trailing CRC-32C over its [len][utf8] bytes, the same checksum MmapSegment uses. openExisting verifies every entry and stops at the first torn or mismatched one, so recovery trusts only the intact prefix and the guard then forces a resend of the rest -- turning a silent misattribution into a fail-clean "resend required". open() strips the CRCs into the wire-shaped loaded-entries buffer, so the catch-up mirror seed and appendRawEntries keep the file==wire contract and the send loop stays untouched. The tail truncate is now failure-checked: a file open() cannot trim is untrusted and recreated empty rather than left exposing stale bytes. Bumps the on-disk format to v2 with a version check; existing files are recreated. A tear that leaves a CRC-matching byte run is still undetected, a 1-in-2^32 collision per entry, no weaker than the frames. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Two documentation-only additions to the QWP store-and-forward client, making previously implicit contracts explicit. No behavior change. flushPendingRowsSplit: add a "Not atomic across frames" note. A sealAndSwapBuffer() failure on split frame k>1 leaves frames 1..k-1 published as deferred; the throw skips the trailing table-buffer reset, so the next flush re-emits the whole batch and the eventual commit commits the already-published prefix twice. This is pre-existing and within store-and-forward's at-least-once contract (absorbed by a DEDUP table or a durable-ack await); the note names the atomic-split fix (roll back or skip the published prefix on retry) as the larger follow-up. PersistedSymbolDict: mark loadedEntriesAddr(), loadedEntriesLen() and readLoadedSymbols() as construction-phase only. They read the native entry region that close() frees and nulls, with no closed-guard, so they are safe only before the slot's I/O thread and any producer append start; reading them from a running thread would risk a use-after-free. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

flushPendingRowsSplit published each table's frame one at a time (deferred, uncommitted) and only checked a frame against the server batch cap after encoding it. An oversized non-first table therefore threw after an earlier table's frame was already on the ring, where a later commit delivered it as a partial batch -- while resetTableBuffersAfterFlush discarded every source row. The caller saw a failure for a batch that had partly committed. Pre-flight every split frame's size before publishing any of them, so the split is all-or-nothing: either every frame fits and all publish, or none publish and it throws with nothing stranded. simBaseline mirrors advanceSentMaxSymbolId, so a measured size equals the frame the publish loop builds; the pass is read-only on the table buffers and touches no delta/persist state. The cost is a second encode pass over the split batch, already the exceptional large-batch path. testOversizedTableSplitStrandsNothing reproduces the stranding -- it fails on the old code with the earlier frame reaching the server -- and passes with the pre-flight. Also add testReconnectPreservesMonotonicDeltaBaseline: it pins that the producer's sent-symbol watermark survives a reconnect, asserting the first post-reconnect data frame carries a delta above the baseline (deltaStart >= 1) rather than re-shipping the whole dictionary from 0. The existing catch-up tests assert only that the final dictionary is complete, which a reset-then-redefine also satisfies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Fold in low-severity hardening found in review; none changes behavior on any reachable input. PersistedSymbolDict: - appendRawEntries fails loudly (rather than flushing an uninitialised scratch tail and mis-advancing size) if a caller ever passes an (addr, len, count) triple whose entries do not fill len; the sole caller derives both from one beginMessage, so this cannot fire today. - openExisting sets entriesLen alongside the malloc, so the catch frees the right size if the copy walk throws -- the comment already claimed this held. - ensureScratch grows in long so scratchCap*2 cannot overflow negative past ~1 GB, matching ensureSentDictCapacity. - Two varint decode loops gain the shift > 35 bound the other decoders already carry. Close a file-descriptor leak in the rmDir test helpers: Files.walk returns a Stream backed by an open directory handle, so wrap it in try-with-resources (PersistedSymbolDictTest calls rmDir 13 times). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mtopolnik · 2026-07-10T23:13:00Z

[PR Coverage check]

😍 pass : 471 / 540 (87.22%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java	14	17	82.35%
🔵	io/questdb/client/cutlass/qwp/client/GlobalSymbolDictionary.java	5	6	83.33%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/PersistedSymbolDict.java	212	251	84.46%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	55	63	87.30%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	175	193	90.67%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketEncoder.java	4	4	100.00%
🔵	io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java	6	6	100.00%

glasstiger added the enhancement New feature or request label Jul 9, 2026

glasstiger mentioned this pull request Jul 9, 2026

feat(qwp): stop resending the full symbol dictionary on every message questdb/questdb#7374

Open

glasstiger and others added 10 commits July 9, 2026 17:10

Merge branch 'main' into qwp-delta-symbol-dict

01f251d

bluestreak01 requested changes Jul 9, 2026

View reviewed changes

glasstiger and others added 8 commits July 9, 2026 22:09

Merge branch 'main' into qwp-delta-symbol-dict

f9430bc

Merge branch 'qwp-delta-symbol-dict' of https://github.com/questdb/ja…

9602440

…va-questdb-client into qwp-delta-symbol-dict

glasstiger added the tandem label Jul 9, 2026

glasstiger and others added 5 commits July 10, 2026 00:43

glasstiger and others added 17 commits July 10, 2026 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(qwp): stop resending the full symbol dictionary on every message#66

feat(qwp): stop resending the full symbol dictionary on every message#66
glasstiger wants to merge 41 commits into
mainfrom
qwp-delta-symbol-dict

glasstiger commented Jul 9, 2026 •

edited

Loading

Uh oh!

glasstiger commented Jul 9, 2026

Uh oh!

bluestreak01 left a comment

Uh oh!

mtopolnik commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

glasstiger commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Memory mode

Store-and-forward (file mode)

Catch-up split

Durability

Tradeoffs

Test plan

Uh oh!

glasstiger commented Jul 9, 2026

Uh oh!

bluestreak01 left a comment

Choose a reason for hiding this comment

Review of PR #66 — feat(qwp): stop resending the full symbol dictionary on every message

Critical

C1 — Persisted .symbol-dict accumulates duplicate entries when appendBlocking fails and a later flush succeeds → silent symbol corruption on recovery (file mode, delta enabled). [in-diff]

C2 — Required Enterprise failover tandem is missing/unlinked; the HA path this feature targets is UNTESTED in CI (Step 2.7 gate). [tandem]

Moderate

M1 — One Files.write syscall per new symbol on the producer thread. [in-diff]

M2 — accumulateSentDict silently drops symbols on a partial-overlap delta. [in-diff]

Minor

m1 — Stale "self-sufficient / delta from id 0" comments now contradict delta mode.

m2 — Memory-mode mirror double-stores the dictionary.

Downgraded (false positives — verified against source)

Coverage map

Summary

Uh oh!

mtopolnik commented Jul 10, 2026

[PR Coverage check]

file detail

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

glasstiger commented Jul 9, 2026 •

edited

Loading

Review of PR #66 — `feat(qwp): stop resending the full symbol dictionary on every message`

C1 — Persisted `.symbol-dict` accumulates duplicate entries when `appendBlocking` fails and a later flush succeeds → silent symbol corruption on recovery (file mode, delta enabled). [in-diff]

M1 — One `Files.write` syscall per new symbol on the producer thread. [in-diff]

M2 — `accumulateSentDict` silently drops symbols on a partial-overlap delta. [in-diff]