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

[jovfer]

Fixes #80

Tandem:

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

Summary

A QWP WebSocket sender keeps a dictionary of every distinct symbol value it has sent, and that dictionary only ever grew. A long-lived sender that saw enough distinct values reached a hard cap, and from then on every new symbol value failed. A pooled sender made this worse: it went back to the pool still full, so the next borrower inherited the failure (#80).

The sender now recycles its dictionary. When the dictionary reaches a threshold (100,000 distinct symbols by default), the sender waits for a safe moment, starts a fresh dictionary on a new connection and carries on. No rows are lost, application code does not change, and the feature is on by default.

The change is client-only, with no wire-protocol change and no on-disk format change. It relies on the server clearing its per-connection dictionary state on disconnect, which QuestDB 10.0.0 and later already do. The tandem OSS PR adds a server test that keeps it that way.

How it works

1. After each flush the sender checks the dictionary size. At or above the threshold, it arms a recycle.
2. The recycle runs at the next table() call that finds every published frame acknowledged, no row in progress and the connection up.
3. The sender then closes its send loop, starts a new dictionary and reconnects in the background. In store-and-forward mode it reuses the same slot, which is empty at that point. Rows written in the meantime are buffered as usual.

Because the swap happens only when nothing is unacknowledged, no frame built against the old dictionary can be replayed against the new one.

A producer that never pauses seldom has everything acknowledged at the start of a row. For that case, once a recycle has been armed for symbol_dict_reset_max_wait_millis (2 s by default), one table() call pauses for up to that long to let the backlog drain. This happens at most once per armed recycle, and on a healthy connection it takes about one acknowledgement round trip. If the backlog does not drain in time, the call returns normally and the recycle stays armed until a quieter moment.

Three more rules keep the recycle out of the way:

• It never runs during an outage. While the sender is reconnecting, the recycle stays armed and runs once the connection is back. A recycle never surfaces a transport error or a reconnect timeout to the producer.
• The bar rises after each recycle. If the live symbol set is larger than the threshold, recycling at the threshold would repeat forever. After each recycle the bar rises to twice the dictionary size at that recycle, up to 1,000,000. A static set below 1,000,000 therefore stops recycling after about log2(set size / threshold) recycles: four for 900,000 symbols at the default threshold. A set of 1,000,000 or more, or one whose values keep changing, recycles again at each 1,000,000 symbols the sender registers, and each of those recycles re-sends the symbol strings on a new connection. A set that stays below 2,000,000 avoids them with symbol_dict_reset=off. A manual resetSymbolDictionary() ignores the bar for that one recycle.
• It never hides a terminal error. If the connection reports a terminal error, such as a security rejection, while a recycle is stopping it, the recycle does not run and the sender throws the error from that table() call or the next call, and from every later one, as on a sender that never recycled.

Configuration

All three settings apply to the WebSocket transport only.

Connect-string key	Builder method	Default	Meaning
symbol_dict_reset	symbolDictReset(boolean)	on	Enables recycling. off also turns resetSymbolDictionary() into a no-op.
symbol_dict_reset_threshold	symbolDictResetThreshold(int)	100000	Distinct symbols that arm a recycle. Accepts 1 to 1,000,000.
symbol_dict_reset_max_wait_millis	symbolDictResetMaxWaitMillis(long)	2000	Longest pause of one table() call for a recycle that has not found a quiet moment. 0 never pauses.

New API

• Sender.resetSymbolDictionary() asks for a recycle regardless of dictionary size. The request is advisory: the recycle still runs at the next safe table() call. It is a no-op on transports without a symbol dictionary, and PooledSender forwards it to the sender it wraps. Call it on the producer thread, like every other Sender method.
• QwpWebSocketSender gains monitoring getters: getSymbolDictEpoch() (recycles completed), isResetArmed() and getSymbolDictResetStarvationTimeouts() (pauses that ended without a recycle).
• QwpWebSocketSender.setEngineRebuildFactory(...) lets a sender created through the low-level connect(...) overloads recycle. Senders from Sender.builder(), Sender.fromConfig() and SenderPool are set up automatically.
• QwpWebSocketSender.connect(...), QwpWebSocketSender.connectWithCredentialSupplier(...) and the CursorWebSocketSendLoop constructor each gain an overload that carries the recycle settings. The two new sender overloads reject a threshold or a wait outside the builder's ranges. Every existing overload is kept. CursorWebSocketSendLoop.markEverConnected() is new.

Release notes

• A sender no longer runs out of symbol capacity over its lifetime. The dictionary is recycled automatically at 100,000 distinct symbols; see Configuration to tune or disable it.
• QuestDB server 10.0.0 or later is required. Earlier servers cap the dictionary at 1,000,000 entries, and the protocol does not negotiate the limit.
• The client-side dictionary cap is now 2,000,000, up from 1,000,000 and equal to the server's. The error raised at the cap now points to the reset settings.
• Frame sequence numbers keep increasing across recycles. flushAndGetSequence(), getAckedFsn(), awaitAckedFsn(), progress callbacks and SenderError ranges stay on one continuous scale for the life of a sender instance, and getAckedFsn() does not fall back to −1 during a recycle. The scale is not persisted: a process restarted on the same store-and-forward slot numbers from the slot's on-disk state.
• Sender counters are lifetime totals. getTotalFramesSent(), getTotalAcks(), getTotalReconnectAttempts() and the related counters carry across recycles and remain readable after close(). A recycle counts as one reconnect attempt and one success.
• A recycle's reconnect is reported without a disconnect. SenderConnectionListener receives RECONNECTED (or FAILED_OVER) with no preceding DISCONNECTED.
• SenderErrorHandler can be called on the producer thread. If the sender finds a damaged store-and-forward slot while rebuilding during a recycle, it reports it synchronously from the table(), flush() or drain() call that ran the rebuild, as it already does at build time.

Limits

• A recycle needs a quiet moment. A producer that continuously outruns the connection may never give it one, and the dictionary then keeps growing toward the 2,000,000 cap. The sign is isResetArmed() staying true while getSymbolDictEpoch() does not advance; the same reading during an outage only means the sender is between connections. Raise symbol_dict_reset_max_wait_millis, or call drain(...) at a quiet point. Moving the swap to the I/O thread, so that it no longer depends on a quiet moment, is the planned follow-up.
• A long outage defers the recycle. If the dictionary reaches 2,000,000 entries before the connection returns, symbol() fails with the dictionary-full error.
• An interrupted recycle resumes. If a recycle cannot finish for a transient reason, such as a thread interrupt, a stalled disk or a failed file open, the next table(), flush() or drain() call completes it; an at() or atNow() call made first fails and rolls back its row. While the disk under the slot is stalled mid-recycle, one call can block for up to 30 s and rows are refused until the disk recovers. The sender becomes unusable in one case only: the slot it has just emptied holds unacknowledged frames when reopened, which means the store-and-forward guarantee was broken.
• Low-level connect(...) senders need two things to recycle: a rebuild factory, and in store-and-forward mode a slot directory whose parent can hold the slot's lock files. If the sender cannot take the slot's lock when a recycle starts, it logs one warning, stops recycling and keeps accepting rows.
• Pooled senders hand an armed recycle to the next borrower. On a healthy connection the borrower's first table() call recycles without waiting. On an unhealthy one it may pay the pause while holding its lease, which is why the default wait is well below the pool's 5 s acquire timeout. The pooled path has no end-to-end test yet.

Design decisions

• Recycle by reconnecting, not by a new protocol message. The server already drops its dictionary state when a connection closes, so the feature needs no server change.
• Swap only when everything is acknowledged. This is what makes the recycle lossless without translating symbol ids on replay: nothing in flight refers to the old dictionary.
• table() is the only trigger. The check sits at the start of a row, so the per-row hot path has no new branch. A caller that never starts another row never recycles.
• The default wait is 2 s. Zero would leave the recycle unreachable for continuous producers, the very senders that reach the threshold. A long wait would let a pooled borrower hold its lease past the pool's acquire timeout and starve other borrowers.
• The sender keeps its store-and-forward slot locked across the swap. Another sender started with the same sf_dir and sender_id fails fast, as it does at any other time, instead of taking the slot mid-recycle.
• The sequence-number offset stays in memory. Persisting it would add to the on-disk format, which this change avoids.
• Sender stays single-threaded. Calling close() from another thread during a recycle is outside the documented contract.

Tests

• Client: new suites cover arming and thresholds, the swap in store-and-forward and memory modes, sequence-number continuity, the bounded wait, outages and reconnect races, a terminal error arriving while a recycle stops the connection, interrupted and resumed recycles, slot ownership, and backward compatibility of the public API.
• OSS (test(qwp): pin the disconnect-clear contract, e2e and fuzz coverage for client symbol-dict recycle questdb#7525): a server test for the disconnect-clears-dictionary contract, real-client end-to-end tests in both modes, and a seeded fuzz test that interleaves recycles with server restarts.
• Enterprise (questdb/questdb-enterprise#1171): continuous ingestion through a primary-to-replica failover with recycles in flight, asserting that no row is lost.

🤖 Generated with Claude Code

[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

[bluestreak01]

@jovfer

Reviewing PR #91 at level 3, the full review: change-surface map, six discovery reviewers, and independent falsifiers with runtime probes at head and base.

Scope. No submodule pointer moved; core/src/main/c/share/zstd is unchanged, so no provenance check was needed. The PR metadata is in order: the title follows Conventional Commits, Fixes #80 is at the top, and the labels fit. One PR-body item is listed under Minor.

Critical

1. On some slot paths the recycle permanently stops a connect()-built sender from accepting rows

• Problem: The recycle wedges the sender when the slot's logical lock cannot exist.
• Net impact: connect()+factory senders on such paths stop ingesting after 100k symbols.
• Evidence: In a probe at 17f04409, 24 of 25 rows were refused. The same app at c9968f24 accepted 25 of 25.

Where (in-diff): QwpWebSocketSender.java:5915-5928, the step-0 code in recycleForDictReset.

Trigger: two slot-path variants reproduce it.

• Relative path: new CursorSendEngine("sf-data", …) uses a single-segment path with no parent.
• Read-only parent: an absolute slot directory whose parent the process cannot write and which has no .slot-locks directory, e.g. a container volume mounted at /data and used directly as the slot.

In either case, the engine is passed to the public QwpWebSocketSender.connect(...) and setEngineRebuildFactory(() -> new CursorSendEngine(sameDir, …)) is installed. Every connect overload's javadoc says the factory is required for recycling (:1055, :1108, :1164, :1221).

Path:

1. After 100k distinct symbols the recycle arms.
2. At the first table() call that finds the backlog acknowledged and the link up, step 0 calls SlotLock.acquireLogical(sfDir).
3. That call throws either IllegalArgumentException("slotDir must contain a parent and slot name") (SlotLock.java:148-150) or SfOperationalException("could not create logical slot lock dir") (SlotLock.java:305).
4. Step 0 wraps the exception as "symbol dictionary recycle deferred … retried at a later row start" and leaves the sender armed.
5. The refused row publishes nothing, so the next table() finds the backlog acknowledged again and fails the same way. sfDir is final, so this never clears.

Symptom: while the link is up, every row is refused. getSymbolDictEpoch() stays at 0 and isResetArmed() stays true. The error message calls the condition transient, but it is permanent.

Runtime evidence:

• Probe: the public 9-arg connect, default recycle settings, 100k distinct symbols.
• At head, both path variants accepted 1 row and refused 24 over 9 s, including an idle period longer than the 2 s max wait. The server received 1 new frame.
• Controls at head:
  • With a writable parent the recycle commits and 25 of 25 rows are accepted.
  • Without a factory the sender never arms.

Base, same trigger: N/A, this is a new surface, since setEngineRebuildFactory does not exist at c9968f24. The identical probe minus that line accepts 25 of 25 rows there.

Why build() is unaffected: build() always uses sfDir/senderId and takes the same lock at startup (Sender.java:1664), so a path like this fails build() before any data is sent.

Net determination:

• Population: users of the public connect(..., CursorSendEngine) API who enable recycling as documented, on a slot directory whose parent is missing or cannot hold .slot-locks.
• Delta and magnitude: ingestion stops for good at the first acknowledged-backlog row after the threshold; at base it never stops.
• Offsets: none documented. setEngineRebuildFactory(null) explicitly does not cancel an armed recycle.
• Net: net-negative.

Fix:

• At step 0, defer only on SlotLockContentionException.
• When the logical lock cannot exist or cannot be created for this path, proceed without it. removeOrphanLogical already treats such paths as a silent no-op (SlotLock.java:184-188), and a colliding build() cannot hold the lock either, because build() takes it at startup and fails there. Step 3's close(recycleSlotLock == null) already degrades correctly in that case. Alternatively, disarm with a single WARN.
• Add connect()-based tests for both path variants.

Moderate

2. No test pins the connect-gate CAS that closeIfLinkUp() adds

• Problem: No test pins closeIfLinkUp()'s connect-gate CAS.
• Net impact: Removing the CAS keeps the suite green and reopens a 30 s stall.
• Evidence: A mutant at 17f04409 without the CAS passes all 92 recycle and gate tests, five runs out of five. A seam-free probe fails on it every time.

Location: CursorWebSocketSendLoop.java:1474-1479 (from 83bcaa4). The 83bcaa4 commit message says the new gate test "pins both outcomes of the gate deterministically", but none of the three tests checks the CAS itself:

• testClaimedStopKeepsIoThreadOutOfConnectWalk writes STOP_CLAIMED by reflection and drives only connectLoop.
• testCloseIfLinkUpRefusesDuringConnectWalkThenStopsLiveLoop calls closeIfLinkUp() after the walk has set lastReconnectError (:1950), so isLinkUp() refuses before the CAS runs.
• SymbolDictRecycleOutageTest#testServerCloseBehindDrainingAckDoesNotStallProducer targets the window before the gate existed, which the isLinkUp() re-check now closes on its own.

Mutation: if (!isLinkUp()) return false;

• The gate test plus every SymbolDictRecycle*Test (92 tests): green in 5 of 5 runs.
• The gate and outage tests alone: green in 10 of 10 runs.
• Six more classes that use the recycle or gate API (225 tests): green.

If this regresses: the I/O thread can enter its connect walk after isLinkUp() read the gate as OPEN but before close() clears running.

• Step 2 then calls close() on a walk blocked in a credential pull or DNS resolve that ignores interrupts.
• close() waits out the 30 s shutdown budget and throws, the recycle parks at CLOSE_LOOP, and the producer's table() absorbs the stall.
• The natural window is tiny (0 hits in 20,000 forced races), which is why this is Moderate rather than Critical.

Fix (no production seam needed): add a gate test using a WebSocketClient subclass whose isConnected() does three things:

1. drops the link,
2. waits until the I/O thread is inside a reconnect factory that ignores interrupts,
3. returns true.

isConnected() is the last check in isLinkUp(), after the gate read, so this forces the window. At head closeIfLinkUp() returns false in about 6 ms (5 of 5 runs). On the mutant, with setShutdownAwaitTimeoutMillis(1000), close() times out and throws (5 of 5). Probe source: /tmp/rev91/fal/f-l7-CloseIfLinkUpGateProbeTest.java.

3. The new engine test skips the suite's memory-leak check

• Problem: A new engine test skips the suite's assertMemoryLeak wrapper.
• Net impact: Native leaks on this path would go undetected; no user impact.
• Evidence: CursorSendEngineTest.java:425-450 at 17f04409; every other @Test in the file wraps it.

testAdoptCountersSharesTheInstanceAndFoldsOnlyItsOwnCounter builds a disk-backed CursorSendEngine (mmap'd segments) without TestUtils.assertMemoryLeak.

Fix: wrap the body in TestUtils.assertMemoryLeak(() -> { … }).

Minor

4. The PR body's release notes announce removing an accessor that never shipped

• Problem: The release note announces removing an accessor that never shipped.
• Net impact: The squashed commit message tells users a nonexistent public API was removed.
• Evidence: getSymbolDictResetsPerformed is absent at c9968f24 and in every tag from 1.0.0 to 1.3.9.

It was added (07ecb27) and later removed on this branch. Fix: drop the "getSymbolDictResetsPerformed() is gone" bullet from the PR body.

Coverage map

• Test gate: passes. There is one admitted coverage gap, finding 2 (Moderate).
  • Search: rg -l "closeIfLinkUp|isLinkUp" core/src/test finds only CursorWebSocketSendLoopConnectGateTest.
  • Failure link: none of its assertions can fail when the CAS is removed (mutation runs above).
• Tandem PRs: questdb/questdb#7525 (real-server e2e and fuzz) and questdb/questdb-enterprise#1171 (failover) both exist on the matching branch, are cross-linked and have green CI. Both pin client 0496f037, so 83bcaa4 (connect gate) and 17f0440 (retained-engine listener) are covered only by local tests until the pins move. That is merge mechanics, not a finding.

Summary

• Verdict: request changes.
• Correctness gate: fails, with one open Critical (finding 1).
• Test gate: passes. There is no Critical coverage gap and one Moderate gap.
• Severity: 1 Critical, 2 Moderate, 1 Minor. All four are in-diff; none is an out-of-diff breakage.
• Submodules: none changed.
• Also checked:
  • The Java 8 floor holds: no post-Java-8 APIs in the diff, and CI at 17f04409 is green for the JDK 8 build and tests and for the JDK 25 and 26 compile smoke.
  • No committed binaries.
  • The steady-state table() and column path adds only field reads, with no allocation.
  • The store-and-forward contract holds: after the first connect, reconnects stay on the I/O thread with no time budget. Finding 1 is a local slot-lock failure, not a transport error reaching the producer.

Falsification probes and logs are under /tmp/rev91/fal/. I removed the scratch worktrees afterwards, and the primary checkout is unchanged at 17f04409.

[mtopolnik]

[PR Coverage check]

😍 pass : 562 / 634 (88.64%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/Sender.java	76	91	83.52%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	56	64	87.50%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	398	447	89.04%
🔵	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 6f58e888, OSS 2bffcfd467, ENT 10b5bc914.

Scope: both submodule pointers point at unmerged feature-branch commits, so the whole PR diff in each repo is in scope. Apart from their pointer lines, the OSS and ENT diffs are byte-identical to the previous review's. The new code under review is client-side:

• the wide connect overloads validate the recycle settings;
• the docs now state when a recycling sender stops recycling;
• a terminal error latched while the recycle stops the I/O loop is thrown instead of discarded;
• tests for all three.

Critical

None.

Moderate

[client] No test pins that close() stays silent after the recycle throws the loop's terminal. Location: QwpWebSocketSender.java:6233-6237.

• Problem: the surfaced-error record on the new recycle throw is untested.
• Net impact: a refactor could make close() rethrow, or raise IllegalArgumentException under try-with-resources.
• Evidence:
  • At 6f58e888, the full client suite (3,595 tests) stays green with the record dropped.
  • Four probe tests go red with the record dropped and pass at head.

retireStoppedLoop() throws through cursorSendLoop.checkError(), which also records the error as already surfaced. close0() reads that record (:1750) to drop the instance the caller already owns (:1918).

Both SymbolDictRecycleLoopTerminalTest tests call table() a second time before close() (:99, :156). That call goes through checkConnectionError() → loop.checkError() and records the error anyway, so a missing record on the recycle path is invisible to them.

The regression is plausible. Step 2 already reads getTerminalError() one line before the call (:6056), and folding the throw into that read would drop the record with nothing failing.

Symptom if it regresses: the trigger is a terminal server rejection, such as SECURITY_ERROR from a read-only node, latching while the recycle stops the loop.

• A caller that catches the terminal from table() gets it thrown again from close().
• A try-with-resources caller gets IllegalArgumentException: Self-suppression not permitted instead of the typed LineSenderServerException.

At base this is a new surface. The same ownership contract already holds for every other terminal path; a non-recycle analog probe passes at base and at head.

Suggested fix: add one test using the existing NackOnDemandClient:

1. Trigger the step-2 latch.
2. Catch the terminal from the first table().
3. Assert that close() returns without throwing.

A try-with-resources variant of the same test asserts that the thrown error is the loop's own terminal.

Minor

[client] The widest connect javadoc gives the max_frame_rejections definition to the recycle keys. Location: QwpWebSocketSender.java:1225-1230.

• Problem: the summary sentence ties the rejection-count definition to the recycle keys.
• Net impact: readers of the public javadoc see the wrong meaning for three keys.
• Evidence: the same block at base a7e7db3d is correct; this PR's 3a30e4d9 inserted the recycle keys before the colon.

Suggested fix: keep the definition right after max_frame_rejections and give the recycle keys their own clause.

Coverage

• Test gate: passes. One coverage gap is admitted, the Moderate above.
• The new tests kill their target mutants:
  • removing checkError() from retireStoppedLoop;
  • removing the step-2 lock release;
  • reverting the CLOSE_LOOP resume arm;
  • all four validation bounds.
• Every new test is red against the previous head 38f8ce04.
• Flakiness: 10/10 consecutive runs passed, and 5/5 with the CPU oversubscribed 2x.

Checked and found sound

• Terminal handling during the stop: close() returns normally only after the I/O thread has passed its shutdown countdown, and recordFatal (the only writer of terminalError) never runs after it.
  • The step-2 guard and the throw always read the same value.
  • A terminal latched inside the graceful-stop window is caught too.
• Sender state after the throw: every entry point (table, the flush family, drain, awaitAckedFsn, ensureConnected) rethrows the same instance, and the CLOSE_LOOP resume is not re-entered.
• Lock release: the step-0 lock is released exactly once, at step 2 or first in close().
• close() after the throw: for default and custom handlers alike, close() is silent, as on a sender that never recycled.
• Pool: it discards the lease.
• Connect validation: it matches the builder's ranges and messages exactly, throws before the engine is attached, and affects no existing caller or released API.
• Doc arithmetic: the "900k symbols → 4 recycles" example and the 1M band are correct.

Verdict

• client feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap #91: approve with comments (the Moderate above).
• OSS #7525: approve.
• ENT #1171: approve.

Correctness gate and test gate pass. Findings: 0 Critical, 1 Moderate, 1 Minor, both in-diff in the client.

🤖 Generated with Claude Code

[bluestreak01]

Reviewing PR #91 at level 3, the full review.

Scope. No submodule pointer moved (core/src/main/c/share/zstd is unchanged), so the whole diff is in scope. The title follows Conventional Commits, Fixes #80 is at the top of the body, and the labels fit. Reviewed head 6f58e888, merge base a7e7db3d.

Critical

None.

Moderate

1. No test pins the cap on the re-arm floor

• Problem: No test pins the re-arm floor's 1,000,000 cap.
• Net impact: If the cap is removed, producers with ever-growing symbol sets silently stop recycling.
• Evidence: With the cap removed at 6f58e888, the full JDK 8 core suite still passes (3,595 tests, 0 failures). A probe fails on that mutant.

Where (in-diff): QwpWebSocketSender.java:5642-5643, the clause Math.min(dictSizeAtSwap * 2, MAX_SYMBOL_DICTIONARY_SIZE / 2).

Why this cap matters: take a producer on default settings whose distinct symbols keep growing.

• After four recycles the floor reaches 1M (100k → 200k → 400k → 800k → 1M).
• From then on every swap commits at 1M or more, and this clause is what holds the floor at 1M.
• Without it, the floor jumps past 2M. armIfEligible (:5435) can then never fire again, because the dictionary is capped at 2M.
• The producer then hits "global symbol dictionary is full", which is the #80 failure again, for exactly the users this PR is for. Base has no recycle, so this is a new surface.

What the tests reach:

• Client floor assertions only use small values (0, 4, 8, 12). The largest dictionary at any swap in the whole suite is 18.
• The tandem OSS and ENT tests use at most 400 symbols per epoch.

Runtime results (probe: /tmp/pr91/probes/PrReviewFloorCapProbeTest.java):

Run	Head	Cap removed
Probe (threshold 500k)	Floor stays 1,000,000 and re-arms at each 1M; no error	Floor 2,200,000, never re-arms, cap error at 2M
Default config, row API	No error over 6M rows	Cap error at about 5.1M rows

Fix: add a test shaped like the probe:

1. Fill 1.1M symbols through getGlobalSymbolDictionaryForTest().
2. Flush, drain, then call table().
3. Assert getResetFloorSymbolsForTesting() == 1_000_000 and that the next refill to 1M re-arms.

It runs in about 1 s on the existing TestWebSocketServer.

2. Sender.table() javadoc doesn't describe its new behaviour

• Problem: The Sender.table() javadoc omits the new pause, the reconnect and the new exceptions.
• Net impact: WebSocket producers past 100k symbols hit table() stalls that the method's own docs don't mention.
• Evidence: Sender.java:793-805 is identical to base. The new behaviour comes from QwpWebSocketSender.java:3650-3658.

Where (out-of-diff): the javadoc is unchanged, but the PR changes the contract it describes. On default settings table() can now:

• park for up to 2 s (maybeBlockForStarvedReset, :5757), and for up to 30 s per wait in a deferred engine close (:185, :5468);
• close the WebSocket and start a reconnect (:5971);
• throw new LineSenderExceptions: transient ones marked "retried on the next send", plus the terminal breach.

None of this could happen at base. It is documented only on resetSymbolDictionary() and the three builder knobs. Sender.table(), QwpWebSocketSender.table() and PooledSender.table() say nothing.

Fix: add a WebSocket paragraph to Sender.table() that links to resetSymbolDictionary() and symbolDictResetMaxWaitMillis(long) and gives the worst-case blocking bound.

Minor

3. The master connect javadoc attaches a definition to the wrong keys

• Problem: The master connect javadoc applies the rejection-count definition to the recycle keys.
• Net impact: Readers of the low-level API get the wrong meaning for three keys.
• Evidence: QwpWebSocketSender.java:1226-1231 at head, against :859-862 at base.

The recycle knobs were inserted before the colon, so "consecutive server-active rejections of the same head-of-line frame…" now reads as their definition. Fix: keep that clause next to max_frame_rejections and describe the knobs in their own sentence.

Coverage map

Test gate: passes. 1 coverage gap admitted.

Change	Search	Failure link
Floor cap, :5642-5643	rg getResetFloorSymbolsForTesting finds only SymbolDictRecycleArmingTest (floors 0/4/8) and SymbolDictRecycleTest (12). OSS #7525 and ENT #1171 use at most 400 symbols per epoch.	None: with the cap removed, 3,595 tests pass and 0 fail

Summary

• Verdict: approve with comments. Please address 1 (add the test) and 2 (doc sentence). Neither blocks the merge.
• Gates: the correctness gate passes (no Critical findings) and the test gate passes.
• Provenance: no submodule pointer changed.
• Split: 2 in-diff (1 and 3) and 1 out-of-diff, the documentation contract in 2.
• Severity: 0 Critical, 2 Moderate, 1 Minor.
• Tandem PRs: questdb/questdb#7525 and questdb/questdb-enterprise#1171 are on the matching branch, cross-linked in both directions, and pinned to this head: OSS pins 6f58e888, ENT pins OSS 2bffcfd4. Client CI is green. The OSS macwin legs and ENT CI were still pending at review time.
• Checked, nothing found:
  • Public API: a base→head javap diff removes no public or protected signature.
  • Build constraints: the Java 8 floor holds and there are no committed binaries.
  • Hot path: the per-row path only adds field reads and allocates nothing.
  • Store-and-forward contract: after initialization no transport error or reconnect time limit reaches the producer.
  • Durable-ack and transactional modes: in durable-ack mode ackedFsn moves only on durable acks. The server withholds acks for uncommitted deferred frames (QwpIngressUpgradeProcessor.java:1676-1695).
  • Local run: the 181 recycle-related tests pass on JDK 8 at head.

The primary checkout is unchanged at 6f58e888, my scratch worktrees have been removed, and I haven't posted anything to the PR.