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.
This commit is contained in:
parent
3bc6fd14de
commit
631a08939b
4 changed files with 167 additions and 3 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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 |
|
||||
|
|
|
|||
|
|
@ -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 <errno.h>
|
||||
#include <pthread.h>
|
||||
|
|
@ -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();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue