[jovfer]

Speeds up the producer side of flush() on the WebSocket sender. This cost dominates when an application flushes every few rows, or borrows a sender from the QuestDB pool and closes it per message. The wire format and the public API do not change.

What changes

Four cuts on the flush path, each in its own commit:

• MicrobatchBuffer no longer reads System.nanoTime() on the first row of every microbatch. The stamp only fed getAgeNanos(), which had no caller.
• The sender encodes table and column names to their wire form once per schema instead of on every frame.
• ColumnBuffer.reset() zeroes the null bitmap only after a batch that held nulls, and clears the local symbol dictionary only when it has entries.
• A flush walks the list of tables that received rows since the last flush, instead of every table the sender has seen with a hash probe each, and empties its scratch lists with setPos(0).

One ordering change follows from the last cut: tables in a multi-table frame now ship in the order they first received a row since the last flush (rows that reset() discarded count), instead of the order the sender first saw them.

Measurements

Apple M4 Max, JDK 25. Producer thread only, median ns per row, 12 runs of 500k rows against the client's loopback test server. "Before" is #121 at 05b31067, measured before #121 merged; main has since changed only build-time validation, failure paths and native-memory accounting.

Shape	Before	After	Change
One Sender, flush() per row, 1 table	151	101	-33%
One Sender, flush() per row, 100 tables round-robin	437	134	-69%
QuestDB facade, borrowSender() and close() per row, 1 table	180	143	-21%
One Sender, rows only (auto_flush_rows=1000)	40	40	0

End to end against QuestDB master (which includes questdb/questdb#8110), 1M rows written with a borrow and a close() per row, time until every row is applied: 304 ms before, 199 ms after. The faster producer lets the I/O thread send full batches, which the server commits together (about 50 WAL commits instead of about 1,900). With 100 tables in round-robin the end-to-end time does not change (1.34 s on both), because the server commits each frame on its own when consecutive frames are for different tables; questdb/questdb#8195 (open) addresses that.

🤖 Generated with Claude Code

[jovfer]

PR #108 tandem review, round 4 (level 3)

Reviewed at level 3, tandem mode, as one logical change across three PRs:

PR	Head	Base
#108	47603c71	70a7f853 (round 3 ended at b2de8e4f)
questdb/questdb#7979	1f84849722	69796aab30
questdb/questdb-enterprise#1308	3087dda29	73dd5c863

Verdicts:

• Client perf(qwp): speed up frequent flushes on the WebSocket sender #108: approve with comments. Please address Moderate 1.
• OSS #7979: approve with comments. Please fix Moderate 2 in this PR; otherwise OSS master gets a flaky test.
• Ent #1308: approve.

Every behavioural finding below was executed on both the head and the base revision.

• PR metadata:
  • All three titles follow the convention and pass the title check.
  • Labels are set: enhancement, tandem, QWP on perf(qwp): speed up frequent flushes on the WebSocket sender #108; tandem, non-breaking-tandem, QWP on #7979; tandem, QWP on #1308.
  • The OSS body's claim holds. The base version of testTwoSendersPerTableWhileReaderThreadStreamsTwoQueries fails against the new client with txn timed out [table=alpha, expectedTxn=2, writerTxn=1], and the PR version passes.
  • The Enterprise PR body's claims about its tests hold.
• Submodules:
  • Enterprise questdb, e0b540f913 -> 1f84849722: OFF-DEFAULT, in scope. The move also carries 69796aab30 (#7640), which is already on OSS master and out of scope.
  • OSS java-questdb-client, c41a4172 -> 47603c71: OFF-DEFAULT, in scope. The move also carries 21e32683 and 70a7f853 (test(qwp): close TestWebSocketServer connections gracefully so scripted drops deliver their acks #95), which are already on client main and out of scope.
• CI:
  • Client and Enterprise CI are green.
  • The OSS Azure test pipeline did not run on draft #7979, so no OSS test has run against the new client in CI.
  • A local run of the OSS QWP ingress, QWP e2e, WebSocket and facade test classes against the head client passed: 594 tests, 0 failures, 1 skipped.

Critical

None.

Moderate

1. [client] QuestDB.close() returns while the settle step still holds a slot blocked on a full ring

• Problem: a settle flush blocked on a full store-and-forward ring outlives QuestDB.close().
• Net impact: pool users who close the handle while the SF ring is full. The slot's lock and connection stay held for up to about 111 s after close() returns.
• Evidence: probe at 47603c71 and 70a7f853, all timeouts at their defaults, ring capped at 128 KiB, server never acks:
  • Head: the lease's close() returned in 0 ms. QuestDB.close() returned after 9,009 ms. An immediate QuestDB.connect on the same sf_dir failed with SlotLockContentionException: sf slot already in use by another process [... default-0, holder=pid=<own pid>]. The lock was released 110,981 ms after close() returned.
  • Base: the lease's close() blocked 120,001 ms and threw cursor SF append failed. QuestDB.close() then returned in 0 ms, and the immediate re-open succeeded.
  • The same result holds after a real outage (server closed after connect), and in two other probes with shorter deadlines.

Trigger:

• Facade default flush_on_close=off, with sf_dir set.
• The server is down or not acking long enough to fill sf_max_total_bytes (10 GiB by default with sf_dir, 128 MiB in memory).
• A lease closes with rows still in the slot. The housekeeper's settle takes the slot and blocks in CursorSendEngine.appendBlocking for up to sf_append_deadline_millis (30 s).
• The application calls QuestDB.close(), then reconnects in the same process.

Path:

• SenderPool.settleIdle() (SenderPool.java:1769-1816) takes the slot off the free list and flushes it outside the lock. appendBlocking deliberately does not stop on an interrupt.
• QuestDBImpl.close() (QuestDBImpl.java:237) first runs PoolHousekeeper.stop(): a 2 s join, an interrupt, then another 2 s join.
• SenderPool.close() then waits min(acquire_timeout_ms, 5 s) for the internal lease (SenderPool.java:1626). It logs "1 borrowed sender lease(s) still outstanding ... or leaks if it never is", and returns.
• Later, on the housekeeper, the append fails. requeueSettled sees closed (:1997) and calls retireLease, so delegate.close() appends again (another 30 s) and then drains for up to close_flush_timeout_millis (60 s).

What the application sees on head:

• close() returns normally, yet a re-open on the same sf_dir fails until the housekeeper finishes.
• The close-time WARN describes the internal settle lease as a borrowed lease that may leak, although the application closed every lease.
• The dropped rows are reported only by an ERROR on the housekeeper about 100 s after close() returned ("slot 0 retired with 1 unshipped row(s); delegate close failed"). If the JVM exits first, that line is never written. Base threw on the lease's close().
• In memory mode there is no slot lock, but the delegate's I/O thread and native buffers still outlive close() (4.5 s close, delegate still open, in a separate probe).

Base comparison:

• For the same steps, base returns from QuestDB.close() with the slot released, and re-open works.
• Base can show the same symptom through other triggers:
  • An idle reap that is closing a delegate on the housekeeper when close() runs. This was reproduced on both revisions: close() took 4,015 ms, the re-open failed, and the lock was released about 10.8 s later.
  • A startup-recovery drain.
• What this PR adds is a deterministic path. A single thread that runs lease.close(); db.close(); and then reconnects hits it every time once the ring is full.

Why Moderate, not Critical:

• The rows are lost on both revisions.
• The trigger needs a full ring.
• The kernel releases the lock at process exit.
• The window is bounded at about two minutes with defaults.

Suggested fix:

• Make the settle flush non-blocking: one append attempt, and on backpressure requeue the slot and retry on the next tick. This is the "non-blocking settle" follow-up the PR already lists, and it also removes the borrower wait behind a settle. A settle then never holds a slot across close(), and close()'s idle loop closes the delegate on the caller's thread, as on base.
• PoolHousekeeper.runLoop sets nextSettleAt from a now read before settleIdle() runs (PoolHousekeeper.java:246-251). After a 30 s settle the next one therefore starts with no wait. Read the clock after the settle.
• At minimum, report the internal lease separately from borrowed leases in the close-time WARN. Also state in the QuestDB.close() Javadoc that a settle in progress can outlive it.

2. [OSS, cross-repo] testElasticPoolGrowsUnderBurstAndShrinksWhenIdle is flaky with the new client

• Problem: the test assumes one WAL transaction per pooled close().
• Net impact: OSS CI. Under CPU load about one run in three fails once #7979 pins the new client.
• Evidence: the unmodified test, at OSS 1f84849722:
  • In a 40-iteration loop under CPU load (14-core Mac, 16 busy processes):
    • Head client 47603c71: 13 of 40 failed with txn timed out [table=el, expectedTxn=4, writerTxn=3] (10 runs) or writerTxn=2 (3 runs).
    • Base client 70a7f853: 0 of 40 failed.
  • Without load, in a warm-JVM loop:
    • Head: 6 of 100 iterations reused a slot, and all 6 ended at 3 transactions.
    • Base: 2 of 100 reused a slot, and both still reached 4.
  • Forced reuse (threads 1-3 wait until thread 0's lease has closed): head failed, base passed.

Details:

• The test is at core/src/test/java/io/questdb/test/client/QuestDBFacadeE2ETest.java:838-910; #7979 does not touch it.
• Four latch-released threads each borrow a sender, write one row and close it. The comment at :884 says "Each of the 4 burst senders flushes once on close -> 4 WAL txns", and :886 then waits for awaitTxn("el", 4).
• With the new client, a release ships nothing until a threshold trips or the settle runs (PooledSender.java:595-599). borrow() hands out the most recently returned slot (SenderPool.java:1252).
• A burst thread that borrows after another thread's lease has come home therefore appends to that slot. The settle ships both rows in one frame (server log: seqTxn=2, rowLo=0, rowHi=2), the table reaches 3 transactions, and awaitTxn throws after 15 s.
• On base every close flushed, so a reused slot still produced 4 frames.
• This test never ran in CI against the new client, because OSS Azure did not run on #7979.

Suggested fix:

• Call s.flush() inside the burst lease before it closes, the same remedy #7979 applied to testTwoSendersPerTableWhileReaderThreadStreamsTwoQueries.
• Correct the comment at :884.
• Waiting on the row count instead of the transaction count would also work.

Minor

• [client] Stale PooledSender Javadoc. The class Javadoc (PooledSender.java:50-52, "by default, ships nothing") and the close() Javadoc (:150-154, "By default nothing is shipped") still say that a release ships nothing.
  • The ship() Javadoc, corrected in round 3 (:576-578), says a release ships every pending row once the oldest is older than auto_flush_interval or another threshold trips.
  • A directly built SenderPool flushes on close by default (SenderPool.java:258-260).
  • Suggested: reuse the ship() wording and name the direct-pool default.
• [OSS] Stale bytes=0 in a test comment. QuestDBFacadeE2ETest.java:700 still reasons with "rows=1000 (> both row counts) and bytes=0". With the new client, an unset auto_flush_bytes means 90 percent of the advertised cap, not 0.
  • The conclusion (exactly two flushes per table) still holds, because the batches are a few KB.
  • #7979 edited the lines on either side of this one.

Coverage map

The test gate passes. There are no admitted coverage gaps.

• Round-3 tests: mutation runs at 47603c71 turn at least one test red for each of these mutations:
  • removing the reapIdle guard;
  • removing the SenderPool.close() ERROR log;
  • setting the direct-pool default flushOnClose to false;
  • removing QuestDBImpl's setFlushOnClose(...);
  • changing the pendingRowCount < 2 gate to < 1;
  • making the hold-back retry non-transactional.
• OSS facade fix: a working regression test. The base version of the test is red against the new client and the PR version is green.

Summary

• Verdicts:
  • Client perf(qwp): speed up frequent flushes on the WebSocket sender #108: approve with comments (Moderate 1).
  • OSS #7979: approve with comments (Moderate 2, plus the Minor OSS comment).
  • Ent #1308: approve.
• Severity: 0 Critical, 2 Moderate, 2 Minor.
• Where the findings sit:
  • Moderate 1 is in-diff: the new settle step meets the unchanged close budgets.
  • Moderate 2 is out-of-diff breakage: an unchanged OSS test is broken by the client contract that #7979 pins.
  • The Minor items are in-diff: Javadoc from this PR, and an OSS comment block that #7979 edited.
• Trade-off: Moderate 1 is the cost of moving the close-time flush off the borrower's thread. With a full ring, base blocked the borrower's close() for up to two minutes and then threw.
• Store-and-forward contract:
  • The pool's new calls on borrower and housekeeper threads (checkError(), autoFlushIfDue(), the settle flush()) surface only latched terminal errors and cap rejections, the same set base surfaced through the close-time flush().
  • A settle that fails transiently requeues the slot instead of retiring it.
  • No reconnect budget or new terminal path was added.
• Submodule provenance:
  • Enterprise questdb: OFF-DEFAULT, in scope, except the upstream commit 69796aab30.
  • OSS java-questdb-client: OFF-DEFAULT, in scope, except the upstream commits 21e32683 and 70a7f853.
• Callsite analysis covered:
  • every OSS and Enterprise consumer of the client:
    • the OSS facade E2E tests;
    • the OSS QWP e2e tests, including the tight-cap and oversize-row tests, under the new 90 percent byte trigger;
    • the Enterprise consumers in the tandem PR;
  • the QuestDBImpl constructor and direct SenderPool callers (none outside the client).

Artifacts

Probe sources and run logs for every finding, on head and base, are kept locally and are available on request.

[jovfer]

PR #108 review (level 3)

Reviewed the four commits on top of #121: f643ffb1..d632ea70 (base is the head of perf/ws-send-first). Client only, no tandem. No submodule pointer moves in the range.

Verdict: approve. No Critical or Moderate findings, three Minor test-code items.

PR metadata: the title is fine. The tandem label is stale now that questdb/questdb#7979 and questdb/questdb-enterprise#1308 are closed. The body describes the table-order change but not the trade-off below.

Minor

1. A table-order check passes even when bravo is missing.

• Problem: in QwpWebSocketSenderTest.java:539-540, indexOf(frame3, "bravo") < indexOf(frame3, "alpha") is true when bravo is absent, because indexOf returns -1.
• Fix: assert indexOf(frame3, "bravo") >= 0 first, as the checks on frames 0 and 2 already do.

2. FrameCapture re-implements existing test helpers.

• FrameCapture.tableCount (QwpWebSocketSenderTest.java:1112) duplicates the public QwpWireTestUtils.tableCount(byte[]).
• The ack at :1106 uses CloseDrainTest.buildAck instead of the shared QwpWireTestUtils.buildAck(long).
• Both helpers are in the same package and have matching signatures.

3. Header size written as a literal 12.

• QwpWebSocketEncoderTest.java:1448 and QwpTableBufferTest.java:1784 hard-code 12.
• QwpConstants.HEADER_SIZE exists, and QwpWebSocketEncoderTest already uses HEADER_SIZE elsewhere.

What was checked

• getAgeNanos removal: nothing calls it in the client, questdb/core or questdb-enterprise.
• Header encoding: the cached table-name and column-definition bytes match the old putString output byte for byte, including unpaired surrogates (both paths write '?'). The UDP sender's size estimate still matches.
• putEncoded: safe on NativeBufferWriter, SegmentedNativeBufferWriter and WebSocketSendBuffer.
• Null bitmap: a bit is only ever set while hasNulls is true. That holds through markNull, truncateTo, clearToEmptyFast, compactNullBitmap and capacity growth, so skipping the memset after a batch without nulls is safe. symbolDict and symbolList always change together, so the guard on the dictionary clear is safe too.
• Pending tables:
  • sendRow is the only place a WebSocket row is committed, and it lists the table right after the commit.
  • The flags are cleared only after a successful flush or a flush that finds no rows.
  • A failed or rejected flush leaves the list intact for the retry.
• CI: all green on d632ea70. The coverage bot shows the changed lines in QwpTableBuffer and QwpColumnWriter at 100%.

Trade-off: the new table order changes which large batches get rejected

Mechanism:

• This applies with the delta symbol dictionary on: memory mode, or store-and-forward once the persisted dictionary is open.
• In that mode, a split flush puts the whole batch's symbol delta in the first frame.
• The first frame used to belong to the table the sender created first. With this PR it belongs to the first table to get a row in the batch.

When it matters: the delta plus the big table's body is over the server cap, but the delta plus the small table's body fits. The outcome then depends on row order.

Scenario: these are the shapes of SelfSufficientFramesTest#testSplitPreflightAdvancesBaselineSoLaterFramesArentSizedWithTheDelta, with the rows reordered:

• Server cap: 200 bytes.
• Config: auto_flush_bytes=off.
• Each table gets a new 64-char symbol.
• The big table also gets a 100-char string.

Run at both revisions:

Scenario	f643ffb1 (base)	d632ea70 (this PR)
Small table created first, big table gets the first row	ships 2 frames	BatchTooLargeForCapException
Big table created first, small table gets the first row	BatchTooLargeForCapException	ships 2 frames
After the rejection: reset(), then resend with the small table first	n/a (the batch shipped)	BatchTooLargeForCapException again, because reset() keeps the discarded batch's order

Why this is not a blocking finding:

• Neither ordering is safe, so this swaps which batches fail rather than adding a new kind of failure.
• It only happens when one batch's new-symbol bytes plus one table body exceed the server cap.

Options:

• State it in the PR body.
• Or have the split pre-flight choose the delta-carrying table by whether it fits, for example the smallest body first in delta mode. Both scenarios would then ship.

[mtopolnik]

[PR Coverage check]

😍 pass : 68 / 69 (98.55%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	19	20	95.00%
🔵	io/questdb/client/cutlass/qwp/client/QwpColumnWriter.java	8	8	100.00%
🔵	io/questdb/client/cutlass/qwp/protocol/QwpTableBuffer.java	41	41	100.00%

[jovfer]

PR #108 review: java-questdb-client (level 3, client only)

Reviewed 38087d56..ba25bf1d (main to head, 4 commits). These are the same commits reviewed on 5 Nov at d632ea70, now rebased onto #121's merge. #121's merge did not touch any of the 4 production files.

Verdict: approve with comments. Nothing blocks the merge. I'd like Moderate 1 fixed before merge.

PR metadata: fine. The title follows Conventional Commits, the labels are enhancement and QWP, and the body states the table-order change.

Moderate

1. The buffer-growth branch of the new putEncoded helper has no test.

• Problem: QwpColumnWriter.java:130-135. No test makes this helper grow the buffer, and it copies table and column names into native memory.
• Impact: if a later edit breaks that ensureCapacity, ordinary flushes write past the native buffer.
• Evidence, at ba25bf1d:
  • With line 132 (buffer.ensureCapacity(n)) deleted, the full client suite stays green: 3,826 tests.
  • An instrumented run of the same suite never once hit the grow branch.
  • A test that drives Sender.fromConfig("ws::...") does hit it.
• How a flush gets there:
  • The sender's encoder starts at 8 KB (QwpWebSocketSender.java:177, :697), and nothing sizes the buffer ahead of the table header.
  • A multi-table batch reaches the branch when the next table's header crosses the encoder buffer's current capacity. One reproduction: an 8,062-char string in table a, then a table with a 100-char name.
  • A wide schema reaches it too: about 64 or more columns with 127-char names.
  • The UDP sender reaches it only with max_datagram_size above about 8.2 KB.
• What a regression looks like: with the line deleted, small overflows produced 8,207 to 8,223-byte frames silently. The JVM then aborted with heap corruption (exit 134), and a larger overflow crashed with SIGSEGV.
• Base: the header went through putString, which checks capacity inside the writer. The raw copy is new code.
• Why it is not blocking: the code is correct today. In a test of the unmutated code, all 101 frames whose header crossed the buffer capacity came through intact. This is about protecting a 4-line helper from a future edit.
• Fix: add a test in QwpWebSocketEncoderTest that encodes at the default 8 KB, with two shapes:
  • a wide schema, comparing the header bytes against getEncodedColumnDefs();
  • a second table whose header crosses the buffer capacity after a large first table.

Minor

1. The ColumnBuffer.reset() javadoc claims more than the code does (QwpTableBuffer.java:1553).

• It says "work is proportional to what the column held".
• For a column that had nulls, the memset at :1578-1579 still clears the bitmap's full capacity, which is sized by the largest batch the column ever held.
• Head is never worse than base, which cleared every nullable column every time. But after one 1M-row batch, a per-row flusher with sparse columns still clears about 125 KB per column on every flush.
• Fix: either reword the javadoc, or clear only the used prefix as clearToEmptyFast already does (:1856-1859), using size captured before it is zeroed. The second option makes the claim true and fits the PR's goal.

2. Two table-order assertions are weak (QwpWebSocketSenderTest). Carried over from the 5 Nov review, plus one new point.

• :539-540: indexOf(frame3, "bravo") < indexOf(frame3, "alpha") is true when bravo is absent. :537 checks only tableCount == 2, so a frame carrying alpha twice would pass. Assert both indexes are >= 0 first.
• :358: the check labelled "tables ship in first-row order" uses a case where creation order and first-row order are the same. Base production code passes it.
  • The comment at :346 ("the three tables stay listed after reset()") describes something the test never observes.
  • Fix: reorder the rows so the check tells the two orders apart, or drop the claim from the assertion message.

3. The new tests re-implement shared helpers (carried over from the 5 Nov review).

• FrameCapture.tableCount (QwpWebSocketSenderTest:1112) duplicates QwpWireTestUtils.tableCount(byte[]) (:150).
• :1106 uses CloseDrainTest.buildAck instead of QwpWireTestUtils.buildAck(long) (:84).
• The 12-byte header size is written as a literal 12 at QwpWebSocketEncoderTest:1448 and QwpTableBufferTest:1784, and as a private constant at QwpWebSocketSenderTest:1065. QwpConstants.HEADER_SIZE exists, and QwpWebSocketEncoderTest already imports it statically.

Coverage map

The test gate passes. There is one gap, Moderate 1, and no Critical gaps.

• Gap row (putEncoded growth):
  • Search: an instrumented full-suite run logged every putEncoded call and found none that needed to grow the buffer. The encoders that did grow grew while writing column data, never in a header.
  • Failure link: none. Every test passes with line 132 deleted.

Mutation runs confirm that the new tests pin the pending-tables list, the null bitmap, and the column-definition cache. Each mutation below turns at least one test red:

• dropping the pendingTables.add call;
• clearing the list before a flush that throws;
• clearing it only when the store-and-forward append fails;
• dropping the isListedPending() guard;
• dropping the row-count filter in collectNonEmptyTables;
• never clearing the null bitmap;
• keeping stale encoded column definitions.

Summary

• Severity: 0 Critical, 1 Moderate, 3 Minor. All are in the diff; none is out-of-diff breakage.
• What I checked and found safe:
  • No WebSocket table can hold committed rows without being on the pending list. sendRow is the only place a row is committed, and it lists the table right after.
  • A flush that throws keeps the batch for the retry. A successful flush, or one that finds no rows, clears the list.
  • The cached names give byte-identical headers, including unpaired surrogates.
  • Skipping the bitmap and symbol-dictionary clears is safe: a null bit is only ever set while hasNulls is true.
  • The UDP sender still rebuilds column definitions once per schema change, not per datagram, and its size estimate still matches the header.
  • Nothing in OSS or Enterprise calls getAgeNanos.
• Trade-off:
  • In delta-dictionary mode, a split flush puts the whole symbol delta in the first frame, and the first frame now belongs to the first table that got a row, not the first table created.
  • So a batch where the delta plus one table's body exceeds the server cap can now be rejected where base shipped it, and the reverse also happens.
  • reset() keeps the discarded batch's order. That is no worse than base, where creation order never changes.
  • The body states the order change but not this consequence. This was raised on 5 Nov.
• Store-and-forward contract: unchanged. The diff touches no drainer, reconnect or error-surfacing code.
• Submodules: no pointer moves.
• Callers checked: every caller of a changed method in the client (WebSocket sender, UDP sender, encoder, split flush, close-time flush, sender pool), plus the OSS and Enterprise consumers of the client.
• CI: GitHub Actions are green on ba25bf1d. The Azure legs were still pending when I checked.

Mutation and probe logs are kept locally and are available on request.

🤖 Generated with Claude Code