[WIP] Systematic testing: Test the full KV + History + Consensus stack, with deterministic and randomly-explored interleavings

[Eddy Ashton (eddyashton)]

This is extremely sketchy, just opening it here for early discussion.

Robot brief explanation

src/commit_concurrency/: a new test suite for KV/Raft/History concurrency

Problem: no existing suite drives a real ccf::kv::Store, a real aft::Aft, and a real MerkleTxHistory together under concurrency. Every existing suite stubs at least one of the three, so bugs in how they interact under real locking/threading go unfound.

What's added, two complementary test binaries:

• commit_concurrency_test — real OS threads, real timing. A Checkpoint primitive (interleaving.h) lets scenarios pin specific rendezvous points; a randomized fuzzer runs writers, elections, and readers concurrently and checks cross-component invariants (history vs. store vs. raft all agreeing on committed state).

• commit_concurrency_model_test — a deterministic scheduler that wholesale-swaps every ccf::pal::Mutex in the binary for a scheduler-aware type (via -include, so no call site opts in individually), and treats every lock acquisition/release (not just contended ones) as a branch point in the search. It can exhaust a scenario's interleavings or sample them randomly with a reproducible seed, and every failing schedule prints a full, human-readable decision trace showing real, semantic lock labels from production code itself (e.g. writer (serialise concurrent Store::commit() calls), elector (roll version and history back to tx_id)), not just an opaque mutex handoff.

Status: two known-bad scenarios are currently red on purpose (tagged NOTE_REJECTED_COMMIT_STALL at the specific broken assertions, with one canonical explanation of the underlying bug), reproducing a real gap in how a commit in flight interacts with a concurrent election: a transaction rejected for a stale view can leave a local write applied to the Store that never reaches consensus, and subsequent ordinary commits can keep failing to replicate until a further election restores agreement. Random sampling shows this is reachable via a wide swath of the interleaving space, not one freak timing (though note: uniform sampling over the abstract schedule space is not representative of real-world timing likelihood — it says something about the shape of the bug, not its production frequency). Both binaries build clean under TSAN and pass the existing kv_test/raft_test/history_test suites unchanged.

Known limitation flagged in-repo (TODO in deterministic_scheduler.h): branching on every single lock/unlock makes the search space for any real-stack scenario enormous (millions to 10^26+ schedules), forcing reliance on random sampling rather than exhaustive search. The next planned step is a per-scenario filter — using the labels already threaded through — to let a test choose a small, targeted set of "interesting" decision points (e.g. specifically the unlock of version_lock in Store::commit()) while everything else fast-passes through unbranched. Intended workflow: random search over the full space to find violations, promote each violation to a pinned deterministic regression test, then fuzz narrowly around those known points for cheap ongoing coverage.

Note for reviewers: this PR is for discussion/sharing, not merge-ready — scripts/ci-checks.sh will fail on the intentional TODO: comment (repo convention normally bans TODO/FIXME outright) and on the two deliberately-red test scenarios described above.

Robot verbose explanation

Motivation

We currently have no test suite that drives a real ccf::kv::Store, a real aft::Aft, and a real ccf::MerkleTxHistory together under concurrency. Every existing suite stubs at least one of the three:

• kv_test — real Store, but consensus is always a stub (StubConsensus/PrimaryStubConsensus/etc.), and TxHistory is absent or hand-rolled.
• raft_test — real Aft, but the store is LoggingStubStore, with no real KV semantics or conflict detection.
• history_test — real Store and MerkleTxHistory, but consensus is a hand-rolled, single-threaded fake.

This gap matters: a prior investigation this session found and fixed a real bug in Store::commit()'s interaction with Store::rollback() under a concurrent view change, and confirming it required building bespoke pause/rendezvous machinery by hand rather than reusing anything. This PR is the generalized version of that machinery, built as its own suite rather than living inside kv_test.

This is additive only. No existing stub classes or test files are touched, consolidated, or removed — they remain legitimate, narrowly-scoped unit tests for their own layers.

What's in src/commit_concurrency/

Two complementary test binaries, both wired into CMakeLists.txt via add_unit_test(...) with DETECT_DEADLOCKS, both under a new concurrency CTest label:

commit_concurrency_test — real OS threads, real timing.

• interleaving.h provides a Checkpoint primitive: any thread can pause at a named point and a controller thread decides when to release it, generalizing the ad hoc pattern from the original bug investigation into one reusable, named primitive.
• threaded/deterministic.cpp ports the two hand-pinned scenarios from that investigation across as a harness sanity check, plus additional scenarios covering election-churn variations your review specifically asked about (stepping down to backup vs. reaching candidate again mid-commit).
• threaded/fuzzer.cpp layers a randomized multi-actor fuzzer on top: N writer threads, an election-churn actor, and a reader thread continuously polling store->current_txid() against history's reported state — mirroring the actual production access pattern in frontend.h that made the original bug externally observable. Seeded (std::mt19937), logged on failure for reproducibility.

commit_concurrency_model_test — a deterministic, single-process scheduler for exhaustive/random exploration of interleavings, rather than relying on real OS thread timing to hit a specific ordering.

• The core trick: ccf::pal::Mutex itself is wholesale-swapped for a scheduler-aware type across the whole binary via -include (interleaving_lock_override.h), so every real lock in store.h/raft.h/history.h participates without any call site opting in. ccf::tasks' own sources are recompiled into this target (not linked from the precompiled ccf_tasks.a), because it keeps a process-wide singleton job board that needs the same scheduler-aware locking to stay consistent across explored schedules.
• deterministic_scheduler.h treats every lock acquisition and release as a decision point (not just contended ones — see "Known limitations" below), plus explicit yield_point()s for branching at points with no lock at all.
• ccf::pal::unique_lock<LockType> (new, in include/ccf/pal/locking.h) is a labeled drop-in replacement for std::unique_lock/std::lock_guard, applied at all 62 real lock call sites across the three production headers. A failing schedule's trace shows real, human-readable reasons a lock was held (e.g. writer (serialise concurrent Store::commit() calls), elector (roll version and history back to tx_id)), not just an opaque sequence of mutex handoffs — this was deliberately built to make failures diagnosable by a human, not just a pass/fail signal.
• explore_all_interleavings() exhausts a scenario via depth-first search with replay; explore_random_interleavings() samples a fixed, seeded number of schedules when the space is too large; estimate_schedule_count() gives a cheap random-walk estimate of a scenario's true size before committing to either.
• model_checked/rejected_commit_stall.cpp reproduces the same bug as the threaded suite, but via this exhaustive/random mechanism instead of hand-pinned timing.

Four production headers (kv/store.h, consensus/aft/raft.h, consensus/aft/impl/state.h, node/history.h) gained a CCF_STATIC_LIBRARY_BUILD-guarded #error, enforcing they can never be compiled into ccf_kv/ccf_tasks/ccfcrypto — this is what makes it safe for commit_concurrency_model_test to recompile them under a different lock type without an ODR violation, and it's enforced at compile time, not just by convention.

Current status: two scenarios are deliberately red

Per explicit direction this session: tests should assert the correct expected behavior and fail honestly when that behavior isn't yet implemented, rather than being skipped or asserting the current (buggy) behavior. Both deterministic.cpp and rejected_commit_stall.cpp have a NOTE_REJECTED_COMMIT_STALL-tagged test/assertions documenting a real gap: a transaction rejected by Store::commit() for a stale view can leave a local write applied to the Store with no corresponding entry ever reaching consensus — and, worse, every ordinary transaction committed afterwards can keep succeeding locally without reaching consensus either, until a further election restores agreement.

Randomly sampling the model-checked version of this scenario (500 samples each, two variants) shows 733 of 1000 sampled schedules that reach the risky state go on to violate the invariant. That's evidence the bug is reachable through a broad part of the interleaving space, not one narrow, contrived ordering — though I want to flag explicitly that this is not a claim about real-world frequency: the scheduler samples uniformly over abstract decision points, which has no relationship to real timing (a lock held for nanoseconds vs. an election taking milliseconds of network round trips). Treat it as evidence about the shape of the bug, not its production likelihood.

Validation

• Normal build: kv_test, raft_test, history_test all pass unchanged (28k+/1M+/46 assertions respectively).
• commit_concurrency_test and commit_concurrency_model_test both build and run cleanly, producing the two known-bad results deterministically across repeated runs.
• Both new binaries build and run clean under -DTSAN=ON with zero data race warnings — meaningful given the whole point of this suite is real concurrent access to shared production state.
• scripts/cpp-format-checks.sh, copyright-checks.sh, includes-checks.sh, and ascii-checks.sh all pass.

What's deliberately not merge-ready, and why this is a draft

• scripts/todo-checks.sh will fail. There's an intentional TODO: comment in deterministic_scheduler.h (repo convention otherwise bans TODO/FIXME outright) describing the single most valuable next step, left in deliberately for this PR's discussion rather than filed elsewhere and forgotten.
• Two test cases are red by design (see above) — this is the point, not an oversight, but it means ctest will report failures for this suite until the underlying store.h bug is fixed.

Known limitation and the planned next step

Branching on every lock/unlock (rather than only contended ones) makes the search space for any real-stack scenario enormous — estimate_schedule_count() reports roughly 16.7 million to 4.4 trillion schedules for the single-writer scenario alone, and up to ~10²⁶ for the two-writer one. That forces reliance on random sampling rather than exhaustive search for anything beyond toy scenarios, which is a real loss of rigor compared to what exhaustive search would give.

The planned fix (see the TODO in deterministic_scheduler.h): treat every lock/unlock/yield_point() as only a candidate decision point, and let a per-scenario predicate — matched against the semantic labels already being reported, so no further production code changes are needed — decide which of them actually branch the search versus fast-passing through unchanged. This lets a test dial the search space down to exactly the handful of points it cares about (e.g. specifically the unlock of version_lock in Store::commit()), rather than an all-or-nothing choice between "every lock branches" (intractable) and "only explicit yield_point()s branch" (may silently miss semantic-lock-ordering bugs). The intended workflow once this exists: random search over the full, unfiltered space to find violations, promote each violation into a deterministic regression test pinned to its exact decision sequence, then fuzz narrowly around those known points for cheap, ongoing, targeted coverage.

Explicitly out of scope for this PR

Consolidating or replacing the existing stub zoo (StubConsensus family, LoggingStubStore, DummyConsensus/CompactingConsensus/RollbackConsensus) is a separate, higher-risk decision, not bundled here.