feat: non-blocking concurrent checkpoint (WAL rotation + MVCC snapshot)

[loganpowell]

Summary

This PR ports Vela-Engineering/kuzu's concurrent multi-writer checkpoint implementation
into ladybug. The implementation was developed by Aikins Laryea (Vela Partners) across
4 phases plus a read-only lock fix; the full diff is 2,360 lines across 45 files.

Before: checkpoint drains all transactions (reads + writes) and blocks new writes
until the entire checkpoint completes.

After: checkpoint only drains write transactions, releases the write gate as soon
as the WAL is rotated, and reads continue concurrently via MVCC snapshot isolation.
Shadow pages are applied with per-page CAS locking so optimistic readers detect updates.

What changed

Core mechanism

• WAL rotation: at checkpoint start, the active .wal is atomically renamed to
  .wal.checkpoint (frozen WAL). New write transactions immediately create a fresh
  active WAL, isolated from the checkpoint.
• Write gate release: once WAL is rotated the gate is released; storage
  materialization runs concurrently with new writers via per-node-group locking.
• MVCC catalog snapshots: Catalog::serializeSnapshot(snapshotTS) and
  CatalogSet::serializeSnapshot(snapshotTxn) use a pinned snapshot timestamp for
  consistent serialization after the gate is released.
• Epoch-based change tracking: Table::hasChanges (bool) replaced by
  changeEpoch (atomic uint64). captureChangeEpochs() snapshots per-table
  watermarks under the write gate; Table::checkpoint() uses watermarks to skip
  tables that haven't changed since the last gate.

Thread safety

• StorageManager::mtx upgraded from exclusive std::mutex to
  mutable std::shared_mutex (reads take shared lock, DDL takes unique lock).
• NodeTable::schemaMtx protects the columns vector during concurrent
  checkpoint vacuum.
• PageManager::version and Catalog::version use std::atomic<uint64_t>
  with a stable lastCheckpointVersion baseline.
• ShadowFile::applyShadowPages now calls BufferManager::updateFrameIfPageIsInFrame
  (CAS-loop locked variant) instead of the lock-free variant.

WAL recovery

• WALReplayer::replay now handles both frozen (.wal.checkpoint) and active
  (.wal) WAL files independently via replayFrozenWAL / replayActiveWAL.

Read-only databases

• File lock is skipped when readOnly=true, enabling concurrent read-only opens.

Apple clang compatibility

• Explicit constructors added to SegmentCheckpointState and LazySegmentData.

Testing

• Updated test/transaction/checkpoint_test.cpp to use the new
  logCheckpointAndApplyShadowPages(bool walRotated) signature, WAL rotation-aware
  flaky checkpointer classes, and ShadowFileDatabaseIDMismatchExistingDB now
  validates the frozen WAL (.wal.checkpoint) rather than the active WAL.
• CI workflow at embed-graph-db/.github/workflows/ladybug-concurrent-writes-ci.yml
  covers build (ubuntu + macos), full ctest suite, checkpoint/concurrent-write test
  filter, and benchmark comparison (blocks on >5% single-writer regression or
  zero read-during-checkpoint throughput).

Checkout Latest Benchmark Run

Attribution

Concurrent checkpoint implementation: Aikins Laryea, Vela Partners
(Vela-Engineering/kuzu, commits 3ad9071..d298e9c)

Ported and adapted for ladybug (lbug namespace, mainStorageManager field,
2-arg applyShadowPages API, std::format, graph catalog support) by Logan Powell.

[adsharma]

@loganpowell we use a clang-format inside a docker container on CI. The ubuntu version has weird patches applied on top of upstream sources. Please use scripts/run-clang-format-docker.sh.

[adsharma]

clang-format-18 on ubuntu is not the same as the clang-format-18 in debian, alpine or homebrew. There are patches in the version that's shipped with ubuntu that annoys people because of this.

#207

[loganpowell]

@adsharma, I rebased and made the clang change as per your suggestion.

[adsharma]

Overall: The "do not block readers while checkpointing" part is more interesting than concurrent write throughput. I don't think the ladybugdb user base cares about concurrent write throughput.

Historically, this project is very closely related to DuckDB, so I'd be careful about deviating substantially from DuckDB's concurrency model. DuckDB supports single writer, but allows multiple-readers during checkpointing.

https://duckdb.org/docs/stable/connect/concurrency

• The CI doesn't need clang-format (already handled in ci-workflow.yml) and this doesn't need to run on every PR.
• Running it as a part of nightly scheduled CI is a good idea

We should also do some antithesis based runs to flush out bugs arising out of this PR or pre-existing issues. Let me know if it's something you're excited about taking on after this is merged.

[loganpowell]

@adsharma the concurrency model is what allows reads during checkpointing. The Vela use-case for concurrency is mult-agent architectures wherein agents can read/write from/to the same graph safely, without any app-level locking. I really liked kuzu and am just happy folks are coming out to keep it alive (🙏). If you want to merge this in and have some specific tests you'd like me to write, I'd be happy to. If you do merge it, I can reach out to the Vela team to see if we can roll them in instead of maintaining divergent forks. In the meantime, I can pull out the clang-formatting.

[adsharma]

multi-agent architectures wherein agents can read/write from/to the same graph safely, without any app-level locking.

This is an interesting use case, but the question is can LadybugDB offer competitive performance vs OLTP tabular databases?

The way I'm imagining data flow is:

{agents} -> pgembed -> duckdb -> ladybugdb -> icebug

The other three are open source projects hosted here: https://github.com/Ladybug-Memory

Also, you had the concurrent write throughput before vs after as a comment on this page. Am I reading this correctly?

baseline:

 1/3 single_writer_txn_per_sec ...
       → 609.3 txn/s
  2/3 checkpoint_latency_ms ...
       → 31 ms
       ...

after:

  1/3 single_writer_txn_per_sec ...
       → 611.4 txn/s
  2/3 checkpoint_latency_ms ...
       → 32 ms
  3/3 concurrent write gtests (4 tests × 4 threads × 1000 inserts) ...
       EmptyDBTransactionTest.ConcurrentNodeInsertions: 1.8s
       EmptyDBTransactionTest.ConcurrentNodeUpdates: 12.6s
       EmptyDBTransactionTest.ConcurrentRelationshipInsertions: 6.0s
       EmptyDBTransactionTest.ConcurrentRelationshipUpdates: 15.3s
       → 447.6 txn/s aggregate
  4/4 read_during_checkpoint_ops_per_sec ...
       → 26.80 ops/s  (0 = blocked/locked, >0 = non-blocking)

So the aggregate throughput is actually lower than single threaded write throughput? With a postgres storage engine in beta right now, I've measured 100k writes/sec in my past life. The only problem is it's stuck in beta for at least 3 years right now.

[adsharma]

Also I misread the duckdb docs around "single writer" model. Here's the relevant paragraph:

DuckDB supports concurrency within a single process according to the following rules. As long as there are no write conflicts, multiple concurrent writes will succeed. Appends will never conflict, even on the same table. Multiple threads can also simultaneously update separate tables or separate subsets of the same table. Optimistic concurrency control comes into play when two threads attempt to edit (update or delete) the same row at the same time. In that situation, the second thread to attempt the edit will fail with a conflict error.

Good news: this PR doesn't break the duckdb concurrency model. I have no objections to it at a conceptual level.

I have some disagreements with the duckdb folks about VFS level concurrency. I think they're stricter than they need to be to the point where a WAL replication setup is impossible on all platforms. But that's a separate topic.

For this particular PR, we just need to work through the 3 review comments and a post-merge MVCC testing plan to flush out any bugs.

[loganpowell]

Sorry, I don't actually see any in-line reviews/comments. Can you reiterate them?
WRT the benchmarks, yes, the concurrency feature seems to actually speed up single-writer TPS. You can see it more clearly here (sorry for the messy/duplicate job summaries). It also slightly increases checkpoint time.

[adsharma]

I think an earlier version of your benchmarking workflow added comments to this PR with output that looked like what you pointed to. But once you force pushed, they disappeared and more recent runs are failing because they don't have permissions to add comments. So I looked into the runs manually and extracted the relevant data.

Benchmarking on github runners is a bit tricky because of the amount of noise and all the problems github is going through.

So I ran some benchmark like this

wget -o bench.py https://gist.github.com/adsharma/36dd6ae67390ec0d612e8564082ad37a
uv run bench.py

baseline.txt

Generating 1,000 rows...

--- Cypher CREATE (vanilla insert) --- 3 iters ---
    Run 1: 7.546s
    Run 2: 7.533s
    Run 3: 7.547s
  Avg Time : 7.542s
  Rows     : 1,000
  Rate     : 133 rows/s

--- Cypher MERGE (no conflicts) --- 3 iters ---
    Run 1: 7.643s
    Run 2: 7.697s
    Run 3: 7.640s
  Avg Time : 7.660s
  Rows     : 1,000
  Rate     : 131 rows/s

--- Cypher MERGE (50% conflicts) --- 3 iters ---
    Run 1: 7.629s
    Run 2: 7.606s
    Run 3: 7.617s
  Avg Time : 7.617s
  Rows     : 1,000
  Rate     : 131 rows/s

branch.txt:

Generating 1,000 rows...

--- Cypher CREATE (vanilla insert) --- 3 iters ---
    Run 1: 7.505s
    Run 2: 7.516s
    Run 3: 7.537s
  Avg Time : 7.519s
  Rows     : 1,000
  Rate     : 133 rows/s

--- Cypher MERGE (no conflicts) --- 3 iters ---
    Run 1: 7.637s
    Run 2: 7.683s
    Run 3: 7.614s
  Avg Time : 7.645s
  Rows     : 1,000
  Rate     : 131 rows/s

--- Cypher MERGE (50% conflicts) --- 3 iters ---
    Run 1: 7.672s
    Run 2: 7.607s
    Run 3: 7.580s
  Avg Time : 7.620s
  Rows     : 1,000
  Rate     : 131 rows/s

I can't detect any benefit on single threaded writes. There are no regressions either. Which is good news.

Like your link says:

Informational only — MVCC write-pipeline overhead; ~2x slower than main is expected trade-off

The cost of enabling concurrent writes is that your throughput drops. In that particular benchmark run by 2x. The main benefit is that multiple writers are able to write at the same time and reads are not blocked. If you want optimal throughput, you're better off using a mutex in the application.

[adsharma]

lastTimestamp is written under a mutex, but read without a mutex (since this is checkpointNoLock()). This is UB under C++ memory rules. How is safety guaranteed here?

[loganpowell]

Good catch. Fixed in 71cb2a7. lastTimestamp is a plain common::transaction_t (non-atomic). Every writer increments it inside mtxForSerializingPublicFunctionCalls (see commit()), but the old code read it bare in checkpointNoLock(). Even though the acquire/release pair on activeWriteTransactionCount gives ordering for the count itself, concurrent access to a non-atomic variable is still UB under the C++ memory model.

We now snapshot it under a brief lock before calling beginCheckpoint():

transaction_t snapshotTimestamp;
{
    std::unique_lock lck{mtxForSerializingPublicFunctionCalls};
    snapshotTimestamp = lastTimestamp;
}
checkpointer->beginCheckpoint(snapshotTimestamp);

See the fix →

[adsharma]

Is this const_cast<> safe?

[loganpowell]

No, it was not safe — and both casts are now removed (71cb2a7, compile fix in 4d02d95).

The root cause: InMemChunkedNodeGroup::flush() and ChunkedNodeGroup::scanCommitted() took non-const Transaction* even though neither function mutates through the pointer. NodeGroupCheckpointState::transaction is const Transaction*, so both call sites needed a cast to paper over the mismatch.

Fix: propagated const Transaction* through the full class hierarchy:

• ChunkedNodeGroup::flush / scanCommitted base class — signatures changed to const Transaction*
• InMemChunkedCSRNodeGroup::flush override — updated to match
• Both call sites in node_group.cpp now pass txn / transaction directly — no cast

[adsharma]

Assuming the WAL related code handle MVCC checkpoints, we should still review secondary data structures:

• Hash index: index.checkpoint(context, pageAllocator) in NodeTable::checkpoint is still called without a snapshotTxn — it uses whatever live index state is present during materialization.
• Stats: tableEntry->vacuumColumnIDs(0) modifies the catalog entry's column ID set while a read transaction may be using those columns.
• serializeCatalogAndMetadata: The catalog snapshot path (serializeCatalogSnapshot) is gated on snapshotTS > 0, which is only set in the new beginCheckpoint(lastTimestamp) path. But the WAL rotation path calls beginCheckpoint then releases the write gate early — new catalog mutations (DDL)
  could race with serialization in finishCheckpoint.

[loganpowell]

vacuumColumnIDs race — Fixed (71cb2a7). tableEntry->vacuumColumnIDs(0) is now called under a std::unique_lock on schemaMtx. Concurrent readers that iterate the column set also acquire schemaMtx, so they will block until the mutation completes:

See the fix →

DDL race in finishCheckpoint — Safe by timestamp exclusion. snapshotTS is captured under the write gate in beginCheckpoint() before the gate is released. finishCheckpoint() calls serializeCatalogSnapshot() (selected when snapshotTS > 0), which calls catalog.serializeSnapshot(serializer, snapshotTS) — only entries with commit timestamp <= snapshotTS are included. Any DDL that commits after the gate releases gets a strictly greater timestamp and is invisible to the serialized snapshot. We added a comment in finishCheckpoint() making this invariant explicit:

See the comment →

Hash index without snapshotTxn — Acknowledged as a pre-existing limitation. HashIndexLocalStorage has no per-entry timestamps, so there is no mechanism to bound the index to snapshotTS without a deeper refactor of the hash-index infrastructure. I added a TODO comment at the call site in NodeTable::checkpoint() and it is tracked as a follow-up; it is not introduced by this PR:

See the TODO →

[loganpowell]

Give me some time to address your comments