Add FlushOptions.allowWriteStall, and settle flush()/compact() promises on read-only databases

[kriszyp]

What

flush() and flushSync() now take an optional { allowWriteStall } bag, mapping to rocksdb::FlushOptions::allow_write_stall. The default is unchanged.

DBDescriptor::flush() constructed a default rocksdb::FlushOptions, and that was the only occurrence of the type in the binding, so every flush ran with allow_write_stall = false and no caller could say otherwise. False is the waiting behavior — despite how the name reads, it means the flush waits until it can run without causing a write stall — and that wait has no timeout at any layer. It is taken on the calling thread, which for the async flush() is a libuv worker, so a database sitting in a stall condition hands its caller a promise that never settles while the event loop stays alive, with no error and nothing logged. The stall conditions that produce it are ordinary: immutable-memtable backlog, the L0 stop trigger, soft_pending_compaction_bytes_limit, or a WriteBufferManager budget that cannot drain.

A caller whose flush is a durability gate it is already blocked on would rather stall writers for the duration than wait a stall out. That is the situation in harper-pro#678, where the base-copy cursor flush is awaited from the copy's onCommit.

This adds an option; it changes no existing behavior. DBDescriptor::close() is the one internal caller and keeps the waiting default — see the CI finding below for why that is deliberate rather than an oversight.

Also fixes #774

Same class of defect one layer down, found while tracing the above. Database::Flush and Database::Compact returned without invoking either resolve or reject when the database was opened read-only, so await db.flush() there was an unconditional permanent hang — no stall required. They now resolve, matching the no-op flushSync already performed. Recorded as AGENTS invariant 12: a callback-style native method owes its caller exactly one settled callback on every path, and for a promise an early return is not a no-op.

Argument validation in all three methods now precedes the read-only short-circuit, so the same argument gets the same verdict in either mode. In Compact it also precedes the AsyncCompactState allocation, so a rejected key buffer can no longer throw past the new and leak it.

Verification

Route (b) new test file plus (a) extension of the suite that owns this area.

• test/flush-options.test.ts (new) — the option is accepted and flushes durably on both entry points; no-args / {} / explicit false still work; a non-object bag or non-boolean value throws rather than being silently ignored, in both read-only and read-write mode; the two read-only settle cases from flush() and compact() never settle their promise on a read-only database #774; and the compact reorder, exercised through the native handle since RocksDatabase.compact always materializes real Buffers and can't reach the validation.
• Fails-on-base, verified mechanically: reverting only the two read-only settle blocks and rebuilding makes both read-only cases fail with Test timed out in 10000ms. A regression here is a hang rather than an assertion failure — the promise never settles and no in-test timer can surface it — so the per-test timeout is the assertion. Restored and rebuilt afterward.
• test/write-buffer-manager-stall.test.ts extended by one case, since it is the only file that builds a stalling WriteBufferManager (allowStall is fixed when the singleton is created, so it needs its own process). It is labelled plumbing-only on purpose — see decision 4.
• Full suite 762 passed / 2 skipped / 56 files, Node 26.2.0 on Linux x64, RocksDB 11.8.1. pnpm check (type-check + oxlint + oxfmt) clean.
• CI: all checks green at head, across Node 22/24/26, Bun, and Deno on Linux/macOS/Windows, plus the native C++ suites. The one earlier failure and its resolution are described above.

What CI caught, and what it means

An earlier revision of this branch had close() opt in (flush(true)), on the reasoning that a closing database has stopped accepting work so a stall costs it nothing. CI refuted that, and it is the most useful thing to know about this change.

Test on Bun (windows-latest) failed deterministically — both in-job retries, while the same job is green on nine recent runs from other branches. No test reported a failure; a vitest worker died. Diffing per-file test counts against the passing Bun/ubuntu job located it: transaction-log.test.ts ran 30 of 70 tests and died at "should write to same log from multiple workers", which drives concurrent worker_threads writers against one shared DB path while looping purgeLogs({ destroy: true }).

Mechanism: allow_write_stall = true makes the flush switch memtables immediately rather than waiting for a stall-free moment, so it fires OnFlushBegin/OnFlushCompleted (db_descriptor.cpp:189) into TransactionLogStores that the concurrent purge is destroying. Reverting only that line turned CI fully green, which is the confirmation.

Two things follow, and they matter more than the option itself:

• The reasoning was too narrow, not just the code. "Close has stopped accepting work" is true of the closing handle; the descriptor is process-global and shared across worker_threads, so other handles are still live. Any future attempt to stop close from wedging has to avoid flushing into that teardown race — recorded in AGENTS invariant 13.
• The public option does not appear to inherit this. I chased that down: TransactionLogEventListener is registered in dbOptions.listeners (db_descriptor.cpp:969), so those callbacks fire on every flush — including RocksDB's own background flushes, continuously, in normal operation. The option changes when a manual flush runs, not whether the callbacks happen, so it is not a new crash class. What made close special is that it flushed during teardown, when the stores are being destroyed. Moderate confidence: reasoned from registration and the listener's weak_ptr discipline, not from a constructed race.

Open concern, not fixed here

flushBeforeBackup flushes inside RocksDB's BackupEngine, which builds its own default FlushOptions this layer cannot reach — the same unbounded wait, and it is taken after runCreateBackup acquires the exclusive .backup.lock. A stalled database therefore turns a backup into an indefinite hang that also blocks every other backup/delete/purge on that directory, cross-process and cross-container, until the process dies. Pre-existing and not introduced here; recorded in AGENTS invariant 13 as a known uncovered path and worth its own issue.

For the human reviewer

1. The default stays false (waiting). Every existing caller is unchanged, which also means the never-settling promise described above remains the out-of-the-box behavior. The alternative is defaulting Harper's flush() to true and making the unbounded wait opt-in. Flipping later is a one-line change but a semver-visible behavior break. A "no" here costs nothing today and closes off the argument that upstream's default is a trap we're propagating.
2. Is my reading of the teardown race right? I concluded the public option does not inherit the crash, because the flush listeners already fire on every background flush — so the option shifts timing rather than introducing callbacks. That is reasoning, not a constructed race, and it is the load-bearing claim for shipping the option at all. If you think a caller can drive flush({ allowWriteStall: true }) into a concurrent purgeLogs({ destroy: true }) in a way a background flush cannot, this needs a guard before merge.
3. Option parsing lives in C++, with a string-matched error, rather than in src/database.ts where the type already exists. That costs error-message quality and duplicates the surface for every future flush option. Cheap to move, but the tests currently assert the native message.
4. The behavioral half is deliberately unproven. No test shows allowWriteStall changes RocksDB's behavior, so nothing would catch the flag being dropped between TS and FlushOptions. Building a deterministic stall fixture is possible, but Fix permanent write stall when the derived memtable-history target exceeds the WriteBufferManager budget #755 removed the stall this fixture could produce on demand, and the failure mode of such a test is a wedged libuv thread rather than a red assertion. If you want the coverage anyway, that is a reasonable call to overrule me on.
5. The compact reorder rides in this PR rather than a separate one — it is the same finding as the flush validation reorder, but it does widen review scope and revert granularity.

Worth knowing while reading: opting a flush in is database-wide. DBDescriptor::flush covers every column family on a descriptor that is process-global and shared across worker_threads, so the stall reaches every other table and every other handle on that path, not just the caller's. The JSDoc says so now; it did not in the first draft, and it is the thing most likely to surprise a call site.

Review coverage

Four rounds. Round 1 (codex + gemini + Harper domain, adjudicated) returned CHANGES on three findings, all fixed: the close path inheriting the waiting default, mode-dependent argument validation, and a stall test that claimed behavioral coverage it did not have. Round 3 (full, adjudicated) settled at severity minor and produced the decisions above. Round 4 (delta) is codex LGTM.

Honest degradations: cursor-grok and cursor-composer were auto-pruned on every round; the gemini leg produced no output on round 2. Gemini raised four findings across rounds that were refuted rather than fixed — three claiming uninitialized argv slots (napi_get_cb_info pads unsupplied slots with undefined, and Compact/Get have used the pattern since before this PR), and one claiming an async-flush use-after-free (AsyncFlushState holds a shared_ptr<DBHandle> whose descriptor is itself a shared_ptr, and close gates on registerAsyncWork). The review-need grade below is pinned at 4 by a run of mine that pruned the adjudicator and so left those unrefuted; the adjudicated severity is minor.

Generated by Claude Opus 5.

_{Review-Coverage: authored=unknown; ran=none; rounds=1 @ 0c71a3e}

_{Human-Review-Need: 4 @ 0c71a3e}

[gemini-code-assist]

Code Review

This pull request addresses a hang issue (#774) where flush and compact operations on read-only databases would return without settling their promises. It ensures that callback-style native methods always settle their callbacks. Additionally, it introduces support for the allowWriteStall option in both synchronous and asynchronous flush operations, allowing callers to control whether a manual flush should wait for or cause a write stall. New tests and documentation have been added to verify and describe these behaviors. I have no further feedback to provide as there are no review comments.

[github-actions]

📊 Benchmark Results

get-sync.bench.ts

getSync() > random keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	24.50K ops/sec	40.82	39.36	596.174	0.121	122,481
🥈 rocksdb	2	11.29K ops/sec	88.59	85.52	31,594.843	1.25	56,441

getSync() > sequential keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	28.94K ops/sec	34.55	33.69	1,701.965	0.090	144,706
🥈 rocksdb	2	11.37K ops/sec	87.92	85.39	3,406.691	0.141	56,867

ranges.bench.ts

getRange() > small range (100 records, 50 range)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	25.43K ops/sec	39.32	36.19	1,961.607	0.288	127,173
🥈 rocksdb	2	16.37K ops/sec	61.11	52.18	1,079.602	0.120	81,826

realistic-load.bench.ts

Realistic write load with workers > write variable records with transaction log

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	384.33 ops/sec	2,601.964	94.43	22,745.854	8.26	771
🥈 lmdb	2	26.61 ops/sec	37,585.064	452.958	1,185,854.998	136.892	64.00

transaction-log.bench.ts

Transaction log > read 100 iterators while write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	38.23K ops/sec	26.16	11.98	13,549.337	0.574	191,157
🥈 lmdb	2	434.72 ops/sec	2,300.348	72.38	25,138.446	1.54	2,174

Transaction log > read one entry from random position from log with 1000 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	728.48K ops/sec	1.37	1.20	534.318	0.067	3,642,388
🥈 lmdb	2	463.59K ops/sec	2.16	1.08	7,744.739	0.524	2,317,953

worker-put-sync.bench.ts

putSync() > random keys - small key size (100 records, 10 workers)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	834.65 ops/sec	1,198.106	1,054.329	2,318.875	0.315	1,670
🥈 lmdb	2	1.14 ops/sec	874,570.64	807,784.123	974,924.363	3.96	10.00

worker-transaction-log.bench.ts

Transaction log with workers > write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	22.99K ops/sec	43.49	29.77	543.18	0.574	45,986
🥈 lmdb	2	839.91 ops/sec	1,190.6	90.98	15,707.189	5.25	1,680

---

Results from commit 0c43e4b