feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap

[jovfer]

Fixes #80

Tandem:

• OSS (contract pin + e2e + fuzz): test(qwp): pin the disconnect-clear contract, e2e and fuzz coverage for client symbol-dict recycle questdb#7525
• Enterprise (failover losslessness): questdb/questdb-enterprise#1171

The WebSocket sender no longer has a hard symbol-cardinality stop. When its global symbol dictionary reaches a configurable threshold (symbol_dict_reset_threshold, default 100 000), the sender recycles its send stack at the next provably-safe instant — all published frames acknowledged, no row in progress, no open transaction: it closes the send loop, retires the store-and-forward slot with the existing fully-drained close (emptying it), starts a fresh dictionary epoch on the same slot, rebuilds the engine through the same code path build() uses, and reconnects. The server needs no change: supported servers (10.x+) already clear per-connection dictionary state on disconnect, and this branch pins that behavior as a wire contract. Zero wire change, zero on-disk format change; all production code is client-side.

User-visible surface

Three connect-string keys / builder methods — symbol_dict_reset (default on), symbol_dict_reset_threshold, symbol_dict_reset_max_wait_millis (bounded wait for a starved reset; default 2000, at most one wait per armed window; 0 makes the recycle opportunistic-only); an advisory Sender.resetSymbolDictionary() (no-op on non-QWP transports; producer-thread-only, and PooledSender forwards it to the live delegate); observability getters on QwpWebSocketSender (getSymbolDictEpoch, getSymbolDictResetStarvationTimeouts, isResetArmed, isSymbolDictResetEnabled, getSymbolDictResetThreshold, getSymbolDictResetMaxWaitMillis). Externally visible frame sequence numbers stay strictly monotone across recycles via an epoch-base offset applied on every public surface (flush sequence, ack watermark, drain, progress stream, error spans). The client-side dictionary cap rises 1M → 2M (now equal to the server cap, with an OSS ordering test), and the cap error now points at the reset valve. A recycle also naturally heals a mid-life full-dict degrade (the fresh engine re-derives delta capability). Senders built via the public connect(...) overloads (no rebuild factory) and senders that don't own their engine never recycle and never arm, so isResetArmed() cannot read true against a permanently-zero resets counter.

Release-note items:

• CursorWebSocketSendLoop's policy-aware constructor and QwpWebSocketSender.connectWithCredentialSupplier(...) each gain their recycle parameters as a new overload; the 13-argument constructor shipped in 1.3.x and the 23-argument connectWithCredentialSupplier present at the merge base (on main since the OIDC device-flow change, not in any release) are retained (delegating with externalFsnBase = 0 and the recycle defaults respectively), and ExportedApiCompatibilityTest pins every public CursorWebSocketSendLoop constructor present in the published 1.3.9 jar plus both connectWithCredentialSupplier overloads present at the merge base, copied from javap output rather than derived from the current classes. An exported-signature diff of this head against the merge base shows no other removed public or protected member (against the 1.3.9 jar: 18 lines, all synthetic Outer$1 anonymous-class artifacts already absent at the merge base). QwpWebSocketSender.connect(...) is additive: a new overload carries the recycle settings and every pre-existing overload is unchanged.
• The per-connection counters (getTotalFramesSent, getTotalFramesReplayed, getTotalReconnectAttempts, getTotalReconnectsSucceeded, getTotalServerErrors, getTotalAcks, getTotalBackpressureStalls) are sender-lifetime: the sender owns one counter holder that every rebuilt I/O loop and every attached engine adopts before use, so the values carry across recycles and are retained after close(). A recycle's own reconnect counts as one attempt and one success.
• Servers released before QuestDB 10.0.0 cap the symbol dictionary at 1M, and QWP has no wire-level negotiation of the limit. Only 10.0.0+ servers are supported; note that the re-arm floor lets a default sender's dictionary grow to 1M entries per epoch, so this is not an edge case of exotic configuration.
• getAckedFsn()'s contract is restated on the Sender interface: on a live sender it never collapses back to −1 and keeps reporting the last durable watermark across a recycle.
• markEverConnected() is a new public method on CursorWebSocketSendLoop (loop-side seeding of the sender-lifetime ever-connected flag; QwpWebSocketSender calls it when it rebuilds the loop).
• symbol_dict_reset_threshold accepts 1..1,000,000 (half the protocol cap, the re-arm floor's own ceiling); symbol_dict_reset_max_wait_millis rejects values whose nanosecond conversion overflows.
• getSymbolDictResetsPerformed() is gone — it could never differ from getSymbolDictEpoch().
• The recycle barrier and its bounded wait run only while the I/O loop holds a live connection; the resetSymbolDictionary(), symbolDictReset and symbolDictResetMaxWaitMillis javadocs and both monitoring paragraphs state the gate (a table() call made while the loop is reconnecting skips the wait and leaves the recycle armed; armed with a flat epoch also means the sender is between connections).

Recycle robustness

The through-line: the recycle prefers abandoning and resuming over latching or re-arming.

• Reconnect is deferred to the I/O loop. The post-swap reconnect never re-runs the initial-connect policy on the producer thread: ensureConnected() latches hasInitialConnectRun on its first completion and routes every later entry through the ASYNC branch — the loop is built and started synchronously, the socket connect happens on the I/O thread with indefinite retry, and the producer keeps buffering into the fresh epoch's slot. A default (OFF) or SYNC store-and-forward sender therefore never sees a transport error or a reconnect budget past its initial connect, matching the steady-state store-and-forward contract. The barrier swaps only while the I/O loop holds a live connection (CursorWebSocketSendLoop.isLinkUp()): step 2's loop close cancels a live socket in milliseconds, but a reconnect blocked in a hostname resolve or a credential pull ignores the cancel and would hold table() for the join budget, so while the loop is between connections the recycle stays armed and runs at the first drained barrier after the reconnect; SymbolDictRecycleOutageTest pins both halves. Deferring has a bound: the dictionary keeps growing until the reconnect, and the protocol caps a sender at 2,000,000 distinct symbol values (MAX_SYMBOL_DICTIONARY_SIZE), so a producer that registers that many new values above the threshold within one outage fails symbol() with the dictionary-full error instead of swapping mid-outage. The rebuilt loop is seeded with the sender-lifetime ever-connected flag (markEverConnected, read after the outgoing loop's close() has joined the I/O thread), so wasEverConnected() stays sticky and endpoint-policy failures keep their post-init retry classification across the swap.
• Anti-thrash re-arm floor. A live symbol set larger than the threshold would otherwise re-arm on every refill, recycling (teardown + reconnect + dictionary re-ship) every ~L·ln(L/(L−T)) rows forever. Each swap raises the re-arm bar to max(threshold, 2 × size-at-swap) and never lowers it -- a manual resetSymbolDictionary() swap bypasses the floor for that one swap without resetting it --, capped at half the protocol cap so genuinely unbounded-cardinality producers keep recycling. No new config key; the default threshold is unchanged. Red-proofed: without the floor a bounded 6-symbol set recycles 4×, with it once. The floor arithmetic itself is pinned (doubling per swap, organic re-arm refused below it, advisory request bypasses it) — the integration suites' bounded symbol sets rely on that bypass to keep recycling.
• The slot stays owned across the swap. The recycle takes the slot's parent-anchored logical lock before it closes the outgoing loop, closes the outgoing (and any healing) engine without reclaiming that lock, rebuilds through the already-locked construction path, and releases the lock once the fresh engine holds the slot's directory flock — also on the breach latch and in close(), so an abandoned swap never keeps the slot from a successor. A build() colliding on the same sf_dir/sender_id therefore fails fast throughout the swap, as the senderId contract states; a contention on the lock itself (a sibling's startup orphan drainer holding it for microseconds) refuses one row before anything is torn down and the recycle re-fires at a later drained barrier. SymbolDictRecycleSlotOwnershipTest attempts the colliding build through the real builder's factory at the rebuild boundary, while a rebuild is pending, after the breach latch and after close(), each red against the mutation it guards. One residue remains: a swap abandoned at the rebuild and then closed leaves the slot's .slot-locks lock-file pair on disk (no engine retires the slot in that state); it is one tiny pair per sender_id, and the next fully-drained retirement of that slot reclaims it.
• A deferred engine close is awaited. The fully-drained close can return with the slot flock retained when the SF worker is wedged in a syscall past SegmentManager's bounded join (a stalled disk/NFS — exactly what the deferred-close machinery exists to survive). awaitDeferredEngineClose parks until the worker's exit path confirms the release (30 s budget) instead of letting the rebuild throw SlotLockContentionException. SymbolDictRecycleDeferredCloseTest pins both branches with a wedged-worker harness and a positive parked witness; red-proofed against the pre-fix code (dies with the exact SlotLockContentionException trace). The same protocol closes an engine the rebuild cannot keep (recovered fully-acked leftovers): a wedged worker there retains the engine and resumes on the next send instead of colliding with the next rebuild, and close() keeps reporting the flock held until the worker exits; pinned with a wedged recovered engine both riding the stall out and exhausting the await. Step 3 detaches the outgoing engine's slot-lock listener before closing it, so a release completing after the swap cannot mark the rebuilt engine's flock released; a recycle-retained engine reaches pools through the isSlotLockReleased() re-probe. The await budget is spent once per pending close: a resume while the worker is still wedged probes and rethrows without parking (pinned: the retried table() returns in well under the budget).
• build() never throws for an error the I/O thread latched during construction. The rebuild-factory setter refuses only a closed sender (a rejection latched while build() runs surfaces on the first send, as before, and close() cleans up), and both post-connect setters sit inside build()'s close-on-failure guard, so a throw there can no longer leak the connected sender, its threads and its slot lock. This was the Windows CI failure of SenderPoolDataLossNotificationTest on the pool's startup recovery.
• The recycle is resumable instead of terminal. The swap commits only after a fresh engine stands on the emptied slot (dictionary, epoch counters, and wiring move after the rebuild), and every transient mid-recycle failure — an interrupt arriving while the loop close joins the I/O thread (a flag the producer carries in is cleared for the join and handed back, so it never abandons), a wedged SF worker exhausting the deferred-close budget, a momentary EMFILE at rebuild, a post-cleanup fsync warning — records a two-state resume point (CLOSE_LOOP/REBUILD) that the next send completes. The terminal latch survives for exactly one case: a rebuilt engine that recovered unacknowledged frames, which proves the outgoing close's fully-drained contract was breached. Benign recovered fully-acked leftovers (a transiently failed segment unlink) heal by closing the recovered engine — which retries the unlink by design — and rebuilding. Error passes through the recycle machinery unwrapped and unlatched. Suites pin the interrupt, rebuild-fault, await-exhaustion, heal, and breach paths, each red-proofed.
• A rebuild-time quarantine reaches the handler installed on the sender. The rebuild factory receives the sender's current user handler (rebuild(SenderErrorHandler)), not the builder's field captured at build time, so setErrorHandler() after build() is honoured when a recycle rebuild sets a torn slot aside; the builder's factory also snapshots sf_dir/sender id at build time.
• A failed post-swap reconnect never latches. A @TestOnly fault hook drives a real failed reconnect: one test proves ids registered mid-window survive into the flushed delta (fails with the resetSymbolDictStateForNewConnection guard reverted to the unconditional clear), the other proves the failure leaves the sender usable (fails with recycleFailure latched in the reconnect catch). Both counterfactuals re-run red against the final tree.
• close()'s drain guard is split by true dependency (flush/commit/seal need only the engine; checkUnsurfacedError/drainOnClose need the loop). Staged rows cannot coexist with a null loop — sendRow() calls ensureConnected() before a row commits and every failure path rolls the in-progress row back — so this is hardening that keeps the guard honest for any future state, not a fix for a reachable row drop.
• Accessors are truthful across the recycle window. getAckedFsn()/awaitAckedFsn() snapshot cursorEngine/cursorSendLoop into locals and, while the engine is null (mid-swap), report the durable watermark the recycle barrier proved instead of collapsing to −1; both accessors read the epoch base before the engine on every pass (awaitAckedFsn() polls through getAckedFsn()'s clamped read, pinned by a monitor thread parked across a real recycle), so a torn pair can never fabricate an FSN above anything published nor pin a wait to the outgoing engine; while the engine is null mid-swap the wait keeps polling to its deadline with the durable watermark standing in, instead of answering false at once for a target above it; cursorEngine, cursorSendLoop, fsnEpochBase, lastRecycleDurableFsn and the ever-connected flag are volatile for monitoring readers, and the seven getTotal* counters read one sender-lifetime holder; the getAckedFsn javadocs state the real post-recycle sentinel behavior, including the HTTP/TCP/UDP always-−1 caveat. The torn (old base, fresh engine) clamp is pinned by a test that parks a monitor thread between the accessor's two reads across a real recycle; the null-engine window is asserted from inside the deferred-close park witness — both go red with the clamp or the durable write removed. checkConnectionError() snapshots the loop before checking it, so a monitor thread cannot race the swap's null write.
• Delta-dict healing is epoch-scoped by design. The rebuilt engine re-derives delta capability rather than inheriting a one-way degrade latch: SymbolDictRecycleHealingTest pins that a degraded sender heals back into delta mode once the fault clears, and that a persistent fault degrades the fresh engine again as a normal, catchable error.
• A rebuild that recovers unacknowledged frames latches the sender terminal (empties-the-slot contract breach); fully-acked leftovers heal. The stale slot-lock latch a pool-style re-probe can leave behind is cleared as soon as the outgoing close is confirmed, before the rebuild takes the flock (pinned through a rebuild-factory probe), and the fresh dictionary is allocated before the rebuild so the commit block genuinely cannot throw.
• The fresh dictionary is allocated at the outgoing dictionary's size (the recovery path already did this), so an epoch's refill no longer re-climbs the rehash ladder.
• Diagnostics use the external FSN scale. drainOnClose's two WARNs (the abandon-uncommitted-frames one and the timeout one) and its thrown message, plus the I/O loop's two replay log lines, report epoch-rebased FSNs — the same scale flushAndGetSequence() and the SenderError spans use (pinned after a real recycle).
• Contracts the recycle touches are stated where readers look. A recycle's reconnect fires RECONNECTED/FAILED_OVER with no DISCONNECTED (now on SenderConnectionEvent.Kind and SenderConnectionListener, and pinned); a rebuild-time quarantine is delivered synchronously on the producer thread like the build-time one (SenderErrorHandler); setEngineRebuildFactory is documented and refuses a closed sender. setEngineRebuildFactory(null) stops future arming but never cancels an armed or pending recycle, and its javadoc now says so.

This update sets the recycle's default starvation wait to 2 s and closes two resume gaps found under interrupt and fault injection. symbol_dict_reset_max_wait_millis defaults to 2000: a recycle armed that long without a table() call finding the backlog drained pauses the next table() call for at most 2 s so the backlog can drain, then swaps — at most once per armed window; 0 opts out to opportunistic-only. A continuous producer keeps frames in flight at every row start and never exposes a drained instant on its own, so the wait is what makes the recycle reachable for the population that crosses the threshold; on a healthy link the pause is about one ack round trip, because the paused producer stops refilling the ring. The value is sized against the sender pool's acquire timeout (see Design decisions) and pinned by SymbolDictResetDefaultsTest. The two park loops a recycle can enter (maybeBlockForStarvedReset()'s starvation wait, awaitDeferredEngineClose()'s deferred-close wait) and the loop-close join share one interrupt policy — clear the flag for the park or join and hand it back on every exit, so the recycle never consumes the caller's interrupt — after a carried interrupt flag was found manufacturing a false CLOSE_LOOP abandon out of an otherwise successful wait. A CLOSE_LOOP resume now re-registers the delta baseline it used to drop, republishing it as deferred dictionary chunks the fresh loop replays ahead of any data frame, and closes the outstanding chunk-commit debt on both the resume's Throwable and Error paths, so an interrupted or faulted recycle converges the same way a clean one does. resetSymbolDictionary(), the dictionary-cap error, and armIfEligible()'s javadoc now state the trigger point in one sentence each: table(...) is the only place a recycle ever starts; a recycle that failed transiently mid-swap is resumed and completed by the next table()/flush()/drain() call (an at()/atNow() that finds the swap still pending fails and rolls back its row, and the next table() completes it), and SenderErrorHandler's threading contract says so too, since a rebuild-time quarantine can therefore arrive synchronously on the producer thread from any of those calls. Tests pin these: the default wait's sizing (positive, and at most half the pool's acquire timeout), the interrupt policy under both park loops, the CLOSE_LOOP re-registration and its fallback/debt-close paths (including a crash-replay run proving the abandoned resume group survives a restart), a post-recycle endpoint-policy-rejection retry, and FSN epoch-base accumulation across two real recycles.

Design decisions

• A 2 s bounded wait by default (symbol_dict_reset_max_wait_millis=2000): the recycle runs when a
  table() call finds the backlog drained; a recycle armed for 2 s without one pauses the next table()
  call for at most 2 s so the backlog can drain, at most once per armed window. A continuous producer
  keeps frames in flight at every row start and never exposes a drained instant on its own, so an
  opportunistic-only default would leave the recycle unreachable for exactly the population that crosses
  the threshold; with the producer paused, a healthy link drains in about one ack round trip. The wait
  surfaces no transport error, carries no reconnect budget and never latches: on timeout the call
  returns, the sender stays armed, and no second wait runs until a swap commits, so a drop that lands mid-wait
  costs at most the rest of that one pause, and a loop already between connections when the barrier
  runs is skipped outright (the live-connection gate above). The earlier 30 s default failed on pool arithmetic: a give-back flush arms the sender,
  the slot idles past the window (idle_timeout_ms 60 s), and the next borrower paid the wait while
  holding its lease, longer than acquire_timeout_ms (5 s), so every other acquirer timed out. 2 s
  clears that with margin, covers one round trip plus a modest backlog, and survives the first four
  attempts of the reconnect backoff ladder. A producer that outruns the wire still times out and stays
  armed until a natural gap; the signal is getSymbolDictResetStarvationTimeouts() at 1 with
  isResetArmed() true while getSymbolDictEpoch() does not advance, and the planned escalation for
  that population is an asynchronous handover of the dictionary swap to the I/O loop, not a longer
  wait.
• Recycle bounds mirror close()'s stall class, not always its blocking behavior. The swap's
  loop-close wait is literally close()'s own budget (CursorWebSocketSendLoop.close()'s 30 s
  shutdown-await) — the same method a plain close() calls. The deferred-engine-close wait is a
  separate, dedicated budget (RECYCLE_DEFERRED_CLOSE_MAX_WAIT_MILLIS, also 30 s) sized to survive the
  same disk/NFS stall class SegmentManager's bounded worker join is built to survive; unlike a plain
  close(), which spends only its own join budget and returns, leaving eventual release to a pool's
  background re-probe (isSlotLockReleased()), the recycle parks the producer on this wait so the
  rebuild never collides with a retained flock. Rows are refused only for that window, and only while
  the SF worker is genuinely wedged — a durability-failure class, the same kind of signal
  SegmentRing.checkDurability() already surfaces on the row-send path — never on a transport outage,
  and no reconnect budget applies to the producer. Both parks' interrupt policies are pinned: SymbolDictRecycleStarvationTest for the starvation wait, SymbolDictRecycleDeferredCloseTest for the deferred-close wait (an interrupt carried into either park is handed back on every exit; both tests fail against a mutated exit policy).
• The CLOSE_LOOP resume's re-registration has its own producer-visible bound. Republishing the
  dropped delta baseline runs through the same sealAndSwapBuffer/appendBlocking path as any other
  publish, on the producer thread, bounded by sf_append_deadline_millis (default 30 s) — stated as its
  own bound, like the deferred-engine-close wait. A failure mid-publish clamps the delta baseline to
  the chunks that reached the ring and closes their group best-effort; a failure after the publish
  keeps the baseline (coverage is full) and retries the closing commit (commitOrphanedDictionaryChunks,
  which never throws); if that best-effort close still leaves the group open, every subsequent flush()
  and close() re-attempts the closing commit whenever a group is open (hasDeferredMessages), so the
  residual is a retried, not permanent, state. The re-registration is one-shot, so a remainder wider than
  the server cap after a mid-publish failure stays wedged on the split pre-flight -- a narrower
  population than the former baseline drop, which re-shipped the whole range.
• Server support floor is 10.x. QWP was experimental through the 9.x line and never released with a
  9.x server, so no compatibility shim for 9.x server behaviour exists or is argued from.
• Sender stays single-threaded. A concurrent close() while a recycle waits is outside the
  documented contract (Sender's own thread-safety javadoc).
• The trigger point is table(), by contract. Sender.resetSymbolDictionary(), the dictionary-cap
  error message, and armIfEligible()'s javadoc all state the trigger explicitly: the recycle runs at
  a table() call that finds the backlog acknowledged, and a caller that never starts another row never
  recycles. No per-row branch is added to the hot path.
• Pooled senders inherit an armed recycle at give-back. On a healthy link the ring drains during the
  idle window, so the next borrower's first table() swaps without waiting; only an unhealthy link makes
  it pay the bounded wait while holding its lease, which is why the default stays at half the pool's
  acquire_timeout_ms. An abandon during a give-back flush ejects the slot via discardBroken, which removes it from the pool's slot
  set so the pool can grow back into a fresh one on demand — safe, at a capacity cost until it does. The
  pooled-path recycle has no end-to-end coverage yet.
• FSNs are scoped to the sender instance. The epoch offset a recycle applies is memory-only; a process restarted on the same slot numbers from the slot's on-disk state, so replayed frames are reported on the new process's scale. flushAndGetSequence(), getAckedFsn() and SenderError say so. Persisting the offset would be an on-disk format addition and is deliberately not part of this change.

Tests

New client suites cover arming, FSN continuity, the swap itself, memory mode, starvation, refusals, outage/orphans, catch-up skip, crash windows, partial re-registration, healing/metrics, deferred close, slot heal/breach, reconnect fault injection, and the reset defaults. Alongside them: the OSS disconnect-clear contract pin, real-client e2e in SF and memory modes, a seeded recycle×reconnect fuzz, and ENT failover losslessness with recycles in flight. Verified green from a clean reactor build; the reconnect-fault counterfactuals re-run red at the final tree.

Two pins close coverage gaps the review found: InitialConnectAsyncTest#testAsyncAuthFailureLeavesRebuildFactorySetterUsable reaches a 401-latched, still-open async sender (the state build() can race into) without timing and asserts setEngineRebuildFactory accepts it, red if the setter goes back to the connection-error check; the two timeout-exit interrupt tests (SymbolDictRecycleStarvationTest#testInterruptedWaitTimesOutAndRestoresFlag, SymbolDictRecycleDeferredCloseTest#testInterruptAtParkEntryIsRestoredWhenAwaitTimesOut) bound the caller's CPU time over the call to half its wall time, pinning the per-iteration interrupt clear that keeps parkNanos sleeping (deleting the clear in either park turns them red; skipped where the JVM cannot report thread CPU time).

🤖 Generated with Claude Code

[jovfer]

Tandem code review — level 3

questdb/questdb-enterprise#1171 · questdb/questdb#7525 · #91 (branch qwp-dict-recycle-r1), reviewed as one logical change.

Verdict: client approve with comments, OSS approve, ENT approve with comments. Nothing blocks. The three fixes this round are correct, and CI shows no failure this change causes.

PR	Head reviewed	Base
#91	91fd4451	981bdb02
questdb/questdb#7525	b8acf7098a	6f1d196f57
questdb/questdb-enterprise#1171	5532b1590	0e4f18001

Scope. Both submodule pointers move to commits that exist only on the feature branch, so both are in scope:

• questdb: OFF-DEFAULT, in scope.
• java-questdb-client: OFF-DEFAULT, in scope.

The OSS and ENT test files are byte-identical to the previous review. The only new code is the four client commits. The OSS/ENT merges brought in upstream code only.

PR metadata. The titles follow the convention, Fixes #80 heads the client body, and the labels fit. The body issues are Minor 5.

---

Moderate

1. [client] The public javadocs don't mention the new live-connection gate

• Problem: the javadocs describing when the recycle runs, and the monitoring advice, leave out the live-connection gate.
• Net impact: API users and operators will misread a recycle that is waiting for an outage to end.
• Evidence: compare Sender.java:698-704 with QwpWebSocketSender.java:5632-5635, plus scratch runs at 91fd4451 and ce20e09b.

Commit 91fd445 makes the table() barrier return early while isLinkUp() is false, before it checks the ring or enters the starvation wait. That is the design the PR body describes. These texts were not updated:

• resetSymbolDictionary(), Sender.java:698-704: says "The reset runs at the next table(...) call that finds all published data acknowledged and no row in progress", and blames indefinite deferral only on "sustained saturation".
• symbolDictReset, Sender.java:1923: states the same condition.
• symbolDictResetMaxWaitMillis, Sender.java:1979-1995, and the constant comment at QwpWebSocketSender.java:149-158: say the next table() "BLOCKS … an outage costs the producer one bounded pause". Now a table() call made while the loop is reconnecting skips the wait.
• Monitoring advice, Sender.java:2001-2005 and QwpWebSocketSender.java:2616-2625: say that "armed while the epoch does not advance" means no row start saw a drained backlog, or that the one wait already timed out. During an outage the backlog can be drained and the wait is never entered.

Scratch tests, same source at both revisions:

Trigger	91fd445	ce20e09
All data acked, resetSymbolDictionary(), server killed, reconnect started, then table()	0 ms, epoch 0, still armed	epoch 1
Armed longer than max wait (500 ms) with unacked rows, server killed, then table()	0 ms, 0 starvation timeouts, no WARN	500 ms, 1 timeout

Base has no recycle, so this is a new surface.

Fix:

• Add "and the I/O loop holds a live connection" to both trigger sentences.
• In the max-wait javadoc and the constant comment, say that a call made while the loop is reconnecting skips the wait.
• Add "or the sender is between connections" to both monitoring paragraphs.

2. [ENT] The failover test's javadoc describes mid-outage swaps the pinned client no longer does

• Problem: the test's javadoc says a swap can run mid-outage, and the pinned client no longer allows that.
• Net impact: readers will misjudge what the test and its recycledDuringOutageWindow log line exercise.
• Evidence: compare SqlFailoverQwpRecycleLosslessTest.java:52-68 with client QwpWebSocketSender.java:5632-5635 and CursorWebSocketSendLoop.java:1848.

What the javadoc says:

• :52-53: "What actually withholds a swap during the outage is isRingDrained()".
• :59-61: "if the ring happens to be drained the instant the outage begins, the very next table() call runs the swap".
• :63-64: "the swap and the epoch bump return promptly even mid-outage".

What happens at 91fd445:

• Once the I/O thread sees the drop, connectLoop sets lastReconnectError, isLinkUp() reads false, and the barrier returns before it checks isRingDrained().
• The client's rewritten SymbolDictRecycleOutageTest asserts exactly this.
• The same gap applies to :117-120 ("an arm attempt that catches the ring undrained during the outage gives up quickly"): during an observed outage, an arm attempt never reaches the wait.

How it happened: the javadoc was last edited in f522c77e5, when the pin pointed at a client without the gate. d82692f34 moved the pin without touching the javadoc. The ENT PR body now says the javadoc is "written against that path", which isn't true.

No assertion depends on this text. The test passed on the Windows lifecycle leg in both builds.

Fix: rewrite :52-68 and :117-120 in terms of the gate. A swap between the demote and the promote can now only land in three places: before the drop is observed, after reconnecting to B, or inside the connect race that earlier reviews accepted. That is what recycledDuringOutageWindow measures. Once the javadoc is rewritten, the body sentence becomes true.

3. [client] Coverage gap: the fix for the previous review's build() leak has no deterministic test

• Problem: the closed-only check in setEngineRebuildFactory is not pinned by any test.
• Net impact: if the check reverts, build() throws again, and if the setters also move back outside the guard, the leak returns.
• Evidence: with the setter changed back to checkNotClosed() at 91fd445, the whole client suite (3,550 tests) stays green.

Only five test classes call the setter, and all of them use healthy senders. SenderPoolDataLossNotificationTest catches the full regression only when a delay is inserted into build():

• Setter reverted and both setters moved back outside the guard: 4/4 red, MMAP_DEFAULT … 4206592.
• Setter reverted alone: the pool logs "could not open slot … deferring this and remaining slots", but the test still passes.

A regression would expose the same users as before:

• pool startup recovery where the server rejects the replayed rows as terminal;
• initial_connect_retry=async with a 401;
• an SF restart whose backlog gets a terminal NACK.

Fix: add a timing-free test that needs no production hook and runs in 0.2 s:

1. Use a raw server that answers every upgrade with 401, and build with initial_connect_retry=async.
2. Wait until the error handler receives the TERMINAL error.
3. Check that table() throws, which shows the error is latched but the sender is not closed.
4. Call setEngineRebuildFactory(f) directly and assert it does not throw.

In a scratch run this test was green 6/6 at head and red 2/2 with the setter reverted.

4. [client] Coverage gap: inverting the interrupt assertions removed the only test of the parks' anti-spin clear

• Problem: nothing tests any more that each loop iteration in the two recycle parks clears the interrupt flag.
• Net impact: if that clear is removed, an interrupted producer spins a core for up to 2 s (starvation wait) or 30 s (deferred close).
• Evidence: with the clear deleted in both parks, 79/79 recycle tests are green at 91fd445. The same edit at ce20e09 turns 2 tests red.

Without wasInterrupted |= Thread.interrupted() (:5337, :5597), parkNanos returns immediately while the flag is set, so the loop spins instead of parking.

• The old swallow assertions caught this indirectly.
• The inverted "flag handed back" assertions also pass with the clear deleted, because the flag is simply never cleared.
• The timeout tests don't catch it either, because the loop still respects its deadline.
• Both test javadocs still say the park "cannot busy-spin".

Producer-thread CPU time vs wall time with the clear deleted:

• 150 ms drained wait: head uses 7–8 ms of CPU, the mutant 149–154 ms.
• 300 ms timeout wait: head uses 14–17 ms, the mutant 300 ms.
• Under 2× CPU oversubscription the mutant still used 215 of 300 ms.

Fix: in the two timeout-exit interrupt tests, assert that ThreadMXBean.getCurrentThreadCpuTime() over the table() call stays below half the wall time. Guard it with Assume on isCurrentThreadCpuTimeSupported(). The API has existed since JDK 5, so it respects the Java 8 floor.

---

Minor

5. [OSS][ENT] The PR bodies predate the gate

• Problem: the OSS body leaves the gate out of its list of client changes, and the ENT body says its test javadoc matches the gate when it does not.
• Net impact: reviewers read a description that disagrees with the pinned client.
• Evidence: the OSS body text, compared with QwpWebSocketSender.java:5632-5639.

The OSS body says "the fuzz producer can pause up to 2 s … when a bounce catches the ring undrained". With the gate, the producer pauses only if the wait began while the link was up. For ENT, see finding 2: the body claims the javadoc is "written against that path", and it repeats the "gives up quickly rather than parking the writer" wording.

---

Coverage map

The test gate passes: there is no Critical coverage gap, and two Moderate gaps (findings 3 and 4). The new gate itself is well tested:

• Deleting the gate, or its lastReconnectError == null condition, turns both rewritten outage tests red.
• Reverting either interrupt hand-back turns its inverted assertion red.

Fix verification

build() leak. A 300–500 ms delay was forced after connect:

• At head, build() returns, the first call surfaces the terminal error, and nothing leaks.
• At ce20e09, it leaks MMAP_DEFAULT … 4206592, the same number as the earlier Windows CI failure.
• The client's Windows CI leg is now green.

Live-connection gate.

• With a 2 s token pull that ignores interrupts, ce20e09's table() took 2,001 ms. At head it returns in 0 ms and the recycle stays armed.
• On healthy links the gate read "up" in all 12 combinations of connect mode × memory/SF × durable-ack, and every barrier swapped.
• A stress run made 2.45 billion concurrent isLinkUp() calls across 4,145 client swaps, with no exceptions.

Interrupt hand-back. With the interrupt flag set on entry, all of these committed and handed the flag back:

• the heal pass;
• a deferred close of a recovered engine;
• a memory-mode REBUILD resume;
• 20,000 rows.

Store-and-forward contract. Nothing changed this round lets a transport error, a reconnect time limit, or a hard failure reach the producer.

CI

• Client: all 16 checks green, including Windows.

• ENT PR-merge build 272038: green.

• enterprise-ci build 272039: one red test on windows-amd64-lifecycle, EntLifecycleOrchestratorTest.testSwitchRoleOwnershipRefusalTornArmFuzz. It timed out after 30 s inside LOG.advisory → nextBully. It is not caused by this change:

  • Lines logged within the same millisecond reached the agent 63–65 s later. The log consumer was blocked writing to the agent's stdout, so the log ring filled up.
  • The PR changes no ENT or OSS production code, and the test imports nothing from the client.
  • The same test passed in 281 ms in build 272038.
  • SqlFailoverQwpRecycleLosslessTest passed on that same leg.

  Rerun the stage.

Summary

PR	Verdict
#91	approve with comments (findings 1, 3, 4)
questdb/questdb#7525	approve (Minor 5 only)
questdb/questdb-enterprise#1171	approve with comments (finding 2)

• Severity: 0 Critical, 4 Moderate, 1 Minor, all inside the diff.
• Test gate: passes.

🤖 Generated with Claude Code

[bluestreak01]

@jovfer

PR #91 — level 3

Critical

Problem: A released public constructor no longer links after upgrading.
Net impact: Direct policy-aware loop users cannot construct their ingestion loop.
Evidence: Identical 1.3.8-compiled caller passes release/base 981bdb02; HEAD 91fd4451 throws NoSuchMethodError.

Location: core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java:945–964 — in-diff.

Adding externalFsnBase replaces the published 13-argument constructor instead of preserving it. Applications compiled against that constructor fail when replacing the dependency, before connection or ingestion begins. This is a deterministic, net-negative compatibility regression; facade-only callers are unaffected.

Independent falsification checked whether this was internal or unpublished: the constructor is documented, exported, and present in the published 1.3.8 JAR. Retained shorter overloads cannot satisfy its JVM descriptor.

Fix: Preserve the 13-argument overload, delegating with externalFsnBase = 0L, and pin its signature in compatibility tests. That bridge passed the unchanged caller and 135 focused tests in scratch.

Reproducer · Base output · HEAD failure

Coverage and validation

• Test gate: pass — 0 admitted coverage gaps.
• JDK 8: 490 tests across focused runs; zero failures/errors, one skipped.
• JDK 25: core production and test compilation passed.
• Reviewed linked OSS #7525 real-server/fuzz coverage and Enterprise #1171 failover coverage, including assertions and CI wiring; not executed locally.

Provenance

• Client PR #91: no submodule-pointer changes.
• Enterprise → OSS dd7ad37a: OFF-DEFAULT.
• OSS → client 91fd4451: OFF-DEFAULT.

Summary

Verdict: request changes.
Severity: 1 Critical, 0 Moderate, 0 Minor. Attribution: 1 in-diff, 0 out-of-diff-breakage.

Primary checkout unchanged; no review posted.

[jovfer]

The released 13-argument CursorWebSocketSendLoop constructor is back as an overload delegating with externalFsnBase = 0, and ExportedApiCompatibilityTest now pins all seven public constructors present in the 1.3.9 jar, copied from its javap output. An exported-signature diff of this head against the merge base (javap over every exported package, at the merge base, at the head and in the 1.3.9 jar) found one more in-place widening, QwpWebSocketSender.connectWithCredentialSupplier (23 → 26 arguments; on main since the OIDC device-flow change, not in any release); its 23-argument form is back the same way and both merge-base overloads are pinned. After both, the diff against the merge base is empty; against the 1.3.9 jar it lists 18 synthetic Outer$1 anonymous-class artifacts that were already absent at the merge base. Head: ba54a13.

[bluestreak01]

@jovfer

PR #91 review, level 3

Verdict: request changes. One Critical finding, one Moderate coverage gap, no Minor.

• Revisions: PR head aaabff3e, merge base 916e51f6.
• Submodules: the PR moves no submodule pointers.
• Title and description: the title follows Conventional Commits, Fixes #80 is at the top of the body, and the labels fit.

Critical

Problem: A recycle unlocks the slot, so a second sender can take it.
Net impact: The running sender with the duplicate sender_id throws on every call while the intruder lives.
Evidence: A probe run on JDK 8. Head aaabff3e reproduced it in 2 of 2 runs. Base 916e51f6 did not reproduce it: 583,500 of 583,500 attempts by B were refused.

Where: this is in-diff. The chain is:

1. QwpWebSocketSender.java:5871-5872: step 3 calls outgoing.close(). This releases the slot's directory lock and removes the logical lock file.
2. :5481: rebuildEngineOrAbandon.
3. :5726: engineRebuildFactory.rebuild.
4. Sender.java:3449: constructEngineOnSlot takes the locks again.

The contract this breaks: the senderId javadoc at Sender.java:2987-2996 says the slot is "locked exclusively for the sender's lifetime via flock" and that "the second sender to start with a colliding id fails fast with 'sf slot already in use'".

Trigger:

• Sender A is running with the default symbol_dict_reset=on and sf_dir set.
• Sender B is configured with the same sf_dir and sender_id, and retries build() after it fails. Realistic sources of B are a rolling-deploy overlap, a supervisor restart loop, or an accidental duplicate instance.
• B's build() lands between A's step-3 close and its step-4 rebuild, and succeeds.
• A's rebuild then fails with SlotLockContentionException and stays in the REBUILD resume state with cursorEngine == null.
• Every later table(), at() or flush() on A throws "symbol dictionary recycle could not rebuild its engine; retried on the next send". The message says the failure is transient, but it lasts as long as B holds the slot.

Probe results: the probe is a scratch JUnit test. Sender A ingests and flushes in a loop while a second thread calls Sender.fromConfig(cfg) for B in a tight loop, with the same sf_dir and sender_id at both revisions.

Revision	B's build()	A while B is open	A after B closes
Head, run 1	succeeded on attempt 95,639, when A was at epoch 1	0 rows ok, 2,630 exceptions	50 of 50 ok
Head, run 2	succeeded when A was at epoch 3	0 rows ok, 2,599 exceptions	50 of 50 ok
Base	0 of 583,500 attempts succeeded, all SlotLockContentionException	2,057,013 rows ok, 0 errors	–

The probe triggers recycles with resetSymbolDictionary() to raise their frequency. Even so, B won within the first few completed recycles, so the window is the whole close-and-rebuild span, not a few microseconds. A natural threshold recycle opens the same window.

Net effect: this is worse than base (net-negative).

• Who is affected: deployments where two senders share sf_dir and sender_id at the same time. The product explicitly anticipates this case and documents the outcome.
• Change from base: at base the newcomer fails fast and the running producer carries on. At head the running producer stops accepting rows for an unbounded time, and its error message is misleading.
• Other safeguards: there is no silent loss, because the barrier proved everything acknowledged and new rows are rejected loudly. Nothing else found holds the slot during the window; I searched SlotLock, constructEngineOnSlot and the recycle steps.
• Store-and-forward rule: this also breaks the rule that a running store-and-forward producer never hard-fails after initialization.

Suggested fix: keep the slot exclusively owned across steps 3 and 4.

1. Before step 3, have the rebuild factory take SlotLock.acquireLogical(slotPath).
2. Close the outgoing engine with close(false), which already exists for callers that hold the logical lock and skips removing the lock file.
3. Rebuild through constructEngineOnSlotLocked while the lock is still held, so a colliding build() fails when it tries to take the logical lock.
4. Add a regression test shaped like the probe, and check that it is red on the current head.

Moderate

Problem: The per-window starvation resets and the age guard are untested.
Net impact: A regression would pause each table() right after arming, or stop recycling after the first starvation wait. Either way the harm is bounded.
Evidence: mutation runs at aaabff3e. Under each mutation below, all 148 tests matching SymbolDictRecycle*Test, SymbolDictResetDefaultsTest and SenderPoolSfTest stay green.

• M1: delete QwpWebSocketSender.java:5348 (starvationWaitDoneThisArm = false).
• M2: delete :5347 (armedSinceNanos = …).
• M3: delete the age guard at :5644.

Why nothing catches them:

• No test arms a second window after a starvation wait has already run.
• Every test that enters the wait first sleeps for maxWait + 50 ms.
• testContinuousFlushesPreserveStarvationWindow, added in this PR's latest commit, only asserts the final count of 1.

Suggested fix: extend that test to cover a second armed window.

1. Release acks until the first recycle commits (epoch 1).
2. Gate acks again and re-arm with resetSymbolDictionary().
3. Assert that a table() inside maxWait returns promptly with the timeout count unchanged. This catches M2 and M3.
4. Assert that the count then reaches 2. This catches M1.

Coverage map

• Test gate: passes. The only admitted coverage gap is the Moderate one above.
• Local run: 484 related tests on JDK 8 at head, with 0 failures, 0 errors and 1 skipped.
• Tandem PRs: both exist and are cross-linked with this PR: OSS questdb/questdb#7525 (e2e and fuzz) and Enterprise questdb/questdb-enterprise#1171 (failover). #7525's submodule pin points at ba54a138; head adds only javadoc and test commits on top of it.

Summary

• Severity: 1 Critical, 1 Moderate, 0 Minor. Both are in-diff; no out-of-diff breakage was admitted.
• Checked and not reported: lost or duplicated rows, epoch-offset errors in the public sequence numbers (FSNs), symbol IDs referenced before registration, durable-ack mode, deferred-commit groups, and binary compatibility.

[mtopolnik]

[PR Coverage check]

😍 pass : 511 / 579 (88.26%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/Sender.java	75	90	83.33%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	41	47	87.23%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	363	410	88.54%
🔵	io/questdb/client/impl/PooledSender.java	2	2	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java	11	11	100.00%
🔵	io/questdb/client/impl/ConfigSchema.java	3	3	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendCounters.java	16	16	100.00%

[jovfer]

Tandem review: client #91 / OSS #7525 / ENT #1171 (level 3)

Reviewed heads: client 0496f037, OSS 186cd29978, ENT f0e373da3.

Scope: both submodule pointers point at unmerged feature-branch commits, so both are reviewed as part of this change. The OSS and ENT test files are unchanged since the previous review. The new code under review is client-side: the recycle now holds the slot's logical lock for the whole swap, awaitAckedFsn re-reads the acked position on every poll, and the docs now say FSNs are scoped to one sender instance.

Critical

None.

Moderate

[client] Recycle refuses every row forever when the slot's logical lock can never be taken. Location: QwpWebSocketSender.java:5870-5881.

• Problem: step 0 treats a failure that will always repeat as brief contention, so the sender stays armed and refuses rows.
• Net impact: a sender built with direct connect() and a custom rebuild factory stops ingesting once the recycle arms.
• Evidence:
  • A probe test at 0496f037: 20 of 20 rows refused, 0 frames sent.
  • The same probe at the previous head ba54a138: 0 refused, the recycle commits.
  • Control run at 0496f037 with a normal slot path that has a parent directory: the recycle commits.

Step 0 turns any non-Error throw from SlotLock.acquireLogical(cursorEngine.sfDir()) into "recycle deferred … retried at a later row start" and leaves the sender armed. Two cases make that throw repeat on every attempt:

• The slot path has no parent directory, for example a relative "sfslot". resolveLogicalLock returns null, so acquireLogical throws IllegalArgumentException (SlotLock.java:148-150).
• The parent directory is not writable. acquireLogical throws SfOperationalException: could not create logical slot lock dir.

The refused row never starts, so the ring stays drained and every later table() goes straight back to step 0. An empty flush() does not re-run armIfEligible(), and setEngineRebuildFactory(null) does not disarm the sender either. On the previous head, step 3's close(true) → removeOrphanLogical silently did nothing for these paths, and the custom factory rebuilt the engine and the recycle committed.

How it is reached: a caller uses the public connect(...) with its own CursorSendEngine and installs a factory with setEngineRebuildFactory, as the new javadoc on all four connect overloads tells them to. build() is not affected: it builds slotPath as sfDir + "/" + senderId and takes acquireLogical itself at startup, so a bad parent directory fails the build instead.

Why this is Moderate rather than Critical: EngineRebuildFactory is new in this PR (not in 1.3.9, not at the base), and the failure also needs a slot path that build() never produces. No caller in any of the three repos does this.

Suggested fix:

• Skip step 0 when resolveLogicalLock would return null, the same way removeOrphanLogical silently does nothing for such a path.
• Keep the refuse-this-row-and-retry-later path only for SlotLockContentionException. For any other failure, disarm the recycle or latch recycleFailure instead of refusing rows forever.
• Add a direct-connect test with a slot path that has no parent directory.

Minor

None.

Coverage

The test gate passes, with no coverage gaps admitted.

A mutation run over 232 recycle-related tests caught three changes:

• removing the step-0 lock;
• removing the lock release when the recycle breach terminates the sender;
• reverting awaitAckedFsn to a single engine snapshot.

These mutants survived, but none changes user-visible behavior beyond leftover lock files:

• keeping the lock after the recycle commits;
• reordering the release in close();
• closing the outgoing engine with close() instead of close(false). This behaves identically, because removeOrphanLogical takes the lock before it unlinks anything.
• dropping the in-loop error check in awaitAckedFsn.

Verdict

• client: approve with comments. The Moderate above should be fixed in this PR.
• OSS: approve.
• ENT: approve.

At review time CI was green on the client and ENT PRs. The OSS macwin run was still pending.

🤖 Generated with Claude Code