Repository navigation
perf(engine): keep checkpoint syncs and the rebind walk off the locks commits wait on - #99
Merged
Merged
Conversation
WriteAheadLog.sync() held the WAL lock for the whole fsync. A checkpoint syncs the log after appending its Checkpoint record, and a committing writer appends while it holds the commit lock, so every writer queued behind that fsync. The same happened to appends waiting on a group commit's sync under durability = sync. sync() now takes a separate sync lock, reads and clears the dirty flag under the WAL lock, and forces the active segment after releasing it. Syncs queue on the sync lock, so a sync that finds nothing dirty only returns after the force that covered its records. If the segment rolls while it is being forced, the roll has already forced and closed it, so the failed force on the closed file is ignored. truncateBefore() also deletes old segment files after releasing the lock. The new WAL_FORCE crash point sits between releasing the lock and the force. CheckpointConcurrencyTest commits from another thread while a checkpoint is held there, crashes checkpoints at CHECKPOINT_CATALOG, WAL_FORCE and CHECKPOINT_JOURNAL under three writers, and runs four writers through back-to-back checkpoints.
After a checkpoint publishes, TransactionManager.checkpointed swapped the pages it wrote into the head generation while holding the commit lock. That walk visits every pending node the checkpoint did not write, and no commit could start until it finished. Materializer.Rebinding now remembers its result for each pending node it visits. The checkpoint runs one pass over the head without the lock, then takes the commit lock, drains the group committer as before, and runs a second pass over the current head. The second pass looks up nodes the first one visited and only walks what commits added in between, so a commit that lands between the passes is kept. A node a commit stamps with the epoch after the first pass saw it stays pending until the next checkpoint, which reuses its page. The new CHECKPOINT_REBIND crash point sits between the two passes. CheckpointConcurrencyTest commits while a checkpoint is held there and checks the commit survives the rebind and a reopen, and crashes there under three writers.
venkat1701
force-pushed
the
perf/checkpoint-off-commit-lock
branch
from
October 14, 2026 13:30
a6effd5 to
04c7bfc
Compare
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
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.
Refs #46
What a commit waited on
The last comment on #46 says a checkpoint publishes the catalog and syncs the WAL while holding the commit lock. On main that is only partly true.
catalog.publish, the reference journal, the feed sync andapplyJournaldon't take any lock a commit needs. Two things do:WriteAheadLog.sync()held the WAL lock for the whole fsync. The checkpoint calls it after appending itsCheckpointrecord. A writer appends to the WAL while it holds the commit lock, so the first writer to arrive waited for the fsync with the commit lock in hand, and every other writer waited behind it on the commit lock. Group commit underdurability = synchad the same problem: appends waited for the leader's fsync.TransactionManager.checkpointedheld the commit lock while it walked the head generation to swap the checkpoint's pages in (rebind). The walk visits every pending node the checkpoint did not write.truncateBeforealso deleted old WAL segment files under the WAL lock.Changes
WriteAheadLog.sync()takes a separate sync lock, reads and clearsdirtyand picks the active segment under the WAL lock, then forces after releasing it. Appends carry on during the force. Syncs queue on the sync lock, so async()that finds nothing dirty only returns after the force that covered its records has finished. If the log rolls during a force, the roll has already forced and closed that segment, so the failed force on the closed file is not an error. Any other failure setsdirtyagain.truncateBeforeremoves the segments from the list under the lock and deletes the files after.Materializer.RebindingreplacesMaterializer.rebound. It remembers its result for every pending node it visits.checkpointedruns one pass over the head without the lock, then takes the commit lock, drains the group committer as before, and runs a second pass over the head as it is now. The second pass looks up what the first already did and only walks nodes that commits added in between.The checkpoint steps run in the same order as before, and nothing written to disk changed.
Commits landing between the steps
Recovery reads the WAL from the catalog's
checkpointLsn, which is the end of the target generation'sCommitrecord. The journal is tagged with the same LSN.Checkpointrecord, so after the LSN. Recovery replays it if it reached the disk. This sync never promised it would: underasyncnothing does, and undersyncthe commit's own group commit forces again, after waiting on the sync lock.truncateBefore(lsn)runs after the catalog with that LSN is durable, same as before. Only the file deletion moved.New crash points:
WAL_FORCE(lock released, before the force) andCHECKPOINT_REBIND(between the two passes).Tests
CheckpointConcurrencyTest:WAL_FORCEorCHECKPOINT_REBIND. The commit must finish within 5 s and still be there after the rebind and after a reopen. Both cases fail on main's locking (moving the force back inside the WAL lock, or taking the commit lock before the first pass, makes the commit wait for the checkpoint).CHECKPOINT_CATALOG,CHECKPOINT_REBIND,WAL_FORCEandCHECKPOINT_JOURNALwhile three writers commit undersyncdurability, reopens twice, and checks that every acknowledged commit is back, that nothing else came back except commits that were in flight, and the reference counts. A crash atWAL_FORCEcan also land in a commit batch the checkpoint drains, so there the writers may fail too.WriteAheadLogTest.aSyncWhoseSegmentRollsAwayDuringTheForceSucceedsrolls the log from inside theWAL_FORCEhook. Without the check for a rolled segment the sync throws.Measurements
Probe: a 20,000-edge store (4 members each),
durability = async, background checkpoints off. One or four writer threads commit single-member inserts into random edges in a loop while the main thread runs 30 checkpoints back to back. JFR records everyjdk.ThreadParkon the writer threads with stack traces. For each checkpoint the probe takes the longest park that overlaps it, split into parks insideTransactionManager.commit(the commit lock) and insideWriteAheadLog(the WAL lock). Ten rounds per variant, main and branch interleaved, so 300 checkpoints each. Same probe class against the two engine jars.The laptop was running other builds the whole time (load average between 30 and 80), so treat these as a rough picture, not as latency numbers.
Longest wait per checkpoint, one writer, in ms:
With four writers the WAL lock wait drops the same way (p50 10.73 to 0.07 ms, p90 29.00 to 6.21 ms). The commit lock wait does not move (p50 124 ms on both). With four writers on a machine this busy, a writer mostly waits for the other writers, and the probe can't tell those waits apart from waits on the checkpoint. The worst commit latency per checkpoint did not change in a way I can separate from the noise on either writer count; at the tail it is CPU scheduling, not locks.
Nothing on disk changed, so bytes written per commit are the same as main.
Left open
mixedwriter p99 needs its own profile. A writer can also still wait for a checkpoint through the write buffer (awaitRoom), which this does not change.durability = syncthat drain can wait for a commit batch's fsync. Dropping the drain needs a different way to keepcurrentfrom running ahead of a commit that is not yet durable, so I left it.SegmentFile.forceclears its dirty flag before forcing, so two concurrentpages.sync()calls can let the second return before the first force finishes. That is on main too and is not touched here.StorageEngineconstructor: they change thePageStore.opencall on the line above theWriteAheadLog.opencall this branch changes. Keep both edits.