fix(qwp): keep SF slot locked until manager worker quiesces by bluestreak01 · Pull Request #67 · questdb/java-questdb-client

bluestreak01 · 2026-07-10T14:25:46Z

Tandem PRs: OSS server CI — questdb/questdb#7387 · Enterprise e2e — questdb/questdb-enterprise#1127

Summary

Fix store-and-forward shutdown so a slot is never reused while a manager or I/O worker can still touch its ring, watermark, transport, or segment files. The change also makes deferred cleanup observable by sender pools, makes close-time segment cleanup crash-safe, and keeps transient startup recovery retryable for the life of the pool.

The branch additionally adds periodic store-and-forward syncing (sf_durability=periodic with a new sf_sync_interval_millis key) for bounded-loss protection against host power loss, serializes concurrent QuestDB.close() callers through shutdown completion, hardens the recovery-time segment sort against quadratic input, and fixes an off-by-one in ObjList.remove(from, to).

This addresses the lifecycle race behind the intermittent unsafe-memory failure in QuestDB build 249990 and prevents stale workers from mutating files owned by a replacement sender.

Root cause

SegmentManager.deregister(ring) removed the ring from the live registry, but the worker could already have copied its RingEntry into a snapshot and entered serviceRing(). Close then unmapped the ring and watermark, unlinked files, and released the slot flock without proving that the in-flight pass had ended.

A replacement engine could acquire the same directory while the stale pass was still provisioning, trimming, or unlinking files. The outcomes included slot corruption, data loss after restart, and mmap/SIGBUS-style failures.

Changes

Worker-quiescence barrier

SegmentManager.serviceRing() claims each entry with an atomic registered/in-service state transition.
Entries deregistered before claim are skipped.
Deregistration of an active pass remains visible until that pass finishes.
Shared managers support a bounded, interrupt-preserving per-ring quiescence wait.
Owned managers use the stronger whole-worker stop/reap barrier.
A timed-out owned close transfers its preallocated terminal cleanup to worker exit, after the final service pass.
Ring, watermark, unlink, and flock cleanup is protected by an exactly-once CAS so retried close and worker-exit cleanup cannot race or deadlock.

Confirmed slot release and pool recovery

SlotLock.release() explicitly unlocks through platform JNI and reports whether release was confirmed.
closeCompleted and isSlotLockReleased() become true only after confirmed unlock.
The sender retains incomplete engines so a later worker/I/O exit remains observable.
Deferred completion notifies SenderPool immediately; parked borrowers do not wait for a timeout or housekeeper tick.
Retired in-range and startup-recovery slots are retained and returned to capacity after late release.
Zero-timeout and expired-budget borrows perform a final retired-slot probe before failing.

Crash-safe segment cleanup

Close persists the final acknowledged FSN through the still-mapped watermark before closing or unlinking segments.
Segment enumeration completes before deletion starts.
Fully acknowledged segments are removed in generation order and deletion stops on the first failure, leaving a safely covered contiguous residue.
The watermark is removed only after every segment is confirmed gone.
Recovery therefore skips acknowledged residue after an unlink failure or crash instead of replaying it.

Periodic store-and-forward syncing

A new durability mode, sf_durability=periodic, checkpoints mmap-published segment data to disk in the background, extending SF protection from process restarts to host power loss with a bounded loss window. It requires sf_dir and WebSocket transport.
A new ingress config key, sf_sync_interval_millis (builder: storeAndForwardSyncIntervalMillis(long)), sets the target checkpoint cadence in milliseconds. Default: 5000 in periodic mode. Accepted range: positive and at most Long.MAX_VALUE / 1_000_000 (the nanosecond-conversion bound); values outside it throw sf_sync_interval_millis is out of range.
The builder rejects the key unless sf_durability=periodic (sf_sync_interval_millis requires sf_durability=periodic) and rejects it for non-WebSocket transports.
The manager worker msyncs each ring's live published data on the cadence; a failed checkpoint is recorded for producer visibility and retried within one second.
Checkpoints use checked mmap and file-descriptor barriers; segment rotation waits until the predecessor segment is durable, and hot-spare installation syncs the segment header and parent directory.
The interval is a target, not a guarantee: JVM scheduling and storage-sync latency add to the actual loss window. request_durable_ack=on remains the option for end-to-end server durability.
The README documents the key and the periodic-durability contract.

Retryable recovery and flock release

Managed-slot startup recovery advances its cursor only after a successful attempt; transient build/connect/drain failures remain pending for a later housekeeper tick.
Failed flock releases use one shared retry driver rather than one thread per engine.
The driver applies exponential backoff from 100 ms to a 5 s cap, resets after progress, and is unparked when new work arrives.
Pool probes re-arm retry scheduling after an injected driver-start failure.

Hardened recovery segment sort

SegmentRing recovery sorts enumerated segments by unsigned baseSeq. The previous median-of-three Lomuto quicksort was O(N²) on organ-pipe, duplicate-heavy, and med3-killer orders — reachable only through corrupted-yet-parseable or operator-copied headers, but the quadratic stall landed before validation could reject the slot.
The sort is now an introsort: the median-of-three fast path is unchanged, each partition path carries a 2*floor(log2(N)) budget, and ranges that exhaust it fall back to in-place heapsort (still Long.compareUnsigned, zero allocation), capping every adversarial pattern at O(N log N) comparisons.

Bounded transport shutdown

Socket and WebSocket layers expose traffic shutdown separately from final close.
Sender close first shuts down active traffic to break a worker blocked in native send/receive, then joins the worker, then performs final socket/resource close.
POSIX and Windows native implementations provide the same shutdown contract.

Serialized QuestDB.close()

QuestDBImpl.close() set the volatile closed flag before running the pool teardown chain, so a second concurrent closer could observe closed == true and return while the first was still draining and releasing pool resources — breaking the expectation that shutdown has completed once close() returns.
close() is now synchronized: the losing caller blocks until the winner finishes teardown, then enters, sees closed, and no-ops. The teardown steps never re-enter close() on another thread, so the monitor cannot deadlock.

ObjList.remove(from, to) backing-array fix

ObjList.remove(int from, int to) filled [pos, buffer.length - 1) with null, leaving the final backing-array slot holding a stale object reference after removal. The fill now covers the whole tail, so removed elements cannot be retained through the last slot.

Attachment and observability guards

A sender cannot replace or detach an already attached cursor engine.
Test-only lifecycle hooks expose close entry, close-drain wait, worker passes, cleanup handoff, retry progress, and final release without changing production control flow.

Review follow-ups

SegmentRing.findSegmentContaining0 now binary-searches the sealed list (strictly ascending and contiguous in baseSeq) instead of linear-scanning it, so cursor repositioning on reconnect is O(log live-sealed) even under a producer-outpaces-drain backlog. Boundary-matrix guard tests in SegmentRingTest were pinned green against the pre-rewrite linear implementation.
AckWatermark now routes its lifetime mmap/munmap through the injected FilesFacade, matching MmapSegment, which makes watermark-mmap fault injection possible from test facades. AckWatermarkTest adds facade-routing and mmap-fault tests, and two SegmentManager tests replace reflective access with direct calls against the now-public open(FilesFacade, String).

Failure behavior

Safety takes precedence over capacity: if worker quiescence or slot release cannot be confirmed, the slot remains unavailable rather than being handed to a replacement. Normally, worker-exit cleanup, immediate pool notification, and the shared retry driver restore capacity once the blocking condition clears. A genuinely wedged worker or permanently failing OS unlock can keep the slot retired until process exit.

Test coverage

Deterministic tests cover:

shared- and owned-manager mid-pass close races;
stale snapshot rejection and quiescence interrupt preservation;
worker-exit cleanup handoff and exactly-once terminal cleanup;
same-slot reacquisition only after safe release;
delegated I/O-thread close and blocked native send/receive shutdown;
confirmed unlock publication and unlock-failure retry;
shared retry-driver backoff, progress reset, and start-failure recovery;
close-time unlink/enumeration failure and watermark-preserved recovery;
transient in-range and out-of-range startup-recovery retries;
immediate wakeup of parked pool borrowers after deferred release;
zero-timeout and final-timeout retired-slot probes;
periodic-sync scheduling, checkpoint-barrier failures, PERIODIC rotation gating (verified mutant-killer: re-neutralizing the gate now fails), transient-failure recovery, and multi-segment stop-on-first-failure/retry passes (SegmentManagerPeriodicSyncTest); the manifest boundary monotonic clamp, verified mutant-killer (SfManifestClampTest); sender-facing durability-latch surfacing (CursorSendEngineTest); the close-path second bounded join (SegmentManagerCloseRaceTest); torn close-time directory enumeration (CursorSendEngineClosePartialEnumerationTest); a MUTANT-marker source tripwire (SourceHygieneTest); plus config acceptance and rejection of sf_sync_interval_millis (SfFromConfigTest, WsSenderConfigHonoredTest);
adversarial recovery-sort orders (organ-pipe, all-duplicate, few-distinct, med3-killer, sign-bit keys) at N=16_384 with comparison-count bounds (SegmentRingTest);
latch-controlled two-concurrent-closer serialization of QuestDB.close() (QuestDBImplCloseTest);
final-backing-slot clearing in ObjList.remove(from, to) (ObjListTest); and
pool capacity recovery across normal, startup, shared-manager, and real worker-wedge paths.

Enterprise PR #1127 adds real-server multi-slot crash recovery, committed-but-unacked replay with WAL/DEDUP, and close-drain failover across deterministic and seeded role-change schedules.

Compatibility

The implementation retains the Java 8 language/API floor. Platform-specific slot unlock and traffic shutdown are implemented for POSIX and Windows and exercised by the tandem CI matrix.

…ing slot resources SegmentManager.close() gives up after a bounded join, but CursorSendEngine.close() could not observe the incomplete shutdown: it closed the ring and watermark, unlinked segment files and released the slot flock while the worker could still be mid service pass - able to unlink a spare/trim path inside a slot directory that a replacement engine had already re-acquired (SF data loss after restart). - serviceRing now claims the entry as in-service under the manager lock and skips entries deregistered before the pass starts - new SegmentManager.awaitRingQuiescence(ring): bounded, interrupt-preserving barrier that confirms the worker can never touch the ring/slot again - CursorSendEngine.close() releases ring/watermark/segment files/slot lock only after confirmed quiescence (or a reaped owned worker); otherwise it leaks them deliberately, logs, and allows close() to be retried - a timed-out SegmentManager.close() hands pathScratch ownership to the worker, which frees it on exit - no permanent native leak when the worker outlives the join - regression: CursorSendEngineSlotReacquisitionTest (slot retained while worker mid-pass + retry completes cleanup; same-slot reacquisition after normal close) and an awaitRingQuiescence contract test

Every production CursorSendEngine (Sender.build, BackgroundDrainer, QwpWebSocketSender.connect) owns its SegmentManager, so close() takes the manager.close() + isWorkerReaped() branch - yet all deterministic retention tests exercised the test-only shared-manager branch (awaitRingQuiescence). A regression confined to the owned path - reporting quiescence unconditionally, or isWorkerReaped() returning true while the worker is alive - would have gone green through the whole suite and silently reintroduced the SF-data-loss hazard on the only path production runs. testOwnedEngineCloseRetainsSlotWhileWorkerIsMidServicePass builds the production shape (2-arg ctor, private owned manager), waits for the initial hot-spare install so the park hook can neither be missed nor fire early, rotates onto the spare to force the worker back into an install pass, parks it there, and drives close() with a 50 ms join budget. It asserts the incomplete close stays observable (isCloseCompleted() == false), the slot flock is retained (SlotLock.acquire throws), and a retried close after worker release completes the full cleanup and frees the slot. Mutation-verified red on all three reverts: - owned branch forcing workerQuiescent = true (only this test catches it) - finally gate reverted to unconditional slotLock.close() - SegmentManager.isWorkerReaped() returning true while the worker is alive (previously zero coverage anywhere)

serviceRing's per-pass finally called lock.notifyAll() unconditionally - with the default 1 ms poll that is ~1000 wakeups/sec per registered ring on the production worker, paid for a barrier (awaitRingQuiescence) that production never takes: all three production constructions own their manager, so close() goes through manager.close()+isWorkerReaped() and never parks on the lock. - new quiescenceWaiters count, incremented/decremented around the awaitRingQuiescence wait loop under the same lock as the worker's check - no lost-wakeup window, and the timed wait remains a fallback - the per-pass finally notifies only when quiescenceWaiters > 0; in steady state it never fires - notifyAll (not notify) retained when a waiter exists: with a shared manager, distinct waiters may await different rings

… retired pool slots When an owned manager's bounded join timed out during engine close, the engine leaked ring + watermark mmaps and the slot flock until process exit, the sender latched slotLockReleased=false forever, and SenderPool retired the slot permanently (leakedSlots++) - a transiently-slow SF filesystem op at close time (> workerJoinTimeoutMillis, default 5 s) permanently ratcheted pool capacity down, even though the worker often exits moments later. - new SegmentManager.deferUntilWorkerExit(cleanup): hands an action to the worker-loop exit block, which runs strictly after the final service pass - the last point the worker can touch any slot path. Registration and the exit block's workerLoopExited flip share the manager lock, so the handoff is exactly-once: accepted while the worker is live, rejected (caller cleans up inline) once it exited - CursorSendEngine.close(): on an owned-manager join timeout, terminal cleanup (ring, watermark, drained-file unlink, flock release) now transfers to the worker's exit path instead of leaking; the quiescence gate is unchanged - the slot stays locked until the pass provably ends - exactly-once via a terminalCleanupClaimed CAS, deliberately not the engine monitor: a retried close() holds the monitor while joining the worker, and the worker cannot die until its cleanup returns - monitor-based exclusion would stall that close() for its full join budget. With the CAS the worker never blocks and the join returns as soon as the pass ends - QwpWebSocketSender.isSlotLockReleased() is now monotonic, not frozen: it re-probes the retained engine (volatile reads only, safe under the pool lock) and flips true once the deferred cleanup - worker exit path or delegated I/O-thread close - releases the flock - SenderPool re-probes retired slots (housekeeper reapIdle tick and capacity-starved borrows just before parking) and returns a recovered index to the free set: leakedSlots goes back down and a would-be borrow timeout becomes an immediate creation. A persistent non-zero leakedSlotCount() now means a genuinely wedged worker - shared-manager engines (test-only construction) keep the old leak-and-retry-close contract: their worker serves other rings and has no exit to defer to; startup-recovery retirements also stay permanent - regression: deferUntilWorkerExit contract test, owned-close handoff test (slot reacquirable after worker exit with NO close() retry), pool recovery via reapIdle and via capacity-starved borrow; the SlotLockReleasedContractTest leak-path pin updated to the new monotonic-getter contract

…ails A throw from deferUntilWorkerExit (allocation failure while building the handoff) carries no worker-liveness information, so close() must retain every worker-reachable resource instead of running terminal cleanup inline under a possibly-live worker. Add a test seam that throws from the registration path while the worker is provably mid service pass and assert the slot flock, ring and watermark are retained, close stays incomplete, and a retried close() after worker exit converges and releases the slot.

@testonly

…lease finishClose() wrote closeCompleted=true before slotLock.close(), so a pool thread could observe completion (isSlotLockReleased -> reprobeRetiredSlots), free the slot index, and admit a replacement sender whose SlotLock.acquire collided with the still-open flock fd -- a spurious "sf slot already in use" naming the process's own pid. SlotLock.close() also discarded the Files.close() result, reporting an unconfirmed release as completion. Reorder the terminal cleanup: release the flock first via the new SlotLock.release() (checks the close rc, retains the fd on failure so the lock state is never misreported), and publish closeCompleted only on a confirmed release. An unconfirmed release keeps closeCompleted false, degrading into the existing retired/leaked-slot accounting with the kernel's process-exit backstop. Add a @testonly hook between cleanup and release so the otherwise microsecond-wide window is deterministically testable; the new test parks the closer inside it and asserts completion is not observable while the flock is provably still held, then latches once released.

…estores pool capacity isSlotLockReleased() is no longer a one-shot snapshot: deferred engine cleanup on a worker/I/O-thread exit path can release the SF slot flock after close() returned. The runtime reclaim paths (discardBroken/reapIdle via reclaimSlot) already keep such slots in retiredSlots and re-probe them, but the in-range startup-recovery pass only ticked leakedSlots and dropped the recoverer, making the retirement permanent even after the release -- fatal at maxSize=1, where every later borrow timed out until process restart. Hand the retained recoverer out of drainCandidateSlotForRecovery (retainedOut replaces the flockHeld boolean) and add it to retiredSlots alongside the leakedSlots tick, so the existing reprobeRetiredSlots() drivers (capacity-starved borrow, housekeeper tick) recover the capacity once the worker finally exits. Out-of-range recoverers stay excluded: they carry no leakedSlots tick and freeSlotIndex(idx) would index past the slotInUse array.

…fore the timeout check borrow() ran the terminal timeout check before reprobeRetiredSlots(), so a zero-timeout (try-once) borrow threw without its one probe, and a borrower whose awaitNanos budget expired mid-wait timed out on capacity that a deferred engine cleanup had already returned (the delegate-side flock release never signals slotReleased). Hoist the probe above the timeout check so both paths recover the capacity instead of failing. Also pre-size retiredSlots to maxSize: every entry keeps a distinct in-range slot index reserved, so add() can never grow the backing array -- a retire (leakedSlots++ then add, under lock) can no longer fail on allocation and strand a counted-but-untracked slot that reprobeRetiredSlots() could never recover. Tests: - testZeroTimeoutBorrowProbesRetiredSlotBeforeThrowing (red pre-fix) - testParkedBorrowerGetsFinalProbeAfterBudgetExpiry (red pre-fix) - testPoolRetiresAndRecoversSlotThroughRealManagerWorkerWedge: full-stack retire/recover cycle with no forged flags -- real wedged manager worker, real timed-out close handoff, real flock release, and a re-borrow on the recovered index proving the slot dir is genuinely reusable

Deterministic regression tests for the shutdown paths the coverage review flagged as untested: - SegmentManagerCloseRaceTest#testStaleSnapshotEntrySkippedAfterDeregisterBeforeServiceClaim: a snapshot entry deregistered before the worker claims it must be skipped at claim time (serviced rings recorded from the worker's own trim-sync point via inService, so the assertion is exact -- no sleeps). - SegmentManagerCloseRaceTest#testWorkerAloneFreesPathScratchAfterTimedOutClose: after a timed-out close hands pathScratch to the worker, the worker's exit block alone must free it -- no retried close() runs in production, so the sibling test's retry-then-assert shape masked a regression here. - CursorSendEngineSlotReacquisitionTest#testTerminalCleanupRunsExactlyOnceWhenRetriedCloseRacesWorkerHandoff: a retried close() racing the worker parked mid-finishClose must lose the terminalCleanupClaimed CAS -- no double cleanup, no premature completion, flock untouched until the worker's release. - CursorSendEngineSlotReacquisitionTest#testMemoryModeOwnedCloseHandsCleanupToWorkerExit: memory-mode (null sfDir/slotLock/watermark) timed-out close must take the same worker-exit handoff without NPE and free the ring's native memory. - EngineClosePublishAfterFlockReleaseTest#testUnconfirmedFlockReleaseKeepsCloseIncomplete: a failed flock release must never publish closeCompleted, and a retried close() must neither throw nor fabricate completion. - SlotLockTest#testFailedCloseRetainsFdAndReportsFalse: release()==false retains the fd for retry and keeps reporting false while the failure persists. - SlotLockReleasedContractTest#testDelegatedIoThreadEngineCloseFlipsSlotLockReleased: the delegated-I/O-close branch (delegateEngineClose()==true) must retain the engine so isSlotLockReleased() flips true once the I/O thread's exit path releases the flock; the existing forged I/O-refusal test throws from delegateEngineClose() before the retained-engine assignment and can never reach this branch. Mutation-verified: dropping the retainedEngine assignment fails the test with the pinned message.

Release the retired slot from a test-only hook after the positive borrow wait has actually exhausted its budget. This removes the scheduler-dependent sleep and guarantees the test reaches the final post-wait probe.

…code Five doc sites documented behavior the code deliberately does not have; each invited a future 'fix' that would reintroduce the hazard the quiescence work eliminated. Comment-only change. - CursorSendEngine.closeCompleted field doc: carve the failed-flock- release case out of the retry sentence. A retried close() exits at the consumed terminalCleanupClaimed CAS and never calls SlotLock.release() again — deliberate, pinned by testUnconfirmedFlockReleaseKeepsCloseIncomplete. - CursorSendEngine.finishClose javadoc: 'must hold the engine monitor' was false for completeDeferredClose (deliberately monitor-free to avoid the join livelock). State the real contract: monitor (close path) OR worker exit path; in all cases the CAS must be won. - CursorSendEngine.isCloseCompleted javadoc: admit the third, unrecoverable state — failed flock release never flips; only process exit frees the flock. - SegmentManager.isWorkerReaped javadoc: the fall-through reap nulls workerThread while the thread may still be running deferred engine cleanups; the engine-side CAS, not this predicate, is the exclusion. - SegmentManager.serviceRing0 trim comment: narrow 'stale snapshots' to mid-pass-deregistered entries — the claim gate makes the trim block unreachable for pre-pass-deregistered entries. - SenderPool reclaimSlot/retireLease: 'retired permanently' contradicted retiredSlots.add + reprobeRetiredSlots recovery three lines down. - QwpWebSocketSender post-guard comment: the incomplete-close branch is not only the owned-manager handoff; document the no-handoff cases (shared manager, failed handoff registration, failed flock release) where the re-probe never flips.

…fix/qwp-worker-quiescence # Conflicts: # core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

…se retries Close-time segment cleanup (review finding: failed cleanup published as reusable slot): - finishClose now persists the final acked FSN through the still-mapped watermark before closing the ring and before any unlink, so any cleanup failure or crash leaves the residue covered by a current watermark - unlinkAllSegmentFiles enumerates fully first, aborts with no unlinks on a failed directory walk, removes segments in ascending generation order and stops at the first failure, so residue is always a contiguous top slice that recovery seeds as fully acked (no replay, no duplicates) - the ack watermark is removed only after every segment is confirmed gone Flock-release retry driver (review finding: unbounded retry threads): - the shared retry driver now backs off exponentially per fully-failed round (100ms base, 5s cap), resets on progress, and is unparked when a fresh engine enqueues - after an injected driver-start failure, pool probes re-arm the retry via ensureFlockReleaseRetryScheduled() from isSlotLockReleased(), so retained capacity recovers without a second explicit close() New regression tests: CursorSendEngineCloseUnlinkFailureTest (close-time unlink fault injection; successor must not see replayable frames) and three FlockReleaseRetryDriverTest cases (backoff schedule, reset-on-progress, pool-probe recovery after start failure).

Make acknowledged segment deletion crash-consistent, bound retry and cleanup paths, improve pool recovery complexity, and add deterministic lifecycle/native regression coverage.

Treat ENOTCONN and WSAENOTCONN as successful traffic shutdown because the peer may finish a graceful close before the owner cancels the I/O thread. Preserve all other failures and descriptor ownership. Add real loopback coverage for peer-first close and a synthetic failure case that verifies non-benign shutdown errors remain visible.

QuestDBImpl.close() set the volatile `closed` flag before running the pool teardown chain, so a second concurrent close() caller could observe closed==true and return while the first caller was still draining and releasing pool resources -- a premature return that breaks the AutoCloseable expectation that shutdown has completed once close() returns. Make close() synchronized so the losing caller blocks on the monitor until the winner finishes teardown, then enters, sees `closed`, and no-ops. A bare CAS is insufficient: the losing caller would still return early. Adds a latch-controlled two-closer regression test (QuestDBImplCloseTest) that is red without the fix.

CursorWebSocketSendLoop.close() did an untimed shutdownLatch.await() while the I/O thread could be blocked in an in-flight foreground connect (connect_timeout=0 => OS SYN-retry ~60-130s). The connecting WebSocketClient is a walk-local in QwpWebSocketSender.connectWalk, invisible to close() (the `client` field is null on async-initial connect / stale on reconnect), so closeTraffic() could not reach it and close() hung -- risking Sender.close() exceeding the sidecar's 120s deadline. Publish a race-safe per-loop cancellation handle (ConnectCancellation) through the transport seam: connectWalk publishes the client it is about to block on before connect(); close() calls closeTraffic() on it to unwind a black-holed connect. The ReconnectFactory seam gains a Java-8 `default reconnect(ConnectCancellation)`, so every existing implementor stays source/binary compatible. Add a bounded backstop: close() awaits the shutdown latch for at most DEFAULT_CLOSE_SHUTDOWN_AWAIT_MILLIS (30s, ~4x under the sidecar deadline) and, on timeout, runs the same loud failed-stop protocol as the interrupt branch (delegates final teardown to the I/O thread's exit path, frees nothing under the live worker) -- so close() returns bounded even in the rare TOCTOU window where cancellation is a no-op. Also clear the in-flight handle on connect-failure paths so it never dangles at a disposed client. Adds CursorWebSocketSendLoopConnectPhaseCloseTest (async-initial + mid-flight cancellation, plus bounded-backstop-without-cancellation).

…anifest SF crash recovery previously failed OPEN on operational errors: - findFirst() < 0 (permission/transient enumeration error) was treated as an empty directory, so the engine started fresh and openCleanRW(O_TRUNC) truncated the existing durable log (sf-initial.sfa) and deleted the ack watermark; - findNext() < 0 silently accepted an incomplete directory listing; - per-file openRW/mmap failures (EMFILE, ENOMEM, transient I/O) on valid segments were swallowed by the same catch as data corruption and skipped; - the FSN contiguity check only compares adjacent pairs, so a skipped LEADING segment silently dropped unacked rows (ackedFsn seeded past them) and a skipped TRAILING segment silently re-issued its FSNs to new payloads (overlapping FSNs on disk). Recovery now fails closed and proves slot integrity before mutating: - Enumeration errors (findFirst/findNext < 0) abort startup. - Operational open/mmap failures on recognized segments abort startup; only positively-identified corruption (new MmapSegmentCorruptionException: bad magic, sub-header size, negative baseSeq, unreadable header page) is skippable, and quarantine renames (<name>.corrupt) are deferred until the surviving chain validates, so a failed recovery mutates nothing. Unsupported-version segments stay fatal (they belong to another client build; renaming them would strand that build's frames). - sf-manifest.bin: dual-slot, CRC-protected, generation-versioned boundary record (headBase/activeBase) written on rotation and ack-driven trim. Recovery validates the chain against the committed boundaries, which catches missing leading/trailing segments that contiguity alone cannot. Segments carry a manifest-required header flag so a deleted manifest next to migrated segments fails closed. Boundary updates are monotonic (clamped) and serialized with trim under the ring monitor; the trim path recomputes the successor under that monitor so a concurrent rotation can never make the head leapfrog a still-unacked sealed segment. - Segment files are created with O_EXCL (new Files.openRWExclusive natives) instead of O_TRUNC, so no code path can truncate an existing log. - Crash-window protocol, verified state by state: fresh start orders initial-segment durability -> manifest create -> flag stamp; rotation syncs the promoted spare's rebased header before the manifest references it and commits the manifest before any ring mutation; clean-drain close durably collapses boundaries to head==active before unlinking and removes the manifest last. Every window either recovers (equivalent empties from a fresh-start kill, an empty active at the chain end after a rotation crash, record-less manifest creation debris, drain-window survivors, stale below-head files) or fails closed with intact evidence. The one deliberate mutation on a failing path is quarantining a provably record-less manifest (it contains no boundaries by construction; flagged segments still fail closed afterwards). Regression tests (SegmentRecoveryIntegrityTest, 24 cases) cover the full required matrix: findFirst=-1, partial findNext=-1, openRW=-1, mmap failure -- each asserting byte-for-byte slot preservation; the engine-level O_TRUNC reproduction; leading/trailing/interior boundary loss; corrupt stray quarantine with sibling recovery; deferred-quarantine no-mutation on failure; flagged-segments-without-manifest; manifest creation-crash debris self-heal (zero-byte and record-less); drain-crash spare survivor; corrupt-active-with-empty-stand-in fail-closed; dual-slot generation selection and torn-record fallback. Independently reviewed for crash-consistency holes, lock ordering (manager lock -> ring monitor -> manifest monitor, acyclic), unacked-frame loss, and fd/mmap leaks; both review blockers (creation-debris brick, drain-window spare brick) are fixed and regression-tested. Windows native openRWExclusive0 uses CREATE_NEW; CI rebuilds platform binaries from the .c change.

sortByBaseSeq was a median-of-three Lomuto quicksort. Median-of-three covers the readdir orders a healthy slot produces (lexicographic enumeration yields already-sorted baseSeqs, hashed directory order is effectively random), but Lomuto partitioning is O(N^2) on organ-pipe, duplicate-heavy and median-of-three-killer orders. Exact simulation at the documented 16K-segment ceiling: 22.6M comparisons for organ-pipe, 50.7M for Musser's med3-killer permutation, 134M (full N^2/2) for mass-duplicate baseSeqs -- versus ~221K on the healthy paths. Such orders are only reachable through corrupted-yet-parseable or operator-copied headers, and recovery validation rejects those slots -- but the quadratic stall lands BEFORE validation gets to reject them. The sort is now an introsort: the median-of-three fast path is unchanged, and each root-to-leaf partition path carries a budget of 2*floor(log2(N)) passes. Loop-on-larger iterations count against the budget, so bad-split chains cannot hide in the loop. Ranges that exhaust it fall back to in-place heapsort (still Long.compareUnsigned, zero allocation), capping every adversarial pattern at ~3.8*N*log2(N) comparisons (organ-pipe 773K, duplicates 507K, med3-killer 861K at N=16384). Recursion depth stays under log2(N). The sortComparisons counter ticks +2 per sift-down level so it remains a strict upper bound in the fallback. New adversarial test drives the sort directly with in-memory segments at N=16384 across organ-pipe, all-duplicate, few-distinct, med3-killer and sign-bit-key patterns, asserting unsigned order plus comparisons < 8*N*log2(N) -- >2x headroom over the worst measured pattern, ~12x below the mildest quadratic blow-up. The existing sorted-input regression test is unchanged and still passes.

Protect the durable ACK watermark with redundant CRC records so torn writes cannot skip unacknowledged frames after restart. Bound sender and query pool shutdown waits while retaining late cleanup ownership, make segment recovery cleanup linear and ownership-safe, and count only genuine reconnect sends as replay telemetry. Add deterministic Java 8 regression coverage for crash recovery, pool shutdown, segment-count complexity, cleanup failures, and reconnect metrics.

EngineClosePublishAfterFlockReleaseTest started the shared retry driver after injecting an invalid slot-lock descriptor. It then returned before the driver's initial park expired. A following test could observe the global driver as active and fail, as Windows CI did. Inject a synchronous retry-driver start failure for this explicit-close retry test. Restore the factory in a nested finally block. Dedicated retry-driver tests continue to cover the asynchronous lifecycle.

Direct SenderPool instances now continue store-and-forward recovery after a transient failure or startup budget exhaustion. The recovery driver uses elapsed-time budget accounting, throttles retries, and shuts down before delegate teardown. Guard driver startup so constructor failures quiesce the driver and close prewarmed delegates. Add deterministic recovery, lifecycle, cleanup, and mutation-sensitive tests.

bluestreak01 · 2026-07-20T15:12:24Z

Scope note: This is the tandem PR review of Enterprise questdb-enterprise#1127 + OSS questdb/questdb#7387, posted here because this client PR (java-questdb-client#67) is the third leg of the tandem and its revision (9c9e1ddbda9a) is what those two PRs pin. The client change was verified for store-and-forward-contract compliance in that context (result below) but was not given a full standalone line-by-line review — that belongs to this PR's own review. The findings below are almost entirely on the ENT/OSS side; the client-relevant result is the SF-contract clearance in the header and the "Downgraded" section.

Client SF-contract clearance: the bumped revision keeps the steady-state SF drainer reconnect unbounded — reconnectMaxDurationMillis is explicitly "NOT consulted by the background loop: Invariant B removed the wall-clock give-up from connectLoop" and only bounds the non-lazy initial connect (which the SF contract permits); per-attempt backoff is capped while the retry loop stays unbounded. No reconnect time budget, no hard-fail on transient outage, and no watermark-advance-past-NACK was observed in the drainer path. Compliant.

Review — QWP multi-slot recovery & close-drain failover (tandem)

Tandem review of Enterprise #1127 + OSS questdb/questdb#7387 (client #67 SF-contract-checked). Full mission-critical pass: 14 review dimensions across independent fresh-context reviewers (Rust N/A — no .rs); every load-bearing finding re-verified against source.

What this PR is

An e2e test PR (ENT, ~3.5k lines of harness/tests) plus a submodule bump carrying one OSS server behavioral change: gracefulCloseAndDisconnect → gracefulCloseAndDrain, a bounded post-CLOSE read-drain so a server-initiated fatal WebSocket CLOSE lingers (FIN + keep reading/discarding) instead of closing the fd under in-flight client bytes. An fd close under in-flight bytes forces a TCP RST that destroys the final durable ACK in the peer's recv queue → SF client replays committed-but-unacked batches → duplicates on non-DEDUP tables.

Tandem chain verified consistent (despite stale descriptions — see Minor): OSS #7387 head 1898c786a6 == ENT-pinned questdb@1898c786a6 → java-questdb-client@9c9e1ddbda9a == OSS transitive bump. Client SF-contract sanity: the steady-state drainer reconnect is unbounded (reconnectMaxDurationMillis is "NOT consulted by the background loop"; it only bounds the non-lazy initial connect — permitted); per-attempt backoff capped, retry loop unbounded — compliant with the store-and-forward invariants.

Critical

None. The production change was deep-traced end-to-end and confirmed correct; its fix ships with an effective regression test. No confirmed correctness, data-integrity, resource, concurrency, or performance/IO defect on a reachable production path remains open. (One "Blocker" candidate was raised and dropped after verification — see Downgraded.)

Moderate

Test-efficacy / coverage-clarity items (they do not touch production correctness/perf). All are cheap to fix in this PR.

M1 — [ENT] test_close_drain_failover.py is not a regression guard for the RST-destroys-ACK bug (misattributed coverage). The test withholds the 40-row backlog from node A via paused forward gates before demote (its own docstring: "forward gates retain the backlog before both endpoints, then A demotes while those bytes cannot reach it") and durably-acks the 30-row baseline beforehand. So A holds no committed-but-unacked data at demote — there is no final durable ACK for an RST to destroy. Reverting the production hunk (immediate RST) yields the identical dense [0,70) on B (client reconnects, replays from ackedFsn+1); the close_pending/ordering assertions are also insensitive to RST-vs-clean-CLOSE. It is a valid, well-built test of the close-rides-failover choreography and should be kept. Fix: either (a) add a deterministic variant where the backlog is durably committed on A before demote with the final durable-ACK withheld by a paused reverse gate — a reverted RST then replays committed rows and breaks the dense oracle on the non-DEDUP table; or (b) adjust the docstring so it isn't represented as the RST-fix witness and cite the actual guards (OSS QwpServerCloseDrainTest + pre-existing ENT SqlFailoverQwpDeferredCloseExactlyOnceTest). The RST fix itself IS covered (see Coverage map) — this is an attribution gap, not an uncovered bug.

M2 — [OSS] Peer-close fast-exit (recv < 0) is executed but never asserted. In QwpIngressUpgradeProcessor.resumeRecv drain branch, the drained < 0 → ServerDisconnectException path (the common conformant-client exit) is only implicitly exercised at QwpServerCloseDrainTest.testFatalCloseLingersWhileStreamingPeerDrainsItsReceiveQueue socket-close teardown; nothing asserts the server disconnects on the peer FIN. A regression (fall-through to would-block/park) would surface only non-deterministically via assertMemoryLeak/pool halt. Fix: after closing the client socket, assert the server tears the connection down promptly (poll connection count → 0, or that a fresh connect succeeds, within a bounded window).

M3 — [OSS] The SEND_STATE_RESUME_CLOSE send-drain path is UNTESTED. Arming the drain from a partially-flushed CLOSE frame (resumeSend RESUME_CLOSE → gracefulCloseAndDrain) needs a PeerIsSlow under a small send-fragmentation cap; QwpServerCloseDrainTest uses default large send buffers, so the CLOSE always leaves via the READY path. This is also the only path where the drainBufferedFrames !isCloseDraining() guard matters. Fix: add an e2e case forcing getForceSendFragmentationChunkSize() below the CLOSE-frame size (as QwpUpgradeRejectFragmentationTest already does for handshake rejects) to exercise resume-close arming + buffered-frame discard.

Minor

[OSS] resumeRecv drain loop does not re-check the deadline in-loop (optional hardening). The while ((drained = socket.recv(...)) > 0) discards to would-block and checks isCloseDrainExpired() only on re-entry. This is the same loop shape as the pre-existing HttpConnectionContext.drainRecvBuffer() and the new code yields to the dispatcher between read events, so it is self-limiting — but a per-N-iteration isCloseDrainExpired() recheck + bounded chunk would make the 5s budget a hard guarantee even against a continuous within-invocation flood. Not a regression.
[OSS] Stale javadoc on sendDeferredFatalClose: still says "half-closes the write side and raises ServerDisconnect" — it now calls gracefulCloseAndDrain and returns into the read-drain. Update the one-liner.
[ENT] QwpSidecarMain.connectPool cleanup NPEs on null senders: the catch does for (Sender s : senders) before db.close(); if new Sender[count] itself throws (pathological CONNECT_POOL <n> / OOM), senders is null → NPE masks the real error and leaks the connected QuestDB facade, defeating the method's own comment. Test-only + unreachable in practice (count validated >=1, test-controlled). Guard with if (senders != null) { … } so db.close() always runs.
[OSS] closeDrainDeadline defensive reset: reset only in onDisconnected() (safe under current teardown wiring; mirrors the shipped roleChangeCloseDeferredDeadline pattern). A defensive reset in of() would harden against future teardown-path changes that could otherwise start a reused pooled context with isCloseDraining()==true.
[BOTH] Stale submodule commit hashes in both PR descriptions — ENT body cites questdb→d078ef13/client→a167e650; OSS body cites ENT pins cfece81562/client→fee9903ca6fc; the actual final pins are 1898c786a6/9c9e1ddbda9a. Descriptions-only; refresh before merge so provenance verification isn't misled.
[ENT] Scope creep: the PR adds .pi/skills/fix-pr/SKILL.md (430 lines of agent tooling) + .gitignore .pi-subagents/, unrelated to the QWP change. Flag for author decision (separate tooling PR / personal config) unless the team has decided to vendor .pi/skills/.
[OSS] Duplicated test helpers: QwpServerCloseDrainTest.createMaskedFrame/performWebSocketHandshake are byte-for-byte copies of the same helpers in QwpWebSocketProtocolTest (both extend AbstractQwpWebSocketTest). Hoist into the shared base to avoid framing-fix drift.
[ENT] ReverseTrafficGate._pause() has no timeout — waits unbounded for an in-flight sendall on a backpressured peer; a misuse surfaces as a whole-suite hang rather than a clean assertion. [ENT] QwpSidecarMain.Client methods not alphabetical within kind (close/closeWithWitness/closeQuietly) — auto-sort reconciles.

Downgraded (false positives / refuted after source verification)

"Blocker: post-CLOSE drain loop pins an IO worker indefinitely / DoS / regression." Refuted. (1) The unbounded while (socket.recv(recvBuffer, recvBufferSize) > 0) is not new — the pre-PR gracefulCloseAndDisconnect ran the identical loop via context.drainRecvBuffer(), so no regression in loop shape. (2) The new code yields between read events: on would-block it returns → handleProtocolSwitchedRecv throws registerDispatcherRead(), so the worker services other connections between drain reads — it does not monopolize across the 5s. (3) Non-blocking recv reaches would-block far faster than any real network delivers (drain = memcpy ≫ link rate); the legit demote-time streamer stops and closes within ms of observing the CLOSE (→ recv<0 exit). The "re-check deadline in-loop" idea survives as a Minor hardening, not a blocker.
"CLOSE/CLOSE_BEGIN on null client now replies ERR (was OK)" — intentional test-harness behavior change, no production impact.
OSS "formatting" files carry hidden behavior — refuted: ColumnType, Micros/Dates/Nanos, CharSink, FunctionParser, SqlCodeGenerator, Concurrent*HashMap, etc. (16 files) are pure nested-ternary reindentation — every changed line differs from its pair only in leading whitespace. EXEMPT.
Client bump imposes a reconnect budget on the SF drainer — refuted: reconnectMaxDurationMillis is explicitly "NOT consulted by the background loop" (bounds only the non-lazy initial connect, which the SF contract permits).

Coverage map (OSS QWP close-drain — the only production behavioral change)

#	Branch / path	Test	Fails on regression?	Verdict
1	drain deadline expiry → disconnect	e2e `testCloseDrainDeadlineBoundsLingerAgainstLiveWriter` (`cutOffMillis>=3000 && <=20000`) + unit `testCloseDrainLifecycle`	Yes — revert (immediate RST) → cutoff ≈0–300ms fails `>=3000`	TESTED
2	`recv>0` discard loop (linger, no RST)	e2e `testFatalCloseLingersWhileStreamingPeerDrainsItsReceiveQueue`	Yes — revert → RST → follow-up write throws	TESTED
3	`recv==0` park → re-register read	e2e tests 1 & 2 (idle gaps)	implicit only	weak
4	`recv<0` peer-close → disconnect	e2e test 2 teardown	not asserted	UNTESTED (M2)
5	send-path RESUME_CLOSE → drain arm	—	—	UNTESTED (M3)
6	`sendFatalClose` arming	e2e tests 1 & 2 (TEXT reject → CLOSE 1003)	Yes	TESTED
7	deferred/durable-ACK-then-close arming (demote)	pre-existing ENT `SqlFailoverQwpDeferredCloseExactlyOnceTest`	Yes	TESTED (out-of-diff)
8	`drainBufferedFrames` skip guard	—	implicit	untested directly
9	`processWebSocketFrames` discard-rest	—	implicit	untested directly
10	`beginCloseDrain` idempotency	unit `testCloseDrainLifecycle` (deadline anchored at origin → expires at `now==TIMEOUT`)	Yes	TESTED
11	`onDisconnected` reset	unit `testCloseDrainLifecycle`	Yes	TESTED
12	`closeDrainDeadline` survives `clear()`/`clearMessageState()`	unit `testCloseDrainLifecycle`	Yes	TESTED
—	16 OSS non-QWP files	—	pure reindentation, verified no behavior	EXEMPT

Summary

Verdict — ENT #1127: approve · OSS #7387: approve, with the Moderate coverage clarifications (M1–M3) and Minors recommended for THIS PR (all cheap; not deferred).

Correctness & performance gate: PASS. The OSS close-drain change was traced end-to-end (recv semantics; recv- and send-path re-registration; state survival across reset()/clear(); pooled-reuse reset via onConnectionClosed→onDisconnected; idempotent arming; all fatal-close paths arm the drain incl. the demote durable-ACK path; FIN-after-CLOSE ordering; ~zero added steady-state cost). No open production defect. The single "Blocker" was refuted (pre-existing loop shape + yields between events + self-limiting).
Test gate: PASS. The fix ships with a regression test whose failure link is verified by revert-reasoning: OSS QwpServerCloseDrainTest fails pre-fix (immediate RST → cutoff <3000 / IOException). The state machine has a strong non-vacuous unit test. The ENT e2e suite complies with the SF store-and-forward test-application checklist (no producer-visible role errors as write-gate evidence; replay-aware oracles via withhold-from-A construction and a DEDUP table; acknowledged-boundary ordering, not timing).
Findings: ~14 draft findings; 1 major false-positive dropped (drain-loop Blocker) + 3 other refutations. Split: 0 Critical · 3 Moderate · ~8 Minor. Essentially all in-diff; the drain-loop shape is pre-existing (drainRecvBuffer) and the demote regression witness (row 7) is a pre-existing out-of-diff test. Cross-context (egress processor, framework re-registration, pooled-context reuse, client SF drainer) all cleared.
Highest-value residual: M1 — attribute the RST-fix regression coverage correctly (OSS QwpServerCloseDrainTest + pre-existing SqlFailoverQwpDeferredCloseExactlyOnceTest), and consider the committed-on-A-before-demote variant so the ENT deterministic test also discriminates the fix.

_{🤖 Generated with tandem PR review (level 3).}

On POSIX, shutdown(SHUT_RDWR) unblocks a recv() already in progress on another thread, which is how closeTraffic() wakes a worker parked in recv without releasing the fd. Winsock's shutdown(SD_BOTH) does not do this: an already-blocked recv() stays parked until data arrives, the peer resets, or the socket is closed. That left the QWP I/O worker stuck and failed SocketTrafficShutdownTest.testPlainSocketShutdownWakesWindowsRecvAndRetainsFd on the windows-msvc-2022-x64 job. Cancel outstanding I/O on the socket handle with CancelIoEx before the shutdown(). This wakes the blocked recv() while leaving the descriptor allocated, so the fd is retained (no reuse race) until close() releases it, matching the POSIX/macOS behaviour the sibling tests already rely on. Net.shutdown() is only reached via PlainSocket.closeTraffic(), so no other caller is affected.

…ger.close() The bounded-join fallback in close() reaped the manager worker (workerThread=null => isWorkerReaped()) as soon as it observed workerLoopExited, which the worker publishes at the START of its exit block -- BEFORE it runs the deferred exit cleanups (the owning engine's finishClose(), which releases the slot flock and only then publishes closeCompleted). When the bounded join timed out mid-finishClose, close() reaped while the flock release was still in flight, so a caller reading isCloseCompleted() saw a stale false and a spurious flock-release retry could be scheduled. On the slow JDK 8 CI leg this raced SenderPoolSfTest.testPreallocatedExitHandoff* to failure ("worker exit must complete startup-recoverer cleanup without a sender or engine close retry"), and the leaked shared retry driver then cascaded into the FlockReleaseRetryDriverTest suite. Once workerLoopExited is true the worker has left its (possibly wedged) service loop and is running only finite exit cleanups, so give it a second bounded join to terminate before reaping. This reuses the same join-under-monitor pattern as the first join -- the worker uses a lock-free CAS for exactly-once cleanup and never blocks on the manager monitor -- and still reaps on timeout, so a pathologically slow cleanup cannot hang close(). The second join is budgeted with the fixed WORKER_JOIN_TIMEOUT_MILLIS, not the tunable workerJoinTimeoutMillis: the first join bounds a possibly-wedged service pass (tests shrink it to force the timed-out path), but the finite exit cleanups must be allowed to finish regardless. In production both values are equal. Validated under JDK 8: a deterministic repro (400ms finishClose delay) reproduces the exact CI assertion without this change and passes with it; the full previously-failing set (SenderPoolSfTest, FlockReleaseRetryDriverTest, EngineClosePublishAfterFlockReleaseTest, CursorWebSocketSendLoopRotationRaceTest) is green.

…open() In the no-valid-record branch, SfManifest.open() closed the fd and then called quarantineDebris(), which throws when both rename and remove fail (e.g. a permission-degraded slot dir). The enclosing catch then closed the same fd a second time. With concurrent threads opening files, the OS can reuse the fd number in that window, so the second close could kill an unrelated live descriptor and silently corrupt whatever it backed. Mark ownership released (fd = -1) after the first close and guard the catch, so the fd is closed exactly once on every path. Adds a SegmentRecoveryIntegrityTest case: correctly-sized manifest with both records CRC-broken and a facade failing rename+remove asserts the manifest fd is closed exactly once (fails with 2 closes pre-fix).

…Append The append hot path computed the frame CRC-32C with two Crc32c.update JNI calls -- one over the u32 payloadLen field, one over the payload -- even though the layout [u32 crc][u32 payloadLen][payload] places the payload immediately after payloadLen (at lenAddr+4). The two CRC'd regions are physically contiguous, so a single update over 4 + payloadLen bytes yields the byte-identical CRC while crossing the JNI boundary once per frame instead of twice. This is the same form the recovery scanner already recomputes (reader.crc32c(pos + 4, 4 + payloadLen) in scanForRecovery), which validates every on-disk frame against the producer-written CRC, so the fused value is guaranteed identical and the round-trip is unchanged. No behavior change, no change to hashed or written bytes; saves one native-call dispatch per non-empty frame on the producer's zero-alloc hot path. Verified by MmapSegmentTest and MmapSegmentRecoveryFaultTest.

FlushFileBuffers is documented for file/volume handles; on NTFS it refuses a directory handle, commonly returning ERROR_ACCESS_DENIED (or ERROR_INVALID_FUNCTION on filesystems that do not implement it), and a GENERIC_WRITE open of a directory can itself be refused by ACLs. Because fsyncDir0 failures are fatal to their callers -- SfManifest.create, SlotLock.acquire (reached whenever sf_sync_interval_millis > 0), and the SegmentManager hot-spare/trim barriers -- SF disk mode with sf_durability=periodic could hard-fail to start on Windows, a Windows-only regression on the durability path. Treat the documented "directory cannot be flushed" signatures (ERROR_ACCESS_DENIED, ERROR_INVALID_FUNCTION), and an ACL-refused GENERIC_WRITE open, as best-effort success: create/rename/unlink of directory entries are made crash consistent by NTFS metadata journaling ($LogFile). This mirrors libgit2/PostgreSQL/SQLite, which do not rely on directory fsync on Windows. Genuine I/O errors, and a missing directory, still propagate as fatal. File-data fsync (Files_fsync) is unchanged.

…ntaining0 findSegmentContaining0 walked sealedSegments[sealedHead..size) front to back to find the segment covering an fsn: O(live-sealed) per call, and the live window is unbounded when the producer outpaces the drain. The callers are cursor (re)positioning on reconnect (findSegmentContaining / pinSegmentContaining via CursorSendEngine), so the cost was per reconnect, not per row -- but it scaled with exactly the backlog a reconnect storm tends to coincide with. The sealed list is strictly ascending and contiguous in baseSeq on every mutation path: rotation rebases the promoted spare to previous.baseSeq() + previous.frameCount() before sealing, recovery sorts by unsigned baseSeq and rejects any gap via validateContiguous, and all removals are prefix-only. The only candidate that can contain fsn is therefore the rightmost sealed segment with baseSeq <= fsn. Binary-search for it (unsigned compare, matching sortByBaseSeq's key order) and re-check that single candidate with the original containment predicate; the active-segment fallback is unchanged. O(log live-sealed), zero allocation, same nulls/misses on every input including negative fsn, empty window, and post-trim sealedHead > 0. Adds two SegmentRingTest guards pinning the full boundary matrix (segment bases, interiors, last fsns, below/above range, single-frame segments, post-trim windows); both were run green against the pre-fix linear implementation first, so the rewrite is provably semantics-preserving.

…cade AckWatermark.open(FilesFacade, ...) routed exists/length/openRW/ allocate/close/msync/fsync through the facade but mapped the watermark with static Files.mmap and released it with static Files.munmap. Harmless in production -- the facade defaults delegate verbatim -- but inconsistent with MmapSegment, which routes mmap/munmap through the facade, and it made watermark-mmap fault injection impossible from a test facade: the injected override was silently bypassed on the one mapping that lives for the watermark's whole lifetime. Route both calls through the facade. The instance already stores the mapping facade in a final field used by sync() and releaseStorage(), so map and unmap always go through the same facade on every path, and the error-cleanup ordering (mmap failure closes the fd via the facade; release unmaps then closes exactly once) is unchanged. Widen open(FilesFacade, String) to public, matching the established facade-factory precedent (MmapSegment.create/openExisting, SegmentRing.openExisting) in the same exported package. This lets SegmentManagerCrashConsistencyTest and SegmentManagerManifestFsyncTest drop their reflective Method lookups for compile-time-checked calls. Adds AckWatermarkTest coverage with a delegating facade: mapping and release must be observed by the injected facade, and a facade that rejects mmap must fail open() with the fd closed and no mapping left. Both tests fail pre-fix (the static call bypassed the facade), pass post-fix.

Recovery resumed appending at lastGood but left the stale bytes in [lastGood, fileSize) on disk. Two ordinary crashes could then brick the client permanently: crash #1 tears the active near its end; recovery resumes and the session fills the segment, but the last frame stops short of the old residue, so rotation reseals the recovered file with a non-zero suffix; recovery #2 correctly refuses non-zero sealed suffixes ("corrupt torn tail in sealed SF segment") -- on every startup, since the plain exception is not quarantined. Primary slot: Sender.build() fails forever; orphan slot: the drainer drops a permanent .failed sentinel and the unacked data is silently stranded. Sibling hazard: the frame envelope binds neither position nor FSN, so a byte-aligned stale frame with a valid CRC past the tear could be resurrected at a recycled FSN by a later scan (stale replay). openExisting now zeroes the residue right after the scan validates and makes the zeroes durable with an msync+fsync barrier before the segment is returned, restoring the invariant fresh segments already have: all bytes past the append cursor are zero. The barrier is load-bearing in MEMORY durability mode, where rotation does not sync the sealed predecessor's data pages. A failed barrier aborts recovery fail-closed. tornTailBytes still reports the pre-sanitization observation and the sealed-suffix check in SegmentRing stays fail-closed on first sight. Regression tests: residue zeroed on disk; two-crash reseal at segment level; stale-frame non-resurrection; barrier-failure abort; end-to-end two-crash ring recovery (torn -> recover -> fill+rotate -> recover).

Commit 88d6b79 accidentally captured a concurrent tooling edit that neutralized requestSyncBeforeRotation to 'return false' (marked 'MUTANT: gate neutralized'): a mutation-testing session shared this worktree and injected the edit between test validation and git add. The neutralized gate allowed rotation in PERIODIC durability mode to proceed without requesting a sync for a predecessor whose published range was not yet durable. This restores the original logic byte-for-byte; the worktree now matches exactly the change set the 369-test sf suite validated. SegmentManagerPeriodicSyncTest, SegmentManagerCrashConsistencyTest, SegmentRingTest and MmapSegmentTest re-run green on the restoration.

@testonly

… win32 errno at failure point Review-finding fixes: - C1: SegmentManager.servicePeriodicSync now clears SegmentRing.durabilityFailure after a subsequent fully-successful sync pass, so a transient fsync/msync failure no longer permanently bricks the producer; recovery is logged like the trim path. Regression test: testTransientSyncFailureClearsOnNextSuccess. - C2: startup SF recovery parks slots whose build persistently fails (immediately on SlotLockContentionException, after 3 consecutive failures otherwise), keeps scanning the remaining slots, rewinds the cursors while parked slots remain, and dedups the per-slot WARN. A wedged slot no longer livelocks the driver, floods the log, or starves higher slots' orphan data; the server-wide transient retry contract is preserved. 3 new regression tests in SenderPoolSfTest. - M1: windows open_file saves GetLastError() at the CreateFileW failure point (before free() can clobber it) and fsyncDir0 classifies the ERROR_ACCESS_DENIED best-effort degrade from the saved value via the new GetSavedLastError() (defined in os.c beside the TLS index it reads), mirroring the FlushFileBuffers branch's capture pattern. - M2: replaced leftover test reflection with the existing @testonly seams in CursorSendEngineSlotReacquisitionTest, QueryClientPoolErrorSafetyTest and SlotLockReleasedContractTest. - Stale comments updated: the durability-latch throw is transient, and the recovery streak constant parks on the 3rd consecutive failure.

…ue dead 88d6b79 zeroed torn-tail residue inside MmapSegment.openExisting, i.e. for every file recovery opens and before any chain-level check runs. The scan stops at the first bad CRC, so after a mid-file tear in a SEALED chain member the frames past the tear -- individually valid CRCs, real unacked payloads, the only surviving copy -- were classified as residue and durably destroyed, while the FSN-gap check then failed recovery closed anyway. Pre-88d6b792 the same fail-closed brick preserved the bytes for operator extraction; post-88d6b792 the first startup attempt erased them and every later startup failed with a misleading FSN gap. openExisting is now a pure observer: it scans, reports tornTailBytes and never mutates. The zeroing moved to MmapSegment.sanitizeTornTail (same setMemory + msync/fsync barrier, same fail-closed abort, idempotent, refused after appends resume) and SegmentRing.recover invokes it only where validation has proven the residue non-load-bearing: - the segment selected as the resumed active, at the single ring-construction funnel after every chain/manifest check has passed (the active is the only segment that takes appends and the only one a rotation can reseal, so this preserves both original hazard fixes: the two-crash reseal brick and stale-frame FSN resurrection); - sealed members whose frame accounting validated complete (contiguity plus boundary matching prove the suffix is dead bytes), which keeps the legacy-poison self-heal: sanitize, fail closed on first sight, restart proves the chain clean. A tear that cost frames now fails closed before any mutation with every byte left on disk. The three stale comments asserting the old behavior ("a failed recovery never mutates the slot", "bytes that already failed frame validation", "the restart re-proves the chain clean") are rewritten to match reality. Tests: openExisting observe-only (byte-identical file across repeated opens); sanitize zeroes durably, keeps the diagnostic, idempotent, refused after appends; barrier failure fails closed at sanitize level with no barrier at open; mid-file tear in a sealed member fails closed twice with byte-identical .sfa files (operator extraction preserved); proven-dead sealed residue sanitized then heals after one restart; ring recovery durably sanitizes the resumed active with zero appends; frame[0]-corruption evidence now provably survives re-opens. The five 88d6b79 regression scenarios remain pinned at ring/segment level. sf package: 385 tests green.

…rage A live mutation accident proved the suite blind to durability-critical code: 88d6b79 shipped requestSyncBeforeRotation neutralized ('return false; // MUTANT: gate neutralized') and 369 tests stayed green; only human re-reading caught it (71cfbe2). Re-running that mutant against today's suite still passed 385/385, as did deleting SfManifest.update's monotonic clamp. Every gap below is now covered, and both mutants were re-applied after writing the tests to verify they die. - SegmentManagerPeriodicSyncTest.testRotationGateDefersRotationUntil- PredecessorDurable: kills the gate mutant at three points (rotation must backpressure while the predecessor is non-durable, the gate's sync request must run the barrier before the interval deadline, the retried append must rotate). Verified: mutant now fails 'expected -1 but was 2'. - SegmentManagerPeriodicSyncTest.testSyncPassStopsAtFirstFailureThen- RetryCoversAllSegments: >= 2 non-durable live segments (recovered segments start non-durable) prove the pass aborts at the first barrier failure and the healed retry covers every segment before the producer unlatches. - SfManifestClampTest: direct pin on the update() monotonic clamp, including independent per-field clamping and durable persistence across reopen. Verified: clamp-deletion mutant now fails 'expected 10 but was 5'. SfManifest and the five members under test are widened to public within the already-exported internal package (JPMS forbids same-package test classes; matches MmapSegment/SegmentRing). - CursorSendEngineTest.testCheckDurabilitySurfacesLatchedFailureTo- SenderEntryPoints: pins the engine delegation seam behind flush()/awaitAckedFsn() (zero references before): quiet when clean, throws the latched instance repeatedly until cleared. - SegmentManagerCloseRaceTest.testSecondBoundedJoinReapsWorker- FinishingExitCleanups: first join times out against a worker parked in its exit cleanups (workerLoopExited set before cleanups run), the second fixed-budget join must reap and close() must free the scratch itself. Every existing close-race test only reached the first join. - CursorSendEngineClosePartialEnumerationTest: torn close-time directory enumeration (findNext < 0 mid-walk, injected via a FilesFacade proxy) must drive ZERO unlinks, keep the watermark, and a successor's fully-drained close retries the cleanup. The sibling unlink-failure test only covered failing unlinks via the root-skipped permission trick. - SourceHygieneTest: tripwire that fails the build on 'MUTANT' markers in main sources -- the exact artifact 88d6b79 shipped. sf package: 392 tests green (385 + 7 new).

The catch (Exception) setup path in BackgroundDrainer logged only t.getMessage() and stored it as lastErrorMessage. For a JVM-raised NPE (getMessage() == null on JDK 8, the release target) that produced 'drainer setup temporarily unavailable for slot ...: null' with no exception class and no stack trace -- and since this path deliberately leaves no .failed sentinel, a deterministic setup bug is retried on every orphan scan while emitting that same contentless line forever. The outer catches in the same method already attach the throwable (LOG.error(..., slotPath, msg, t)); only this inner retryable catch dropped it. Log the throwable (SLF4J attaches the stack trace) and carry t.toString() -- class plus message -- into lastErrorMessage so the telemetry surface shows 'java.lang.NullPointerException' instead of 'null'. Diagnostics only; the retry-not-quarantine policy is unchanged. sf package: 391 tests green.

…tch clear) After a failed writeback the kernel marks the dirtied pages clean and reports the error once per open file description (errseq_t; fsyncgate), so re-running msync+fsync over the same range returns a vacuous 0: durableCursor advanced, the PERIODIC rotation gate opened, and the round-3 unlatch (958a36d) resumed the producer -- all without the data being durable. A clean non-durable page is also evictable, so reclaim could silently replace the only good copy with stale disk content. - MmapSegment.syncPublished now brackets the barrier: best-effort mlock pin over the page-aligned [durableCursor, published) range, then msync+fsync; the failure arm re-dirties the pinned range (one same-value store per page -- kernel dirty tracking is value-blind) before the pin is released. The pin closes the clean-page reclaim race by construction; the re-dirty guarantees the next barrier performs real writeback and reports real errors, keeping the manager's clearDurabilityFailure() honest. - mlock/munlock natives added (POSIX mlock, Windows VirtualLock) and exposed through Files with an UnsatisfiedLinkError guard plus FilesFacade defaults. Refusal (RLIMIT_MEMLOCK, missing capability) is a soft downgrade: one deduped WARN, barrier outcome and latch semantics unaffected. - 4 regression tests: bracket ordering on success, re-dirty-before-unpin on failure, an fsyncgate facade model (first fsync fails, later msync/fsync return vacuous 0 without delegating) proving the latch only clears over re-dirtied pages, and mlock-refusal degrade. The drop-the-redirty mutant was applied and killed (2 failures) before restoring. Native libraries are built from source, so the new symbols ship with every build; the UnsatisfiedLinkError guard remains as defense-in-depth for environments running a stale prebuilt library.

… mutant-killers Round-3 review re-audit of e0ebdf0 found three rows not genuinely closed; each is now pinned by a test whose mutant was applied and verified to die before restoring: - C-2a: close-path unlink stop-on-first-failure was empirically unprotected -- a continue-past-first-failed-remove mutant in CursorSendEngine.unlinkAllSegmentFiles survived the whole suite (2,722 green). e0ebdf0 mapped the row to the periodic sync pass (different subsystem); the enumeration and all-unlinks-fail siblings cannot discriminate stop-vs-continue. New CursorSendEngineCloseUnlinkStopOnFirstFailureTest: legacy (manifest-less) slot, two FSN-contiguous sealed segments recovered from disk, FilesFacade refusing only the lowest .sfa removal once; asserts the higher generation is never attempted and survives, and a successor recovers the contiguous residue and completes the cleanup. Mutant now fails 'expected:<0> but was:<1>'. - C-2b: the sender-level checkDurability call sites (QwpWebSocketSender awaitAckedFsn pre-check, wait-loop, and flushAndGetSequence) had no user-visible-layer coverage -- deleting all three survived the whole suite; only the engine seam was pinned. New QwpWebSocketSenderCursorEngineAttachmentTest. testLatchedDurabilityFailureSurfacesThroughSenderFlushAndAwait: unconnected createForTesting sender + attached engine + latched failure; empty flush()/flushAndGetSequence() and awaitAckedFsn(fsn,0) must each throw the latched instance repeatedly (empty flush and <=0-timeout polls publish nothing, so the ring gate never runs -- these call sites are their only durability act, and they must fire before connection setup). Mutant now dies on the first flush(). - C-2c: testSecondBoundedJoinReapsWorkerFinishingExitCleanups passed under the exact pre-fix revert of 8d962e0: both variants null workerThread (isWorkerReaped() cannot discriminate) and the worker.join(5s) preceding the isAlive assert destroyed the liveness observable. Added the discriminating assertion at the moment close() returns: the second bounded join must hold close() until the parked cleanup finishes (+400ms), while the revert returns at ~200ms with the worker alive. Verified deterministic both ways: fails under the revert, passes with the fix. Full core suite: 2,724 green (2,722 + 2 new).

…ze already healed The fail-closed first-sight throw in sanitizeSealedResidue fires AFTER the proven-dead sealed residue has been durably zeroed (msync+fsync), so the chain on disk is already healed when it propagates. The orphan drainer classified that throw as terminal SfRecoveryException and dropped a .failed sentinel -- stranding a fully replayable backlog behind a quarantine no scan revisits, for an incident recovery had just repaired. Introduce SfSanitizedResidueException (a refinement of SfRecoveryException, so producer startup keeps its fail-closed restart-proves-clean semantics) and have the drainer intercept it for a single in-place construction retry. Any second failure, including a repeat of the refinement from a non-sticking heal, falls through to the existing terminal classification. New drainer test proves the full arc: poisoned sealed suffix, heal durably on disk before the throw, one retry, connect reached, no sentinel, backlog still scanner-eligible. SegmentRingTest now pins the thrown type at the sanitize site.

… parked slots A managed slot whose flock is held by another live owner is parked as CONTENDED and re-probed on every startup-recovery retry cycle -- every second on the direct pool's private driver, for as long as the owner lives. Each re-probe paid a full recovery build just to reach SlotLock.acquire and throw: two isCandidateOrphan dir enumerations, a complete config re-parse plus builder graph, parent-dir fsync barriers in periodic durability mode, and an owned SegmentManager allocation torn straight back down. Add SlotLock.probeHolder: a side-effect-light non-blocking flock probe that never creates dirs or files, never throws, and routes its momentary hold through the standard close() so an unconfirmed unlock can never leak from a probe. The recovery scan asks the probe first and parks on a held flock for a few syscalls; an indeterminate probe falls through to the full build, which keeps sole ownership of error classification. Races are benign both ways: a free probe can still lose the acquire (parks exactly as before) and a stale held probe is re-observed next cycle. New SenderPoolSfTest proves the arc with a real externally held flock: three parked cycles cost zero builds, and the first cycle after release recovers the slot with exactly one build.

… as it is Two in-diff comments claimed more than the code proves. The recovery-scan comment asserted 'a failed recovery never mutates the slot (single exception: ...sanitizeSealedResidue)'. There are more windows where a recovery that later fails has already durably mutated: the legacy-migration sanitize zeroes residue before SfManifest.create (or any later step) can fail, and validated-extra cleanup plus corrupt-file quarantine run before the active-sanitize barrier or ring construction can fail. Restate the invariant as what the code actually guarantees -- a failed recovery never mutates COMMITTED CHAIN BYTES -- and enumerate the windows, each confined to proven-dead zeroes or preserve-by-rename quarantines. The db8938e-derived javadoc claimed sanitizeTornTail runs 'only on residue that validation proves non-load-bearing', gliding over the resumed active with '(reclaimed by appends anyway)'. No such proof exists for the active -- it has no successor to bound its accounting, and past a mid-file tear its residue can hold valid-CRC frames of real unacked payloads. Zeroing them is a deliberate POLICY (replay cannot cross the tear; unzeroed residue risks the reseal-brick and stale-frame resurrection hazards), not a validation-derived proof. Say so at every site that carried the stronger wording: openExisting's javadoc, the scan-time observe-only comment, sanitizeTornTail's contract, and the active-sanitize call site in SegmentRing. Comments only; no behavior change.

The in-range recovery scan skipped any reserved index as "live" without a deferral tick. A RETIRED index (wedged close() kept the slot flock) aliases into that branch: the scan could finish a cycle with zero deferrals and latch recoveryComplete while the retired slot's dir still held unacked durable data. From there nothing in-process ever drained it -- reprobeRetiredSlots() only restores capacity -- so the data waited for a restart or a lucky borrow of that exact index, which steady low load may never produce. Treat a retired index whose dir is still a candidate orphan as a deferral, the same rule as a CONTENDED park: the end-of-scan rewind keeps the cycle alive, and once the deferred cleanup releases the flock the scan reserves the freed index and drains it itself -- no borrow required. Data-inert (durable on disk; a restart already rescanned); this closes the drain- liveness gap for the life of the pool. The regression test drives the scan by hand: forge the wedged retire, walk the scan (pre-fix it latched complete right here), heal the flock, and assert the scan itself delivers the stranded data and only then completes.

…chinery Seeded, single-threaded (replayable) randomized schedules of recovery steps, housekeeper ticks, borrows and late flock releases against stranded slot dirs and wedge-forged recovery builds. The forged recoverers model a wedged worker faithfully: alive but streaming into a silent sink, with close-flush off, so neither the forge nor the heal can deliver data on the scan's behalf. Oracle: after every wedge heals and the pool quiesces, the scan must converge and an ordinary-lifecycle close must leave no slot dir with undelivered durable data -- no restart, no lucky borrow. Iteration 0 pins the wedged-retire shape deterministically (prologue drives the retire before any random traffic, heals only at the end) so the suite reds on the scan- abandonment bug class regardless of seed; the remaining iterations randomize freely. Failure messages carry the reproducer seeds.

… holds data Two remaining drain-liveness windows around retired-slot flock releases: 1. Post-latch runtime retire: discardBroken/reapIdle can retire a wedged slot AFTER the startup scan legitimately latched recoveryComplete. The late release then restored capacity only -- nothing re-drained the dir (the scan was done, close() never owns an unowned dir), so the data waited for a restart or a lucky borrow of that exact index. 2. Mid-cycle release (found by the drain-liveness fuzz): a retire AND its release can both land while the scan is alive but after the cursor already passed that index -- it was LIVE at walk time, so no retired-candidate deferral was counted, and recoveryComplete was still false at release, so no post-latch path applied. A cycle ending with zero deferrals then latched past the freed dir's stranded data. recoverRetiredSlotAt now probes the freed dir: if it is still a candidate orphan it either un-latches and rewinds the scan (post-latch case; the volatile latch publishes the rewound cursors to the unlocked driver gates) or raises recoveryRearmRequested (alive case; cursors stay driver-owned). The end-of-cycle latch decision consumes the flag under the pool lock -- the producer runs under the same lock, so no release can slip between the check and the latch write. Deferred pools re-arm through the housekeeper's unconditional per-tick step; a direct pool whose private driver already exited gets the driver revived (daemon, same loop, joined by close()). Three deterministic pins, each red before this change: the post-latch re-arm, the revived direct driver draining with no external help, and the mid-cycle release that must not latch past data it never revisited.

Extends the drain-liveness fuzz with a runtime-wedge action -- a borrow built against the silent sink, written through, then wedged-closed and discarded, so reclaimSlot retires it with its rows durably unacked -- and a second pinned iteration for the post-latch residual: the scan completes legitimately over clean dirs first, then the runtime wedge lands, and only the end-of-schedule heal may deliver the data back through the re-armed scan. Random iterations draw the runtime wedge freely so runtime retires also interleave with a still-running scan (that interleaving is what surfaced the mid-cycle release gap this change's sibling fix closes). The fault injector switches off at the end-of-schedule heal (faultsHealed): without it, a wedge[idx] flag whose first recovery build only happened DURING the quiescent convergence would forge a brand-new wedge after "every fault healed" -- one the schedule can never heal -- and the scan would correctly refuse to complete, failing the audit for the wrong reason. Discrimination matrix: both fixes green; residual fix removed reds at the pinned iteration 1; both fixes removed reds at the pinned iteration 0.

The revive path decided "will a driver observe my un-latch?" by probing Thread.isAlive() -- an out-of-band signal not ordered with the driver's own exit decision. A driver past its final latch check could still report alive, so a release landing in that window skipped the spawn and left the un-latched scan ownerless until the next release event or restart (a microscopic, documented-but-real lost-wakeup corner on direct pools). Ownership is now a lock-guarded token (recoveryDriverRunning). The loop drops it transactionally: releaseRecoveryDriverOwnership re-checks the latch under the pool lock before the drop, so an un-latch that raced the exit keeps this thread driving; the producer (recoverRetiredSlotAt) clears the latch and reads the token under the same lock, so it spawns a successor exactly when no owner remains. Mutual exclusion makes the two decisions serial: no interleaving leaves an un-latched scan ownerless, and cursor ownership hands off cleanly (predecessor's last cursor touch happens-before its token drop happens-before the successor's spawn, all via one lock). Loop semantics are otherwise preserved: waiter only after a workless step on a live un-latched scan, prompt exits on close/interrupt (which also drop the token so a still-live pool can revive later).

…ings Completes the errno-capture pattern 958a36d established for open_file/ fsyncDir0: seven sites across five natives -- length0 (CreateFileW and GetFileSizeEx failures), mkdir0, remove0, rename0, and findFirst0 (both the utf8_to_wide and FindFirstFileW failures) -- called SaveLastError() only AFTER free()/CloseHandle(), either of which may overwrite the thread's last-error value (HeapFree can SetLastError). The saved errno for a failed operation could therefore be an unrelated code or 0. Inert to behavior: repo-wide, nothing branches on the saved value for these operations -- fsyncDir0 was the sole control-flow consumer and already reads its own capture via GetSavedLastError() -- so the exposure was wrong [errno=N] diagnostics in Windows failure messages. Save now happens at the failure point; cleanup follows on every path unchanged. Windows-only TU: verified by mechanical review (order-only transform, brace/paren balance identical, statement-level clobber audit clean); compile coverage via the native build pipeline.

@testonly

…end to end The build -> startOrphanDrainers -> BackgroundDrainer -> CursorSendEngine inheritance was verified-correct but mutation-invisible: passing 0 anywhere along the chain would drain a PERIODIC slot without checkpoints and still pass the suite (the only startOrphanDrainers test call sites used the 4-arg overload, which itself hardcodes 0L). The new integration test adopts a fabricated orphan under sf_durability=periodic against a never-acking server -- the drain can never finish, so the drainer and its engine stay deterministically observable in the pool snapshot -- and asserts the configured interval at the foreground engine, the drainer, and the drainer's engine, failing on value rather than timeout. Seams: @testonly drainer-pool accessor on the sender plus engine capture and inherited-interval getter on the drainer.

…ed residue Deleting sanitizeSealedResidue(chain, false) in the no-manifest migration branch passed all 395 SF/recovery tests: every legacy-migration test migrated a clean chain (the sanitize ran as a no-op), and the existing residue test poisons the sealed suffix only after the first recovery has created the manifest, exercising exclusively the fail-closed (chain, true) pass. The new black-box twin plants the pre-fix-client junk in the sealed suffix gap BEFORE the first recovery ever runs and pins the one-pass silent heal: no first-sight throw, residue durably zeroed on disk, frames untouched, manifest created, and a clean restart -- no spurious SfSanitizedResidueException deferred to production's next start.

…ground thread Direct SF pools no longer run inline, construction-time recovery, and no longer revive a per-attempt recovery driver. Each direct pool now owns a single long-lived background recovery thread: it scans retired/stranded slots off the construction path, parks indefinitely when the scan is complete, and re-parks with a bounded wait between failed attempts. It exits only on close or interrupt. Release callbacks no longer create or revive threads. They re-arm the scan cursor under the pool lock and then LockSupport.unpark the single driver; the retained-permit ordering closes the callback-before-park lost-wakeup window. This removes the double-driver race and retires the transactional revival token (recoveryDriverRunning, releaseRecoveryDriverOwnership, reviveDirectRecoveryDriverIfNeeded, revivedStartupRecoveryThread, startupRecoverySignal) in favor of one scan with one owner. Construction no longer blocks on replay. The clean-release re-arm fix is preserved: reclaimSlot() calls rearmRecoveryScanIfStranded() so a clean flock release that frees a dir still holding stranded data re-arms and unparks the scan. Deferred pools own no private thread and remain PoolHousekeeper-driven; unpark(null) there is a harmless no-op. Shutdown unparks and joins only the single driver and never blocks on delivery; durable leftovers wait for restart. Java 8 stays green via Compat.onSpinWait, and the test-only manual drive helpers now refuse direct pools to keep the one-owner invariant.

mtopolnik · 2026-07-23T01:26:41Z

[PR Coverage check]

😍 pass : 2274 / 2584 (88.00%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegmentCorruptionException.java	2	4	50.00%
🔵	io/questdb/client/std/Files.java	14	22	63.64%
🔵	io/questdb/client/Sender.java	24	31	77.42%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	121	155	78.06%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java	279	330	84.55%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	60	70	85.71%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SfManifest.java	106	122	86.89%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java	497	568	87.50%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java	356	407	87.47%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java	65	72	90.28%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java	31	34	91.18%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegment.java	186	204	91.18%
🔵	io/questdb/client/impl/SenderPool.java	346	373	92.76%
🔵	io/questdb/client/network/JavaTlsClientSocket.java	20	21	95.24%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/AckWatermark.java	76	80	95.00%
🔵	io/questdb/client/impl/PooledSender.java	3	3	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SfSanitizedResidueException.java	2	2	100.00%
🔵	io/questdb/client/network/NetworkFacade.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SfRecoveryException.java	2	2	100.00%
🔵	io/questdb/client/network/NetworkFacadeImpl.java	1	1	100.00%
🔵	io/questdb/client/cutlass/http/client/WebSocketClient.java	2	2	100.00%
🔵	io/questdb/client/std/ObjList.java	1	1	100.00%
🔵	io/questdb/client/std/DefaultFilesFacade.java	7	7	100.00%
🔵	io/questdb/client/std/FilesFacade.java	9	9	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SfOperationalException.java	2	2	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SenderErrorDispatcher.java	1	1	100.00%
🔵	io/questdb/client/impl/ConfigSchema.java	1	1	100.00%
🔵	io/questdb/client/network/PlainSocket.java	5	5	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLockContentionException.java	2	2	100.00%
🔵	io/questdb/client/impl/SenderSlot.java	4	4	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SenderConnectionDispatcher.java	1	1	100.00%
🔵	io/questdb/client/impl/QueryClientPool.java	35	35	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SenderProgressDispatcher.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/client/QwpQueryClient.java	8	8	100.00%
🔵	io/questdb/client/network/Socket.java	1	1	100.00%
🔵	io/questdb/client/impl/QuestDBImpl.java	2	2	100.00%

bluestreak01 added 11 commits July 10, 2026 14:28

test(qwp): pin slot retention until worker quiescence

a0dd202

fix(qwp): preserve slot ownership after incomplete close

d976fd5

bluestreak01 mentioned this pull request Jul 10, 2026

chore(qwp): carry crash-safe SF cleanup and pool-capacity recovery questdb/questdb#7387

Open

bluestreak01 added the tandem label Jul 10, 2026

bluestreak01 added 17 commits July 11, 2026 02:15

Make final-probe regression deterministic

79e0847

Release the retired slot from a test-only hook after the positive borrow wait has actually exhausted its budget. This removes the scheduler-dependent sleep and guarantees the test reaches the final post-wait probe.

fix(qwp): recover SF slots after deferred cleanup

b89384f

Merge remote-tracking branch 'origin/fix/qwp-worker-quiescence' into …

6b373cb

…fix/qwp-worker-quiescence # Conflicts: # core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

fix(qwp): harden cleanup and recovery lifecycle

a086575

fix(qwp): harden SF crash recovery and cleanup

a531164

Make acknowledged segment deletion crash-consistent, bound retry and cleanup paths, improve pool recovery complexity, and add deterministic lifecycle/native regression coverage.

Fix client durability and creation shutdown

4b6ba13

fix(client): batch manifest updates during segment trim

429153a

bluestreak01 added 28 commits July 20, 2026 19:15

fix(qwp): harden orphan recovery and metadata crash safety

80ad4ca

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(qwp): keep SF slot locked until manager worker quiesces#67

fix(qwp): keep SF slot locked until manager worker quiesces#67
bluestreak01 wants to merge 63 commits into
mainfrom
fix/qwp-worker-quiescence

bluestreak01 commented Jul 10, 2026 •

edited

Loading

Uh oh!

bluestreak01 commented Jul 20, 2026

Uh oh!

mtopolnik commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

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

Uh oh!

Summary

Root cause

Changes

Worker-quiescence barrier

Confirmed slot release and pool recovery

Crash-safe segment cleanup

Periodic store-and-forward syncing

Retryable recovery and flock release

Hardened recovery segment sort

Bounded transport shutdown

Serialized QuestDB.close()

ObjList.remove(from, to) backing-array fix

Attachment and observability guards

Review follow-ups

Failure behavior

Test coverage

Compatibility

Uh oh!

bluestreak01 commented Jul 20, 2026

Review — QWP multi-slot recovery & close-drain failover (tandem)

What this PR is

Critical

Moderate

Minor

Downgraded (false positives / refuted after source verification)

Coverage map (OSS QWP close-drain — the only production behavioral change)

Summary

Uh oh!

mtopolnik commented Jul 23, 2026

[PR Coverage check]

file detail

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bluestreak01 commented Jul 10, 2026 •

edited

Loading