Run a database's async commits on up to four concurrent commit threads

[kriszyp]

I don't why previous benchmarks showed no gains from writer concurrency, but it clearly seems to help now.

Stacked on #897 (fix/shared-occ-lock-buckets): this branch uses its occValidation / occLockBuckets settings and updates their guidance. Merge #897 first; GitHub will then retarget this PR to main.

Closes #898.

⊙ Problem

The default commit lane from #694 runs every async RocksDB commit for a database on one thread, and under concurrent load that thread is the bottleneck. With four workers committing 64-key transactions to one database (transaction-log entries on, WAL off), it committed 4.7k transactions/s against 9.5k for the legacy libuv path. A perf profile showed the rocksdb-commit thread at 96-100% of a core: 38% in optimistic validation (CheckKeysForConflicts), 33% in memtable inserts, 10% in the transaction-log write and 0.4% in completion dispatch. RocksDB runs validation and memtable inserts in parallel when it sees concurrent writers, but a single lane never gives it any.

❓ Your call: should concurrent commits be the default? This PR makes commitThreads default to min(4, cores), so commits to one database finish and resolve out of dispatch order, as legacy commits and commitSync() callers always have. The alternative is to ship the setting with a default of 1 and let Harper opt in. Either way it is one config value to change. The Harper audit found nothing that depends on dispatch-order completion (details under Alternatives).

💡 Solution

CommitWorker now runs up to commitThreads dedicated threads per database (RocksDatabase.config({ commitThreads }), default min(4, cores), read when a database is first opened). A thread starts only when queued commits outnumber idle threads, so a database that is never committed to concurrently keeps one. Each thread runs a commit's transaction-log write and its RocksDB commit back to back. Log writes serialize on the store's write mutex, as legacy commits did. Commits never use the libuv threadpool, so #694's fix for fs/dns/crypto starvation holds. commitThreads: 1 restores the previous lane exactly.

Concurrent completion exposed a pre-existing durability race, which this PR fixes. commitFinished() pairs the committed log prefix with a RocksDB sequence for flush correlation, and the sequence was read before taking dataSetsMutex. Between the read and the lock, an earlier log position could commit at a later sequence and finish. A flush at the smaller sequence would then record a replay start past the earlier transaction's unflushed data, and crash replay (startFromLastFlushed, WAL off) would skip it. Legacy mode and commitSync() racing the lane already had this race. The sequence is now read through a callback under that mutex.

⚠️ Look hardest: the flush-correlation invariant. After the position leaves the uncommitted set, under dataSetsMutex, every lower position has already returned from Commit(), so its sequence is no greater than the one read there.

Results on an i7-12700H, with one database, transaction log on, pinned to P-cores, and 3 runs (median and range):

load	lane (before)	concurrent (after)	legacy
4 workers × 64 keys	4.7k/s (4.6-4.8k)	9.7k/s (9.4-9.9k)	9.5k/s (9.2-10.0k)
8 workers × 64 keys	4.6k/s (4.6-4.7k)	9.8k/s (9.6-10.1k)	10.0k/s (9.5-10.2k)
8 workers × 1 key	112.5k/s (108-114k)	194.2k/s (185-197k)	195.3k/s (184-198k)
4 workers × 1,000 keys	311/s (303-315)	342/s (305-342)	334/s (331-337)
no log, 8 workers × 1 key	152.0k/s (143-156k)	288.3k/s (287-290k)	262.9k/s (261-265k)
1 caller, 1 commit in flight, 1 key	32.8k/s, p50 21 µs	33.1k/s, p50 21 µs	30.7k/s
1 caller, 1 commit in flight, 64 keys	1.6k/s, p50 269 µs	1.9k/s, p50 229 µs	1.4k/s, p50 452 µs

The main thread timed fs.stat during a 4 worker × 64 key burst with UV_THREADPOOL_SIZE=4. With the lane, p50/p99 were 14/23 µs. With concurrent commits they were 18/25 µs. With legacy they were 19/560 µs, and in earlier runs legacy reached a p50 of 4-14 ms at 8 workers × 64 keys and 78-121 ms at 1,000 keys.

At 1,000 keys the gain is capped by lock-bucket collisions, not by threads. See the #897 follow-up below.

⚖️ Alternatives

• Route log-bearing commits through the ordered rocksdb-txnlog lane, then to the commit threads. This was the issue's plan, and it was built and measured. It was no faster under concurrency (8 × 1 key: 200k vs 195k/s; 8 × 64 keys: 10.2k vs 9.9k/s). For a caller with one commit in flight it was 13% slower at 1 key and 25% slower at 64 keys, from an extra thread handoff per commit. That caller is Harper's replication receiver, which awaits each commit before applying the next. It remains selectable as ROCKSDB_JS_COMMIT_THREAD=2.
• One process-wide commit pool shared by all databases. This bounds total threads, but a database in a write stall, or holding OCC buckets across one, would occupy shared threads and block commits to every other database. That is the coupling Make optimistic commit lock buckets and validation policy configurable #897 keeps OCC buckets private to avoid. The cost of staying per database is at most commitThreads threads per database, and only for databases that are committed to concurrently.
• Merge queued transactions into one Write() on the lane. RocksDB's optimistic API has no multi-transaction commit, so this would re-implement OCC validation, including conflicts within the merged batch.
• Make legacy mode (ROCKSDB_JS_COMMIT_THREAD=0) the default. It has the same throughput, but it puts commits back on the libuv pool. The --stat-probe runs above measured fs.stat p50 at 4-121 ms under legacy load.

❓ Your call: should Harper drop its planned occValidation: 'serial' default? Under concurrent commits, serial validation gives back the whole gain because RocksDB never batches serial-validated writers. At 4 workers × 64 keys it ran at 4.5k/s against 9.9k/s for parallel, and the lane plus serial ran at 5.1k/s. At 8 workers × 1 key it was 160k against 202k. It came out ahead only at 1,000 keys with the default bucket count (361 vs 336/s). Raising occLockBuckets does better there: 466/s at 2^20 and 685/s at 2^22. My recommendation is to drop it and keep parallel. Harper databases with large transactions should raise occLockBuckets instead.

❓ Your call: two Harper paths get weaker with out-of-order completion. Both races exist today and neither is fixed here. First, same-thread aftercommit delivery, used by crossThreads: false subscriptions, can deliver B's entries before A's. Second, the subscribe cursorMaxTime gate in Table.ts can drop a lower-timestamp commit that lands after a higher one is already visible. Should they be fixed in Harper before it adopts this version, or after?

❓ Your call: if the first commit thread cannot be created (thread exhaustion), the commit runs inline on the calling thread, which may be the JS thread. Before this change the std::thread exception escaped the N-API callback. Rejecting with a retryable error instead would mean unwinding the commit's admission state. Is inline execution acceptable for this edge?

🔧 Changes

• src/binding/database/commit_worker.h: CommitWorker holds a vector of threads and a maxThreads limit. enqueue starts a thread when the queue outnumbers idle threads. A woken thread counts as idle until it retakes the mutex, so testing for an idle thread alone let one thread drain a burst. If no thread can be started, the task runs inline. With one thread the worker is still a lane that drains its queue in order; with more, each thread pops one task. A thread counts itself idle only while it waits. threadCount() backs the new gauge.
• src/binding/database/db_settings.{h,cpp}: the commitThreads setting (default min(4, hardware_concurrency), validated as an integer from 1 through 64 before any setting is stored; getter).
• src/binding/database/db_descriptor.{h,cpp}: the descriptor builds its commit worker with the setting current at open, and the member comment no longer promises dispatch order.
• src/binding/transaction/transaction.cpp: the mode comments describe concurrent threads. Both commitFinished call sites pass a sequence getter (async, sync); commitSync uses its pinned descriptor.
• src/binding/transaction_log/transaction_log_store.{h,cpp}: commitFinished takes the getter and calls it under dataSetsMutex.
• The commitPipeline.commitThreads gauge: db_handle.cpp, src/stats.ts, and docs/stats.md, which also updates the queue-depth descriptions.
• src/load-binding.ts and README.md document commitThreads (typings, README, example). The README notes that log position is not same-key apply order after a retried commit, so consumers must order by transaction timestamp. Both revise the occValidation/occLockBuckets guidance with the measurements above (serial, buckets, typings, validation typings) and the flush-stall notes.
• The commit-threads design note covers why threads are per database, why log writes stay on the commit thread, and the out-of-order invariants. The Make optimistic commit lock buckets and validation policy configurable #897 bucket and serial text is updated for concurrent commits, and the root index links it. AGENTS.md updates the commit-execution map, the env var, and the three invariants that assumed one commit thread (14, 16, 24).
• benchmark/occ-commit-contention.mts: --log writes one transaction-log entry per key, as Harper writes one audit entry per record. --stat-probe times fs.stat on the main thread (report), and --commit-threads sets the limit. Retryable commits are now retried on the same transaction, so --log no longer abandons them.
• stress-test/workers/stress-transaction-put-worker.mts: awaits every commit in a batch rather than the last one, which can settle first.
• binding.gyp: adds the new native test.

✅ Verification

The end-to-end route is fork-fixture integration tests through the public API, plus native tests for the scheduler and the store.

• test/commit-threads.test.ts with test/fixtures/fork-commit-threads.mts, run under modes 1 and 2. Both modes cover these cases:
  • Thread limit: (fixture) with commitThreads: 1, eight commits stalled by ROCKSDB_JS_COMMIT_EXECUTE_DELAY_MS=100 take 800 ms or more. With 4 they take under 600 ms and start four threads after a warm-up, so the pool grows past an idle thread.
  • Out-of-order completion: (fixture) with one thread, a 100k-key commit resolves before a 1-key commit dispatched after it. With four, the small commit resolves first. Each time a commit resolves, every visible log entry belongs to a transaction whose data is readable.
  • libuv starvation: (fixture) with commits stalled and UV_THREADPOOL_SIZE=2, fs.stat returns in under 150 ms. It is skipped on Deno, which runs node:fs outside the pool N-API async work uses, so it cannot fail there. That a stalled legacy commit does delay fs.stat is a runtime property, not a rocksdb-js one, so it is demonstrated by the benchmark's --stat-probe (legacy p50 4-121 ms above) rather than asserted in the unit suite.
  • Config: validation of the setting, lazy start, and the limit staying with the descriptor across handles while a new path picks up a changed setting.
• test/native/commit_worker_test.cc: a burst against a warm idle thread must run four tasks at once. With the old growth rule it failed in every round. It also covers in-order execution with one thread, and shutdown draining queued tasks and then running later tasks inline.
• test/native/transaction_log_flushed_state_test.cc: forces the race schedule with a condition variable. The later position samples sequence 10, the earlier one commits at 11 and publishes, and a flush at 10 must not record a replay start past the earlier position. With the sample moved back before the lock, the test fails (flushed position past the earlier one); with the fix it passes. The existing purge test moves to the callback signature.
• pnpm check, pnpm test (77 files, 1,120 passed), pnpm test:native (303 passed) and STRESS_MODE=essential pnpm test:stress (8 passed) on macOS arm64.
• Benchmarks were run on the Linux box as above, with variant order rotated per run.
• CI: the first run's 13 red legs were 11 jobs that never got a runner (no runner name, no steps, cancelled after 15 minutes in the queue, the same pattern on other PRs' runs at that time) plus the legacy fs-starvation demonstration failing on Deno, which is fixed above. No job hung.
• Crash replay with WAL off, through a real interleaving, is not covered end to end. The window is a few instructions, so the native test pins the invariant instead.

_{🤖 Generated with Claude Code (Claude Opus 5.5); posted via @kriszyp.}

Related PRs: #742 independent (shares DBSettings/load-binding.ts surface only), #767 independent (shares the DBDescriptor initializer list and binding.gyp native-test list; merge-check on combination), #842 independent (check lease admission/drain still holds under concurrent commit threads), #897 overlaps (base branch: this PR is stacked on it; its commits are inherited into this branch), #900 independent, #901 independent (check VT intent release and retry interaction under multiple commit threads), #890 overlaps (merged; both touch the transaction-log store — keep the commitFinished callback signature on merge)
Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex,gemini; adjudicated=domain; declined=cursor-grok,cursor-composer,cursor-kimi,cursor-muse; rounds=3; full=1 @ 679533c}

_{Review-Attention: study ~20m (critical: transaction.cpp, transaction_log_store.cpp +2; decisions: default-commit-threads, per-database-thread-sets, thread-start-failure-fallback, apply-order-contract, stacked-on-897, do-less-alternative) @ 679533c}

[gemini-code-assist]

Code Review

This pull request introduces concurrent commit threads per database (commitThreads) to run async transaction commits, defaulting to min(4, cores). This replaces the single-threaded commit worker bottleneck, allowing RocksDB to validate and apply concurrent commits across multiple cores and group them into single writes. To support out-of-order commit resolution safely, the transaction log's flush correlation logic was updated to sample the latest sequence number under the dataSetsMutex lock, preventing replay gaps. The changes also include new benchmarks, stats, and comprehensive native and integration tests. I have no feedback to provide as there are no review comments.

[cb1kenobi]

Looks good!