Skip to content

fix(transaction): in BEGIN, take the write gate before the public mutex - #1516

Merged
adsharma merged 2 commits into
LadybugDB:mainfrom
zahariash:fix/auto-checkpoint-gate-wait
Nov 6, 2026
Merged

adsharma merged 2 commits into
LadybugDB:mainfrom
zahariash:fix/auto-checkpoint-gate-wait

Conversation

@zahariash

Copy link
Copy Markdown
Contributor

This work was produced with the help of language models.

Fixes #1514.

With a write transaction open on B, A's commit starting an automatic checkpoint that waits for B, and a write from C arriving meanwhile, B's COMMIT took 3.6 s instead of 4 ms and a read on a fourth connection took 4.1 s instead of 6 ms. The checkpoint then failed. C's BEGIN held the mutex every BEGIN, COMMIT and ROLLBACK takes while it blocked on the write gate, so B could not leave. Under steady load with three or more writers this repeated until the load stopped. Multi-writer mode is the default since #1435.

Fix. A write BEGIN now takes the write gate first and the public mutex second, so one waiting for a checkpoint holds nothing that open writers or readers need. #1507 solved this for forced checkpoints; its separate wait is now part of the mustWait loop.

The checkpoint now samples the open transactions with a plain lock, since no holder of the public mutex waits on the gate. A failed try_lock there kept the WAL even with nothing open, and every later commit ran another checkpoint. The capture before the gate and both fallbacks are gone.

Writers, time in each transaction Commits/s, main → this PR Worst commit Worst read Checkpoints per run
2, 50 ms 27-28 → 34 0.20 s → 0.13 s 54 ms → 11 ms 1 → 4
3, 50 ms 16, 40 → 50 11.1 s, 0.15 s → 0.14 s 11.1 s, 0.13 s → 8 ms 0, 2 → 3
4, 50 ms 12-13 → 67 11.1 s → 0.15 s 11.1 s → 12 ms 0 → 5
8, 50 ms 25-26 → 136-137 16.7 s → 0.12 s 11.1 s → 11 ms 0 → 5
4, 500 ms 2 → 7 10.9 s → 0.46 s 5.6 s → 13 ms 0 → 5
4, 2 ms, 1 MB threshold 12, 105 → 250, 321 11.1 s → 0.38 s 11.1 s → 6 ms 0, 17 → 40, 51

Two 40-60 s runs per row through the C API, with a reader at 50/s and the default 16 MB threshold unless noted. Four writers at 50 ms with auto_checkpoint=false reach 61-68 commits/s on main, the ceiling for that load.

Cost. Instructions per operation stay within 0.04% of main for a point read and for BEGIN, CREATE and COMMIT, single-threaded. No class layout changes.

Not fixed here. While a long write transaction stays open, a commit that starts the automatic checkpoint still waits out the 5 s timeout; #1515 tracks it.

Tests. WriterWaitingBehindAutoCheckpointLetsOthersThrough has a writer arrive while a checkpoint waits for an open one, then expects a read and the open writer's COMMIT to finish within 5 s against a 10 s checkpoint timeout; on main they take 11 s. WriterWaitingAtWriteGateDoesNotPinCheckpoint has a BEGIN arrive as the checkpoint starts and expects an empty WAL; main leaves 216 bytes. BusyPublicMutexDoesNotPinCheckpoint holds the public mutex for 100 ms at the same point and expects the same; it fails with only the BEGIN change. Full ctest: 3360/3360.

zahariash and others added 2 commits November 5, 2026 12:42
A checkpoint closes the write gate, then waits up to the checkpoint wait
timeout for open write transactions to leave. A write BEGIN arriving
meanwhile took the public mutex and then blocked on the gate while holding
it. Every BEGIN, COMMIT and ROLLBACK needs that mutex, so the open writers
could not leave, reads could not start, and the checkpoint timed out. Its
next attempt, on the next commit past the threshold, met the same state, so
under steady load with three or more writers the database stalled for the
timeout over and over and no checkpoint completed.

A write BEGIN now takes the gate first and the public mutex second. A BEGIN
waiting for a checkpoint holds nothing the open writers or readers need. The
wait for a pending forced checkpoint at the top of beginTransaction existed
for this same reason and is now part of the mustWait loop.

A BEGIN waiting at the gate also made the checkpoint's try_lock on the public
mutex fail, which pinned the WAL with nothing open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The checkpoint sampled the open transactions after closing the write gate
with a try_lock, because a write BEGIN could hold the public mutex while
blocked on the gate. When the try_lock failed it fell back to a horizon
captured before the gate, or to its own snapshot timestamp if nothing had
been open. Either way the checkpoint counted as pinned and kept the WAL, even
with no transaction open, so the WAL grew past the threshold and every later
commit ran another checkpoint.

No holder of the public mutex blocks on the gate any more, so the checkpoint
now takes the mutex and always samples what is really open. The capture before
the gate and both fallbacks are gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adsharma

adsharma commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Note on what the two mutexes protect (for future readers):

mtxForSerializingPublicFunctionCalls ("public") protects the transaction registry — all state mutated by BEGIN/COMMIT/ROLLBACK: activeTransactions, openTxnStartTS (txn ID → start timestamp for the checkpoint retention horizon), cachedHorizon, lastTransactionID, forcedCheckpointPending, nextWALCommitSequenceToPublish and the two CVs. Every public entry point (beginTransaction, commitAndCheckpoint, rollback, waitForOtherWritersBeforeForcedCheckpoint, snapshotHorizon*()) takes it briefly.

mtxForStartingNewTransactions ("gate" / write gate) protects no data — it is admission control. A checkpoint holds it for its entire run: stopNewWriteTransactionsAndWaitUntilAllWriteTransactionsLeave() locks it, polls activeWriteTransactionCount == 0 (drain), and keeps it held through storage phase + finish (releasing early corrupts version chains — see the NOTE in checkpointNoLock). A write/recovery BEGIN must acquire it; read-only BEGIN skips it via snapshot isolation. While held, new writers queue on gate.lock() holding nothing, so open writers can still leave via COMMIT/ROLLBACK (public only) and readers can still start (public only).

Two mutexes exist so the drain can wait without blocking the very writers it waits for. The ordering invariant after this PR is gate → public: nothing may block on the gate while holding public, which is what lets checkpointNoLock() (holding the gate) safely take the public mutex with a blocking lock to sample openTxnStartTS.

@adsharma
adsharma merged commit 3b5a3b6 into LadybugDB:main Nov 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: with three or more writers, an automatic checkpoint freezes reads and commits

2 participants