Skip to content

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

Open
glasstiger wants to merge 67 commits into
mainfrom
qwp-delta-symbol-dict
Open

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

Conversation

@glasstiger

@glasstiger glasstiger commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Tandem

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

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 rather than silently trusted. Each side-file entry carries a CRC-32C (the same checksum the SF segment frames use), so recovery stops at the first torn or mismatched entry 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-entry CRC-32C catches an interior page lost out of order (or a stale entry 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 and recreates it empty 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-entry 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/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>
@glasstiger glasstiger added the enhancement New feature or request label Jul 9, 2026
glasstiger added a commit to questdb/questdb that referenced this pull request Jul 9, 2026
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

Copy link
Copy Markdown
Contributor Author

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

glasstiger and others added 10 commits July 9, 2026 17:10
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 bluestreak01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glasstiger

Review of PR #66feat(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, ReconnectTest15 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

sealAndSwapBufferappendBlocking 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-dictempty. 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 (swapClientsetWireBaselineWithCatchUpsendDictCatchUp), 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=0fsnAtZero = -catchUpFrames) corrupts ack accountingdismissed. 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 appendSymboldismissed. 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 watermarkdismissed. 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 reconnectdismissed. 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 taildismissed. 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-updismissed. 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 codedismissed. 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 Logdismissed. 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.

glasstiger and others added 8 commits July 9, 2026 22:09
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>
glasstiger and others added 5 commits July 10, 2026 00:43
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>
glasstiger and others added 29 commits July 10, 2026 21:48
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>
open() cast the on-disk file length to int before reading the whole
file into one native buffer. A .symbol-dict that grew past 2GB (an
extreme ~100M+ distinct symbols on one slot) therefore reopened with a
negative malloc, a malloc(0) followed by a 4-byte out-of-bounds read,
or a small-positive prefix whose validLen<len path then destructively
truncated the multi-GB file. open() now rejects a length above
Integer.MAX_VALUE up front and recreates the dictionary empty --
fail-clean, matching every other unreadable-file case, so the sender
falls back to full self-sufficient frames.

Thread a FilesFacade through PersistedSymbolDict (production keeps
FilesFacade.INSTANCE, so behavior is unchanged; callers use the
INSTANCE-default overloads) so a test can fault-inject a failing
truncate. Add testTruncateFailureRecreatesEmpty: a torn-tail file whose
tail truncate fails is recreated empty rather than trusted over stale
bytes, covering the previously untested fail-clean branch the
per-entry-CRC hardening added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ensureSentDictCapacity threw a bare LineSenderException when the
lifetime-monotonic sent-dictionary mirror would exceed
MAX_SENT_DICT_BYTES. accumulateSentDict runs after the frame's
sendBinary, so that throw unwound to ioLoop -> fail() -> connectLoop
with running still true, which reconnected and replayed the same frame;
the mirror never shrinks, so the replay re-overflowed and the loop
reconnected again -- an unbounded livelock instead of the loud failure
the ceiling promises.

recordFatal now flips running=false before the throw, so connectLoop's
!running guard winds the loop down and checkError() surfaces the
terminal to the producer. This matches sendCatchUpChunk's guard for the
same ceiling. The throw stays, to unwind past the pending copyMemory.
The path needs ~100M+ distinct symbols on one connection to reach, so
it is defensive; the producer's GlobalSymbolDictionary would OOM first.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two hardening fixes to the delta symbol-dictionary path.

Torn-dictionary resume (data integrity): the persisted .symbol-dict is
not fsync'd, so a host crash can lose its newest entries while the
segment frames that introduced those ids survive and replay. A resumed
producer, seeded from the shorter dictionary, would reuse ids the
surviving frames already define and silently misattribute symbols. The
send loop's replay guard only catches a gap (deltaStart > mirror); the
contiguous tear self-heals the mirror and leaves the producer diverged.
CursorSendEngine now records the highest symbol id the recovered frames
reference via a read-only MmapSegment/SegmentRing walk, and
seedGlobalDictionaryFromPersisted fails clean when it reaches the
recovered dictionary size -- the affected data must be resent. Only the
resuming producer refuses; the orphan drainer still self-heals and
drains the slot. testTornDictSubsetFailsCleanOnResume covers it.

Split-flush sizing (performance): flushPendingRowsSplit re-encoded the
whole batch a second time just to size each split frame. It now derives
each frame size arithmetically from the combined encode flushPendingRows
already performed, capturing each table's body bytes and the delta-entry
length. Body encoding is context-free, so the sizes are exact; the
publish loop's assert cross-checks them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two hardening fixes to the split/catch-up paths for heterogeneous /
rolling-cap clusters.

Catch-up cap-gap settle budget (M1): when a symbol-dict catch-up entry
is too large for the fresh server's advertised batch cap, sendDictCatchUp
latched a terminal on first sight. A homogeneous cluster never trips this
(an entry that fit its data frame under a cap always fits its bare
catch-up frame under that same cap), but a failover to a smaller-cap node
could kill the sender for an entry an earlier node accepted. It now
retries across MAX_CATCHUP_CAP_GAP_ATTEMPTS consecutive reconnects
(throwing a retriable CatchUpSendException, no recordFatal), riding out
the transient window until a larger-cap node returns; only a persistent
gap latches the terminal. A successful catch-up resets the budget, and a
transient reconnect never reaches the catch-up so it neither increments
nor burns it. Matches the orphan drainer's durable-ack settle budget.

Split-flush cap snapshot (M2): flushPendingRows now snapshots the
volatile serverMaxBatchSize once and threads it into flushPendingRowsSplit
for both the pre-flight sizing and the publish-loop assert. A mid-flush
failover (the I/O thread lowers the cap) could previously make the two
passes size against different caps, breaking the all-or-nothing guarantee
and firing the assert on a legitimate race. The next flush picks up the
new cap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round of minor cleanups on the delta symbol-dictionary work.

Tests for previously-uncovered defensive guards (all proven to fail
without their guard): PersistedSymbolDict.open recreates empty on a bad
version byte (a valid entry behind a corrupted version must not parse)
and on a > Integer.MAX_VALUE length (a >2GB dictionary can't map into one
int buffer -- driven by a fault facade); and the send loop's
ensureSentDictCapacity latches a terminal instead of overflowing the int
capacity math when the mirror would exceed MAX_SENT_DICT_BYTES.

openExisting nulls buf after freeing it on the bad-magic/bad-version
path, so the catch block cannot double-free if ff.close ever threw.

PersistedSymbolDict.appendSymbol is marked @testonly -- production
persists a frame's whole new-symbol range via appendSymbols /
appendRawEntries; only tests append one symbol at a time.

Documented that symbol-dict catch-up frames sit outside the poison-frame
detector: a deterministically-NACKed catch-up recycles forever (paced),
which is fine since a catch-up only re-registers already-accepted symbols.

Consolidated four reinvented recursive rmDir/rmDirRec test helpers into
one TestUtils.removeTmpDirRec (the existing removeTmpDir is flat and can't
clean the store-and-forward slot subdirectory layout), and removed the
imports that freed up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
seedGlobalDictionaryFromPersisted returned early when the persisted
.symbol-dict recovered empty (size 0), skipping the torn-dictionary
guard on the line just below. A host crash -- or a prior full-dict-mode
session whose openClean failed and never wrote the side-file -- can
leave a size-0 dictionary while the segment frames that introduced its
ids survive. Resuming then seeds the producer with an empty dictionary,
so the next new symbol reuses id 0; once that frame is trimmed and the
connection recycles, the catch-up re-registers the recovered ids and
the producer's rows are silently misattributed.

The send loop's replay guard cannot catch this: a deltaStart=0 frame
self-heals the catch-up mirror rather than tripping the gap check, so
the seed-time guard is the only defense. Run it before the size-0
short-circuit. A genuinely empty slot (no symbol-bearing frames) still
has recoveredMaxSymbolId == -1, so -1 >= 0 is false and the empty case
returns without throwing.

Add testTornDictTotalLossFailsCleanOnResume covering the size-0 total
tear (frames replay from deltaStart=0, so only the seed-time guard can
reject it). Repoint two tests that had reached the I/O-thread guard
through the bypassed early return: testTornDictionaryFailsCleanly...
now expects the build-time guard, and testTornDictionaryOneIdGap...
reaches the I/O guard through the unopenable-dictionary path instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CursorSendEngine.recoveredMaxSymbolId walked every recovered frame,
including the aborted orphan-deferred tail -- the frames trySendOne
retires without ever transmitting. Those frames typically introduce the
highest symbol ids, so a host crash that tore the persisted .symbol-dict
down to the committed ids while an orphan-tail frame referenced a higher
one inflated recoveredMaxSymbolId and made seedGlobalDictionaryFromPersisted
reject an otherwise fully recoverable slot. The producer never reuses an
orphan id on the wire -- the tail retires before its new frames send -- so
the rejection was a false positive.

Bound the maxSymbolDeltaEnd walk to recoveredCommitBoundaryFsn so only
committed frames count. MmapSegment and SegmentRing take a maxFsnInclusive
argument; a frame whose fsn (baseSeq + i) exceeds it is skipped, and the
walk still advances past it. A slot with no orphan tail is unchanged: the
boundary equals publishedFsn, so every frame still counts.

Add testRecoveredMaxSymbolIdExcludesOrphanTailFrames: a committed delta
frame registering ids 0,1 plus a deferred (orphan) frame registering id 2
must recover recoveredMaxSymbolId == 1, not 2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PersistedSymbolDict.openExisting frees the loaded-entries buffer in the
truncate-failure branch, then calls ff.close(fd). If that close threw, the
enclosing catch re-freed the same buffer -- a double-free -- because,
unlike the bad-magic branch, this branch did not null the pointer first.

Null entriesAddr/entriesLen right after freeing them, matching the buf
discipline above, so the catch's guard skips the second free. Correct the
catch comment, which wrongly claimed nothing between the malloc and the
return could throw -- the truncate branch's free-then-close is exactly
such a point.

This is defensive hardening: FilesFacade.close returns a status code and
never throws (the native close never raises), so the path is unreachable
today, but the nulling keeps it safe against a future facade or edit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The catch-up-NACK routing guard (fsn <= ackedFsn) had its equality case
untested: the common single-frame catch-up maps its only catch-up frame
to fsn == ackedFsn exactly, yet the existing test only exercised the
strictly-below case (fsn = -2 < ackedFsn = -1). A "<=" -> "<" mutation
therefore survived the suite while routing the single (or last) catch-up
frame's NACK onto the poison-strike path and laundering the detector.

Add testPostSendCatchUpNackAtAckedFsnBoundaryDoesNotStrike, which drives
fsn == ackedFsn == -1 and asserts poisonStrikes stays 0. poisonStrikes,
not poisonFsn, is the discriminator here: a wrongly-charged strike sets
poisonFsn to the rejected fsn -1, which is the "no suspect" sentinel, so
a poisonFsn check is blind at this boundary. The mutation charges the
strike (0 -> 1) and the test fails; the production guard is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cap-gap settle budget (catchUpCapGapAttempts) counts consecutive
catch-up cap gaps across reconnects and resets to 0 on a successful
catch-up, so gaps interspersed with successes -- a rolling-cap cluster --
never accumulate to a spurious terminal. The existing latch test only
accrues gaps under one fixed cap with no success interleaved, so a
dropped reset would have gone unnoticed.

Add testSuccessfulCatchUpResetsCapGapBudget: accrue max-1 cap gaps, let a
larger-cap node accept the dictionary, then assert the budget is back to
0 and that max-1 further gaps still latch no terminal. Make the stub
client's advertised cap mutable so the test can model the rolling cap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
flushPendingRowsSplit's write-ahead dictionary persist (the
appendRawEntries fast path) runs only in the exceptional over-cap split
path, and both split tests were memory-mode, so the split-times-persist
path was never exercised with a live PersistedSymbolDict -- a recovered
slot relies on it to rebuild the delta frames.

Add testFileModeSplitPersistsDictBeforePublish: a file-mode two-table
batch that exceeds the server cap splits into two frames, and the first
frame's persist must record both new symbols in .symbol-dict. Dropping
the split persist leaves the file empty and fails the test; the non-split
file persist test is unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review findings on the delta symbol-dictionary change.

Tests (close two coverage gaps):
- SelfSufficientFramesTest pins the split-flush pre-flight baseline
  advance: a large-delta two-table split whose second frame fits only
  with an empty delta. Removing the simBaseline advance mis-sizes that
  frame with the delta and wrongly rejects a shippable batch.
- CursorWebSocketSendLoopOrphanTailTest pins that a zero-count delta
  frame anchors recoveredMaxSymbolId at its baseline, not 0. Skipping
  it would under-strand a torn dictionary and risk a silent id shift.

Docs (correct load-bearing recovery semantics):
- seedGlobalDictionaryFromPersisted no longer claims the drainer always
  self-heals: it does so only when a surviving frame straddles the tear;
  a higher-baseline survivor quarantines the slot, a deliberate
  conservative over-strand.
- maxSymbolDeltaEnd documents that a zero-count delta frame contributes
  its deltaStart, not 0.
- The catch-up cap-gap budget documents that a transient reconnect never
  increments or resets the counter (only a successful catch-up resets
  it), so a transient cannot burn the terminal budget.

Hardening:
- writeVarint and putVarint assert a non-negative value: the signed loop
  writes one truncated byte for a negative long while varintSize returns
  10.
- PersistedSymbolDict.open diverts a file at exactly Integer.MAX_VALUE to
  a clean re-create instead of a doomed 2GB malloc (> becomes >=).
- readLoadedSymbols bounds its varint decode (shift > 35) like its
  sibling parsers.

Production behavior changes are limited to the two varint asserts, the
>= boundary, and the defensive varint bound; the rest is tests and
documentation. Full delta-dict suite green on JDK 25 with -ea.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The failed-publish regression tests exercise a persist that SUCCEEDED
(the publish is what fails), so nothing pinned the persist-FAILURE
trigger: a short write (disk full / quota) mid-persist. Add a
ShortWriteOnceFacade that lands one entry append short and two tests --
one per production persist path (appendRawEntries fast path,
appendSymbols slow path) -- asserting the load-bearing idempotency
invariant the write-ahead persist relies on: a short write throws
WITHOUT advancing size()/appendOffset, so a retry keyed off pd.size()
re-persists the same range at the same offset and recovers a gap-free,
duplicate-free dictionary.

Verified as regression guards: reordering the production code to advance
size before the written==len check fails both tests with "short write
must NOT advance size expected:<0> but was:<2>".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three review follow-ups on the delta symbol-dictionary change:

- PersistedSymbolDict relinquishes fd (sets it to -1) before each
  in-branch ff.close and guards the catch-block closes with fd >= 0,
  so a throwing in-branch close cannot double-close via the catch.
  Completes the null-before-free discipline the buffers already use.

- QwpWebSocketEncoderTest pins the split path's context-free-body
  invariant: a table's body bytes must encode identically solo and
  combined, since flushPendingRowsSplit sizes each frame from the
  combined encode's splitFrameBodyBytes rather than re-measuring. A
  future column encoder that carried cross-table state now fails here
  instead of silently stranding a deferred prefix.

- CursorSendEngine imports QwpConstants instead of spelling it out
  fully-qualified eight times, matching its sibling send loop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A store-and-forward slot written in full-dict fallback -- the
.symbol-dict could not open when writing, so every frame re-ships the
whole dictionary from id 0 -- leaves self-sufficient frames behind but
no side-file. On recovery the engine opens a fresh empty .symbol-dict,
so the surviving frames out-reach it, and the sender's seed-time guard
mistook the empty dictionary for a host-crash tear: it failed
Sender.build() for a slot the orphan drainer drains fine.

CursorSendEngine now tells the two cases apart with a new
maxSymbolDeltaStart walk (MmapSegment, SegmentRing). When the persisted
dictionary is a subset of the ids the frames reference but every such
frame is self-sufficient (maxSymbolDeltaStart == 0), the engine discards
the empty side-file so isDeltaDictEnabled() reports false and the slot
recovers in full-dict mode, exactly how it was written. A genuine torn
delta dictionary keeps a deltaStart > 0 frame and still fails clean.

testFullDictFramesRecoverInFullDictModeInsteadOfBricking covers the new
path -- red without the discard, green with it. The torn-delta
fail-clean tests are unchanged.

Also regroups CursorWebSocketSendLoop's six new mutable delta-dict
fields with the other mutable fields (deltaDictEnabled stays final, in
its alphabetical slot) and adds the missing thousands separator to a
test's 16_384 batch-size literal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The QWP store-and-forward recovery path treated a damaged .symbol-dict as
unrecoverable in two places, stranding slots that are in fact replayable.
Both stem from keying on the dictionary FILE rather than on what the
surviving frames themselves define.

CursorWebSocketSendLoop ran the torn-dictionary guard unconditionally but
gated accumulateSentDict, the reconnect catch-up and the mirror seed on
engine.isDeltaDictEnabled(). That flag goes false exactly when a disk
slot's dictionary fails to open -- fd exhaustion, a read-only remount,
ENOSPC -- which is precisely when the frames already on disk are still
deltas. With the mirror gated off, sentDictCount froze at 0 while the
ungated guard kept comparing against it, so the frame at deltaStart=1
latched a terminal and the background drainer quarantined the slot behind
a .failed sentinel. Yet replaying that sequence from id 0 rebuilds the
dictionary on the server contiguously and drains perfectly. The loop now
always mirrors, always catches up and always guards: the mirror is its
model of what the SERVER holds, so it must track every mode.
isDeltaDictEnabled() now governs only what the producer emits and
persists.

seedGlobalDictionaryFromPersisted threw whenever the surviving frames
out-reached the persisted dictionary. That bricked Sender.build() for
slots the drainer replays fine, and permanently: build()'s catch releases
the slot lock, but the sender that would have hosted the orphan drainer is
the one that just failed, so the retry recovers the same slot and throws
again. Relaxing the guard would have been worse than the bug -- with an
empty dictionary the producer hands its next symbol id 0, an id the frames
already define, and accumulateSentDict skips it, so the next catch-up
replays the stale symbol and misattributes it silently. The producer now
seeds from the dictionary's intact prefix AND THEN from the surviving
frames' own delta sections, through collectReplaySymbolsAbove
(MmapSegment -> SegmentRing -> CursorSendEngine). Those are the same two
sources, in the same order, that the send loop's mirror is built from, so
the producer's delta baseline and the mirror's coverage land on the same
number by construction.

A genuine gap -- ids introduced by frames since acked and trimmed away --
still fails clean, and now on exactly the condition the send loop's replay
guard trips on, so producer and drainer agree on which slots are
recoverable.

The two cap-gap settle-budget tests derived their retriable loop bound
from MAX_CATCHUP_CAP_GAP_ATTEMPTS itself, so a regression of that constant
to 1 -- the very bug they name -- ran the loop zero times and let the test
pass green. They now pin the budget against a literal.

Tests: an untrimmed unopenable-dictionary slot must replay gap-free; a
torn (subset) and a wholly lost dictionary must rebuild from the surviving
frames and place a new symbol above the recovered tip. The two
watermark-stamped siblings still fail clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Four fixes to the QWP delta symbol-dictionary work, all on the
store-and-forward recovery and failover paths.

Set an unreplayable slot aside instead of bricking the sender. A
host crash can tear the unsynced .symbol-dict relative to the frames
it serves, and the resulting frames genuinely cannot be replayed.
Detecting that is right; throwing out of build() was not. senderId
defaults to a stable name, a not-fully-drained slot is retained on
close, and so every subsequent build() re-recovered the same slot and
threw again -- forever, until an operator removed the directory by
hand. The application could not construct a Sender at all, so it could
not even buffer new rows: one already-lost batch became an unbounded
outage of everything after it. build() now renames the slot aside,
marks it .failed for the orphan drainer, and continues on a fresh
empty slot. The bytes are kept for resend; not one of its frames ever
reaches the wire.

Never recreate a dictionary over an existing file. open() fell through
to openFresh() -- which is O_TRUNC -- on any read or parse failure, so
a single transient EIO, or a rollback to a client that does not know a
newer version byte, destroyed the only copy of load-bearing state and
made every surviving delta frame permanently unreplayable. It now
degrades to null, leaving the file intact: the sender falls back to
full self-sufficient frames and a later attempt can still recover the
slot in full.

Give the catch-up cap-gap terminal a wall-clock dwell. The settle
budget was attempt-based, and 16 attempts at the capped reconnect
backoff burn through in about two minutes -- less than an ordinary
rolling restart of the larger-cap node, which is the very transient
the budget exists to ride out. Escalation now needs both the strike
count and a minimum dwell, anchored at the first cap gap of the
episode, defaulting to five minutes and tunable through
catchup_cap_gap_min_escalation_window_millis.

Checksum the persisted dictionary per chunk, not per entry. Crc32c is
a native call, so a per-entry checksum put one JNI transition, one
sub-cache-line copy and one redundant varint decode on the producer
thread for every new symbol -- a thousand native calls per flush on
the one-new-symbol-per-row batch this feature exists to serve. A chunk
is exactly the set of symbols one frame introduces, and every frame's
deltaStart is a chunk boundary, so per-entry granularity bought no
extra recoverable prefix. Recovery also folds its two passes into one.

ensureScratch now throws instead of clamping below the requested size,
which would have turned a clean out-of-memory into a native-heap
overflow on the dictionary write path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WsSenderConfigHonoredTest enforces that every key registered in
ConfigSchema is actually plumbed through to the sender. The new
catchup_cap_gap_min_escalation_window_millis key needed its
assertion.
Four behaviours the delta symbol-dictionary work relies on had no test
that could fail if they regressed. Each mutation below leaves the whole
suite green today.

Pin the split path's write-ahead ordering. Moving the persist after the
publish in flushPendingRowsSplit left every test passing, because the
only split-path test checked the dictionary after a SUCCESSFUL flush --
where the symbols land either way. Only a publish that FAILS while the
symbols are new can tell the orderings apart, so the new test caps the
server batch size to force the split and shrinks the segment so the
split frame cannot be published. With the ordering broken, a process
crash in that window would leave a recorded frame whose deltaStart
exceeds the recovered dictionary, and recovery would quarantine a slot
that should have drained cleanly.

Pin the persist-failure translation. A short write to .symbol-dict --
a full disk, an exhausted quota -- throws a low-level
IllegalStateException that persistNewSymbolsBeforePublish wraps in a
LineSenderException. Deleting the wrap left the suite green, yet the
raw exception would sail past every caller's catch around flush().
Nothing could reach that path: PersistedSymbolDict has facade-aware
overloads but CursorSendEngine called only the FilesFacade.INSTANCE
ones, so the engine now takes a FilesFacade and the test drives a real
short write through the real producer path.

Pin the sealed-segment walk. Deleting the sealed half of
SegmentRing.maxSymbolDeltaEnd left the suite green because every test
fits its slot in one active segment. It is not a benign wrong answer:
recoveredMaxSymbolId collapses to -1, and the seed guard fires on
`>= pd.size()`, so a value that is too low never fires at all -- a torn
dictionary is trusted and the producer silently reuses ids the
surviving frames already define. The walk earns its keep exactly when
the active segment holds nothing but an aborted deferred tail, which is
the crash the guard was built for, so the test reproduces that shape.

Hoist DelegatingFilesFacade into test/tools so a fault seam can be
shared rather than stamped into each test that needs one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The full-dictionary rebuild and the unreplayable-slot quarantine answer
two halves of the same question and have to be ordered, not merged.

Rebuilding the dictionary from the surviving frames' own delta sections
rescues most of the slots that used to fail, so it must run FIRST -- a
quarantine that pre-judged the slot on its own coverage arithmetic would
set aside slots recovery can still replay in full, and strand data that
drains perfectly.

What it cannot rescue it still throws on, and that throw still escaped
build(). senderId is stable and a not-fully-drained slot is retained on
close, so every retry re-recovered the same slot and threw again: the
application could not construct a Sender at all, and so could not even
BUFFER new rows. One already-lost batch became an unbounded outage of
everything after it, which inverts the guarantee store-and-forward
exists to give.

So seedGlobalDictionaryFromPersisted is now the single authority on
whether a slot is replayable -- it is the only code that has tried every
source of truth -- and its give-up is typed. build() waits for that
verdict rather than forming its own, and on it sets the slot aside: the
bytes are kept for forensics and resend, the .failed sentinel tells the
orphan drainer the copy is human-in-the-loop, and the producer continues
on a clean slot. The engine's own isRecoveredDictionaryIncomplete()
guess goes away with it.

Not one frame of a set-aside slot ever reaches the wire; that safety
property is unchanged and still asserted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A slot whose symbol dictionary was damaged was being condemned as
unreplayable while every symbol it needed was sitting on disk. The
sender told the user to resend data that had never been lost, and after
the previous commit it set the slot aside instead.

Three things kept the rebuild from ever running.

setCursorEngine gated the producer's seed on deltaDictEnabled. That flag
is false exactly when the slot's dictionary failed to open -- fd
exhaustion, a read-only remount, ENOSPC -- which is precisely when the
frames on disk are the only surviving copy of the symbols. The rebuild
was dead code for the very case it was written for.

collectReplaySymbolsAbove walked from ackedFsn+1, so it skipped the
acked frames. But an acked frame is not trimmed the instant it is
acked: trim unlinks whole SEALED segments. The frames that registered
the early ids are almost always still there, and they are exactly the
ids the replay set's deltas start above. It walks every frame on disk
now, and a slot whose registering frames really HAVE been trimmed still
reports the gap, because then nothing holds them.

And the send loop's mirror seeded only from the dictionary file, never
from the frames -- so even with the other two fixed, the loop would
still condemn (deltaStart > sentDictCount) a slot the producer had just
seeded successfully. The mirror is the loop's model of what the SERVER
holds and it is what the catch-up ships, so it now seeds from the same
two sources in the same order, and the two land on the same number by
construction. Only when a surviving frame actually depends on ids it
does not carry: at maxSymbolDeltaStart == 0 every frame re-registers
from id 0 as it replays, so a full-dict slot stays catch-up-free.

The recovery tests now assert the strong form -- the fresh server's
reconstructed dictionary must come back COMPLETE and in order, which
can only happen if the catch-up carried the ids the acked frames still
hold. A null-padded or shifted dictionary fails there, which is the
corruption the old guard condemned these slots to avoid. It never had
to.

The quarantine now covers only what is genuinely unrecoverable: the
registering frames trimmed away and the dictionary torn, so nothing
anywhere holds those ids.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The seed's ungating had no test: nothing wrote a NEW symbol after
recovering a slot whose dictionary could not be opened, which is the
only place the difference shows.

It shows on the wire. Re-gate the seed and the resumed producer restarts
its id space at 0, on top of ids the surviving frames already define --
the server's dictionary comes back as

    [after-recovery, sym-1, sym-2, sym-3, sym-4, sym-5]

with sym-0 overwritten. Two symbols on one id, and the send loop's
mirror still reads that id as sym-0, so mirror and producer disagree
about what it MEANS. That is the misattribution the whole dictionary
machinery exists to prevent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sendDictCatchUp tested `catchUpCapGapFirstNanos < 0` to decide whether a
symbol-dictionary cap-gap episode was already open. A System.nanoTime()
instant is only meaningful as a difference: its origin is arbitrary and
the spec permits negative values, so its sign cannot encode state.

CursorWebSocketSendLoopCatchUpAlignmentTest backdates the episode anchor
by two hours to prove that a cap gap outliving the escalation window
latches a terminal. On Linux nanoTime() is nanos-since-boot, so on a
freshly booted CI agent it sits far below two hours of nanos and the
backdated anchor came out negative. The loop then read that real anchor
as "unset", re-anchored it to now on every strike and pinned
episodeNanos at ~0: the dwell was never satisfied, no terminal latched,
and the test failed. It fails on every machine with under two hours of
uptime and on no long-lived dev box, which is why only CI saw it.

Key the episode off the strike count instead, exactly as
recordHeadRejectionStrike keys its own off poisonStrikes. -1 survives as
a debug marker but no longer decides anything, so no state rides on a
timestamp's sign.

Add testCapGapEpisodeWithANegativeAnchorStillEscalates, which plants the
negative anchor directly and so pins the sentinel whatever the host's
uptime; the existing test only reddens where uptime happens to fall under
its two-hour backdate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CursorWebSocketSendLoop constructor mallocs the sent-dictionary
mirror from the persisted dictionary's prefix, then extends it from the
surviving frames' deltas. If that second step throws -- a native realloc
OOM, or the MAX_SENT_DICT_BYTES ceiling -- the throw leaves the loop
unpublished, so neither ensureConnected's catch nor BackgroundDrainer's
finally can close() it, and the already-malloc'd mirror leaks. Free it on
any throw, mirroring the loopNeverRan free in close()/ioLoop's exit.

Also close two test-coverage gaps the review flagged:

- The send loop's torn-dict guard (deltaStart > sentDictCount in
  trySendOne) is the drainer path's sole defense against replaying a
  frame whose symbols were trimmed away; the foreground path is
  quarantined earlier, so this fire direction was unexercised. Cover it
  at the send-loop level against a stub client (the drainer integration
  path cannot connect on darwin), asserting zero frames ship and the
  terminal latches.

- quarantineTornSlot must fail loudly rather than drop bytes when it
  cannot set a slot aside. Saturate every quarantine candidate and
  assert build() throws and preserves the slot on disk.

A TestOnly seam (forceMirrorSeedFailureForTest) makes the ctor seed
throw deterministically; the unreplayable-slot setup is now a shared
helper.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment-only follow-ups from the delta symbol-dict review; no behaviour
change.

- quarantineTornSlot marks the set-aside slot .failed unconditionally, so
  the orphan drainer never re-examines it. Fix the two comments that
  claimed the drainer could still deliver a quarantined slot; state that
  it is a human-in-the-loop copy whose bytes are kept for a manual resend.

- sendCommitMessage's full-dict-mode comment claimed the commit frame
  stays self-sufficient, but the prior flush reset currentBatchMaxSymbolId
  to -1, so the frame carries an empty delta. Say so -- it is harmless,
  the preceding data frames already registered the dictionary.

- Move three javadoc blocks stranded above the wrong method back onto
  their own: MmapSegment.maxSymbolDeltaEnd, SegmentRing.maxSymbolDeltaEnd
  and Sender.closeFlushTimeoutMillis were left undocumented, and the
  stranded @param names no longer matched the method beneath them.

- Drop a double blank line in CursorSendEngine.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PersistedSymbolDict.open() computed the routing length as
ff.length(path) and sent anything below HEADER_SIZE to openFresh(),
which O_TRUNCs the file. A stat that fails transiently (an EIO on a
flaky disk) returns -1 for a fully populated dictionary, so that
routing could truncate the only copy of load-bearing recovery state --
the exact destruction the class's "never recreate over an existing
file" contract forbids, and which the openExisting read path already
guards against but the routing check did not.

A stat error reports -1, distinguishable from a genuine sub-header stub
in [0, HEADER_SIZE), so open() now degrades to null when the file
exists but its length reads negative: it falls back to full
self-sufficient frames and leaves every byte on disk for a later
attempt, once the transient clears.

Also move appendRawEntries' per-entry (addr, len, count) consistency
walk behind an assert. It re-reads every entry's length prefix on the
common flush path, yet its sole caller derives count and len from one
beginMessage, so it cannot fire today. The assert keeps the structural
guard in the client's -ea test suite while eliding it from production,
where client apps run without -ea.

PersistedSymbolDictTest now covers a transient length fault leaving the
file intact and recovering once it clears, and an inconsistent
raw-entries triple being rejected under -ea.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mtopolnik

Copy link
Copy Markdown
Contributor

[PR Coverage check]

😍 pass : 743 / 851 (87.31%)

file detail

path covered line new line coverage
🔵 io/questdb/client/Sender.java 35 46 76.09%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegment.java 99 124 79.84%
🔵 io/questdb/client/cutlass/qwp/client/GlobalSymbolDictionary.java 5 6 83.33%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/PersistedSymbolDict.java 235 278 84.53%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java 29 33 87.88%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java 219 237 92.41%
🔵 io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java 85 91 93.41%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java 1 1 100.00%
🔵 io/questdb/client/cutlass/qwp/client/QwpWebSocketEncoder.java 4 4 100.00%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/UnreplayableSlotException.java 2 2 100.00%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java 20 20 100.00%
🔵 io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java 8 8 100.00%
🔵 io/questdb/client/impl/ConfigSchema.java 1 1 100.00%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request tandem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants