perf(qwp): event-driven I/O loop, zero CPU when idle; producer fast paths by jerrinot · Pull Request #116 · questdb/java-questdb-client

jerrinot · 2026-10-27T12:31:28Z

Problem

A QWP sender burns ~70% of a core per connection even when idle: the I/O thread polls the socket and the segment ring every 50 µs.

Change

Event-driven I/O loop (perf(qwp): event-driven I/O loop, zero CPU when idle #114, squashed into one commit). The I/O thread blocks until the socket is readable, a producer published a frame, close() runs, or a PING is due. Producers ring a native doorbell (eventfd on Linux, kqueue EVFILT_USER on macOS/FreeBSD, a kernel Event on Windows) only when the thread is parked. Ack waiters, the segment manager and the dispatchers are event-driven too.
Producer fast paths, no behaviour change:
- one-compare capacity check in OffHeapAppendMemory; identity check first in column lookup
- ASCII fast path in OffHeapAppendMemory.putUtf8
- 8-byte word moves for native copies up to 64 bytes (Vect.memcpy)
- no frame CRC in memory-backed store-and-forward segments: they are never recovered; disk-backed segments are unchanged

No change to the Sender API or configuration.

Numbers

I/O loop: see perf(qwp): event-driven I/O loop, zero CPU when idle #114. Idle CPU 0%, flush-to-ack latency 3–15x lower, saturation +20%. Above ~25k flushes/s per connection the client uses 1.1–1.45x the CPU of main.
Fast paths: each was measured on its own against a mock server, on a longer branch: string columns +8.6% rows/s (putUtf8), 1 row/flush +8.3% (small copies) and +7.2% (CRC skip). They have not been re-measured together on this branch.

Testing

mvn -pl core test: 3748 pass on JDK 25; JDK 8 test-compile clean.
New: randomized putUtf8 against String.getBytes(UTF_8); Vect.memcpy over every length 0–160 at every source and destination alignment.

A QWP sender burned ~70% of a core per connection even when idle: the I/O thread polled the socket and the segment ring every 50 us. Kafka Connect users saw 30% CPU on QWP vs 3% on HTTP for the same quiet sink. The I/O thread now blocks until there is something to do: socket readable, a producer published a frame, close(), or a PING is due. The producer wakes it through a native doorbell (eventfd on Linux, kqueue EVFILT_USER on macOS/FreeBSD, a kernel Event on Windows), and only when the thread is parked, so a busy connection never pays a syscall per flush. Ack waiters, the segment manager and the dispatchers are event-driven as well: a waiter in awaitAckedFsn/drain is unparked on every ack advance, terminal and durability latch; the segment manager parks until a rotation, ACK or timer needs it and persists the ack watermark on a 1 ms cadence while ACKs stream; the progress dispatcher thread starts only when a handler is installed. No change to the Sender API or configuration. Idle CPU drops to 0, flush-to-ack latency falls 3-15x and saturation throughput rises ~20%. Above ~25k small flushes per second per connection the client uses 1.1-1.45x the CPU of the polling loop: the I/O thread takes two prompt wakes per flush (doorbell, then ACK). design/tla holds TLA+ models of the two wake protocols.

… in column lookup ensureCapacity() carried the realloc in the same body, which was large enough that the JIT would not inline it into every putLong/putDouble. The check is now one compare against a precomputed limit address with the realloc in grow(). lookupColumn() first compares the column name by identity, which callers passing String constants hit every row. asipc, mock server, 60M rows, 1000 rows/flush, 3 interleaved reps: instructions/row -26..-68, rows/s +1..2% across numeric/symbol/string.

Only recovery reads a frame's CRC, and a memory-backed segment is never recovered, yet every frame paid a CRC pass over all its bytes: 13% of the producer's cycles at one row per flush. The CRC field stays, zeroed, so both backings keep one frame layout; disk segments are unchanged (inverting the condition fails the recovery tests). Measured on asipc against the batching mock, producer and I/O thread pinned to CPUs 2 and 4, 5 interleaved reps, medians: 1 row/flush: rows/s +7.2%, producer instructions/row -4.4% 1000-row numeric: rows/s +2.5% against the baseline's fast mode (+22.9% against its median: the baseline runs are bimodal, 28.0-28.8M or 34.6M; this one is steady at 34.8-36.4M), instructions/row -3.2% 1000-row symbol: +4.3%; 1000-row string: +3.9%

Encode on a local cursor and leave for the general loop at the first non-ASCII char. Writing the appendAddress field per char, interleaved with Unsafe stores, kept the JIT from holding the address in a register. String column (80M rows, 1000 rows/flush, pinned): +8.6% rows/s, producer instructions -9.0%. Other shapes unchanged.

Vect.memcpy copies up to 64 bytes with 8-byte moves instead of the Unsafe.copyMemory stub, and NativeBufferWriter.putBlockOfBytes goes through it. At one row per flush each column is a few bytes, and the stub's call and dispatch cost more than the copy: 7.3% of the producer. One row per flush, pinned: +8.3% rows/s, producer instructions/row -7.5%. 1000-row shapes unchanged.

jerrinot · 2026-10-27T13:04:03Z

Review (level 3): approve with comments

Reviewed HEAD 5981feab against BASE 82b2e144. No submodule pointers moved.

Nothing blocks the merge. Two Moderate findings have runtime evidence on HEAD and base; please address #1 before merging.

Critical

None.

Moderate

1. A failed rotation followed by close leaks one segment from the pool's shared budget

Problem: A segment kept after a failed manifest update is never released from the budget.
Net impact: Pooled disk-mode senders permanently lose one segment (4 MiB by default) of sf_max_total_bytes per occurrence.
Evidence: A scratch test ran the same trigger on both revisions. After deregister + close, the budget read 65536 on HEAD and 0 on base.

Path:

The manifest write or fsync fails during a rotation (EIO/ENOSPC).
SegmentRing.appendOrFsn calls SegmentProvisioner.discard(), which puts the segment in retained. It is still charged to the budget.
The flush throws, and PooledSender.close() discards the sender.
CursorSendEngine.close calls SegmentManager.deregister, which releases only ring.totalSegmentBytes(). That covers active, sealed and pending trims, not retained.
SegmentProvisioner.close() closes the segment without releasing its charge.

SenderPool keeps one SegmentBudget for its whole lifetime (SenderPool.java:560, handed to every sender at :2086). The leak therefore lasts until the pool restarts.

Base, same trigger: the spare stayed installed as hotSpare. Base totalSegmentBytes() counted it, so deregister released it.

Limits:

It needs a real manifest I/O failure.
A successful retry on the same sender reuses the segment and restores the balance.
The liveness floor still lets a ring go one segment past the cap, so the result is earlier backpressure, not a deadlock.

Suggested fix: make SegmentRing.totalSegmentBytes() include the source's retained bytes, e.g. through SegmentSource.retainedBytes(). That keeps base's rule that deregister releases everything the ring holds. Add a discard → deregister → close test that checks budget.getSegmentBytes() goes back to its pre-register value.

Repro output (scratch test: disk mode, 64 KiB segments, shared budget, manifest fsync made to fail)

HEAD:
FZ1 budget after register=65536 ringBytes=65536
FZ1 appended=63 thrown=...MmapSegmentException: could not sync SF manifest
FZ1 budget after throw=131072 ringBytes=65536
FZ1 budget after deregister=65536
FZ1 budget after ring+manager close=65536 (pre-register=0)

BASE:
FZ1 budget after register=65536 ringBytes=65536
FZ1 appended=63 thrown=...MmapSegmentException: could not sync SF manifest
FZ1 budget after throw=131072 ringBytes=131072
FZ1 budget after deregister=0
FZ1 budget after ring+manager close=0 (pre-register=0)

2. With `sf_dir` on tmpfs, producer-thread segment creation adds about 0.4–0.7 ms to every rotation

Problem: SegmentProvisioner.next() now runs open, posix_fallocate, mmap and the fsyncs inside Sender.flush().
Net impact: Disk-mode SF with sf_dir on tmpfs. A rotation append goes from about 25 µs to 340–510 µs, and unpaced throughput drops by about 24%.
Evidence: 5 interleaved runs per revision (JDK 25, Ryzen 9950X, 4 MiB segments, 64 KiB appends). strace shows fallocate on tmpfs averaging 419 µs.

Setup	Base	HEAD
tmpfs, unpaced	3009–3042 MB/s, rotation 24 µs	2192–2443 MB/s, rotation 338–496 µs
tmpfs, 100 MB/s	0.63 ms/s in rotations	11.7 ms/s
tmpfs, 300 MB/s	1.8 ms/s	33.7 ms/s
ext4/NVMe	—	equal or better: no spare-wait stalls, lower max latency, about 1.75× unpaced throughput

Magnitude: one stall per 4 MiB of encoded data, about 1–3% of producer time at 100–300 MB/s. The stall lands on whichever flush() triggers the rotation.
Base: the manager thread pre-created a hot spare, so the producer only rebased it and updated the manifest. Nothing rejects tmpfs for sf_dir.
Optional fix: keep synchronous creation as the fallback, but have the manager pre-create a spare off the producer thread once the active segment is about ¾ full. That keeps the ext4 gain and takes the tmpfs fallocate out of flush().
Description: the PR description doesn't mention this design change. Please add it there: the hot spare is gone, segments are created on the producer thread, and the ack watermark is now written by the I/O thread on every ACK.

Minor

None.

Coverage

Test gate: passes. There is no Critical coverage gap.
Moderate gap, no tandem: the change touches store-and-forward behaviour (idle wait, reconnect replay, ack waiters), but there is no questdb or questdb-enterprise PR on perf/qwp-idle-cpu-low-risk (searched with gh pr list --head).
- The local tests and the TLA+ models cover the wake protocol.
- A real-server failover/reconnect run against the event-driven loop is missing.
- Please run the Enterprise failover suite against this branch before bumping the submodule.
CI: GitHub Actions, the macOS Azure leg and the coverage check are green. The Windows and linux-x64 Azure legs were still pending when I checked; the Windows WSAEventSelect wait code only runs there.

Summary

Verdict: approve with comments. Please address ci(build): set file encoding to utf-8 for logging #1.
Findings: 0 Critical, 2 Moderate, 0 Minor. Both are in the diff itself.
Tradeoff: idle CPU drops to zero and disk-mode throughput on ext4/NVMe goes up. In exchange, rotations are slower when sf_dir is on tmpfs (chore(conf): update JDK version from 17 to 11 and change tag to 1.0.0 #2).
Checked, no defect found:
- the I/O loop's park/publish handshake (ioParked against publishStamp)
- the doorbell's lifecycle against close
- recvDrained and TLS buffered input
- AckWaiters
- concurrent writes and syncs of the ack watermark (a torn record only causes duplicates)
- manager wakeups covering every trim case
- all Vect.memcpy callers (none depends on overlap-safe copies at 64 bytes or less)
- OffHeapAppendMemory's capacity limit staying consistent
- readers of the frame CRC that memory segments now leave at 0
- the store-and-forward invariants: no reconnect time budget, and no transport errors surfaced to the producer
The perf(qwp): event-driven I/O loop, zero CPU when idle #114 review items are fixed on this branch.

A rotation whose manifest update throws hands its new segment back to the SegmentProvisioner, still charged to the SegmentBudget, so a retry can reuse it. SegmentManager.deregister released only SegmentRing.totalSegmentBytes(), which did not count that segment, and SegmentProvisioner.close() closed it without a release. A pooled sender closed after such a flush failure therefore left one segment charged to the pool's shared budget for the life of the pool. SegmentSource gains heldBytes(), which the provisioner reports for its retained segment, and totalSegmentBytes() includes it. Deregister now releases the whole ring, as it did for the hot spare before. Close still does not release, so the charge is returned exactly once. SegmentManagerSharedBudgetTest injects a manifest fsync failure and checks that deregister returns the budget to zero, and that repeated failed rotations and the retry that finally commits charge one segment only.

Preserve event-driven I/O and ACK wakeups with tier-aware durable acknowledgements. Keep constructor entry points from both parents and cover idle local and replicated ACK waits.

Drop boolean QWP forwarding overloads, obsolete HTTP factories and polling constructors. Remove the unused progress-dispatcher capacity and migrate internal callers and tests to the current interfaces.

jerrinot · 2026-10-29T15:57:36Z

Review (level 3): approve with comments

Reviewed HEAD 084cb471 against BASE 28c84830. No submodule pointers moved.

Critical

None.

Moderate

1. With `sf_dir` on tmpfs, the flush that triggers a rotation now pays for creating the segment

Problem: in disk mode, the user's thread creates the next segment at rotation, inside flush().
Net impact: applies to disk-mode senders with sf_dir on tmpfs. The rotating append is about 16× slower, and unpaced throughput drops about 25–35%.
Evidence: a scratch probe, CursorSendEngine with 4 MiB segments and 64 KiB appends, base and head alternated, 6 runs each. Output is in the details block below.

Backing	Metric	Base	Head
tmpfs	rotating append	28–35 µs	485–570 µs
tmpfs	throughput	2300–2800 MB/s	1770–2030 MB/s
ext4/NVMe	rotating append	~31 ms	~18 ms
ext4/NVMe	throughput	~119 MB/s	~197 MB/s
memory	p50 append	21 µs	1 µs

Path: Sender.flush() → QwpWebSocketSender.sealAndSwapBuffer → CursorSendEngine.appendBlocking → SegmentRing.appendOrFsn:1061 → SegmentProvisioner.next. That method runs MmapSegment.create (preallocates 4 MiB), syncHeader and fsyncDir (SegmentProvisioner.java:169-176). In base, the manager thread built the spare ahead of time, triggered at ¾ fill. The producer's rotation then only synced the header and updated the manifest.

Who and how often:

Only senders whose SF directory is on tmpfs, such as /dev/shm or a memory-backed emptyDir. Memory mode and ext4 are faster on head.
One stall per 4 MiB published, landing on whichever flush() triggers the rotation. Real encoding caps throughput far below the probe's rate, so the share of producer time is smaller than the unpaced figures suggest.
It is a bounded cost and not blocking.

Suggested fix:

Record the design change in the description, since it becomes the squash commit message:
- the hot spare is gone;
- the producer thread now creates segments, including fsyncDir;
- the I/O thread now writes the ack watermark on every ACK.
Optionally, have the manager pre-create the next segment off-thread (for example at ¾ fill) and keep synchronous creation only as the fallback.

Probe method and raw output

Setup: scratch JUnit probe in worktrees at 28c84830 and 084cb471, same libquestdb.so, JDK 25.0.4 (Corretto), Linux x86-64. tmpfs = /dev/shm, ext4 = NVMe.
Workload: new CursorSendEngine(dir, 4 MiB) (owned manager, default MEMORY durability). 200 warmup appends, then 4000 × 64 KiB appendBlocking, with acknowledge(fsn) after each append.
topN: the mean of the 62 slowest appends, about one per rotation.
Command: mvn -q -o -pl core test -Dtest=RotationLatencyProbeTest -Dprobe.runs=3 -Dprobe.appends=4000 -Dprobe.dirs=/dev/shm/pr116probe,/tmp/pr116probe-ext4,MEMORY
Order: base, head, base, head, with 3 runs inside each.

== base iter 1
PROBE /dev/shm/pr116probe      MB/s=  2793.7 p50=    22us p99=    26us max=     52us topN(62)mean=     28us
PROBE /tmp/pr116probe-ext4     MB/s=   118.5 p50=    27us p99= 30890us max=  40949us topN(62)mean=  31666us
PROBE memory                   MB/s=  2801.4 p50=    21us p99=    31us max=    222us topN(62)mean=     34us
PROBE /dev/shm/pr116probe      MB/s=  2301.8 p50=    27us p99=    32us max=     42us topN(62)mean=     34us
PROBE /tmp/pr116probe-ext4     MB/s=   119.4 p50=    29us p99= 30806us max=  36547us topN(62)mean=  31283us
PROBE memory                   MB/s=  2799.3 p50=    21us p99=    30us max=    104us topN(62)mean=     32us
PROBE /dev/shm/pr116probe      MB/s=  2438.6 p50=    24us p99=    34us max=     50us topN(62)mean=     35us
PROBE /tmp/pr116probe-ext4     MB/s=   120.1 p50=    25us p99= 30853us max=  36363us topN(62)mean=  31302us
PROBE memory                   MB/s=  2799.1 p50=    21us p99=    33us max=     55us topN(62)mean=     38us
== head iter 1
PROBE /dev/shm/pr116probe      MB/s=  1959.8 p50=    22us p99=   502us max=    802us topN(62)mean=    570us
PROBE /tmp/pr116probe-ext4     MB/s=   199.1 p50=    25us p99= 18437us max=  24718us topN(62)mean=  18218us
PROBE memory                   MB/s= 45431.0 p50=     1us p99=    10us max=     13us topN(62)mean=     10us
PROBE /dev/shm/pr116probe      MB/s=  1874.7 p50=    27us p99=   443us max=    870us topN(62)mean=    485us
PROBE /tmp/pr116probe-ext4     MB/s=   196.0 p50=    29us p99= 18615us max=  23879us topN(62)mean=  18420us
PROBE memory                   MB/s= 54012.7 p50=     1us p99=     3us max=      7us topN(62)mean=      3us
PROBE /dev/shm/pr116probe      MB/s=  1771.5 p50=    27us p99=   448us max=    689us topN(62)mean=    489us
PROBE /tmp/pr116probe-ext4     MB/s=   195.3 p50=    29us p99= 18580us max=  22721us topN(62)mean=  18483us
PROBE memory                   MB/s= 53671.2 p50=     1us p99=     3us max=      6us topN(62)mean=      3us
== base iter 2
PROBE /dev/shm/pr116probe      MB/s=  2800.8 p50=    21us p99=    26us max=     44us topN(62)mean=     28us
PROBE /tmp/pr116probe-ext4     MB/s=   118.8 p50=    25us p99= 30874us max=  41352us topN(62)mean=  31612us
PROBE memory                   MB/s=  2798.6 p50=    21us p99=    31us max=    212us topN(62)mean=     37us
PROBE /dev/shm/pr116probe      MB/s=  2398.8 p50=    26us p99=    30us max=     49us topN(62)mean=     32us
PROBE /tmp/pr116probe-ext4     MB/s=   118.1 p50=    29us p99= 30873us max=  39855us topN(62)mean=  31737us
PROBE memory                   MB/s=  2800.2 p50=    21us p99=    31us max=     68us topN(62)mean=     36us
PROBE /dev/shm/pr116probe      MB/s=  2694.6 p50=    21us p99=    29us max=     57us topN(62)mean=     30us
PROBE /tmp/pr116probe-ext4     MB/s=   120.4 p50=    29us p99= 30695us max=  35333us topN(62)mean=  31092us
PROBE memory                   MB/s=  2810.4 p50=    21us p99=    31us max=     46us topN(62)mean=     31us
== head iter 2
PROBE /dev/shm/pr116probe      MB/s=  2030.2 p50=    22us p99=   488us max=    722us topN(62)mean=    510us
PROBE /tmp/pr116probe-ext4     MB/s=   198.9 p50=    24us p99= 18576us max=  24121us topN(62)mean=  18301us
PROBE memory                   MB/s= 46759.5 p50=     1us p99=     9us max=     15us topN(62)mean=      9us
PROBE /dev/shm/pr116probe      MB/s=  1820.5 p50=    27us p99=   512us max=    927us topN(62)mean=    553us
PROBE /tmp/pr116probe-ext4     MB/s=   195.4 p50=    29us p99= 18528us max=  21929us topN(62)mean=  18483us
PROBE memory                   MB/s= 54079.9 p50=     1us p99=     3us max=     30us topN(62)mean=      4us
PROBE /dev/shm/pr116probe      MB/s=  1811.0 p50=    27us p99=   541us max=    811us topN(62)mean=    542us
PROBE /tmp/pr116probe-ext4     MB/s=   200.3 p50=    29us p99= 18150us max=  27440us topN(62)mean=  17942us
PROBE memory                   MB/s= 52999.2 p50=     1us p99=     3us max=     16us topN(62)mean=      3us

2. `design/wake-protocol.md` doesn't match the code it describes

Problem: the new wake-protocol contract has dead references and some rows that are wrong.
Net impact: affects maintainers, who are told to keep this document as the list of every wake source.
Evidence: static, at 084cb471.
- All 17 commits it cites (459f909, 3362671, …) fail git merge-base --is-ancestor <sha> HEAD. They exist only on qwp-event-driven-io-loop and perf/client-breakthrough, so they will dangle after the squash merge.
- Its line numbers are "as of 3362671". For example, it puts ioLoop step 5 at :2838-2847; at head it is :2797-2808.
- The Consumer C table still marks recycleFailure and closed as "none … Open 1/Open 2", yet the code checks both in checkAwaitAckedFsnErrors (QwpWebSocketSender.java:1519-1528) and signals the waiters at :5863. The same table calls recycleFailure a "plain field", but it is now volatile.
- It says the fix adds ackWaiters.signal() right after closed = true. There is no such call (:1698-1700). Waiters are actually woken by the loop's close() → signalAckProgress() after it clears running (CursorWebSocketSendLoop.java:1391,1407). Behaviour is correct; only the document is wrong.

Suggested fix:

Replace the SHAs and line numbers with symbol names.
Update the two Consumer C rows.
Describe the close wake as it is actually implemented.

3. `testHandlerInstalledAfterSkippedOffersDelivers` doesn't test the wake it describes

Problem: the test passes with the wake in setHandler removed.
Net impact: nothing protects a late progress handler from never receiving the current watermark.
Evidence: in a scratch worktree at 084cb471, I replaced the setHandler guard (SenderProgressDispatcher.java:188) with if (false). The test still passed (tests="1" failures="0"). The cause is that offer(1001) at SenderProgressDispatcherTest.java:195 runs after setHandler and starts delivery by itself. No other test calls setHandler after skipped offers.

Suggested fix: don't offer after setHandler. Wait for delivery of 1000 instead.

Minor

Comments still describe the removed path scratch or the 1 ms polling:
- CursorSendEngine.java:95, 362, 371, 397, 846
- SegmentManager.java:50 ("Polls each ring on a configurable tick (default 1 ms)")
- SegmentManagerUnlinkFailureTest.java:126 ("A spare is already in place…")
SegmentProvisioner.java:169-170 now allocates on the producer thread at every disk rotation: a String.format call, string concatenation, and the String overload of MmapSegment.create (a byte[] plus a native path malloc). Base reused a native buffer (pathScratch) on the manager thread. It happens once per 4 MiB; reusing a sink would restore zero allocation.

Coverage

Test gate: passes, with 1 Moderate coverage gap.
Gap: no real-server or failover run of the new event-driven I/O loop.
- This touches the store-and-forward drainer: the idle wait, replay after reconnect, and ack waiters. That calls for a questdb matching-branch PR, and an Enterprise one for failover.
- I searched with gh pr list --head perf/qwp-idle-cpu-low-risk against both repos and found nothing. The local tests use loopback servers only.
- If it regressed: a missed wake would hold delivery until the next publish, and close() could hit its drain timeout.
- Why not blocking: the TLA+ models, CursorWebSocketSendLoopIdleWaitTest, the TLS idle tests and CI on Linux, macOS and Windows already cover the wake protocol.
- Cheapest fix, no new test code: a questdb PR that bumps the submodule to this branch, so its QWP e2e suite runs, plus the Enterprise failover suite.

Summary

Verdict: approve with comments. Please address ci(build): set file encoding to utf-8 for logging #1 (at least the description) and chore(conf): update JDK version from 17 to 11 and change tag to 1.0.0 #2.
Findings: 0 Critical, 3 Moderate, 2 Minor, all in the diff itself.
Trade-off:
- Idle CPU goes to zero.
- Memory-mode appends are much faster; the CRC skip took p50 from 21 µs to 1 µs.
- ext4 disk mode is faster too.
- The cost is slower rotations with sf_dir on tmpfs.
Previous review: its Moderate ci(build): set file encoding to utf-8 for logging #1 (budget leak after a failed rotation) is fixed by b48a0127. Its Moderate chore(conf): update JDK version from 17 to 11 and change tag to 1.0.0 #2 is still open; it is ci(build): set file encoding to utf-8 for logging #1 above.
API removals: removing the QwpWebSocketSender, CursorWebSocketSendLoop and createLineSender overloads is consistent with design/entra-id-qwp-auth.md:186 (only Sender and its builder are public API). Nothing in questdb master or questdb-enterprise main references the removed symbols.
Scope: the out-of-diff callers I checked were:
- every Vect.memcpy caller (including the one overlapping copy);
- the tryReceiveFrame and hasBufferedInput callers;
- the SegmentManager.register callers;
- the cursorEngine assignments that wire up the ack listener.
None of them is broken by this change.

jerrinot · 2026-10-29T21:46:31Z

Review (level 3): approve with comments

I reviewed HEAD fe98739f against BASE 28c84830.

No submodule pointers moved, and there are no committed binaries.
There is no tandem PR in questdb/questdb or questdb/questdb-enterprise (searched with gh pr list --head perf/qwp-idle-cpu-low-risk).
Nothing in questdb origin/master or enterprise origin/main uses the removed internal overloads. design/entra-id-qwp-auth.md:186 says these are internal factories, so removing them follows a documented decision.

Critical

None.

Moderate

1. With `sf_dir` on tmpfs, every segment rotation now stalls `flush()` for ~0.5 ms

Problem: the producer creates the next segment inside flush(). In base, a background thread created it ahead of time.
Net impact: disk-mode senders on tmpfs pay 0.4–0.7 ms extra on each rotation, which happens once per 4 MiB.
Evidence: probes on HEAD and BASE, run alternately. One set is mine; an independent falsifier ran another and also traced it with strace.

Path: SegmentRing.appendOrFsn:1061 → SegmentProvisioner.next (SegmentProvisioner.java:169-176). That method runs MmapSegment.create (including a 4 MiB fallocate), syncHeader and fsyncDir on the producer thread. strace confirms it: at HEAD every fallocate runs on the producer thread; at BASE, 15 of 18 ran on the manager thread.

The last column is how many ms per second the producer spent inside appends (BASE → HEAD), except for the unpaced row, which shows throughput.

Backing	Rotating append, BASE	Rotating append, HEAD	BASE → HEAD
tmpfs, paced at 100 MB/s	36–50 µs	640–720 µs	41–43 → 55–57 ms/s
tmpfs, paced at 300 MB/s	28–46 µs	530–610 µs	115–126 → 150–157 ms/s
tmpfs, unpaced	26–37 µs	440–600 µs	2.6–2.7 → 1.9–2.1 GB/s
ext4/NVMe	3.4–10 ms	3.3–6 ms	HEAD equal or better
memory mode	p50 18 µs	p50 1 µs	HEAD much better

Counterevidence:
- Pinning the JVM to one CPU closes most of the gap.
- ext4 and memory mode come out ahead.
- Real producers spend most of their time encoding rows, so they lose a smaller share than this raw-engine probe shows.
Severity: it is a bounded cost, paid once per 4 MiB and only on tmpfs, so it doesn't block the merge.
Fix:
- At minimum, describe the trade-off in the PR description (see build: 1.0.0 #3).
- Optionally, have the manager pre-create the next segment off-thread when the active one is about ¾ full, and keep synchronous creation as the fallback.

2. Disk rotation now allocates on the producer's `flush()` path

Problem: SegmentProvisioner.next builds the segment path with String.format and the String-based overloads.
Net impact: about 0.8 KB (JDK 25) or 2.4 KB (JDK 8) of garbage per 4 MiB rotation, on the producer thread.
Evidence: getThreadAllocatedBytes around appendBlocking:
- HEAD: 776–824 B per rotation on JDK 25, 2320–2480 B on JDK 8.
- BASE: 0 B on the producer thread.
- Non-rotating appends: 0 B in both.
- Whole process: about 0.5 KB (JDK 25) or 1.8 KB (JDK 8) more garbage per rotation than BASE. BASE allocated the rest on the manager thread.

Where it allocates:

String.format("%016x", …) and the string concatenation (:169);
the String overload of MmapSegment.create (:170);
ff.fsyncDir(String) (:174).

BASE avoided this on purpose with a reused native pathScratch. Fix: build the path in a reused DirectUtf8Sink with Numbers.appendHex and pass the pointer to MmapSegment.create, as the removed nextSparePath did.

3. The PR description, which becomes the squash commit, leaves out the structural changes

Problem: it says "no behaviour change", but several behaviours change.
Net impact: the commit message misleads maintainers.
Evidence: static comparison of the PR body with the diff.

Not mentioned:

The hot spare is gone; segments are now created on the producer thread at rotation, with the tmpfs cost from ci(build): set file encoding to utf-8 for logging #1.
The I/O thread now writes the ack watermark on every ACK; the manager only syncs it.
A truncated TLS record at EOF now reports a disconnect and reconnects (JavaTlsClientSocket.java:237); BASE returned "no data".
A progress handler installed late now receives the current watermark straight away.
The internal connect/constructor overloads are removed.

4. In `SegmentRingAckOffMonitorTest`, a failing assertion on the producer thread doesn't fail the test

Problem: SegmentRingAckOffMonitorTest.java:82 asserts inside a bare new Thread(...) lambda.
Net impact: the test never checks the rotation's return value, or whether it threw.
Evidence: I changed only the expected value from 4L to an impossible value. The log printed Exception in thread "producer" … expected:<999> but was:<4>, and the result was still Tests run: 1, Failures: 0. An independent falsifier reproduced this and found no rule or handler that would catch it.

Fix: capture the result or throwable in an AtomicReference (or use a FutureTask), and assert after join().

Minor

Comments that the hot-spare removal made stale:
- The SegmentRing.nextSeqHint() javadoc says it serves the manager's spare. Only tests use it now.
- The MmapSegment.rebaseSeq javadoc says the manager pre-creates spares.
- snapshotSealedSegments still says "per tick".
SegmentManagerTrimDeregisterRaceTest: the comments this PR edited (lines 94 and 110-126) still describe spares and say "Acknowledge does NOT wake the worker". An ACK that completes the oldest sealed segment now wakes it.
The PR has no labels.

Coverage

Test gate: passes. There are 2 Moderate coverage gaps and no Critical ones.

Gap A: no test covers the spill-buffer half of JavaTlsClientSocket.hasBufferedInput().

No test catches its removal: with unwrapOutputBuffer.position() != 0 deleted (so only unprocessedRecordPending remains), the full mvn -pl core test still passes: 3780 tests, 0 failures. An independent run got the same result.
The existing test doesn't reach that branch: at 150 KiB, TlsLargeAckIdleWaitTest never takes the spill path.
What breaks without the condition: a sweep of ACK sizes over TLS showed the I/O loop blocking with plaintext stuck in the spill buffer. awaitAckedFsn and close() then time out ("drain timed out … ackedFsn=-1").
- With the stock test server, ACKs from about 1.05 MB upward hang: 18/18 at the selected sizes.
- With a server that writes in 10 000-byte chunks, ACKs from 65 KiB upward hang: 123/200 sizes.
- Unmodified HEAD passed every one of those sizes.
Real frames that can get that large: STATUS_OK and durable-ack frames, whose size grows with the number of tables they list.
Cheapest test: a TlsLargeAckIdleWaitTest case with a ~1.1 MB ACK, or one where the server writes the 150 KiB frame in 10 000-byte chunks.

Gap B: no Enterprise tandem run for the HA/failover store-and-forward path.

The OSS real-server suites pass with this client. I ran questdb master with -P local-client on this HEAD: 383 tests, 0 failures. The suites included:
- QwpSenderE2ETest
- QwpWebSocketSenderTlsE2ETest
- QwpIngressServerRestartFuzzTest
- QwpSymbolDictRecycle*
- QwpLocalDurableAckTest
- QwpIngressDeferredCloseDurableAckTest
- QwpServerCloseDrainTest
- QwpSenderFuzzTest
Not run: Enterprise failover and the replicated durable-ack flow.
Cheapest fix: run the Enterprise QWP failover/SF suite against this branch before bumping the submodule. It needs no new test code.

Summary

Verdict: approve with comments. Please address build: 1.0.0 #3, build: 1.0.1 #4 and Gap A. For ci(build): set file encoding to utf-8 for logging #1, at least update the description.
Findings: 0 Critical, 4 Moderate, 2 Moderate coverage gaps, 3 Minor, all in the diff itself.
Trade-off:
- Gains: idle CPU drops to zero, memory-mode appends are about 15× faster at p50, and ext4 rotations are faster.
- Costs: slower rotations on tmpfs (ci(build): set file encoding to utf-8 for logging #1) and a small allocation per rotation (chore(conf): update JDK version from 17 to 11 and change tag to 1.0.0 #2).
Checked, no defect found:
- the ioParked/publishStamp handshake;
- the doorbell lifecycle against close and fd reuse;
- recvDrained;
- every AckWaiters signal source;
- the manager's wake conditions;
- moving the watermark writer to the I/O thread, including the order of engine close;
- the watermark rewrite at recovery;
- every Vect.memcpy caller, including the overlapping TLS compaction;
- the PAYLOAD_TOO_LARGE pre-check;
- the segment-size check in register;
- the dispatchers' take()/POISON shutdown.
CI: green on Linux, macOS and Windows.

mtopolnik · 2026-10-29T23:07:32Z

[PR Coverage check]

😍 pass : 435 / 457 (95.19%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegment.java	24	27	88.89%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java	23	26	88.46%
🔵	io/questdb/client/cutlass/http/client/WebSocketClient.java	26	29	89.66%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	51	57	89.47%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java	57	60	95.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/AckWaiters.java	24	25	96.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java	25	26	96.15%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentProvisioner.java	74	75	98.67%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	56	57	98.25%
🔵	io/questdb/client/std/Files.java	1	1	100.00%
🔵	io/questdb/client/network/JavaTlsClientSocket.java	4	4	100.00%
🔵	io/questdb/client/std/FilesFacade.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/protocol/OffHeapAppendMemory.java	44	44	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SenderErrorDispatcher.java	1	1	100.00%
🔵	io/questdb/client/std/Vect.java	7	7	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SenderConnectionDispatcher.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SenderProgressDispatcher.java	12	12	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentSource.java	1	1	100.00%
🔵	io/questdb/client/network/Socket.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/protocol/QwpTableBuffer.java	1	1	100.00%

jerrinot added 5 commits October 27, 2026 13:20

jerrinot mentioned this pull request Oct 27, 2026

perf(qwp): event-driven I/O loop, zero CPU when idle #114

Closed

jerrinot added 4 commits October 27, 2026 14:23

Merge origin/main into perf/qwp-idle-cpu-low-risk

e65b3b1

Preserve event-driven I/O and ACK wakeups with tier-aware durable acknowledgements. Keep constructor entry points from both parents and cover idle local and replicated ACK waits.

Remove compatibility bridges from internal QWP send loop

4ea23c1

Remove compatibility wrappers from internal sender factories

084cb47

Drop boolean QWP forwarding overloads, obsolete HTTP factories and polling constructors. Remove the unused progress-dispatcher capacity and migrate internal callers and tests to the current interfaces.

jerrinot marked this pull request as ready for review October 29, 2026 15:15

jerrinot added 2 commits October 29, 2026 16:58

Remove branch-added TLA models and design notes

584a7c2

Fix late-handler wakeup test and stale store-and-forward comments

fe98739

Reduce allocations during store-and-forward segment rotation

bc67d5d

Navigation Menu

Repository navigation

perf(qwp): event-driven I/O loop, zero CPU when idle; producer fast paths - #116

perf(qwp): event-driven I/O loop, zero CPU when idle; producer fast paths#116
jerrinot wants to merge 12 commits into
mainfrom
perf/qwp-idle-cpu-low-risk

jerrinot commented Oct 27, 2026

Uh oh!

jerrinot commented Oct 27, 2026

Uh oh!

jerrinot commented Oct 29, 2026

Uh oh!

jerrinot commented Oct 29, 2026

Uh oh!

mtopolnik commented Oct 29, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jerrinot commented Oct 27, 2026

Problem

Change

Numbers

Testing

Uh oh!

jerrinot commented Oct 27, 2026

Review (level 3): approve with comments

Critical

Moderate

1. A failed rotation followed by close leaks one segment from the pool's shared budget

2. With sf_dir on tmpfs, producer-thread segment creation adds about 0.4–0.7 ms to every rotation

Minor

Coverage

Summary

Uh oh!

jerrinot commented Oct 29, 2026

Review (level 3): approve with comments

Critical

Moderate

1. With sf_dir on tmpfs, the flush that triggers a rotation now pays for creating the segment

2. design/wake-protocol.md doesn't match the code it describes

3. testHandlerInstalledAfterSkippedOffersDelivers doesn't test the wake it describes

Minor

Coverage

Summary

Uh oh!

jerrinot commented Oct 29, 2026

Review (level 3): approve with comments

Critical

Moderate

1. With sf_dir on tmpfs, every segment rotation now stalls flush() for ~0.5 ms

2. Disk rotation now allocates on the producer's flush() path

3. The PR description, which becomes the squash commit, leaves out the structural changes

4. In SegmentRingAckOffMonitorTest, a failing assertion on the producer thread doesn't fail the test

Minor

Coverage

Summary

Uh oh!

mtopolnik commented Oct 29, 2026

[PR Coverage check]

file detail

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

2. With `sf_dir` on tmpfs, producer-thread segment creation adds about 0.4–0.7 ms to every rotation

1. With `sf_dir` on tmpfs, the flush that triggers a rotation now pays for creating the segment

2. `design/wake-protocol.md` doesn't match the code it describes

3. `testHandlerInstalledAfterSkippedOffersDelivers` doesn't test the wake it describes

1. With `sf_dir` on tmpfs, every segment rotation now stalls `flush()` for ~0.5 ms

2. Disk rotation now allocates on the producer's `flush()` path

4. In `SegmentRingAckOffMonitorTest`, a failing assertion on the producer thread doesn't fail the test