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

[glasstiger]

Tandem

This change lands together with its counterparts (merge as a set):

• OSS: feat(qwp): stop resending the full symbol dictionary on every message questdb#7374 -- bumps the java-questdb-client submodule, and adds one server-side change: the ingress decoder now rejects a delta symbol dictionary whose start id runs past the connection dictionary, atomically and with a dedicated retriable error, instead of null-padding the hole. See "Server-side gap rejection" below.
• Enterprise: questdb/questdb-enterprise#1122 -- bumps the client so the failover suite (SqlFailoverQwpClientLosslessTest, file-mode failover) runs end-to-end against this change.

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).

Server-side gap rejection (OSS half)

Delta framing makes a non-zero start id reachable on the wire for the first time, so the decoder's handling of one now matters. QwpMessageCursor.parseDeltaSymbolDict grew the connection dictionary with nulls up to deltaStartId + deltaCount, which inflated size() -- the very bound QwpSymbolColumnCursor checks an incoming symbol index against. A row referencing a padded id therefore passed the bounds check, read back null, and landed a NULL symbol value with no error.

The decoder now rejects deltaStartId > size() with its own error code, DELTA_DICT_GAP, surfaced to the sender as a new wire status byte, STATUS_DICTIONARY_GAP (0x0D). The gap verdict depends on this connection's dictionary coverage -- server state, not the frame's bytes -- so unlike a parse error it is retriable: the server sends the NACK and keeps the connection open, and the sender recycles the wire and re-registers from an id the server actually holds. A contiguous append (deltaStartId == size()) and a lower start that re-registers or remaps existing ids both stay allowed. The parse is atomic on failure: a rejected delta restores every entry it overwrote and nulls the slots it grew into, so the connection dictionary is exactly what it was before the frame and can never hold a null.

Wire-compat note: the server now rejects a frame shape it previously (wrongly) accepted, and 0x0D is a status byte no earlier server emitted, under an unchanged protocol version. QWP is experimental and unreleased, and the bundled client moves in lockstep with the server, which is what the tandem labels assert; this client maps an unknown status byte to a retriable category, so an older bundled client against a newer server degrades to retry rather than failing. This client cannot emit a gapped frame -- its send loop refuses to -- so the guard exists for a client bug, a torn store-and-forward dictionary, or a third-party implementation.

Symbol dictionary capacity

The server caps a connection's symbol dictionary at 1,000,000 distinct values (MAX_SYMBOL_DICTIONARY_SIZE, pre-existing). Before this change the practical ceiling was far lower: every message re-shipped the dictionary prefix from id 0, so per-message cost grew with lifetime cardinality and a large dictionary outgrew the frame budget long before the cap. Delta encoding removes that per-message cost, which makes the protocol cap the binding constraint for the first time — and because the producer's baseline is lifetime-monotonic, the reconnect catch-up would trip the server's rejection on every reconnect, including recovered slots and orphan drainers, stranding an already-buffered store-and-forward backlog with no error ever reaching the producer.

The client therefore enforces the cap at registration: creating the 1,000,001st distinct symbol value throws a LineSenderException from symbol() naming the limit and the recovery, before the row is buffered. Rows using already-registered values are unaffected. Everything buffered stays deliverable — the server's check is >, so a dictionary of exactly the cap still catches up cleanly. To reset the id space, close the sender and build a new one: a fully drained close removes the slot's dictionary side-file, so the rebuilt sender starts fresh. Reaching a million distinct values in symbol columns usually means the data belongs in varchar.

The server-side rejection itself keeps its parse-error (terminal) classification: with the registration guard, this client cannot reach it, the same unreachability argument the gap status relies on for old clients.

Recovery-time side-file disposition

PersistedSymbolDict.open() — the recovery entry point — now mirrors the Rust client's open_recovered disposition matrix:

• A transient I/O failure against an existing side-file (stat error, failed open, mmap or short read, failed torn-tail truncate, late mmap fault) throws the retriable SfOperationalException instead of silently degrading to full-dictionary frames. Sender.build() aborts without quarantining and BackgroundDrainer leaves the slot for a later scan, so a transient can no longer permanently quarantine an intact backlog, and a degraded session can no longer write frames next to a stale populated side-file that a later recovery would trust — the silent cross-generation symbol-misattribution chain loses its only organic entry point.
• A provably absent or corrupt side-file (bad magic/version, sub-header stub) still degrades to full-dictionary frames, and the recovery path no longer fabricates a fresh empty side-file next to recovered segments. Both dispositions are sticky across restarts, so consecutive sessions cannot disagree about the slot's mode.
• openClean() (the fresh-slot truncate-or-refuse path) is unchanged.

Coverage equivalent to the earlier generation-stamp test (testRecoveryDiscardsADictionaryFromAnotherGeneration, removed with the stamp) is restored by testTransientDictFaultOnRecoveredSlotFailsLoudAndRetryRecoversInFull, which drives the three-session chain end-to-end and proves it now breaks at session B with the slot byte-identical, nothing quarantined, and a full recovery on retry.

Slot quarantine: deterministic recovery failures set the slot aside

A recovery failure that is deterministic -- a torn slot whose surviving frames cannot be replayed without corrupting data, an unreadable interior segment, a corrupt segment chain -- no longer aborts Sender.build() forever or spins the orphan drainer. Sender.build() and BackgroundDrainer catch the typed exceptions (UnreplayableSlotException, SfRecoveryException, MmapSegmentCorruptionException), rename the whole slot directory aside for operator attention, dispatch a synchronous SenderError, and continue on a fresh slot. Renaming the whole directory guarantees the replacement starts empty and cannot fail the same way twice. Operational failures -- e.g. a drained-slot leftover whose unlink fails -- deliberately stay plain aborts that retry, rather than quarantining data that is still deliverable.

Mmap faults on the dictionary path degrade instead of killing the sender

MmapSegment.isMmapAccessFault recognizes the InternalError HotSpot raises for an access to an unbacked page (delivered asynchronously before JDK 21, JDK-8283699). The dictionary-side consumers (persistNewSymbolsBeforePublish, healPersistedDictionary) treat a recognized fault as a persist failure and degrade the sender to full self-sufficient frames (disableDeltaDict) instead of propagating an untyped Error; an unrecognized InternalError still propagates. Segment recovery itself validates every page through positioned reads before mapping, so the recovery scan cannot hit a late-delivered fault on pages it has not already read.

Reconnect policy: post-connect endpoint rejections retry instead of killing the producer

Once a foreground sender has completed its first connection (including the dictionary catch-up), a later WebSocket upgrade rejection or durable-ack capability mismatch no longer latches a producer-fatal terminal: the send loop retries with backoff while store-and-forward keeps buffering, and the failure is reported through SenderError dispatch. At build/initialization time these failures still surface loudly. Auth failures on the orphan drainer, and initialization-time failures in all modes, keep their previous terminal behaviour. hasEverConnected latches only after the catch-up succeeds, so a first connection that fails inside the catch-up still counts as never-connected and keeps endpoint-policy failures terminal.

P-C8: .symbol-dict bytes count against sf_max_total_bytes

The provisioning cap check compared .sfa segment bytes only, while the
symbol dictionary's side-file grows monotonically over the sender's
lifetime -- so dictionaries could fill the SF filesystem while the cap
reported headroom. SegmentManager now reads a live per-slot gauge
(PersistedSymbolDict.appendedBytes(), wired at engine registration) at
every cap check, and the throttled disk-full warning breaks the
dictionary component out as sideFileBytes=. Memory mode and degraded
full-dict sessions are unaffected (no side-file, no gauge).

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 rather than silently trusted. Each side-file chunk carries a CRC-32C over its header and batched entry bytes (the same checksum the SF segment frames use), so recovery stops at the first torn or mismatched chunk and trusts only the intact prefix; the send loop then detects any surviving delta frame whose start id exceeds that prefix 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, and the affected data must be re-sent. Every detectable tear now fails clean rather than corrupting: the per-chunk CRC-32C catches an interior page lost out of order (or a stale chunk left by a failed best-effort truncate) that would otherwise shift the dense id->symbol mapping, so recovery trusts only the intact prefix and the send loop forces a "resend required" for the rest. A tail truncate that itself fails makes the file untrusted; recovery leaves it intact and falls back to full-dictionary frames rather than exposing stale bytes. The one residual is a tear that happens to leave a CRC-matching byte run -- a 1-in-2^32-per-chunk collision, no weaker than the SF frames' own checksum.
• On failover to a node advertising a smaller batch cap, a symbol accepted under a larger or absent cap can exceed the new cap during the catch-up. A foreground sender retries that indefinitely and recovers on its own once a larger-cap node returns, so store-and-forward contains the window instead of surfacing it to the producer. Only an orphan drainer gives up, and only after both 16 consecutive cap gaps and a minimum wall-clock dwell (catch_up_cap_gap_min_escalation_window_millis, 5 minutes by default); it then sets its slot aside for an operator and that slot's data must be 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.
• The server-side gap rejection turns a previously silent (and silently wrong) frame into a NACK. The rejection is retriable by design -- a gap is a statement about per-connection server state, and re-registering from a held id resolves it -- but a sender that persistently re-sends the same gapped frame escalates through the poison-frame detector to a terminal error rather than looping forever.
• Quarantine trades availability of one slot's data for the rest of the pipeline: a slot set aside must be re-sent (or inspected and restored by an operator), and the sender continues on a fresh slot instead of blocking.

Follow-ups (known, deliberately not in this PR)

• Sender.build()'s rollback closes the cursor engine without the failed-stop check the close-delegation protocol requires, and PersistedSymbolDict makes the send loop's mirror a borrower of the engine's native memory — so a throw landing in the narrow window after the I/O thread starts, combined with a thread that outlives the 30 s stop (in practice an OOM), could free memory a live I/O thread still reads. The reachable window is effectively theoretical, and the fix (move the rollback close into QwpWebSocketSender.connect's catch, which owns the engine and honours the protocol) touches teardown paths not worth destabilizing here. It must land together with narrowing ensureConnected's blanket exception wrap, which currently masks the worse variant of the same defect: fixing either alone makes the other worse.
• In full-dictionary fallback mode accumulateSentDict still runs per frame: it re-walks the already-held dictionary prefix varint-by-varint on the I/O thread and, when the loop was constructed with the delta dict already disabled, accumulates a native mirror nothing ever reads. Both are constant-factor costs on a mode that already re-sends the whole dictionary per frame, so the fix (carrying the encoder's entries-length as sideband on ring entries, plus offset arithmetic for the identical prefix) waits for profiling evidence rather than adding plumbing here.
• The Rust client (c-questdb-client) already enforces a producer-side dictionary cap (SymbolGlobalDict::intern errors at the cap), but its constant MAX_CONN_SYMBOL_DICT_SIZE = 8_388_608 was taken from the egress/result-batch direction, not the ingress server's 1,000,000 — so its guard cannot fire before the server rejection. One-line constant fix (plus comment correction) needed in that repo.
• Known perf debt, pre-existing and unchanged here: each flushed message is copied one extra full time on the producer thread (encoder buffer -> microbatch -> segment mapping; two copies would suffice on the non-split path), and both CRC-32C paths (frame append and recovery scan) run software slice-by-8 -- hardware CRC32 instructions behind runtime dispatch are a native-build change. Neither gets worse with this PR; both dominate their respective paths and are worth a dedicated pass.
• The oversized-single-entry residual of the catch-up cap fix: a single symbol whose solo frame exceeds the server's actual receive buffer still reconnect-loops when the server advertises no cap. Reachable only with a symbol value comparable to the receive buffer (default 128 KiB) on a no-cap server; the halve-and-retry probe is the planned fix. Until then the failure mode is a visible reconnect loop, not data corruption.
• The batch-too-large rejection now names reset() as the non-destructive recovery, but the pre-flight is still whole-flush: one unsplittable table's batch blocks other tables' healthy batches behind the same exception until reset()/close(), and sendRow's per-row guard checks raw column bytes without the frame overhead (header, delta section, table name), so a batch can pass the row guard and still exceed the cap. Per-table pre-flight and an overhead-aware row guard are the follow-up.
• P-C8 (second half, deferred): size the dictionary append window from
  segmentSizeBytes instead of the fixed 4 MiB APPEND_MAP_CAPACITY.
  Until then ensureAppendMap preallocates in 4 MiB steps, so a crash
  can leave up to a 4 MiB allocated-but-unaccounted tail per slot; a
  clean close() truncates it back. Trigger for doing it: tightening
  small-cap configurations (cap comparable to a few segments), where a
  4 MiB tail is a material fraction of the budget.
• P-C8 liveness note: with side-file bytes now counted, a configuration
  where sideFileBytes + 2 * segmentSize > sf_max_total_bytes can no
  longer provision a hot spare, and ACK-driven trim cannot free
  dictionary bytes -- the producer stays backpressured until the cap is
  raised. The disk-full warning names the side-file component
  (sideFileBytes=) so the condition is diagnosable. A cap-vs-dictionary
  validation or escape hatch is deliberately not implemented yet; revisit
  together with the append-window sizing follow-up above.
• P-C8 residue note: a session that degrades to full-dict mode closes and
  discards its recovered .symbol-dict but leaves the file on disk with a
  null gauge, so its bytes sit outside the cap for that session. The
  residue is static (nothing appends to it) and is cleared by a fully
  drained close or the next fresh session's truncate; unlinking at
  discard time needs its own analysis before we do it.

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 is caught by the per-chunk CRC and only the intact prefix is trusted.
• PersistedSymbolDictTest -- side-file append/read/orphan-removal round trips, and a multi-byte UTF-8 round trip across reopen (every other symbol in these suites is ASCII, where a symbol's UTF-8 byte length and its char count agree, so a confusion between the two would otherwise go unnoticed).
• GlobalSymbolDictionaryTest, DeltaDictCeilingTest -- the 1,000,000-entry protocol cap: the boundary entry is accepted, the next is refused without mutation, cancelRow() recovers the row, the sender keeps working with registered values, and the refused symbol never reaches the wire.
• CursorWebSocketSendLoopCatchUpAlignmentTest -- the split catch-up's chunks must tile [0, n) exactly: the captured frames are reassembled through the same decoder the end-to-end tests use and compared per id, so an overlap, a gap or a shift all fail. Also covers a reconnect with an empty dictionary (no catch-up frame at all) and a split over entries of differing widths.
• SelfSufficientFramesTest, ReconnectTest -- full-dict fallback and reconnect replay still hold.
• MmapFaultDegradesTest -- a recognized mmap access fault on the dictionary persist path degrades the sender to full-dict frames; an unrecognized InternalError still propagates.
• MmapSegmentRecoveryFaultTest -- single-segment recovery fault shapes: read errors, short reads, size changes and unbacked pages fail closed before mapping or skip the unbacked tail.
• SegmentSkipQuarantineTest, SegmentRecoveryIntegrityTest, BackgroundDrainerUnreplayableSlotQuarantineTest -- deterministic recovery failures quarantine the whole slot and the replacement starts empty; a drained-slot leftover whose unlink fails aborts and retries instead of quarantining.
• CursorWebSocketSendLoopForegroundReconnectPolicyTest -- post-connect endpoint rejections retry on a foreground sender; initialization-time failures stay terminal; a first connect that fails inside the catch-up does not latch hasEverConnected.
• SlotLockTest -- lock lifecycle, including the pid-sidecar-before-lock unlink order on retirement.
• OSS QwpSymbolDecoderTest -- a gapped delta is rejected with DELTA_DICT_GAP (routed to the DICTIONARY_GAP status), the deltaStartId == size() boundary is still accepted, a rejected delta restores every overwritten entry and leaves no nulls -- including when the rejected frame was the connection's first.
• The enterprise SqlFailoverQwpClientLosslessTest (file-mode failover) passes end-to-end against a real server, asserting per row that every surviving SYMBOL is the value its id implies.
• PersistedSymbolDictTest pins every disposition: each transient (stat, open, mmap, short read, truncate) throws SfOperationalException with the file byte-identical and a subsequent open recovering in full; absent/stub/bad-magic report null with nothing created or destroyed
• DeltaDictRecoveryTest#testTransientDictFaultOnRecoveredSlotFailsLoudAndRetryRecoversInFull drives the three-session misattribution chain: session B fails loudly, the slot stays intact and unquarantined, and the retry replays the backlog with every wire-reconstructed id resolving to the original string
• Both directions of the torn-dictionary defense are re-enabled: the fixtures now trim through the live SegmentManager (prefix-ACK, manifest-correct head trim), so CursorWebSocketSendLoopTornDictGuardTest proves the pre-send guard refuses a gapped frame and ships nothing, and DeltaDictRecoveryTest#testFullyAckedTornSlotResumesInPlaceWithoutQuarantine lands exactly on the ackedFsn == recoveredCommitBoundaryFsn boundary (a >= -> > mutation reddens it) and resumes in place without quarantine.
• The fixture-driven quarantine tests pin the dictionary-gap verdict via the .failed sentinel content, and a deliberate chain-boundary test keeps the missing-head-segment fail-closed path covered on purpose instead of by accident.
• DictionaryGapNackTest -- first end-to-end 0x0D: a real DICTIONARY_GAP NACK recycles the wire, replays from the ack watermark, materialises the server-side dictionary gap-free, and neither latches a terminal nor poison-escalates on a single gap.
• CloseDrainTest covers both branches of close()'s drain-timeout outage naming: a 401-after-upgrade produces "the wire is not draining: WebSocket upgrade rejected with HTTP 401", and the never-dropped wire keeps the generic guidance tail. The test-only writeAckWatermark helper now writes the real AckWatermark format (its legacy 16-byte stamps were silently reset on open, i.e. no-ops).

🤖 Generated with Claude Code

[glasstiger]

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

[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.

[mtopolnik]

[PR Coverage check]

😍 pass : 1547 / 1686 (91.76%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SfOperationalException.java	0	2	00.00%
🔵	io/questdb/client/cutlass/qwp/client/WebSocketResponse.java	0	1	00.00%
🔵	io/questdb/client/std/Files.java	2	3	66.67%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java	30	37	81.08%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java	18	21	85.71%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/OrphanScanner.java	6	7	85.71%
🔵	io/questdb/client/Sender.java	85	98	86.73%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/PersistedSymbolDict.java	399	451	88.47%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	158	176	89.77%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java	89	98	90.82%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	387	412	93.93%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/RecoveredFrameAnalysis.java	162	168	96.43%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java	35	36	97.22%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketEncoder.java	54	54	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegment.java	7	7	100.00%
🔵	io/questdb/client/cutlass/http/client/WebSocketClient.java	11	11	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java	50	50	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/UnreplayableSlotException.java	2	2	100.00%
🔵	io/questdb/client/std/FilesFacade.java	2	2	100.00%
🔵	io/questdb/client/std/Crc32c.java	27	27	100.00%
🔵	io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java	8	8	100.00%
🔵	io/questdb/client/cutlass/qwp/client/GlobalSymbolDictionary.java	8	8	100.00%
🔵	io/questdb/client/impl/ConfigSchema.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/client/BatchTooLargeForCapException.java	2	2	100.00%
🔵	io/questdb/client/SenderError.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/protocol/QwpTableBuffer.java	3	3	100.00%