fix(qwp): keep SF slot locked until manager worker quiesces#67
Merged
Conversation
…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.
…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.
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.
… 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.
…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.
Both new user-visible rejection branches for sf_sync_interval_millis shipped untested: the out-of-range guard (<=0, or past the Long.MAX_VALUE/1_000_000 nanosecond-overflow boundary) and the non-WebSocket transport guard in the config-string parser. Add three cases to SfFromConfigTest: - testSfSyncIntervalRejectsZero: =0 is a realistic operator typo, rejected with "sf_sync_interval_millis is out of range". - testSfSyncIntervalOverflowBoundary: two-sided pin of the millis->nanos overflow guard -- Long.MAX_VALUE/1_000_000 is honored with exact nanos, one millisecond more is rejected -- so a later change to the operator or the divisor cannot regress silently. - testSfSyncIntervalRejectsOnNonWebSocketTransport: an http config is rejected with "sf_sync_interval_millis is only supported for WebSocket transport".
servicePeriodicSync copied and scanned the whole live sealed list under the ring monitor every tick and called syncPublished() on each. But a segment is made durable before it is sealed (the requestSyncBeforeRotation gate), so every sealed barrier early-returns -- the copy was O(live-sealed) monitor-held work that grows without bound under a producer-outpaces-drain backlog. Track a monotonic durability frontier (firstNonDurableSealed) and copy only [frontier, size) + active via copyPendingSyncSegments. The frontier only advances past segments observed durable and durability never regresses, so it is a conservative lower bound that can never skip a segment still needing a barrier; recovery-resumed non-durable sealed segments are covered because it starts at 0. Maintained under the ring monitor and shifted with sealed-list compaction, so steady-state copy is O(1). Adds a frontier invariant assert plus tests for the durable-prefix skip and the compaction index-shift. Also folds in minor local cleanups (Arrays.fill, RetainedSegmentMembership lambdas, assertTrue).
Contributor
[PR Coverage check]😍 pass : 2298 / 2607 (88.15%) file detail
|
Member
Author
Review —
|
bluestreak01
enabled auto-merge (squash)
July 23, 2026 11:49
bluestreak01
disabled auto-merge
July 23, 2026 11:58
mtopolnik
added a commit
that referenced
this pull request
Jul 23, 2026
Merge main (PR #67, keep SF slot locked until manager worker quiesces, plus its periodic-durability work) into the branch that renames sf_max_bytes to sf_max_segment_bytes. The merge base is one commit behind on each side, so every conflict is this branch's rename landing on top of #67's edits; each was resolved by keeping main's semantic change and re-applying the rename. Conflict resolutions: - Sender.java (3 spots): kept main's reworded comment and its new actualSfSyncIntervalNanos argument threaded into the CursorSendEngine constructor and startOrphanDrainers, with the identifiers renamed to actualSfMaxSegmentBytes / sfMaxSegmentBytes. - SegmentRing.java: took main's version wholesale. The only line this branch changed here was a comment that #67's recover() rewrite deleted, so main's file already reflects the intended state. - SfFromConfigTest.java: kept main's four new testSfSyncInterval* methods and renamed the trailing method to testSfMaxSegmentBytesAcceptsSizeSuffixes. - SegmentRingTest.java: kept main's Javadoc wording (it matches the deterministic comparison-count assertion) and applied the rename. Verified no conflict markers or stale sf_max_bytes spellings remain, and that the merged call sites resolve against the five-argument CursorSendEngine constructor and startOrphanDrainers overload. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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=periodicwith a newsf_sync_interval_milliskey) for bounded-loss protection against host power loss, serializes concurrentQuestDB.close()callers through shutdown completion, hardens the recovery-time segment sort against quadratic input, and fixes an off-by-one inObjList.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 itsRingEntryinto a snapshot and enteredserviceRing(). 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.Confirmed slot release and pool recovery
SlotLock.release()explicitly unlocks through platform JNI and reports whether release was confirmed.closeCompletedandisSlotLockReleased()become true only after confirmed unlock.SenderPoolimmediately; parked borrowers do not wait for a timeout or housekeeper tick.Crash-safe segment cleanup
Periodic store-and-forward syncing
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 requiressf_dirand WebSocket transport.sf_sync_interval_millis(builder:storeAndForwardSyncIntervalMillis(long)), sets the target checkpoint cadence in milliseconds. Default:5000in periodic mode. Accepted range: positive and at mostLong.MAX_VALUE / 1_000_000(the nanosecond-conversion bound); values outside it throwsf_sync_interval_millis is out of range.sf_durability=periodic(sf_sync_interval_millis requires sf_durability=periodic) and rejects it for non-WebSocket transports.request_durable_ack=onremains the option for end-to-end server durability.Retryable recovery and flock release
Hardened recovery segment sort
SegmentRingrecovery sorts enumerated segments by unsignedbaseSeq. 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.2*floor(log2(N))budget, and ranges that exhaust it fall back to in-place heapsort (stillLong.compareUnsigned, zero allocation), capping every adversarial pattern at O(N log N) comparisons.Bounded transport shutdown
Serialized QuestDB.close()
QuestDBImpl.close()set the volatileclosedflag before running the pool teardown chain, so a second concurrent closer could observeclosed == trueand return while the first was still draining and releasing pool resources — breaking the expectation that shutdown has completed onceclose()returns.close()is nowsynchronized: the losing caller blocks until the winner finishes teardown, then enters, seesclosed, and no-ops. The teardown steps never re-enterclose()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
Review follow-ups
SegmentRing.findSegmentContaining0now binary-searches the sealed list (strictly ascending and contiguous inbaseSeq) 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 inSegmentRingTestwere pinned green against the pre-rewrite linear implementation.AckWatermarknow routes its lifetime mmap/munmap through the injectedFilesFacade, matchingMmapSegment, which makes watermark-mmap fault injection possible from test facades.AckWatermarkTestadds facade-routing and mmap-fault tests, and twoSegmentManagertests replace reflective access with direct calls against the now-publicopen(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:
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); aMUTANT-marker source tripwire (SourceHygieneTest); plus config acceptance and rejection ofsf_sync_interval_millis(SfFromConfigTest,WsSenderConfigHonoredTest);SegmentRingTest);QuestDB.close()(QuestDBImplCloseTest);ObjList.remove(from, to)(ObjListTest); andEnterprise 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.