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

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

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, 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 INVALID_DICTIONARY_INDEX; a contiguous append (deltaStartId == size()) and a lower start that re-registers existing ids both stay allowed. 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.

Segment recovery: late-delivered mmap faults

Independent of the dictionary work, this branch also fixes recovery of a store-and-forward slot containing an unbacked or sparse segment page. MmapSegment converts the resulting InternalError around its own reads, but that only works when HotSpot delivers the fault at the faulting instruction. Before JDK 21 (JDK-8283699) delivery is asynchronous -- the error surfaces at the next return or safepoint check, which can be an arbitrary caller frame -- so on JDK 8/11/17 it escapes every handler in that class. SegmentRing caught only MmapSegmentException, so an escaped fault reached its outer catch, which closes every already-recovered segment and rethrows: one bad page aborted recovery of the whole slot and stranded the frames the sibling segments still held.

SegmentRing's per-file arm now also converts a recognized mmap access fault into a skip of that one .sfa, and rethrows anything else so resource exhaustion and programmer errors still propagate as before. This is what makes the two MmapSegmentRecoveryFaultTest guards pass on the shipping JDK 8 target; they had been failing there since the branch was cut.

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 (catchup_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 parse error that closes the connection. That is the intended trade: a gapped frame is deterministic under replay, so retrying cannot help, and the sender must re-register from an id the server holds.

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, 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).
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.
MmapSegmentRecoveryFaultTest -- a beyond-EOF or unbacked segment page is skipped per file, and the sibling segments still recover; an unrecognized InternalError still propagates.
OSS QwpSymbolDecoderTest -- a gapped delta is rejected, a contiguous append at deltaStartId == size() is accepted, and a parsed connection dictionary holds no nulls.
The enterprise SqlFailoverQwpClientLosslessTest (file-mode failover) passes end-to-end against a real server.

🤖 Generated with Claude Code

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

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

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

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

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

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

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

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

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

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

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

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

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

bluestreak01

@glasstiger

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

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

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

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

Critical

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

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

Code-path trace (verified):

flushPendingRows runs, in order:

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

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

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

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

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

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

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

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

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

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

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

Verification (commands recorded):

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

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

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

Moderate

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

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

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

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

Minor

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

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

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

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

Downgraded (false positives — verified against source)

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

Coverage map

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

Summary

Verdict: REQUEST CHANGES.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

testQuarantineRenameFailurePreservesOriginalSlot counted the slot's .sfa files BEFORE build(), then asserted the count was unchanged after the failed rename. Recovery legitimately unlinks an empty never-rotated hot-spare on the way in (SegmentRing drops a recovered segment whose frameCount() is 0 with no torn tail), and whether SegmentManager had provisioned one by then is a race -- so the assertion measured that race rather than the rename. It failed 3 of 3 standalone runs, and inverted (2 vs 3) under load. Snapshot inside the quarantineAfterCloseHook instead: after the engine closed, before the rename. That is exactly what "a failed rename preserves the slot" means, and it is immune to the hot-spare race because both counts observe the post-recovery state. Assert the snapshot is non-zero as well, so the equality cannot pass vacuously on an empty slot. The product was never at fault: the reclaimed segment holds no frames, and close() unlinks nothing -- instrumenting quarantineTornSlot shows the same segments before and after it. Mutating that method to delete a segment during the failed rename still fails the test (expected 2, was 1), so it continues to catch real destruction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Five review findings on the delta symbol dictionary. The three performance ones all sit on paths that scale with symbol cardinality -- the workload this feature exists for introduces a new symbol per ROW -- and the two guards were pinning nothing. RecoveredFrameAnalysis.foldDelta issued one bounds check, one capacity check and a ~12-byte copyMemory per SYMBOL. runningCoverage is loop-invariant (it only advances after the walk) and id ascends, so "id >= runningCoverage" is a step predicate: the entries a frame contributes are always ONE contiguous run at the tail of its delta section. Note where the run starts and copy it once. Recovery walks the whole backlog, so this traded millions of stub-dispatched small copies for one bulk copy per frame. It also drops the partial prefix the per-entry version left behind on a markGap bail-out; that residue was already unreachable (a gap pins coverage() at -1, and a later self-sufficient frame resets the buffer), so not writing it is equivalent and leaves less state to reason about. accumulateSentDict already solved the same problem this way one file over. Crc32c.updateUnsafe read each 8-byte slice-by-8 block with eight getByte calls: 16 loads per block where 9 do. Consume it with two getInt loads. getInt is an Unsafe intrinsic exactly as getByte is, so the catchable InternalError this method exists for is unaffected; the loop becomes little-endian, which costs nothing because the formats it checksums are already little-endian throughout. This is the whole recovery CRC. The differential test against the native intrinsic (8 alignments x 28 lengths x 3 seeds, plus the chaining fuzz) proves it bit-identical. The sent-dict entry-ends index was maintained incrementally by accumulateSentDict and built eagerly by the constructor, yet sendDictCatchUp is its ONLY reader. That put a store per new symbol on the per-frame I/O path and retained 4 bytes per symbol for the connection's whole life, to serve a reconnect that may never come. Build it lazily instead: ensureSentDictEntryIndex() already notices sentDictIndexedCount falling behind and rebuilds, at most once per reconnect, dwarfed by shipping the same bytes over the wire. A connection that never reconnects no longer allocates the index at all. testCtorFreesSeededMirrorWhenFrameSeedThrows could not fail. The seam threw BEFORE ensureSentDictCapacity -- the call that copy-on-writes the borrowed prefix into a loop-OWNED allocation -- so the cleanup it guards, gated on sentDictBytesOwned, freed nothing. Deleting that cleanup left the test green. Move the seam past the grow: deleting the cleanup now fails with a 4096-byte NATIVE_DEFAULT leak. The test's premise comment described the old copy-based design and is corrected. testPersistFailureSurfacesAsLineSenderException drove code production never runs. FilesFacade.isMmapAllowed() defaults to (this == INSTANCE), so installing any fault facade silently swapped the dictionary onto the positioned-write fallback -- the act of injecting the fault replaced the code under test. Fault a refused ff.allocate on the mmap append window instead, with the facade opting into isMmapAllowed(): that is both the real shape of ENOSPC here and the path production takes. Reverting the wrap now escapes as "could not grow mmap append region", from the mapped path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The full-dict-fallback branch discards a persisted .symbol-dict whose frames carry their whole dictionary inline (maxSymbolDeltaStart == 0 and recoveredMaxSymbolId >= size). But the recovery analysis was folded with the dictionary's size as its baseline, and every consumer of it presents the baseline it derived the same way: once pd is null, seedGlobalDictionaryFromPersisted computes baseline 0, and checkedRecoveryAnalysis rejects a baseline that disagrees with the fold. Discarding a dictionary that held entries (size > 0) therefore threw IllegalStateException("recovery symbol baseline mismatch") out of build(). That is not an UnreplayableSlotException, so build()'s quarantine handler could not set the slot aside. With a stable senderId every restart re-recovered the same slot and threw again, so the application could never construct a Sender -- it could not even buffer new rows. Exactly the outage quarantineTornSlot exists to prevent, and on a slot that is fully recoverable, since its frames need no dictionary. It takes one transient plus one crash: a session whose .symbol-dict fails to open (EIO, fd exhaustion, a Windows share lock) runs full-dict fallback and, per the never-recreate contract, leaves the previous session's populated side-file intact; if that session then crashes, this recovery opens a dictionary with size > 0 beside self-sufficient frames that out-reach it. The prior regression test covered only the size == 0 case. Re-fold at baseline 0 after the discard so producer, mirror and replay guard all anchor at 0 -- the full-dict mode the slot was written in. The new test plants a populated survivor beside full-dict frames; reverting the re-fold throws "recovery symbol baseline mismatch [expected=3, actual=0]". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Four fixes across the recovery, orphan-scan and drainer paths, all on inputs a real cluster can produce. Bound the oversized-batch throw. flushPendingRowsSplit's all-or-nothing pre-flight throws when a single table's frame exceeds the server cap, and keeps the batch buffered so a rejected flush does not silently drop the caller's rows -- correct, and better than the old behaviour that discarded every table's rows while reporting failure. But a batch that can never fit the reachable cap then re-rejects on every flush(), growing unbounded if the caller keeps appending. The batch cannot drain until a larger-cap node returns or the sender is closed, so the message now says so, rather than reading as a transient a caller would retry into an ever-larger batch. Stop OrphanScanner counting the legacy-reader guards as data. CursorSendEngine plants .qwp-v2-guard-a/b.sfa in every disk-mode slot, before recovery or any segment, and its failure path does not unlink them. A construction that then throws (ENOSPC on the initial segment) leaves a slot holding only guards, which the scanner reported as an orphan with unacked data: a drainer adopted it, failed its own build under the same disk pressure, and dropped a permanent .failed sentinel plus an ERROR on a slot that never held a byte. Exclude the guards by name (SegmentRing's own predicate, now shared) -- a real segment beside them still counts. Saturate the drainer's capability-gap budget. reconnect_max_duration_millis is validated only as > 0, so a large value (Long.MAX_VALUE is the natural "never give up") wrapped the raw millis*1_000_000 multiply negative, and the OR-gated elapsed check then quarantined the slot on the FIRST capability-gap sweep, skipping the whole 16-sweep settle budget -- asking for more tolerance bought none. Convert with TimeUnit.toNanos, which clamps at Long.MAX_VALUE. The keepalive-interval and poison-escalation windows in CursorWebSocketSendLoop's constructor had the identical raw multiply -- the poison one is the exact twin of the cap-gap dwell the PR already hardened -- and a negative there escalates a transient to a producer-fatal terminal on strike count alone; convert both the same way. Make the legacy-reader barrier durable and stop re-truncating it. Its content never changes, yet it was rewritten via openCleanRW (truncate) on every engine open and never synced, so every open re-opened a window in which a host crash leaves a zero-filled guard beside already-durable v2 segments -- and a rolled-back v1 reader then skips the guard (bad magic) and the segments (bad version) alike, reads the slot empty, and truncates the unacked log. Verify an existing guard and rewrite only a missing or damaged one, and msync the guard on creation (33 bytes, once per slot lifetime) so it is durable before the data it protects. The guard frame is not byte-reproducible, so the test discriminates kept from rewritten by byte-identity across a reopen and asserts only semantic validity after a repair. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

connectWithRetry set its deadline as startNanos + maxDurationMillis * 1_000_000L, a raw millis-to-nanos multiply. A large reconnect_max_duration_millis -- Long.MAX_VALUE is the natural "retry until the server boots" value, and the builder setter enforces no upper bound -- wraps that product negative, so the pre-condition retry loop runs zero iterations. The SYNC initial connect then throws "no attempts made" without ever calling the reconnect factory: the exact opposite of the requested patience. The prior commit hardened this same overflow class in the drainer's capabilityGapBudgetNanos and the loop dwells via TimeUnit.MILLISECONDS.toNanos, but missed connectWithRetry, the primary consumer of the value. connectWithRetry now compares elapsed time against a saturating budget instead of an absolute deadline: toNanos alone still overflows the startNanos + budget sum once it saturates to Long.MAX_VALUE, whereas the bounded nanoTime difference stays correct. A new regression test drives connectWithRetry with a Long.MAX_VALUE budget and asserts at least one attempt runs; it fails on the old code with "failed after 0ms / 0 attempts". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

PersistedSymbolDict.appendSymbols encoded the mapped chunk in two passes over the id range: a first loop called Utf8s.utf8Bytes on every symbol just to size the header varint and the mmap reserve, then a second loop re-walked each symbol's UTF-8 length to write it. It now stages the entry region in the scratch buffer with a single walk per symbol and bulk-copies it into the append window after the header, dropping the redundant walk. The chunk it writes is byte-identical, so the exact-file-size "one chunk, one CRC" test still holds. A direct in-window encode cannot avoid the double walk: the back-to-back chunk format leaves no room to reserve and back-fill the header in place. The path is cold -- only a retry after a failed publish re-encodes from the dictionary; the steady state persists a frame's pre-encoded bytes through appendRawEntries. Add testMappedAppendSpansMultipleWindowsAndRecoversDenseIds: it appends a dictionary larger than one 4 MiB append window, asserts the window remaps (appendMapGrowthCount >= 2) without a positioned write, then reopens and checks every id recovers to its own symbol across the boundary. This exercises the ensureAppendMap remap arithmetic and the single-pass encode as chunks straddle the boundary, both of which the single-window happy path left untested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Production recovery runs one ordered fold (SegmentRing.analyzeRecovery -> MmapSegment.scanRecovery -> RecoveredFrameAnalysis) and seeds the producer through CursorSendEngine.addRecoveredSymbolsTo. The older per-metric walk -- collectReplaySymbolsAbove, maxSymbolDeltaEnd and maxSymbolDeltaStart on MmapSegment and SegmentRing, plus the CursorSendEngine.collectReplaySymbolsAbove wrapper -- had no production caller and diverged from the fold: it lacked the full-dict gap reset, the unacked-gap distinction and commit-boundary checkpointing, so the tests routing through it verified an algorithm production never runs. Delete all seven methods and the now-orphaned RecoveredFrameAnalysis.appendDecodedSymbols helper, then rewire the two tests that reached the walk (RecoveredFrameAnalysisTest, CursorWebSocketSendLoopOrphanTailTest) to the production addRecoveredSymbolsTo path. That is behavior-preserving -- same coverage, same decoded symbols -- and stronger, since the tests now drive the real seeding method. Drop the imports the deletion left unused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@testonly

Two test-quality fixes in the send-loop cursor suite; no production behavior change. De-reflection. CatchUpAlignmentTest, PoisonFrameTest, TornDictGuardTest and MirrorLeakTest reached into CursorWebSocketSendLoop by reflection -- reading private constants and fields, invoking private methods, and writing private state -- so a rename broke them as a runtime ReflectiveOperationException instead of a compile error. Add @testonly accessors and thin ...ForTest wrappers on CursorWebSocketSendLoop, each delegating verbatim to the same private member, and make forceMirrorSeedFailureForTest public; then rewire the four tests to call them. The rewrites drive the identical production paths, and the mirror-overflow guard now catches the exact public LineSenderException type rather than unwrapping an InvocationTargetException. De-flake. PoisonFrameTest asserted absolute wall-clock bounds -- a recycle is "immediate" under 300 ms, and a recycle count fits a fixed sleep window -- that a CI GC pause or starved scheduler can trip. Replace them with predicates only a real regression fails: consecutive recycle gaps must clear 3/4 of the reconnect backoff (an unpaced loop's sub-millisecond gaps fail, while a slow test thread only yields more paced samples); the deterministic zeroProgressRecycles branch value (level 0, then 1, then reset on progress); a first*2 <= second relative bound; and the kept second >= backoff lower bound, which starvation can only lengthen. Split the dwell test into a huge-window case (threshold reached, window still open, so no escalation -- asserting poisonStrikes == 3) and a small-window case crossed by a sleep strictly longer than the window, which must escalate to the PROTOCOL_VIOLATION terminal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@testonly

A cluster of small review follow-ups; only the first changes behavior. - The orphan-tail-retire re-anchor caught a CatchUpSendException and passed fail(e.getCause()), unwrapping the cap-gap marker so connectLoop's isCatchUpCapGap reset the orphan settle episode instead of preserving it. Route unwrap-aware: fail(isCatchUpCapGap(e) ? e : e.getCause()) keeps the attempt count and dwell anchor across the re-anchor recycle for a genuine cap gap, and unwraps to the raw cause otherwise, exactly as a normal send failure does. This only ever errs safe today -- more retries before an orphan drainer quarantines -- but the wrapper exists precisely to carry that signal. - Drop @testonly from BackgroundDrainer.connectWithDurableAckRetry: run() calls it in production, so the annotation was inaccurate. - Rename CatchUpSendException.capGap to isCapGap (boolean naming). - Restore the CursorWebSocketSendLoop final-field group and a QwpWebSocketSender import to alphabetical order. - DeltaDictCatchUpTest: fix a comment naming a test that does not exist. - DeltaDictRecoveryTest: delete the never-instantiated CountingHandler and the orphaned javadoc stacked above FullDiskDictFacade. - CursorSendEngineTest: rename the legacy-guard test to describe what it asserts (the guards force an FSN gap; current recovery adopts the v2 slot) -- it never drives a legacy reader to throw. - PoisonFrameTest: the two catch-up-only "does not strike" tests now assert poisonStrikes() == 0 explicitly instead of relying only on checkError() not throwing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

testLogicalLockReportsLockDirectoryCreationFailure built the lock directory path with Paths.get(parentDir, ".slot-locks"), which yields a backslash separator on Windows. SlotLock.acquireLogical builds it as parentDir + "/" + ".slot-locks" (forward slash), so the injected LockDirectoryFailureFacade -- which fails mkdir only when lockDir.equals(path) -- never matched the production path on Windows. The mkdir then succeeded, acquireLogical did not throw, and the test hit its fail() guard. It passed on Linux and macOS only because Paths.get uses a forward slash there. Build the test's lock-dir (and slot) path with the same "/" concatenation SlotLock uses, so the facade matches on every platform. Test-only change; no production behavior is affected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

sendRow read the volatile serverMaxBatchSize three times while checking one row against the cap. The I/O thread lowers that field mid-stream on a failover -- clearing it to 0 when the new node advertises no cap -- so a read torn across the guard and the throw could reject the row against a "cap" of 0, which means "no cap": the row rolled back and a retry succeeded, but the producer still saw a spurious "row too large for server batch cap" error. Snapshot the field into a local once, exactly as flushPendingRows already does, so the guard, the comparison and the message all use one consistent value. Also add QwpWebSocketSenderTest coverage for the per-row cap guard, which had none: an oversized row under a positive cap throws and rolls back to zero rows, and a large row commits when the server advertises no cap (the invariant the snapshot protects). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

sendDictCatchUp is the only reader of the sent-dictionary entry-ends index, and it walks entries strictly in order, so the index bought nothing: it added an O(n) rebuild pass per capped reconnect and kept N*4 bytes of native memory alive for the connection's whole life. Replace it with a running pointer that derives each entry's byte span from its own [len varint][utf8] framing in a single pass. Removing the index also deletes rebuildSentDictEntryIndex, whose bare LineSenderException on a malformed mirror unwound into connectLoop as a transient and reconnect-livelocked instead of failing loud. The packing loop's inline bound check now latches a terminal via recordFatal (like the sibling ensureSentDictCapacity), so a corrupt mirror stops the loop cleanly. Add testCorruptCatchUpMirrorLatchesTerminalNotLivelock to pin that terminal, and drop the now-obsolete entry-index assertions from the split catch-up test (renamed to testSplitCatchUpStagesOnlyPrefixAcrossReconnects). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CursorWebSocketSendLoop stored reconnectMaxDurationMillis but never read it: the running connectLoop retries with no wall-clock budget (store-and-forward Invariant B), and the initial-connect budget flows through the static connectWithRetry parameter instead. Remove the dead field, its assignment, and the parameter from all eight constructor overloads, updating the delegating calls and the two production and 24 test call sites. QwpWebSocketSender and BackgroundDrainer keep their own reconnectMaxDurationMillis -- both still feed it to connectWithRetry. Also harden two PersistedSymbolDict spots for consistency with their twins: - flushChunk computes bodyLen/recLen in long like commitMappedChunk, so a near-2 GiB scratch chunk cannot wrap the CRC and write length negative on the positioned-write (fault-facade) path. - decodeLoadedSymbols decodes exactly `size` entries and throws on a truncated entry or trailing bytes instead of silently under-filling the dictionary the way a short buffer previously would. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The store-and-forward logical slot lock lives in the shared <sf_dir>/.slot-locks directory, outside the slot dir so it survives a slot rename. Nothing reclaimed it: unlike the slot's .ack-watermark and .symbol-dict side-files, which CursorSendEngine.close() unlinks via removeOrphan on a fully-drained close, the logical lock and its .lock.pid sidecar were left behind. Under rotating senderIds sharing one sf_dir, .slot-locks accumulated one dead lock+pid pair per distinct slot name for the lifetime of sf_dir. SlotLock now exposes removeOrphanLogical, mirroring AckWatermark.removeOrphan and PersistedSymbolDict.removeOrphan, and CursorSendEngine's fully-drained cleanup calls it alongside the sibling side-file removals. acquireLogical and removeOrphanLogical share a resolveLogicalLock helper so the two can never target different files. The unlink is best-effort: the retiring engine still holds the directory-local lock, the real multi-writer guard, and a fully-drained retirement performs no rename, so the logical lock is not in use; a drainer momentarily mid-acquireLogical fails its immediate candidacy and directory-lock check and backs off. SlotLockTest covers the removal and its silent no-op on absent or invalid input; EngineCloseSlotLockReleaseTest adds an end-to-end test that a fully-drained engine close reclaims a build's orphaned logical lock, verified to fail without the CursorSendEngine change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Commit 994554e rewrote the initial_connect_retry=async docs to claim that transport, auth, upgrade and capability failures are all retried indefinitely and never surfaced. That over-generalized the transport retry-forever rule (Invariant B) onto endpoint-policy failures and inverted a deliberately engineered, test-proven contract. endpointPolicyFailureIsTerminal() returns ORPHAN || !hasEverConnected, so on a foreground sender's initial connect an auth, upgrade or durable-ack capability rejection is terminal: connectLoop latches it via recordFatal and dispatches it to the async SenderErrorHandler (a close() rethrow in ASYNC, a throw from build() in SYNC/OFF). Only transport failures, and endpoint-policy failures once the wire has been up at least once, retry forever. InitialConnectAsyncTest proves both halves: testAsyncAuthFailureSurfacesTerminal the terminal path, testAsyncNoServerRetriesForeverNoTerminal the transport path. The wrong wording risked an operator not wiring an error_handler and then silently buffering into store-and-forward on an auth misconfiguration. Restore the carve-out in all four docstrings the commit inverted: the Sender.InitialConnectMode.ASYNC javadoc, the CursorWebSocketSendLoop class-doc and attemptInitialConnect() javadoc, the QwpWebSocketSender ASYNC-case comment, and the InitialConnectAsyncTest class javadoc. Documentation only; no runtime behavior changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

SegmentRing's per-file arm caught only MmapSegmentException, so an unbacked or sparse mapped page in one .sfa could abort recovery of the whole slot and strand every frame the sibling segments still held. MmapSegment installs handlers around its own recovery reads, but they cannot be relied on before JDK 21: HotSpot records an unsafe-access fault and raises the InternalError at the next return or safepoint check rather than at the faulting read, so it surfaces in an arbitrary caller frame. JDK-8283699 made delivery precise only in 21+. Observed on 8 and 17 landing two frames above SegmentRing.openExisting, past every handler MmapSegment installs -- which is what left the two map-past-EOF guards in MmapSegmentRecoveryFaultTest erroring on the JDK 8 job while passing on 25. Convert at the per-file boundary instead, the frame the late delivery lands in. The arm widens to Throwable but rethrows anything that is neither MmapSegmentException nor the recognized mmap access fault, so resource exhaustion and programmer errors still propagate to the outer catch as before. isMmapAccessFault becomes package-private for that caller and carries the delivery caveat in its javadoc. Add a facade-aware SegmentRing.openExisting overload, mirroring the seam MmapSegment already exposes, and two tests that inject the fault through it deterministically on any JDK: one asserting the faulting segment is skipped while its sibling's frames survive, one asserting an unrecognized InternalError still propagates so the widened catch cannot degrade into skip-on-anything. The two pre-existing map-past-EOF tests now drive SegmentRing -- the boundary their own javadocs describe -- keeping the strong assertion where delivery is precise and degrading on pre-21 to what the JVM still makes checkable: the process survived and the escaping error is the recognized fault. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The split catch-up tests counted frames but never read their bytes, so a chunk walk shipping the right NUMBER of frames while overlapping or skipping ids passed them all. That is the silent-NULL corruption vector: the server null-pads a gapped delta, and a row referencing a padded id lands a NULL symbol with no error anywhere. Mutating the walk's start-id advance proves the gap -- the alignment suite stayed 17/17 green while only the 25-second end-to-end test noticed. CatchUpCapturingClient now keeps each frame's bytes, joining the two slices of a multipart send, and assertCatchUpReassembles folds them through QwpWireTestUtils.accumulateDeltaDictionary -- the decoder the end-to-end handler already uses -- then compares per id. Comparing content rather than ranges catches overlap, gap and shift alike, and each frame is checked against the advertised cap. QwpWireTestUtils and that one method turn public for the sf.cursor package; the rest of the class stays package-private. Add three tests: a three-way split whose chunks must tile [0, n) exactly; an empty-dictionary reconnect that must ship no catch-up frame, using cap 0 so the guard rather than an empty walk holds the count at zero; and a variable-width split, the only test that catches a walk assuming a uniform entry stride. Cover multi-byte UTF-8 on the persist path, which nothing exercised: every symbol in these suites was ASCII, where a symbol's UTF-8 byte length and its char count agree. Replacing Utf8s.utf8Bytes with String.length leaves DeltaDictRecoveryTest 23/23 green while the new round-trip test fails on the truncated tail. The literals carry a runtime self-check so the test cannot decay into an ASCII one should the file ever lose its encoding. Drop a comment block left stranded above WebSocketResponse when its field was removed, and rewrite two that described an entry-end index this class never had. Correct the surviving Invariant B text: the reconnect budget reaches connectWithRetry from ensureConnected's SYNC branch and BackgroundDrainer's capability-gap deadline, never buildAndConnect and never this loop. Tear the torn-dict guard test down recursively. It builds the slot layout under <dir>/default, which the flat helper cannot remove, so every run left the whole tree behind. Production behaviour is unchanged: every main/ hunk is comment text. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Two defects in the parent-anchored logical slot lock, which this branch introduced: before it, build() took no SlotLock at all and SlotLock created only the per-process slot directory. The lock now lives under a directory every sender beneath one sf_dir must create a file in, and ensureDirectory created it 0755. Whichever process starts first owns it, so a sender running as a different uid cannot create its lock file and acquireAt throws "could not open slot lock file". That is an IllegalStateException, not a LineSenderException, so a caller following the documented contract does not catch it and build() fails outright. Create that one directory 0777 instead and let the deployment's umask decide the sharing policy, exactly as it already does for the sf_dir the lock lives under: the usual 022 leaves it at the same 0755 as before, while a shared-group deployment gets the group-writable directory it needs. Slot directories keep the restrictive mode -- one process creates each and only that process writes inside it. The drainer then turned that failure into a permanent one. isLockContention matches only "already in use", so every other acquireLogical failure -- the permission case above, or a momentary fd exhaustion -- rethrows into the outer catch, which drops a .failed sentinel. That runs BEFORE CursorSendEngine takes the slot's own .lock, so the drainer quarantines a healthy slot another live process still owns. Nothing ever removes the sentinel and isCandidateOrphan treats it as disqualifying, so when the real owner later died its unacked data would stay stranded until an operator intervened. Write the sentinel only when engine is non-null, which is set once the engine holds the directory-local lock and so marks genuine adoption; a pre-adoption failure now leaves the slot for the next scan. The auth/upgrade and wire sentinels are unchanged -- both fire after adoption and are sanctioned orphan terminals. Both tests fail without their fix: the mode assertion reports 493 where 511 is required, and the drainer test finds the sentinel in a slot it never locked. The drainer test blocks the lock by planting .slot-locks as a regular file rather than by chmod, so it reproduces the failure on any platform and does not silently pass when CI runs as root. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

testPreAdoptionSetupFailureDoesNotQuarantineTheSlot derived the slot's parent directory with slotPath.substring(0, slotPath.lastIndexOf('/')). File.getAbsolutePath() yields '\' separators on Windows, so lastIndexOf returned -1 and substring threw StringIndexOutOfBoundsException before the test reached a single assertion. It was the only failure in the Windows CI run (2684 tests, 1 error). Paths.get(slotPath).getParent() derives the parent portably. The join stays a forward slash on purpose: SlotLock.resolveLogicalLock builds the lock directory as parentPath + "/" + ".slot-locks", and the blocking file this test plants must land on the exact string acquireLogical will mkdir. Deriving the whole path through Paths.get would yield '\.slot-locks' on Windows, putting the file where acquireLogical never looks, so the test would pass while exercising nothing. SlotLockTest already documents that trap for its sibling. Verified both ways on macOS: the test passes with this change, and still fails with "a slot this drainer never adopted must not be quarantined" when the BackgroundDrainer engine != null guard it pins is reverted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

C1. CursorSendEngine.close() unlinked the parent-anchored logical slot lock while Sender.build() still held it. A fresh slot is "fully drained" by definition (publishedFsn() < 0), and build() closes the engine from inside its acquireLogical scope whenever connect fails -- the ordinary "server isn't up yet" startup. On POSIX the unlink frees the pathname without releasing the flock, so the next acquireLogical creates a second inode and locks it successfully: two parties owning the lock that exists solely to serialise the quarantine close->rename->recreate window. close(boolean) lets a caller that holds the lock skip the reclaim; build() and quarantineTornSlot pass false. An unheld lock is still reclaimed, so .slot-locks does not grow without bound. C2. Recovery rebuilds the producer dictionary from the side-file's intact prefix AND the surviving frames' own deltas, then resumes sentMaxSymbolId at the combined tip -- above pd.size(). Every frame published afterwards then carried a deltaStart the side-file could not describe. The steady-state write-ahead did not close that gap: it persists [pd.size() .. currentBatchMaxSymbolId] and returns early when the batch's highest id is below pd.size(), so it healed only if, and only as far as, later traffic referenced the recovered high ids. The frames carrying those ids are the oldest unacked and so the first to be acked and trimmed; once gone, an ordinary process crash -- which store-and-forward promises to survive -- left a slot whose frames reference ids nothing holds, and build() quarantined it. Recovery now heals the side-file eagerly and in full, before any new frame publishes. That heal advances pd.size() before the send loop is built, and the loop seeded sentDictCount from the live size() while taking its bytes from the fixed loaded region -- so the mirror would have claimed symbols it did not hold. PersistedSymbolDict now exposes recoveredSize(), the entry count that matches loadedEntriesAddr/Len, and the loop and the loaded- region decoder both key off that instead. C3. deltaDictEnabled was written once at setCursorEngine with no runtime disable, so a mid-run .symbol-dict write failure killed flush() forever. A full disk reaches exactly that state: SF's segments are pre-allocated mmap files and stay writable while the growing side-file does not. Full self-sufficient frames need no side file at all, so a persist failure now degrades to them. The failing flush still throws -- beginMessage has already baked a delta deltaStart into the staged frame -- but the throw precedes every publish, so the rows stay buffered and the next flush re-encodes them from id 0. C4. Two recovery-seed exits threw raw IllegalStateException, which Sender.build() does not route to its quarantine handler: with a stable senderId every restart re-recovered the same slot and rethrew, so the application could never construct a Sender and could not even buffer. Both now throw UnreplayableSlotException. QwpWebSocketSender.connect's rollback also let a close() failure replace the original exception and demote a recoverable slot back to that brick; it now addSuppressed. Regression tests, each verified to fail with its fix reverted: EngineCloseSlotLockReleaseTest#testCloseDoesNotUnlinkALogicalLockItsCallerHolds ("close(false) must leave the logical lock file alone") and DeltaDictRecoveryTest#testRecoveryHealsThePersistedDictionaryBeforeAnyNewFrame ("expected:<3> but was:<2>"). A companion test pins that an unheld logical lock is still reclaimed. Full client suite: 2687 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Each test was verified to fail with its production fix reverted, and the revert was applied in place rather than via git checkout so no other uncommitted work could be discarded. testPersistFailureDegradesToFullDictInsteadOfKillingFlushForever arms the full-disk facade before the first flush, so the very first ensureAppendMap is refused -- a later append would sit inside the window already mapped and never call allocate at all. The first flush must still throw, because beginMessage has already baked a delta deltaStart into the staged frame, but the second must succeed with deltaStart 0. Reverted, the retry fails with "failed to persist symbol dictionary before publish". testRecoveryBaselineMismatchIsQuarantinableNotAPermanentBrick recovers a slot and asks addRecoveredSymbolsTo for a baseline the fold cannot match. Reverted, it surfaces IllegalState "recovery symbol baseline mismatch [expected=0, actual=9999]", which Sender.build() does not route to its quarantine handler. testUndecodableLoadedRegionIsQuarantinableNotAPermanentBrick hand-writes a CRC-valid chunk declaring 3 entries inside a 2-byte region. Reverted, it surfaces IllegalState "truncated loaded symbol dictionary entry". The fixture also documents a gap worth closing separately: scanRecoveredChunks verifies the chunk CRC and rejects entryCount <= 0, but never checks that entryBytes actually holds entryCount well-formed entries, so it accepts this chunk and only the decoder notices. Validating the triple during the scan would end the trusted prefix there, as a CRC failure already does. Full client suite: 2690 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

C5. The foreground send loop CONSUMED the engine's recovery sources -- pd.takeLoadedEntries() plus engine.releaseRecoveredSymbolStorage() -- making the hand-off one-shot while leaving a second construction reachable: ensureConnected's catch closes and nulls cursorSendLoop precisely so a caller can retry, and `connected` is set only at the very end. Neither call cleared the counts that gate re-seeding, so the retry saw recoveredSize() > 0 with loadedEntriesLen() == 0, left sentDictCount at 0, and then either tripped the baseline mismatch or latched the torn-dictionary terminal on a healthy slot. Both policies now BORROW, as the orphan drainer already did; the engine frees at close. Lifetime holds in both directions because QwpWebSocketSender.close() and BackgroundDrainer's finally both close the loop before the engine. The two transfer methods had no other caller and are deleted. C6. A pre-21 unsafe-access fault is delivered asynchronously, so one taken while scanning a segment can surface at MmapSegment.openExisting's RETURN -- in SegmentRing's frame, before `seg = openExisting(...)` ever assigns. The segment was fully constructed, so its fd and whole-file mapping leaked with nothing referencing them, while the log claimed a skip that had not happened. openExisting now publishes the segment into a caller-supplied holder immediately before returning, and SegmentRing's finally closes whichever handle survived. The holder is cleared at every ownership transfer, so it can never close a segment `opened` now holds. C7. accept() checkpoints committedRawLen/Count only at a commit-bearing frame, while foldDelta's gap-reset rewinds runningRawLen/Count to 0 without touching them -- so the next appendRaw overwrites the committed bytes in place. Harmless when a commit-bearing frame follows, but a reset inside an uncommitted deferred tail never gets that refresh and finish() then published the old counts over the tail's bytes: recovery decoded the tail's symbol under the committed id while the frame that actually replays registered a different one. finish() now detects the rewind and fails clean instead. C8. markFailed is best-effort and returns silently when it cannot open the sentinel -- and a full or read-only disk, the condition that makes a slot unreplayable, is exactly the condition that makes writing it fail. Twelve lines after quarantining, the same build() dispatches its orphan drainers, and the quarantined copy was still a candidate: unbounded churn against data explicitly declared human-in-the-loop, repeating on every restart. OrphanScanner now excludes quarantined slots BY NAME, and owns the reserved infix that Sender.build() renames with. Regression tests, each verified to fail with its fix reverted: testForegroundLoopReSeedsAfterAClosedFirstAttempt (UnreplayableSlot "recovery symbol baseline mismatch [expected=2, actual=0]"), testGapResetInsideTheDeferredTailFailsCleanInsteadOfMisattributing (expected:<-1> but was:<1>), and testQuarantinedSlotIsNotACandidateEvenWithoutTheFailedSentinel. The two MirrorLeakTest cases that pinned the old ownership transfer now pin borrowing, including that closing a borrowed loop leaves the engine's buffer alive. C6 has no regression test: reproducing it needs a fault delivered at one specific instruction boundary on a pre-21 JIT, which is the same JIT-dependent delivery the existing MmapSegmentRecoveryFaultTest guards cannot pin either. The change is a strict improvement over having no handle at all, and the existing multi-segment recovery tests cover the double-close risk it introduces. Full client suite: 2693 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

C9. The full-dict-fallback discard re-folded the whole recovered ring even when the dictionary it discarded was EMPTY -- and that is the ordinary client-upgrade path, not the rare one the comment described. Every frame the shipped client ever wrote passes confirmedMaxId = -1, so deltaStart is 0 and maxDeltaStart is 0; such a slot has no .symbol-dict, so recovery creates a fresh empty one. All three discard conditions hold on the first restart after upgrading, for every non-empty slot, and the first fold was already keyed to that size-0 baseline -- so the re-fold recomputed a bit-identical result over the entire backlog, with a byte-at-a-time varint decode over full-dict frames that carry the whole dictionary, inside build(). It now runs only when the discarded dictionary held entries, which is the only case that can desynchronise the fold from the baseline its consumers derive. C11. PersistedSymbolDict.openExisting walked the file twice: once to CRC every chunk and total the surviving entry bytes, then again -- re-decoding both header varints of every chunk -- purely to copy into an exact-sized allocation. The entry region is by construction a subset of the file, so the file length bounds it for free: one pass now validates and copies together into a length-sized buffer, then shrinks to the exact size. The trusted prefix is unchanged. The same pass gained an entryBytes <= 0 guard. Every entry costs at least its own length varint, so a positive entryCount inside a zero-byte region is self-contradictory -- but the CRC only proves the bytes are what was written, never that the header triple is consistent, so the scan accepted such a chunk and left decodeLoadedSymbols to discover it later, as a throw two layers up rather than a trimmed prefix. C12. Each pass of the flush path re-walked tableBuffers.keys() and re-probed the hash map per table: three probes per table on a plain flush, five on a split, on the producer's thread. The non-empty tables are now collected once into two reusable lists and every pass indexes them directly -- which also removes the bodyIdx bookkeeping that had to stay aligned with splitFrameBodyBytes by hand. Regression tests, each verified to fail with its fix reverted: testEmptyDictionaryDiscardDoesNotReFoldTheBacklog (expected:<1> but was:<2>) with testPopulatedDictionaryDiscardStillReFolds as its counterpart, and testChunkClaimingEntriesInAZeroByteRegionEndsTheTrustedPrefix (expected:<0> but was:<1>). Observing C9 needed a new engine-level recoveryFoldCount: recoveryFramesVisited() cannot see a second fold because the re-fold replaces the analysis instance. C12 has no new test. Its behaviour is unchanged by construction and its real risk -- index alignment across the collected lists and splitFrameBodyBytes -- is already exercised by the two split tests that drive two distinct tables (testSplitBatchShipsDeltaOnFirstFrameOnly, testOversizedTableSplitStrandsNothing). Full client suite: 2696 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

C14. findSegmentContaining walked the sealed list linearly, on the reconnect path, under the ring monitor -- with the list bounded only by the documented ~16K-segment ceiling, and its sibling nextSealedAfter doing the identical predicate in log N right beside it. The list is sorted by baseSeq with disjoint ranges, so the same binary search applies. A comparison probe makes it observable, and the test asserts the counter is non-zero BEFORE asserting the bound: the counter exists only in the binary search, so a bound-only assertion would pass for free against a reverted linear scan -- the trap the sibling perf assertions in this file already fall into. C15. seedGlobalDictionaryFromPersisted poured the recovered symbols into a default-capacity (64) dictionary, rehashing the map ~log2(n/64) times, each pass O(current size). recoveredMaxSymbolId + 1 bounds the seed, so the dictionary is now pre-sized before the pour. The field is no longer final; nothing retains a reference -- the encoder and the persisted dictionary both take it per call -- and the swap happens during construction, before the producer or the I/O thread exist. C16. ensureAppendMap rounded the window START down to a 4 MiB boundary while sizing it to the record's END, so a chunk straddling that boundary produced a window spanning both: 8 MiB mapped and re-allocated, whose lower half covers bytes already written and never touched again. Steady state then advanced 4 MiB per remap while always mapping 8. mmap only requires a page-aligned offset, so the window is now page-aligned. C17. readVarintAt returned its end position through an instance field -- a store per call, and this runs once per SYMBOL: per frame in accumulateSentDict and once per mirror entry in sendDictCatchUp. It now packs (value << 3) | bytes into the return, the shape RecoveredFrameAnalysis.readVarint already uses. That also closes a guard that could not guard. The field version returned 0 with varintEnd == p when p >= limit, so a caller computing p = varintEnd + len got p == limit and its `p > limit` bail-out could not fire at an exact boundary; a frame declaring more entries than it carries would walk zero-length pseudo-entries and advance sentDictCount past bytes the mirror does not hold. A -1 return makes truncation detectable and every bail-out now fires. C13 is RETRACTED, not fixed. The review claimed trySendOne's in-place orphan-tail re-anchor was dead after a catch-up, so every retirement on a dictionary-bearing slot cost a reconnect. The dead-arm half is right; the cost claim is not. Both connection setup sites -- swapClient and positionCursorForStart -- call tryRetireOrphanTail before any send, so that arm is reached only when frames below the tail still needed acks, which means they were sent on this connection and a recycle was always the correct path. Proven: a test written for the fix passed with the fix reverted. The code is restored; only the comment, which claimed the wire mapping is untouched "because no wire sequence has been consumed" (false once a catch-up has run), is corrected. The test is kept under a name describing what it does pin -- that adding a catch-up leaves the start()-time retirement intact -- and says so. Full client suite: 2697 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

C20. BackgroundDrainer's capabilityGapBudgetNanos overflow fix had no test: restoring the raw millis * 1_000_000L left all 38 drainer tests green. reconnect_max_duration_millis is validated only as > 0, and Long.MAX_VALUE is the natural way to ask for "never give up" -- but a raw multiply wraps it negative, and since the escalation gate ORs against the attempt cap, an elapsed of 0 on the very first sweep already clears a negative budget. The slot quarantines on sweep ONE, skipping the whole 16-sweep settle budget, so asking for more tolerance bought none. The new test asserts the full budget is spent; reverted it reports expected:<16> but was:<1>. The CursorWebSocketSendLoop twin of the same conversion was already pinned, which is what made this gap visible. C18. testScanFaultOnMapPastEofIsHandledAnyFilesystem nested every assertion in `if (ring != null)`, under a comment saying the null case was "also handled" -- so on a late-delivered fault, where SegmentRing skips the file and openExisting returns null, the test ran ZERO assertions and passed. It now writes a healthy sibling contiguous below the faulting segment, so the ring is non-null on every delivery path and the invariant can be asserted unconditionally: one bad .sfa must never take the slot down, and the skip must be per-FILE, with the sibling keeping all of its frames. Two limits are worth stating rather than papering over. MmapSegment's own in-class fault conversion still cannot be pinned on the shipping JDK 8 target -- delivery there is asynchronous, so a test cannot require the precise-delivery outcome. And the scanFrames CRC choice cannot be regression-tested at all: reverting it to the native Crc32c.update means a SIGBUS inside JNI, which aborts the JVM rather than failing a test. C19 is NOT fixed and its finding is downgraded. ensureConnected's `cursorSendLoop.close()` is genuinely unguarded, as the review proved, but it is no longer demonstrably load-bearing: the foreground loop now BORROWS its mirror rather than owning it, so an orphaned one holds almost nothing, and the catch-up frame buffer is allocated only once a catch-up is actually sent. A test written against an injected start() failure passed with the fix reverted. The window is also unreachable without a seam -- a null error handler is coerced to a default, the inbox capacity is validated at its setter, and with no reachable server ensureConnected fails before the loop is ever constructed. Rather than add production test-surface for a test that proves nothing, the seam and the test were both removed. The fix stays as defensive hygiene. Full client suite: 2698 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

C22. The full-dict-fallback discard is gated on BOTH recoveredMaxSymbolId >= size AND recoveredMaxSymbolDeltaStart == 0, and the second conjunct had no failing test. Every candidate passed either way: the torn-dict tests have frames covering from id 0, so a re-fold at baseline 0 succeeds anyway, and the covered-prefix test fails the size conjunct first. The new test builds the shape that discriminates -- a side-file holding [a,b] beside frames registering c@2 and d@3, so the frames out-reach the dictionary but are NOT self-sufficient. Delete the conjunct and recovery discards the only source of ids 0..1, re-folds at 0, finds deltaStart 2 above a coverage of 0 and quarantines a perfectly recoverable slot. C23. start()'s catch of CatchUpSendException had no test: no other case combines a seeded mirror with a failing client through start(). It matters because a recovered sender ships its catch-up on the CALLER's thread in SYNC/OFF startup, so without the catch a server that drops mid-catch-up turns a transient outage into a build() failure -- an Invariant B violation. Reverted, the new test reports CatchUpSend "transient wire failure during catch-up" straight out of start(). C25. Nothing exercised a split failing at frame k>1; every other failed-publish test fails at frame 1. The new test injects at the RING rather than the cap -- both frames clear the pre-flight, but the second does not fit a fresh sf_max_bytes segment -- so frame 1 publishes and frame 2 throws. It is labelled a CHARACTERISATION test, not a mutation guard, because it does not discriminate against reverting the pd.size() persist key: frame 1 published successfully, so advanceSentMaxSymbolId had already moved the baseline past the whole batch and the retry is an early return under either key. That is exactly why the k>1 path is safe, and it is what the test records. C21 and C24 are NOT covered, and both are downgraded. C21 -- the advanceSentMaxSymbolId/sealAndSwapBuffer ordering -- needs a publish that fails ONCE and then succeeds. Every failure mode reachable from the public API is permanent for a given batch: an oversized frame stays oversized, and the rows are retained deliberately, so the retry re-encodes the same bytes. Reaching it would need a fault-injection seam at appendBlocking, which is more production test-surface than the row justifies now that C25 covers the k>1 shape. C24 -- trySendOne's re-anchor catch and its cap-gap discrimination -- is unreachable, for the reason that retracted C13 in the previous commit: both connection setup sites call tryRetireOrphanTail before any send, so that arm is only reached once frames below the tail have been sent on this connection, and the catch sits inside the nextWireSeq == 0 branch that then cannot hold. Full client suite: 2701 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Guards that don't guard. Already closed by the varint packing change: readVarintAt returns -1 for a truncated varint instead of 0 with its end at the limit, so a caller's `p > limit` bail-out can fire at an exact boundary. It had no test; one now feeds a delta declaring two entries while carrying one, ending precisely on the boundary that used to slip through. Reverted to the indistinguishable shape it reports expected:<0> but was:<2> -- the mirror claiming symbols it holds no bytes for, which the next catch-up would ship as a chunk whose deltaCount exceeds its payload. DIR_MODE_SHARED. The javadoc claimed umask governed it "exactly as it does for the sf_dir these live under", but sf_dir was created with an explicit 0755 and umask can only CLEAR bits -- so under umask 002 the lock directory became group-writable while sf_dir did not, and a second uid still could not create its slot directory. build() failed one level before the problem the mode existed to solve. sf_dir now gets the same mode, and the mode is 01777 rather than 0777: without the sticky bit any local user with write access can unlink another process's lock file, which does not release that process's flock but does free the pathname, so the next acquirer mints a second inode and locks it successfully. umask masks only the rwx bits, so the restricted-deletion semantics survive every umask and the usual 022 still yields 0755 plus sticky. The slot directory itself stays 0755 -- only its creator writes it. Foreground retry observability. Retrying an endpoint-policy rejection is what the contract demands, but dispatchError ran only in the terminal branches, so the retry was programmatically invisible: a revoked token produced a throttled slf4j WARN and nothing else, in a library that ships embedded in user applications. flush() kept returning success until SF filled, and the failure then surfaced as "cursor ring backpressured ... sf_max_total_bytes too small" -- pointing the operator at disk sizing. The retry branches now dispatch a RETRIABLE SenderError, and lastReconnectError is promoted from a connectLoop local that never escaped to a volatile field, which close()'s drain-timeout throw uses to name the real cause. scanRecoveredChunks. It rejected entryCount <= 0 but never checked that entryBytes actually holds entryCount well-formed entries. The CRC proves the bytes are what was written, not that the header triple is consistent, and the only write-side guard sits behind an assert that production never runs. Such a chunk is now rejected during the scan, so the trusted prefix ends there as it would on a CRC failure, instead of decodeLoadedSymbols discovering it later and quarantining the whole slot. ensureSentDictCapacity. The MAX_SENT_DICT_BYTES arm deliberately latches via recordFatal before throwing, because accumulateSentDict runs AFTER the frame's sendBinary and a bare throw unwinds into a reconnect that replays the same frame. The allocation one line below had no such latch, so a genuine out-of-memory took exactly the livelock the ceiling arm exists to prevent. It now latches too. PersistedSymbolDict.close(). It set closed = true and then ran munmap and truncate unguarded, so a throw from either stranded the scratch buffer and the fd for the process's lifetime -- a retry short-circuits on `closed`. Each step is now in its own try/catch, as CursorSendEngine's close already was. Two tests were updated rather than added: the one asserting the scan ACCEPTED an inconsistent chunk now asserts it ends the trusted prefix (that path's exception-type change stays pinned on its reachable sibling in CursorSendEngineTest), and SlotLockTest's mode assertion follows the constant. The OOM latch and the close hardening have no test: neither an Unsafe allocation failure nor a throwing munmap is injectable through any existing seam. Full client suite: 2702 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@testonly

Review follow-ups: resource hygiene in the poison-frame tests, dead code left behind by the recovery fold, and comments that contradict the code. Wrap the twelve CursorWebSocketSendLoops in CursorWebSocketSendLoopPoisonFrameTest in try-with-resources. They were never closed, and their enclosing bodies leaked on the assertion-failure path too, which TestUtils.assertMemoryLeak hides because it calls skipChecks() when the body throws. Nesting the loop inside the engine's try-with-resources also fixes close order: the loop borrows the engine's persisted dictionary prefix, so it must close first. A probe on all twelve sites confirms none of them actually leaked -- no symbol-dict mirror or catch-up frame buffer was allocated on these paths, and the reconnect factory registers every client in the list closeAll() drains. The value is in removing the trap: releaseSentDictBytes() and freeCatchUpFrameBuffer() are reachable only through close(), so a future test in one of these bodies that triggers a dictionary catch-up would have leaked silently. Carry the slot-qualified path into PersistedSymbolDict so the mmap-trim warning in close() names the dictionary it is about. It logged the bare FILE_NAME constant, which is unattributable in a process holding many slots -- a foreground sender plus N orphan drainers. Delete SegmentRing.findLastFsnWithoutPayloadFlag and MmapSegment.findLastFrameFsnWithoutPayloadFlag. The recovery fold left both without callers, and each walks the whole backlog. Drop the two dead stores in RecoveredFrameAnalysis.finish(): only appendRaw() reads those fields, and it cannot run after finish(). Correct four comments that state the opposite of what the code does: - hasEverConnected is documented as observability only while being half of endpointPolicyFailureIsTerminal(). - PersistedSymbolDict.close() claims that nulling the pointer prevents use-after-free, contradicting loadedEntriesAddr()'s accurate javadoc. It guards a construction-phase reader; ordering is what makes the borrow safe across threads. - sentDictBytesOwned describes borrowing as an orphan-only state, but loops of either policy borrow now. - connectWithDurableAckRetry refers to @testonly callers that no longer exist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address blocking review findings in the store-and-forward paths. SegmentRing.openExisting no longer detaches the active segment from `opened` before the ring owns it: `new SegmentRing` and the sealed-add loop can throw (ObjList growth, native OOM), and the catch iterated only `opened`, leaking the active segment's mapping and fd. Ownership now transfers in one step and `opened` is cleared last. PersistedSymbolDict.open gains the in-flight holder MmapSegment already uses. Recovery reads the file through a mapping, and pre-JDK-21 HotSpot delivers an unsafe-access fault at the method's return, past the inner catch; without a holder the built instance's fd and buffer leak and the fault escapes build() instead of degrading to full-dictionary frames. commitMappedChunk checksums the append mapping with Crc32c.updateUnsafe rather than the native update. ff.allocate can leave the window over uncommitted blocks (ENOSPC, quota, a crash-torn tail); a SIGBUS there aborts the JVM through JNI but is a catchable InternalError at an Unsafe site, matching scanFrames. SlotLock.removeOrphanLogical acquires the lock before unlinking it. Removing a file another party holds frees the pathname without releasing the flock, so the next acquireLogical locks a second inode -- two owners of a mutual-exclusion lock. QwpWebSocketSender.close carries a reclaim-logical-lock flag that connect()'s rollback clears, so a failed connect no longer unlinks the lock Sender.build() still holds. Sender.build reports an abandoned quarantined slot through the configured errorHandler, not only via LOG.error: the client ships slf4j-api with no binding, so an embedder without a provider would learn of the loss nowhere. Files.DIR_MODE_SHARED's javadoc no longer claims the sticky bit survives: Linux mkdir(2) masks it off, so the restricted-deletion guarantee is best-effort and lock safety rests on the acquire-before-unlink above. SlotLockTest pins the held-lock case both ways. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mtopolnik · 2026-07-21T03:47:11Z

[PR Coverage check]

😍 pass : 1592 / 1728 (92.13%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java	30	36	83.33%
🔵	io/questdb/client/Sender.java	75	88	85.23%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/OrphanScanner.java	7	8	87.50%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/PersistedSymbolDict.java	382	434	88.02%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	152	170	89.41%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java	85	94	90.43%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	393	419	93.79%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java	102	106	96.23%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/RecoveredFrameAnalysis.java	162	168	96.43%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegment.java	37	38	97.37%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketEncoder.java	54	54	100.00%
🔵	io/questdb/client/cutlass/http/client/WebSocketClient.java	11	11	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java	54	54	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/UnreplayableSlotException.java	2	2	100.00%
🔵	io/questdb/client/std/FilesFacade.java	4	4	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	6	6	100.00%
🔵	io/questdb/client/impl/ConfigSchema.java	1	1	100.00%

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

glasstiger mentioned this pull request Jul 9, 2026

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

Open

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

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

01f251d

bluestreak01 requested changes Jul 9, 2026

View reviewed changes

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

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

f9430bc

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

9602440

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

glasstiger added the tandem label Jul 9, 2026

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

glasstiger and others added 29 commits July 17, 2026 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

glasstiger commented Jul 9, 2026 •

edited

Loading

Uh oh!

glasstiger commented Jul 9, 2026

Uh oh!

bluestreak01 left a comment

Uh oh!

mtopolnik commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

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

Uh oh!

Tandem

Summary

What changed

Memory mode

Store-and-forward (file mode)

Catch-up split

Server-side gap rejection (OSS half)

Segment recovery: late-delivered mmap faults

Durability

Tradeoffs

Test plan

Uh oh!

glasstiger commented Jul 9, 2026

Uh oh!

bluestreak01 left a comment

Choose a reason for hiding this comment

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

Critical

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

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

Moderate

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

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

Minor

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

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

Downgraded (false positives — verified against source)

Coverage map

Summary

Uh oh!

mtopolnik commented Jul 21, 2026

[PR Coverage check]

file detail

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

glasstiger commented Jul 9, 2026 •

edited

Loading

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

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

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

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