From c41edc515ef44721e786985628f2af65ea85bc29 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 18 Jun 2026 06:29:47 +0000 Subject: [PATCH] feat(ha-commit-durability): cap commit self-contribution at durable_lsn + durable-advance commit wake (task 2.2) --- .kiro/specs/ha-commit-durability/tasks.md | 3 +- src/controller/commit.c | 61 +++++- src/controller/persist.c | 56 +++++- .../testsuite/test_coldstart_lsn0.c | 4 + .../testsuite/test_commit_barrier_rollout.c | 5 + .../testsuite/test_commit_durability.c | 65 +++++++ src/controller/testsuite/test_commit_engine.c | 177 ++++++++++++++++++ .../testsuite/test_demotion_convergence.c | 8 + src/controller/testsuite/test_repl_majority.c | 6 + src/include/persist.h | 23 +++ 10 files changed, 395 insertions(+), 13 deletions(-) diff --git a/.kiro/specs/ha-commit-durability/tasks.md b/.kiro/specs/ha-commit-durability/tasks.md index 0f36ad2..701f527 100644 --- a/.kiro/specs/ha-commit-durability/tasks.md +++ b/.kiro/specs/ha-commit-durability/tasks.md @@ -31,7 +31,7 @@ - _Requirements: 1.1, 1.6_ - _Boundary: DurableCommit_ -- [ ] 2.2 Count self toward the commit majority only when durable +- [x] 2.2 Count self toward the commit majority only when durable - Change the commit-majority evaluation so the local member's self-contribution is capped at the durable-LSN watermark, not the appended tip; clamp the watermark on WAL truncation. - Observable: a unit test shows `committed_lsn` does not advance over an entry until that entry is durable locally, and the self-cap equals the durable-LSN. - _Requirements: 1.2, 1.4_ @@ -124,3 +124,4 @@ - 1.2: Test seam = `persist_test_durable_lsn_on_disk(persist_t*)` (decl `persist.h`, impl `persist.c`, all `#ifdef XCE_TESTING`) — re-opens WAL segment files from disk and CRC-validates the chain (read-only, state-free) to return the highest fully-persisted LSN; new test `testsuite/test_commit_durability.c` (registered in `meson.build` + `controller_test.c`). Production `xce_controller` links without it (proven via `nm`). DRY NOTE: ~115 lines of segment-scan/CRC-chain logic are duplicated from the static `replay_wal`; tasks 3.1 (recovery verdict) and 3.4 (durable floor from WAL tip) touch the SAME scan/CRC logic — consider extracting a shared read-only `wal_scan_durable_lsn`-style helper there and routing this seam through it (avoid a third copy). `XCE_TESTING` is defined only for `controller_unit_test` (`src/controller/meson.build:167`). - 1.3: Deterministic mode = `bash src/tools/test-commit-durability.sh --deterministic` proves the durability GAP via strace, NOT joblist data-loss. SCOPE CLARIFICATION (review-ratified): single-host SIGKILL committed-job loss is physically NON-deterministic — `persist_wal_async` pwrites inline (page cache survives process death) and standbys fdatasync-before-WalAck (persist.c:515), so a committed job is always durable off the primary; a data-loss run flips GREEN/RED/RED. The deterministic slice: with `--wal-sync-ms 30000` the primary acks "jobsubmit ok" with ZERO primary fdatasync in the submit→ack window (exec-wrap `strace -f -tt -e trace=fdatasync,fsync`; PTRACE_ATTACH `-p` is yama-blocked). RED = in-window primary syncs==0 (15/15 across 5 runs; primary=0 vs standby=2). This is a valid RED→GREEN gate: the task-2.x fsync-before-ack fix makes the primary count ≥1 ⇒ GREEN. Original racy soak retained as the default mode. Boundary: only `src/tools/test-commit-durability.sh` (flag plumbed via deploy-ha.sh start.sh `$*` passthrough — no C edits). For task 5.2's E2E, prefer this strace gap-proof over joblist-loss for a deterministic durability assertion. - 2.1: `persist_t.durable_lsn` (`_Atomic uint64_t`, accessor `persist_durable_lsn()`) = prefix watermark advanced ONLY after a successful `fdatasync`. `fsync_worker` is now a demand-driven group-commit flusher: `persist_wal_async` pwrites under `p->lock`, releases, then signals `fsync_cond`. Lost-wakeup guard is LEVEL-triggered on `flusher_work_pending_locked` (`durable_lsn < current_lsn-1`), re-checked under `p->lock` before wait and after each flush (reviewer-confirmed sound). Invariants: monotonic, `durable_lsn <= current_lsn-1`, clamped on truncate (`clamp_durable_lsn_locked` at both truncate paths). fail-loud rc-gate: `durable_lsn` does NOT advance on `fdatasync` error (all 4 sites: flusher main/fallback, `persist_stop`, `persist_flush`) — logs ERROR. Test fault-injection seam: `#ifdef XCE_TESTING` toggle `persist_test_set_force_fsync_fail()` (production-excluded, nm-verified). NOT yet consumed by commit (that is 2.2). FOLLOW-UP (non-blocking, out of scope for 2.1): bound the flusher retry on SUSTAINED fdatasync failure (backoff / rate-limited logging) — currently tight-spins (releases `p->lock` each iter, so liveness OK, but CPU/log burn on a degrading disk). NOTE: a HANGING (not erroring) fdatasync blocks appends since it runs under `p->lock` — pre-existing, not 2.1's boundary. +- 2.2: `majority_candidate_locked` (commit.c:93) self-cap = `persist_durable_lsn(ce->p)` (was `highest_appended_lsn`); appended tip kept only for the `assert(cand <= highest_appended_lsn)` never-beyond-log check. So `committed_lsn` cannot advance over an entry until it is durable locally (Req 1.2/1.4). Durable-advance WAKE (the wire 2.1 deferred): persist defines a callback hook `on_durable_advance(void*)`/`durable_cb_ctx` + `persist_set_durable_cb()`; commit registers `commit_on_durable_advance` (ctx=engine) in `commit_engine_init`, unregisters in `commit_engine_fini`. Flusher invokes it ONLY when `durable_lsn` advanced, AFTER releasing `p->lock` → callback takes `ce->lock` + broadcasts `ce->cond`. Respects commit→persist dependency direction (persist never includes commit). Lock order p->lock→ce->lock only (cycle-free). Lost-wakeup-free: broadcast follows the release-store; waiter re-reads durable under ce->lock; timedwait backstop. Shutdown UAF closed: `persist_stop` JOINS the flusher BEFORE `commit_engine_fini` (cluster.c:1675 vs 1695); final stop-flush does NOT fire the callback; fini also unregisters defensively. GOTCHA (bug-704): self-cap broke 4 existing commit-path fixtures (test_repl_majority/coldstart_lsn0/commit_barrier_rollout/demotion_convergence) that primed `current_lsn` but not `durable_lsn` — fix = prime `durable_lsn = appended tip` in any in-memory fixture that sets current_lsn without writing real WAL bytes (the flusher can't advance durable for a fake log). Any future commit-engine test must prime durable_lsn too. diff --git a/src/controller/commit.c b/src/controller/commit.c index a660872..13b1bc4 100644 --- a/src/controller/commit.c +++ b/src/controller/commit.c @@ -80,11 +80,23 @@ static uint64_t majority_candidate_locked(commit_engine_t* ce) { replicator_t* r; uint64_t cand; - uint64_t cap = highest_appended_lsn(ce->p); + /* DURABILITY PRECONDITION (ha-commit-durability Req 1.2): the PRIMARY's + * self-contribution to the commit majority is capped at its DURABLE + * watermark (the highest LSN fdatasync'd locally), NOT the appended tip. + * An entry that is only in the page cache (appended but not yet synced) + * must not count self toward its majority — otherwise "committed" could be + * durable on a single member and lost across failover (bug-661). The + * standby side already persists-before-ACK (persist_wal_write_replicated), + * so once self is durable-capped, committed_lsn ⇒ durable on a majority. + * highest_appended_lsn is still used below as the never-beyond-the-log + * bound (it can only exceed durable_lsn, so it never loosens this cap). */ + uint64_t cap = persist_durable_lsn(ce->p); int n = 0; int need, k, i; - /* Standalone / single-member: self constitutes the majority (Req 6.3). */ + /* Standalone / single-member: self constitutes the majority (Req 6.3) — + * but self still only counts up to what is durable locally, so a + * single-member commit also cannot advance over a not-yet-synced entry. */ if (ce->n_peers <= 1) { return cap; } @@ -184,6 +196,37 @@ static void wait_deadline(struct timespec* dl, uint32_t timeout_ms) } } +/* Durable-advance wake (ha-commit-durability task 2.2): invoked by the persist + * group-commit flusher AFTER it advances durable_lsn and AFTER it releases + * p->lock. Now that the commit majority caps self at durable_lsn, an advance of + * durable_lsn can newly satisfy a pending entry's majority even when no standby + * ACK arrived — re-evaluate and wake blocked COMMIT_MODE_WAIT callers so they + * complete promptly instead of stalling until their timeout (the lost-wakeup + * window: all standby ACKs landed before the local fdatasync finished, so + * commit_on_ack already fired and nothing else would re-check). + * + * LOCK ORDERING: this runs on the flusher thread with NO persist lock held; it + * takes only ce->lock. The commit side never takes p->lock while holding + * ce->lock, so the only edge is p->lock → ce->lock (cycle-free, deadlock-free). + * ctx is the commit_engine_t registered in commit_engine_init. */ +static void commit_on_durable_advance(void* ctx) +{ + commit_engine_t* ce = ctx; + + if (ce == NULL || ce->p == NULL) { + return; + } + pthread_mutex_lock(&ce->lock); + commit_advance_locked(ce); /* re-derives majority with the new durable cap; + * broadcasts ce->cond on any advance */ + /* Broadcast even when committed_lsn did not advance: the durable watermark + * moved, so a waiter's wait_done predicate (which the durable cap feeds via + * commit_advance_locked) must be re-checked. commit_advance_locked only + * broadcasts on a committed_lsn change, so cover the no-change case here. */ + pthread_cond_broadcast(&ce->cond); + pthread_mutex_unlock(&ce->lock); +} + /* ============================================================ * Lifecycle * ============================================================ */ @@ -201,6 +244,12 @@ int commit_engine_init(commit_engine_t* ce, persist_t* p, int n_peers, int n_vcp ce->inflight_waits = 0; ce->writable_lsn = 0; /* no own-term gate until set by promotion */ memset(ce->scratch, 0, sizeof(ce->scratch)); + + /* Register the durable-advance wake so the flusher can re-trigger commit + * evaluation when durable_lsn moves (task 2.2). Persist owns the hook slot; + * we only register a function + ctx, preserving the commit→persist (never + * the reverse) dependency direction. */ + persist_set_durable_cb(p, commit_on_durable_advance, ce); return 0; } @@ -209,6 +258,14 @@ void commit_engine_fini(commit_engine_t* ce) if (ce == NULL) { return; } + /* Unregister the durable-advance wake BEFORE tearing down the mutex/cond: + * otherwise the flusher could fire commit_on_durable_advance against a + * destroyed engine (use-after-free / ASAN). Callers are expected to have + * stopped the flusher (persist_stop) first, but unhooking is defensive and + * cheap. After this, persist holds no pointer to ce. */ + if (ce->p != NULL) { + persist_set_durable_cb(ce->p, NULL, NULL); + } pthread_cond_destroy(&ce->cond); pthread_mutex_destroy(&ce->lock); ce->p = NULL; diff --git a/src/controller/persist.c b/src/controller/persist.c index 555d38f..0803177 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -345,17 +345,36 @@ static void* fsync_worker(void* arg) * landed during the flush, the level is still pending and we flush * again without needing its signal (lost-wakeup-safe). */ pthread_mutex_unlock(&p->fsync_mutex); - flusher_flush_once(p); + uint64_t before = atomic_load_explicit(&p->durable_lsn, + memory_order_acquire); + uint64_t after = flusher_flush_once(p); - /* COMMIT-EVAL WAKE (ordering fixed here; consumer wired by task 2.2): - * durable_lsn is advanced INSIDE flusher_flush_once (atomic store with - * release) BEFORE we return here. A WAIT caller's commit evaluation - * reads durable_lsn via an acquire load, so once it is signalled it - * observes the advanced watermark. Task 2.2 adds the explicit - * commit-eval condvar broadcast at THIS point (after the durable_lsn - * advance) — the "advance THEN wake" ordering this loop establishes is - * what makes that wake lost-wakeup-safe. Nothing consumes durable_lsn - * for commit yet, so no wake is needed in 2.1. */ + /* COMMIT-EVAL WAKE (task 2.2 consumer wired): durable_lsn is advanced + * INSIDE flusher_flush_once (atomic store with release) BEFORE we + * return here, and flusher_flush_once has already RELEASED p->lock. + * Now — holding NO persist lock — invoke the registered durable-advance + * callback so the commit engine re-evaluates its majority and wakes any + * COMMIT_MODE_WAIT caller whose entry just became durable locally. + * + * LOST-WAKEUP SAFETY: the wake fires strictly AFTER the durable_lsn + * release-store, and the commit engine's predicate re-reads durable_lsn + * via an acquire load under ce->lock — so a WAIT caller either (a) sees + * the advanced watermark on its own re-check, or (b) is blocked in + * pthread_cond_wait and is woken by this broadcast; it can never both + * miss the watermark AND miss the wake. We only fire the callback when + * durable_lsn actually advanced (after > before): a no-op flush (or a + * failed fdatasync that held the watermark) needs no commit re-eval, + * avoiding spurious wakes. + * + * LOCK ORDERING (cycle-free): the flusher releases p->lock inside + * flusher_flush_once, THEN the callback takes ce->lock (commit side). + * The commit side only ever takes ce->lock and reads persist atomics — + * it never takes p->lock while holding ce->lock. So the only ordering + * is p->lock → ce->lock; there is no ce->lock → p->lock edge, hence no + * deadlock cycle. The callback explicitly MUST NOT take p->lock. */ + if (after > before && p->on_durable_advance != NULL) { + p->on_durable_advance(p->durable_cb_ctx); + } pthread_mutex_lock(&p->fsync_mutex); } @@ -415,6 +434,12 @@ int persist_init(persist_t* p, persist_config_t* cfg, cluster_t* c) * flusher thread after a successful fdatasync (ha-commit-durability 2.1). */ atomic_store(&p->durable_lsn, 0); + /* Durable-advance wake hook starts unregistered (ha-commit-durability 2.2): + * the commit engine registers it in commit_engine_init. Until then every + * flusher durable advance is a no-op wake. */ + p->on_durable_advance = NULL; + p->durable_cb_ctx = NULL; + /* Create WAL and snapshot directories */ if (mkdir_p(p->wal_path, 0700) != 0) return -errno; if (mkdir_p(p->snap_path, 0700) != 0) return -errno; @@ -755,6 +780,17 @@ uint64_t persist_durable_lsn(persist_t* p) return atomic_load_explicit(&p->durable_lsn, memory_order_acquire); } +void persist_set_durable_cb(persist_t* p, void (*fn)(void* ctx), void* ctx) +{ + if (!p) + return; + /* Set ctx before fn so the flusher (which checks fn != NULL) never sees a + * live callback with a stale/uninitialised ctx. Registration happens in + * commit_engine_init, before the engine is reachable by any ACK path. */ + p->durable_cb_ctx = ctx; + p->on_durable_advance = fn; +} + #ifdef XCE_TESTING void persist_test_set_force_fsync_fail(persist_t* p, int fail) { diff --git a/src/controller/testsuite/test_coldstart_lsn0.c b/src/controller/testsuite/test_coldstart_lsn0.c index 9116218..dcf28a8 100644 --- a/src/controller/testsuite/test_coldstart_lsn0.c +++ b/src/controller/testsuite/test_coldstart_lsn0.c @@ -79,6 +79,10 @@ static void ce_setup(int n_peers, int n_standbys, uint64_t appended_lsn) * sentinel), so current_lsn = 1 = "first entry will be lsn 1". */ g_p.current_lsn = appended_lsn + 1; atomic_store(&g_p.committed_lsn, 0); + /* ha-commit-durability 2.2: self caps at durable_lsn. Stub fixture (no + * flusher) — treat the appended log as durable up front, preserving the + * pre-durability "self covers every appended entry" model. */ + atomic_store(&g_p.durable_lsn, appended_lsn); TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&g_ce, &g_p, n_peers, 8)); } diff --git a/src/controller/testsuite/test_commit_barrier_rollout.c b/src/controller/testsuite/test_commit_barrier_rollout.c index 402fad4..84546fa 100644 --- a/src/controller/testsuite/test_commit_barrier_rollout.c +++ b/src/controller/testsuite/test_commit_barrier_rollout.c @@ -496,6 +496,11 @@ static void barrier_setup_vcpus(int n_peers, int n_standbys, g_bp.current_lsn = current_lsn; atomic_store(&g_bp.committed_lsn, 0); + /* ha-commit-durability 2.2: self caps at durable_lsn. This stub has no + * flusher, so treat the whole appended log (current_lsn - 1) as durable + * up front, preserving the pre-durability "self covers every appended + * entry" model. (current_lsn == 0 → nothing appended → durable 0.) */ + atomic_store(&g_bp.durable_lsn, current_lsn > 0 ? current_lsn - 1 : 0); TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&g_bce, &g_bp, n_peers, n_vcpus)); diff --git a/src/controller/testsuite/test_commit_durability.c b/src/controller/testsuite/test_commit_durability.c index 33c4df5..164db05 100644 --- a/src/controller/testsuite/test_commit_durability.c +++ b/src/controller/testsuite/test_commit_durability.c @@ -8,6 +8,7 @@ #include +#include "commit.h" #include "config.h" #include "persist.h" @@ -433,6 +434,69 @@ static void test_durable_lsn_holds_on_fsync_failure(void) printf("test_durable_lsn_holds_on_fsync_failure: PASS\n"); } +/* ================================================================== * + * task 2.2 — durable-advance wake drives commit (integration) * + * * + * End-to-end over a REAL persist_t + commit_engine_t (single-member, * + * NULL cluster): the group-commit flusher advances durable_lsn and * + * fires the registered durable-advance callback, which the commit * + * engine uses to advance committed_lsn and wake WAIT callers. Proves * + * the persist↔commit wiring works against the real flusher thread — * + * not just the directly-driven callback (covered in * + * test_commit_engine.c). n_peers=1 means self alone is the majority, * + * so committed_lsn tracks durable_lsn with no standby ACKs needed. * + * ------------------------------------------------------------------ */ +static void test_durable_advance_drives_commit_single_member(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_durcommit_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + persist_t* p = make_durability_persist(tmp); + TEST_ASSERT_NOT_NULL(p); + + /* Single-member commit engine over the real persist. n_vcpus large enough + * to admit a wait. commit_engine_init registers the durable-advance cb. */ + commit_engine_t ce; + TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&ce, p, 1, 8)); + + /* committed starts at 0; nothing appended/durable. */ + TEST_ASSERT_EQUAL_UINT64(0, atomic_load(&p->committed_lsn)); + + /* Append three entries (LSN 1,2,3). They are in page cache only — the slow + * 5s cadence means the append-SIGNAL (not the timer) must drive the flush; + * the real flusher fdatasyncs, advances durable_lsn, and fires the cb, + * which advances committed_lsn for this single-member quorum. */ + TEST_ASSERT_EQUAL_INT(0, persist_wal_async(p, WAL_JOB_UPDATE, "a", 1)); + TEST_ASSERT_EQUAL_INT(0, persist_wal_async(p, WAL_JOB_UPDATE, "b", 1)); + TEST_ASSERT_EQUAL_INT(0, persist_wal_async(p, WAL_JOB_UPDATE, "c", 1)); + + uint64_t tip = p->current_lsn - 1; /* 3 */ + TEST_ASSERT_EQUAL_UINT64(3, tip); + + /* Wait for the flusher to make the tip durable AND for the wake-driven + * commit to catch up. committed_lsn must reach the tip — driven by the + * durable-advance callback, not by any ACK (there are no standbys). */ + uint64_t committed = 0; + for (int i = 0; i < 5000; i++) { + committed = atomic_load(&p->committed_lsn); + if (committed >= tip) + break; + usleep(1000); + } + TEST_ASSERT_EQUAL_UINT64_MESSAGE( + tip, committed, + "durable-advance callback must drive committed_lsn to the durable tip"); + /* committed never exceeds durable (the invariant the wake preserves). */ + TEST_ASSERT_TRUE(atomic_load(&p->committed_lsn) <= persist_durable_lsn(p)); + + /* Tear the engine down BEFORE the persist (unhooks the callback so the + * flusher cannot fire against a destroyed engine). */ + commit_engine_fini(&ce); + teardown_durability_persist(p); + printf("test_durable_advance_drives_commit_single_member: PASS\n"); +} + #endif /* XCE_TESTING */ /* ------------------------------------------------------------------ */ @@ -449,6 +513,7 @@ void test_commit_durability(void) RUN_TEST(test_durable_lsn_never_exceeds_synced_tip); RUN_TEST(test_durable_lsn_invariants_under_load); RUN_TEST(test_durable_lsn_holds_on_fsync_failure); + RUN_TEST(test_durable_advance_drives_commit_single_member); #else printf("test_commit_durability: skipped (no XCE_TESTING)\n"); #endif diff --git a/src/controller/testsuite/test_commit_engine.c b/src/controller/testsuite/test_commit_engine.c index a5f3ee8..8d2c005 100644 --- a/src/controller/testsuite/test_commit_engine.c +++ b/src/controller/testsuite/test_commit_engine.c @@ -112,10 +112,37 @@ static void engine_setup_vcpus(int n_peers, int n_standbys, uint64_t appended_ls } g_p.current_lsn = appended_lsn + 1; atomic_store(&g_p.committed_lsn, 0); + /* ha-commit-durability 2.2: the commit majority now caps self at + * durable_lsn, not the appended tip. By default treat every appended + * entry as already fdatasync'd locally (durable_lsn == appended tip), so + * the pre-durability test semantics ("self covers every appended entry") + * are preserved. Tests that exercise the self-cap lower durable_lsn + * explicitly via set_durable() after setup. */ + atomic_store(&g_p.durable_lsn, appended_lsn); TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&g_ce, &g_p, n_peers, n_vcpus)); } +/* Set the local durable watermark (highest LSN fdatasync'd locally). Mirrors + * what the persist group-commit flusher does after a successful fdatasync; + * here it is driven directly so the commit-engine self-cap can be tested in + * isolation from real disk I/O. */ +static void set_durable(uint64_t durable_lsn) +{ + atomic_store(&g_p.durable_lsn, durable_lsn); +} + +/* Drive the registered durable-advance callback exactly as the persist flusher + * does after advancing durable_lsn and releasing p->lock: bump durable_lsn, + * then invoke the hook so the commit engine re-evaluates + wakes WAIT callers. + * Asserts a callback is actually registered (it is, from commit_engine_init). */ +static void advance_durable_and_wake(uint64_t durable_lsn) +{ + atomic_store(&g_p.durable_lsn, durable_lsn); + TEST_ASSERT_NOT_NULL(g_p.on_durable_advance); + g_p.on_durable_advance(g_p.durable_cb_ctx); +} + static void engine_setup(int n_peers, int n_standbys, uint64_t appended_lsn) { engine_setup_vcpus(n_peers, n_standbys, appended_lsn, 8); @@ -311,6 +338,104 @@ static void test_retain_mode_returns_zero_immediately(void) printf("test_retain_mode_returns_zero_immediately: PASS\n"); } +/* ════════════════════════════════════════════════════════════════════════ + * ha-commit-durability task 2.2 — self counts toward the commit majority only + * when durable. The PRIMARY's self-contribution is capped at durable_lsn (the + * highest LSN fdatasync'd locally), NOT the appended tip. (Req 1.2, 1.4) + * ════════════════════════════════════════════════════════════════════════ */ + +/* ── Test 2.2a: self-cap equals durable_lsn, not the appended tip ───────── + * Appended tip is 5 but only LSN 3 is durable locally. A standby ACKs the + * full tip (5). committed_lsn must stop at 3 — self only covers up to its + * durable watermark, so entries 4,5 are NOT committed despite the standby + * majority covering them. Kills the highest_appended_lsn self-cap. */ +static void test_self_cap_equals_durable_not_appended_tip(void) +{ + /* Arrange: 3-member cluster, entries 1..5 appended, but durable only to 3 */ + engine_setup(3, 2, 5); + append_entries(5); + set_durable(3); + + /* Act: standby 0 ACKs the full appended tip (5) — self + sb0 majority + * covers 5, but self is only durable to 3. */ + standby_ack(0, 5); + + /* Assert: committed caps at the durable self-contribution (3), NOT 5. */ + TEST_ASSERT_EQUAL_UINT64_MESSAGE( + 3, committed(), + "self contribution must cap at durable_lsn, not the appended tip"); + + engine_teardown(); + printf("test_self_cap_equals_durable_not_appended_tip: PASS\n"); +} + +/* ── Test 2.2b: committed_lsn holds over an entry until it is durable ────── + * The core observable (Req 1.2/1.4): with a full standby majority already + * present, committed_lsn does NOT advance over entry 5 until entry 5 is + * durable locally. As durable_lsn steps up, committed tracks it — never + * passing the durable watermark. */ +static void test_committed_holds_until_durable_then_advances(void) +{ + /* Arrange: 3-member cluster, entries 1..5 appended; nothing durable yet. */ + engine_setup(3, 2, 5); + append_entries(5); + set_durable(0); + + /* A full standby majority covers the entire tip up front. */ + standby_ack(0, 5); + standby_ack(1, 5); + + /* Assert: with self durable to 0, NOTHING commits despite the majority. */ + TEST_ASSERT_EQUAL_UINT64_MESSAGE( + 0, committed(), + "no entry may commit while self is not durable for it"); + + /* Durable advances to 2 → committed may advance to 2 (self+majority). */ + set_durable(2); + commit_on_ack(&g_ce, 0, 5); /* re-evaluate (an ACK re-fires evaluation) */ + TEST_ASSERT_EQUAL_UINT64(2, committed()); + + /* Durable advances to 5 → committed reaches the tip. */ + set_durable(5); + commit_on_ack(&g_ce, 0, 5); + TEST_ASSERT_EQUAL_UINT64(5, committed()); + + engine_teardown(); + printf("test_committed_holds_until_durable_then_advances: PASS\n"); +} + +/* ── Test 2.2c: single-member self-cap — standalone still gated on durable ─ + * n_peers=1 self-quorum: committed advances on self alone, but ONLY up to + * durable_lsn. An appended-but-unsynced entry is not committable even with + * no peers. (Req 1.2, 6.3) */ +static void test_single_member_self_cap_gated_on_durable(void) +{ + /* Arrange: single-member, entries 1..3 appended, durable only to 1. */ + engine_setup(1, 0, 3); + append_entries(3); + set_durable(1); + + /* Act: drive an evaluation (membership change re-evaluates). */ + commit_on_membership_change(&g_ce); + + /* Assert: self-quorum commits only the durable prefix (1), not the tip. */ + TEST_ASSERT_EQUAL_UINT64_MESSAGE( + 1, committed(), + "single-member commit must cap at durable_lsn"); + + /* Durable catches up → committed reaches the tip. */ + set_durable(3); + commit_on_membership_change(&g_ce); + TEST_ASSERT_EQUAL_UINT64(3, committed()); + + engine_teardown(); + printf("test_single_member_self_cap_gated_on_durable: PASS\n"); +} + +/* NOTE: test_durable_advance_wake_completes_blocked_wait (the liveness case + * for the durable-advance wake) is defined AFTER the waiter-thread helpers + * (spawn_waiters/poll_inflight/join_waiters), further down this file. */ + /* ════════════════════════════════════════════════════════════════════════ * Task 2.1 — COMMIT_MODE_WAIT blocking semantics, admission control, * own-term gate. RED against the task-1 engine (WAIT = -ENOSYS). @@ -406,6 +531,53 @@ static int poll_inflight(int want, int n_spawned, uint32_t deadline_ms) return v; } +/* ── Test 2.2d: durable-advance wake completes a blocked WAIT promptly ───── + * Liveness for change (B): a COMMIT_MODE_WAIT caller whose standby majority + * already covers its entry, but whose entry is not yet durable locally, must + * complete PROMPTLY when the flusher advances durable_lsn and fires the + * durable-advance callback — NOT only when its timeout expires. The standby + * ACK arrives BEFORE the durable advance, so commit_on_ack alone cannot + * complete it; the durable-advance wake must. (Req 1.2, 1.4) */ +static void test_durable_advance_wake_completes_blocked_wait(void) +{ + uint64_t t0, elapsed_ms; + + /* Arrange: 3-member cluster, entry 1 appended, NOT durable yet. */ + engine_setup(3, 2, 1); + append_entries(1); + set_durable(0); + + /* Full standby majority ACKs entry 1 up front — but self is not durable, + * so this ACK does NOT commit it (committed holds at 0). */ + standby_ack(0, 1); + TEST_ASSERT_EQUAL_UINT64(0, committed()); + + /* A WAIT caller blocks on entry 1 with a long timeout (5s). It cannot + * complete on the ACK (already delivered) — only the durable-advance wake + * can release it before the timeout. */ + spawn_waiters(1, 1, 5000); + (void)poll_inflight(1, 1, 2000); /* ensure the waiter is actually blocked */ + + /* Act: the "flusher" advances durable_lsn to 1 and fires the wake. */ + t0 = mono_ms(); + advance_durable_and_wake(1); + join_waiters(1); + elapsed_ms = mono_ms() - t0; + + /* Assert: the waiter completed with success, PROMPTLY (well under the 5s + * timeout) — proving the durable-advance wake, not the deadline. */ + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, g_wargs[0].rc, + "WAIT must complete on durable-advance, not time out"); + TEST_ASSERT_EQUAL_UINT64(1, committed()); + TEST_ASSERT_TRUE_MESSAGE( + elapsed_ms < 2000, + "durable-advance wake must complete the wait promptly, not on timeout"); + + engine_teardown(); + printf("test_durable_advance_wake_completes_blocked_wait: PASS\n"); +} + /* ── Test 8: WAIT blocks until majority ACK, then returns 0 ─────────────── * (Req 1.1) */ static void test_wait_blocks_until_majority_ack_then_returns_zero(void) @@ -1421,6 +1593,11 @@ void test_commit_engine(void) RUN_TEST(test_committed_lsn_never_regresses); RUN_TEST(test_lagging_standby_does_not_block_majority_commit); RUN_TEST(test_retain_mode_returns_zero_immediately); + /* ha-commit-durability task 2.2 — self counts toward majority only when durable */ + RUN_TEST(test_self_cap_equals_durable_not_appended_tip); + RUN_TEST(test_committed_holds_until_durable_then_advances); + RUN_TEST(test_single_member_self_cap_gated_on_durable); + RUN_TEST(test_durable_advance_wake_completes_blocked_wait); /* Task 2.1 — WAIT semantics, admission control, own-term gate */ RUN_TEST(test_wait_blocks_until_majority_ack_then_returns_zero); RUN_TEST(test_wait_times_out_with_committed_lsn_unmoved_past_entry); diff --git a/src/controller/testsuite/test_demotion_convergence.c b/src/controller/testsuite/test_demotion_convergence.c index 1b46d7b..8e99d4f 100644 --- a/src/controller/testsuite/test_demotion_convergence.c +++ b/src/controller/testsuite/test_demotion_convergence.c @@ -151,6 +151,11 @@ static void dc_setup(const char *suffix, int n_peers, int n_standbys, g_dc.persist.current_lsn = appended_lsn + 1; pthread_mutex_unlock(&g_dc.persist.lock); atomic_store(&g_dc.persist.committed_lsn, 0); + /* ha-commit-durability 2.2: self caps at durable_lsn. This fixture primes + * the WAL tip in-memory (no bytes written → the flusher can't advance + * durable_lsn itself), so mark the primed log durable up front to preserve + * the pre-durability "self covers every appended entry" commit model. */ + atomic_store(&g_dc.persist.durable_lsn, appended_lsn); for (int i = 0; i < n_standbys; i++) { snprintf(name, sizeof(name), "dcsb%d", i); @@ -570,6 +575,9 @@ static void rj_set_local_state(uint64_t tip_lsn, uint64_t committed) g_dc.persist.current_lsn = tip_lsn + 1; pthread_mutex_unlock(&g_dc.persist.lock); atomic_store(&g_dc.persist.committed_lsn, committed); + /* ha-commit-durability 2.2: keep durable_lsn at the primed tip so the + * commit-majority self-cap matches this in-memory-primed WAL state. */ + atomic_store(&g_dc.persist.durable_lsn, tip_lsn); } /* Demote the fixture to STANDBY (the role rejoin reconciliation runs in) and diff --git a/src/controller/testsuite/test_repl_majority.c b/src/controller/testsuite/test_repl_majority.c index 57692ff..931ad68 100644 --- a/src/controller/testsuite/test_repl_majority.c +++ b/src/controller/testsuite/test_repl_majority.c @@ -78,6 +78,12 @@ static void majority_setup(int n_peers, int n_standbys, uint64_t appended_lsn) } g_mp.current_lsn = appended_lsn + 1; atomic_store(&g_mp.committed_lsn, 0); + /* ha-commit-durability 2.2: the commit majority now caps self at + * durable_lsn. This stub has no flusher, so mark every appended entry + * durable up front to preserve the pre-durability model ("self covers + * every appended entry"); the durability precondition is exercised + * separately in test_commit_durability.c / test_commit_engine.c. */ + atomic_store(&g_mp.durable_lsn, appended_lsn); TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&g_meng, &g_mp, n_peers, 8)); repl_set_commit_engine(&g_mp.repl, &g_meng); diff --git a/src/include/persist.h b/src/include/persist.h index 2ae70fd..fa82f9c 100644 --- a/src/include/persist.h +++ b/src/include/persist.h @@ -122,6 +122,19 @@ struct persist_s { pthread_mutex_t fsync_mutex; /* guards the fsync timer wait */ pthread_cond_t fsync_cond; /* signaled by persist_stop for prompt join */ + /* Durable-advance wake hook (ha-commit-durability task 2.2). + * Persistence owns this slot; the commit engine REGISTERS a callback + * (ctx = the commit_engine_t) via persist_set_durable_cb. The flusher + * invokes it AFTER advancing durable_lsn and AFTER releasing p->lock, + * so a COMMIT_MODE_WAIT caller is woken promptly when its entry becomes + * durable locally — without it, a commit whose standby ACKs all arrived + * before the local fdatasync would stall until the WAIT timeout. The + * dependency direction is preserved (commit depends on persist, never + * the reverse): persist only stores+calls an opaque function pointer. + * NULL until registered (every flush is then a no-op wake). */ + void (*on_durable_advance)(void* ctx); + void* durable_cb_ctx; + /* Checkpoint (stubs for now — wired in Task 10) */ atomic_int checkpoint_requested; pthread_cond_t checkpoint_done; @@ -206,6 +219,16 @@ int persist_flush(persist_t* p); * any thread. Returns 0 when nothing is yet durable. */ uint64_t persist_durable_lsn(persist_t* p); +/* Register a durable-advance wake callback (ha-commit-durability task 2.2). + * The group-commit flusher calls fn(ctx) after it advances durable_lsn and + * after releasing p->lock, so a consumer (the commit engine) can re-evaluate + * its commit majority and wake any COMMIT_MODE_WAIT caller whose entry just + * became durable locally. fn MUST NOT take p->lock (it runs after release, on + * the flusher thread) — the commit engine's callback takes only ce->lock, + * keeping the lock order p->lock → ce->lock cycle-free. Pass fn = NULL to + * unregister. Safe to call before persist_start (no flusher running yet). */ +void persist_set_durable_cb(persist_t* p, void (*fn)(void* ctx), void* ctx); + #ifdef XCE_TESTING /* ============================================================ * Durability-verification test seam (ha-commit-durability Req 3.2, 3.3)