From 631a08939bf4372163f4bc52fc3c854c3e62a428 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 11 Jun 2026 15:48:42 +0000 Subject: [PATCH] test(ha-commit-engine): membership re-evaluation regression tests (tasks 3.1+3.2 joint) Tests 17-20: catch-up ACK exclusion vs pending waiter, membership-change wake of covered waiter, removal-below-majority fails at own deadline (no forced early-fail), non-blocking bound. Landed GREEN: task 1.2's commit_on_membership_change already implements the full 3.2 contract, so RED was unachievable; reviewer confirmed mutation-killing power and zero production changes. 20/20 group tests; suite at baseline. --- .kiro/specs/ha-commit-engine/tasks.md | 5 +- .wolf/cerebrum.md | 1 + .wolf/memory.md | 7 + src/controller/testsuite/test_commit_engine.c | 157 +++++++++++++++++- 4 files changed, 167 insertions(+), 3 deletions(-) diff --git a/.kiro/specs/ha-commit-engine/tasks.md b/.kiro/specs/ha-commit-engine/tasks.md index 1c1d15de..6b0dfce9 100644 --- a/.kiro/specs/ha-commit-engine/tasks.md +++ b/.kiro/specs/ha-commit-engine/tasks.md @@ -30,12 +30,12 @@ - _Requirements: 1.1, 4.3, 4.4, 6.3, 7.1, 7.2, 7.3_ - [ ] 3. Membership-change re-evaluation -- [ ] 3.1 Write RED tests for membership re-evaluation +- [x] 3.1 Write RED tests for membership re-evaluation (completed jointly with 3.2 — RED unachievable, see Implementation Notes) - Tests in `test_commit_engine.c`: a catch-up standby's ACK watermark does not count toward a pending entry; after clearing `in_catch_up` and calling `commit_on_membership_change`, the pending waiter completes if the standby's watermark covers it; on standby removal that makes majority unreachable, a pending WAIT never returns success and fails with `-ETIMEDOUT` at its deadline; `commit_on_membership_change` returns without blocking - Observable completion: new cases FAIL (red) - _Requirements: 4.1, 4.2_ -- [ ] 3.2 Implement commit_on_membership_change to green +- [x] 3.2 Implement commit_on_membership_change to green (verified no-op — task 1.2 already implemented the full contract; tests 17-20 are the green evidence) - Re-run the commit evaluation under the engine mutex against current registry state and broadcast the condvar so all pending waiters re-check their predicates; no forced early-fail (reconnection within the deadline may still commit — design decision) - Observable completion: all task 3.1 tests PASS - _Requirements: 4.1, 4.2_ @@ -86,3 +86,4 @@ - (1.1) repl_add_standby returns the SLOT INDEX, not 0 (bug-410 note for fixture writers). - (1.1) commit.c is registered ONLY in controller_unit_test_src for now; production target linkage lands with task 5.3. - (2.1) WAIT/cap/gate tests (8-16) read `g_ce.inflight_waits` under `g_ce.lock` — task 2.2 must keep that field accurate while waiters block. Timing floors have ~20ms slack vs sleeps (reviewer advisory: watch for flake on loaded boxes). Unity assertions are main-thread-only after joins — keep that discipline in future threaded tests. +- (3.1/3.2) Joint completion: task 1.2's commit_on_membership_change (commit.c:334-346) already implemented the full 3.2 contract, making 3.1's RED unachievable without dishonest tests. Tests 17-20 landed green as regression coverage; reviewer confirmed mutation-killing power (each test fails against the relevant mutant) and zero production changes. Lesson for later RED tasks: when a prior task's "minimal" allowance fully implements a later contract, complete the pair jointly under review rather than weakening tests. diff --git a/.wolf/cerebrum.md b/.wolf/cerebrum.md index 4f6636fa..921c27d6 100644 --- a/.wolf/cerebrum.md +++ b/.wolf/cerebrum.md @@ -37,6 +37,7 @@ - repl_add_standby returns the SLOT INDEX (>=0) on success, not 0 (2026-06-11, ha-commit-engine task 1.1): asserting `== 0` breaks on the second standby. Assert `>= 0` or the expected index. - Unity harness quirks in controller_unit_test (2026-06-11): (a) failure lines misattribute the FILE — Unity.TestFile comes from whichever group's UNITY_BEGIN ran, so a failure printed as test_repl_unit.c:294 may live in test_p2_integration.c; locate the actual test by grepping for the test NAME. (b) Each group's UNITY_BEGIN resets counters, so the process exit code only reflects the LAST group — failures in earlier groups are masked (exit=0); meson reports green despite real failures. Grep output for FAIL with `grep FAIL` on the raw log (ANSI codes sit between ':' and 'FAIL', so `grep ":FAIL"` misses them). - 3 pre-existing controller suite failures on persistent_ha baseline (2026-06-11, verified by stash/rebuild without ha-commit-engine changes, see bug-411): test_committed_lsn_does_not_advance_on_timeout (rc -110 vs expected 0 — persist_wal_sync_append now returns -ETIMEDOUT, test expects old rc==0+sync_rc contract), test_repl_ack_after_promote_gate_allows_jobsub and test_rdhup_primary_dispatch_gate_opens (both -10013: ha_has_write_quorum false, standby_endpoints empty with n_peers=2). +- ha-commit-engine: task 1.2 shipped commit_on_membership_change as a FULL re-evaluation (commit_advance_locked + cond broadcast), and majority_candidate_locked already excludes in_catch_up + ep==NULL slots. Task 3.1 membership tests are green-on-arrival; task 3.2 is a no-op (verified 2026-06-11, tests 17-20 in test_commit_engine.c). ## Do-Not-Repeat diff --git a/.wolf/memory.md b/.wolf/memory.md index 795a3e06..ef8eedeb 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -885,3 +885,10 @@ | 08:00 | Edited src/controller/commit.c | added 5 condition(s) | ~684 | | 15:20 | ha-commit-engine task 2.2: implemented COMMIT_MODE_WAIT (admission cap max(1,n_vcpus-COMMIT_WAIT_RESERVED) -> -XCE_ERR_NOT_READY, CLOCK_REALTIME pthread_cond_timedwait loop with per-LSN+writable_lsn predicate, n_peers<=1 self-quorum via commit_advance_locked) | src/controller/commit.c | 16/16 test_commit_engine PASS, full suite = 3 baseline failures only (bug-411), no sanitizer output | ~28k | | 08:24 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~22 | +| 08:28 | Edited src/controller/testsuite/test_commit_engine.c | added error handling | ~191 | +| 08:29 | Edited src/controller/testsuite/test_commit_engine.c | added error handling | ~1650 | +| 08:29 | Edited src/controller/testsuite/test_commit_engine.c | added error handling | ~129 | +| 15:33 | task 3.1 ha-commit-engine: wrote tests 17-20 (membership re-eval) in test_commit_engine.c; ALL GREEN against current commit.c (1.2 already implements full re-eval+broadcast) — no RED possible, task 3.2 likely no-op; reported BLOCKED | src/controller/testsuite/test_commit_engine.c | green-not-red finding | ~30k | +| 08:48 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~35 | +| 08:48 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~43 | +| 08:48 | Edited .kiro/specs/ha-commit-engine/tasks.md | 1→2 lines | ~229 | diff --git a/src/controller/testsuite/test_commit_engine.c b/src/controller/testsuite/test_commit_engine.c index b4027f03..22d3a7f1 100644 --- a/src/controller/testsuite/test_commit_engine.c +++ b/src/controller/testsuite/test_commit_engine.c @@ -30,7 +30,15 @@ * registry + commit_on_ack. All threads are joined BEFORE any assertion so * Unity's longjmp never strands a live thread (ASAN/LSan clean). * - * Requirements: 1.1, 1.2, 2.1, 2.2, 2.3, 4.3, 4.4, 5.1, 6.3, 7.1, 7.2, 7.3 + * Task 3.1 extends the file with membership re-evaluation cases + * (commit_on_membership_change): catch-up exclusion against a pending + * waiter, catch-up completion waking a pending waiter, standby removal + * below majority dying at the waiter's own deadline (no forced early-fail — + * design §Commit Engine), and the non-blocking contract of + * commit_on_membership_change itself. + * + * Requirements: 1.1, 1.2, 2.1, 2.2, 2.3, 4.1, 4.2, 4.3, 4.4, 5.1, 6.3, + * 7.1, 7.2, 7.3 */ #include #include @@ -634,6 +642,148 @@ static void test_wait_gate_never_opening_times_out_not_notready(void) printf("test_wait_gate_never_opening_times_out_not_notready: PASS\n"); } +/* ════════════════════════════════════════════════════════════════════════ + * Task 3.1 — membership re-evaluation (commit_on_membership_change). + * (Req 4.1, 4.2; design §Commit Engine: re-evaluate under the engine mutex + * against current registry state + broadcast; no forced early-fail.) + * ════════════════════════════════════════════════════════════════════════ */ + +/* ── Test 17: catch-up standby's watermark never counts for a pending entry + * A pending WAIT must not complete off an ACK from a standby still in + * catch-up — it dies at its own deadline with committed_lsn unmoved. + * (Req 4.1) */ +static void test_catchup_standby_ack_does_not_count_toward_pending_entry(void) +{ + /* Arrange: 3-member cluster, entry 1 appended; the ONLY ACKing standby + * is in catch-up */ + engine_setup(3, 2, 1); + atomic_store(&g_p.repl.standbys[0].in_catch_up, 1); + spawn_waiters(1, 1, 250); + + /* Act: the catch-up standby's watermark covers the entry */ + sleep_ms(50); + standby_ack(0, 1); + join_waiters(1); + + /* Assert: never success — catch-up ACKs are excluded from majority */ + TEST_ASSERT_TRUE_MESSAGE(g_wargs[0].rc != 0, + "catch-up standby ACK must not commit a pending entry"); + TEST_ASSERT_EQUAL_INT(-ETIMEDOUT, g_wargs[0].rc); + TEST_ASSERT_TRUE_MESSAGE(g_wargs[0].elapsed_ms >= 200, + "WAIT must hold to its deadline, not early-complete"); + TEST_ASSERT_EQUAL_UINT64(0, committed()); + + engine_teardown(); + printf("test_catchup_standby_ack_does_not_count_toward_pending_entry: PASS\n"); +} + +/* ── Test 18: clearing in_catch_up + membership change completes the waiter + * The standby's watermark already covers the entry; once it leaves + * catch-up, commit_on_membership_change must re-evaluate against current + * registry state and wake the pending waiter to success. (Req 4.2) */ +static void test_catchup_clear_then_membership_change_completes_waiter(void) +{ + int done_while_in_catchup; + + /* Arrange: 3-member cluster, entry 1 appended; standby 0 in catch-up + * with a watermark that covers the entry (excluded while catching up) */ + engine_setup(3, 2, 1); + atomic_store(&g_p.repl.standbys[0].in_catch_up, 1); + standby_ack(0, 1); + TEST_ASSERT_EQUAL_UINT64(0, committed()); + spawn_waiters(1, 1, 2000); + + /* Act: waiter stays blocked while the standby is in catch-up... */ + sleep_ms(120); + done_while_in_catchup = atomic_load(&g_waiters_done); + + /* ...then catch-up completes: clear the flag and signal membership */ + atomic_store(&g_p.repl.standbys[0].in_catch_up, 0); + commit_on_membership_change(&g_ce); + join_waiters(1); + + /* Assert (after join): held during catch-up... */ + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, done_while_in_catchup, + "waiter must not complete while the covering standby is in catch-up"); + /* ...then woken to success by the membership re-evaluation */ + TEST_ASSERT_EQUAL_INT(0, g_wargs[0].rc); + TEST_ASSERT_TRUE_MESSAGE(g_wargs[0].elapsed_ms >= 100, + "waiter must block until the membership change"); + TEST_ASSERT_EQUAL_UINT64(1, committed()); + + engine_teardown(); + printf("test_catchup_clear_then_membership_change_completes_waiter: PASS\n"); +} + +/* ── Test 19: removal below majority — pending WAIT dies at ITS deadline ── + * Standby removal that makes majority unreachable must never surface + * success; per the design (no forced early-fail — reconnection within the + * deadline may still commit), the wait fails with -ETIMEDOUT at its own + * deadline, and committed_lsn never regresses. (Req 4.1, 4.2) */ +static void test_standby_removal_below_majority_wait_fails_at_deadline(void) +{ + /* Arrange: 3-member cluster, entries 1..2 appended; entry 1 committed */ + engine_setup(3, 2, 2); + standby_ack(0, 1); + TEST_ASSERT_EQUAL_UINT64(1, committed()); + spawn_waiters(1, 2, 400); + + /* Act: both standbys drop — majority for entry 2 is now unreachable */ + sleep_ms(50); + repl_remove_standby(&g_p.repl, &g_eps[0]); + repl_remove_standby(&g_p.repl, &g_eps[1]); + commit_on_membership_change(&g_ce); + join_waiters(1); + + /* Assert: never success... */ + TEST_ASSERT_TRUE_MESSAGE( + g_wargs[0].rc != 0, + "WAIT must not succeed after removal makes majority unreachable"); + /* ...fails with -ETIMEDOUT at its own deadline (no forced early-fail) */ + TEST_ASSERT_EQUAL_INT(-ETIMEDOUT, g_wargs[0].rc); + TEST_ASSERT_TRUE_MESSAGE(g_wargs[0].elapsed_ms >= 350, + "removal must not abort the wait before its deadline"); + /* ...and the commit index never regresses on membership shrink */ + TEST_ASSERT_EQUAL_UINT64(1, committed()); + + engine_teardown(); + printf("test_standby_removal_below_majority_wait_fails_at_deadline: PASS\n"); +} + +/* ── Test 20: commit_on_membership_change returns without blocking ──────── + * iomgr-callable contract: bounded mutex hold only — it must return + * immediately even while a WAIT is pending (it never waits for waiter + * completion or I/O). (Req 4.2; design §External Dependencies) */ +static void test_membership_change_returns_without_blocking(void) +{ + uint64_t t0, elapsed_ms; + + /* Arrange: 3-member cluster, entry 1 appended, one pending waiter */ + engine_setup(3, 2, 1); + spawn_waiters(1, 1, 2000); + sleep_ms(50); + + /* Act: membership event with the waiter still blocked */ + t0 = mono_ms(); + commit_on_membership_change(&g_ce); + elapsed_ms = mono_ms() - t0; + + /* Complete the waiter so teardown joins cleanly */ + standby_ack(0, 1); + join_waiters(1); + + /* Assert (after join): the membership call itself never blocked... */ + TEST_ASSERT_TRUE_MESSAGE(elapsed_ms < 500, + "commit_on_membership_change must not block"); + /* ...and the pending waiter still completed normally via the ACK */ + TEST_ASSERT_EQUAL_INT(0, g_wargs[0].rc); + TEST_ASSERT_EQUAL_UINT64(1, committed()); + + engine_teardown(); + printf("test_membership_change_returns_without_blocking: PASS\n"); +} + /* ── Test group entry point ──────────────────────────────────────────────── */ void test_commit_engine(void) @@ -656,5 +806,10 @@ void test_commit_engine(void) RUN_TEST(test_wait_no_gate_when_writable_lsn_unset); RUN_TEST(test_wait_gate_blocks_until_committed_reaches_gate); RUN_TEST(test_wait_gate_never_opening_times_out_not_notready); + /* Task 3.1 — membership re-evaluation */ + RUN_TEST(test_catchup_standby_ack_does_not_count_toward_pending_entry); + RUN_TEST(test_catchup_clear_then_membership_change_completes_waiter); + RUN_TEST(test_standby_removal_below_majority_wait_fails_at_deadline); + RUN_TEST(test_membership_change_returns_without_blocking); UNITY_END(); }