From b2598823c0ed1a0abfa4470b020f8df4040c8f23 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 11 Jun 2026 16:15:21 +0000 Subject: [PATCH] test(ha-commit-engine): RED tests for write-admission quorum counting (task 4.1) Tests 21-28: repl_active_standby_count contract (5 RED vs inert -1 stub) + 3 gate-level pins (n_peers<=1 always-true, zero-connected reject, stateless re-accept) that 4.2 must preserve. Declaration in repl.h; ha.c/ha.h untouched (4.2 boundary). 28 tests / 5 RED; suite at the 3-failure baseline with unchanged signatures. --- .kiro/specs/ha-commit-engine/tasks.md | 3 +- .wolf/cerebrum.md | 2 + .wolf/memory.md | 10 + src/controller/repl.c | 14 + src/controller/testsuite/test_commit_engine.c | 290 +++++++++++++++++- src/include/repl.h | 6 + 6 files changed, 322 insertions(+), 3 deletions(-) diff --git a/.kiro/specs/ha-commit-engine/tasks.md b/.kiro/specs/ha-commit-engine/tasks.md index 6b0dfce9..d436e83d 100644 --- a/.kiro/specs/ha-commit-engine/tasks.md +++ b/.kiro/specs/ha-commit-engine/tasks.md @@ -41,7 +41,7 @@ - _Requirements: 4.1, 4.2_ - [ ] 4. Quorum-accounting fix: catch-up exclusion at the dispatch gate -- [ ] 4.1 Write RED tests for write-admission quorum counting +- [x] 4.1 Write RED tests for write-admission quorum counting - Tests: `repl_active_standby_count` counts only slots with `ep != NULL && !in_catch_up`; `ha_has_write_quorum` with n_peers=3 returns false when the only connected standby is in catch-up, true with one active standby, false with zero connected; after simulated reconnect (registry restored) the gate accepts again with no reset call (stateless re-evaluation); n_peers=1 always true - Declare `repl_active_standby_count` in src/include/repl.h so tests compile - Observable completion: new cases FAIL (red) — current `ha_has_write_quorum` counts `standby_endpoints->count` including catch-up (grep ha.c for the current anchor near ha.c:1507) @@ -87,3 +87,4 @@ - (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. +- (4.1) REVIEWER ADVISORY for 4.2: the gate-side fix (ha_state_t.repl back-pointer + catch-up exclusion through ha_has_write_quorum) has NO RED test — 4.1's tests would all pass even if 4.2 only implemented the helper. 4.2 MUST add a wired gate-level case (n_peers=3, catch-up-only standby → NOT_READY via ha_dispatch_gate) as its own green evidence. Tests 26-28 pin the NULL-back-pointer legacy fallback and n_peers<=1 contracts 4.2 must preserve. Stub repl_active_standby_count returns -1 (repl.c:557) — replace, don't wrap. diff --git a/.wolf/cerebrum.md b/.wolf/cerebrum.md index 921c27d6..11b7f326 100644 --- a/.wolf/cerebrum.md +++ b/.wolf/cerebrum.md @@ -76,3 +76,5 @@ - (2026-06-11, ha-convergence-recovery spec) `repl_complete_catchup` (repl.c:521) has NO production caller — `in_catch_up` is set by broadcast-overflow eviction (repl.c:240) and never cleared; re-inclusion trigger is wired by the ACK-driven catch-up continuation. - (2026-06-11) Scheduler-bound job egress has TWO seams, not one: full push `xcesched_push_jobs` (sched.c:169, on scheduler join) AND incremental JOB_NEW push `sched_job_new_fun`→`xcesched_push_joblist` (sched.c:236-248). Status reads (apiserver.c:83, opget.c:173) share `collect_jobs` (dataex.c:294) and must stay unfiltered per umbrella deviation. - (2026-06-11) No saved-argv/execv machinery exists anywhere in src/controller or src/lib — demotion self-restart is net-new; `job_t` is a thin wrapper `{ xce_job_t job; }` at src/include/xce/types.h:483-485 (controller-local fields go on the wrapper, wire type untouched). + +- ha_has_write_quorum (ha.c:1507) is STATIC — testsuite reaches it only through ha_dispatch_gate(ha, opcode) (extern-declared in test files, pattern: test_ha_repl_ack.c). Gate fixtures: ha_init + n_peers + atomic role store + heap zarray standby_endpoints (test_ha_connection_reuse.c pattern). Catch-up exclusion at the gate is untestable until the ha_state_t.repl back-pointer lands (ha-commit-engine 4.2) — assert via repl_active_standby_count instead. (2026-06-11, task 4.1) diff --git a/.wolf/memory.md b/.wolf/memory.md index ef8eedeb..2d12216d 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -892,3 +892,13 @@ | 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 | +| 08:55 | Edited src/include/repl.h | added error handling | ~154 | +| 08:56 | Edited src/controller/repl.c | modified repl_needs_snapshot() | ~244 | +| 08:56 | Edited src/controller/testsuite/test_commit_engine.c | 10→15 lines | ~214 | +| 08:56 | Edited src/controller/testsuite/test_commit_engine.c | expanded (+6 lines) | ~145 | +| 08:57 | Edited src/controller/testsuite/test_commit_engine.c | added error handling | ~2974 | +| 08:57 | Edited src/controller/testsuite/test_commit_engine.c | expanded (+9 lines) | ~255 | +| 2026-06-11 | ha-commit-engine task 4.1: RED tests for write-admission quorum counting (tests 21-28 in test_commit_engine.c), repl_active_standby_count declared in repl.h + inert -1 stub in repl.c | src/controller/testsuite/test_commit_engine.c, src/include/repl.h, src/controller/repl.c | build green; 5 RED fails on assertions; baseline 3 failures unchanged | ~60k | +| 16:20 | Adversarial review of ha-commit-engine task 4.1 (RED quorum tests): boundary clean, 5 RED + 3 pins verified, full census 8 fails (3 bug-411 + 5 RED), APPROVED | test_commit_engine.c, repl.h, repl.c | approved | ~25k | +| 09:14 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~17 | +| 09:15 | Edited .kiro/specs/ha-commit-engine/tasks.md | 1→2 lines | ~280 | diff --git a/src/controller/repl.c b/src/controller/repl.c index 6a000a53..279ad68a 100644 --- a/src/controller/repl.c +++ b/src/controller/repl.c @@ -545,3 +545,17 @@ int repl_needs_snapshot(replicator_t *r, uint64_t standby_lsn, /* Standby LSN predates oldest surviving WAL segment — full snapshot required */ return (standby_lsn < oldest_wal_lsn) ? 1 : 0; } + +/* ============================================================ + * Write-admission quorum accounting (ha-commit-engine task 4) + * ============================================================ */ + +/* RED scaffold for task 4.1: deliberately inert so the task-4.1 tests fail + * on assertions rather than at link time (-1 is never a valid count). + * The real counting — slots with ep != NULL && !in_catch_up under r->lock — + * is implemented in task 4.2. */ +int repl_active_standby_count(replicator_t *r) +{ + (void)r; + return -1; +} diff --git a/src/controller/testsuite/test_commit_engine.c b/src/controller/testsuite/test_commit_engine.c index 22d3a7f1..cf9d937d 100644 --- a/src/controller/testsuite/test_commit_engine.c +++ b/src/controller/testsuite/test_commit_engine.c @@ -37,14 +37,20 @@ * 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 + * Task 4.1 extends the file with write-admission quorum accounting: + * repl_active_standby_count (registry truth: ep != NULL && !in_catch_up) + * and ha_dispatch_gate write-quorum behavior. RED against the inert repl.c + * stub (returns -1); green with the task-4.2 implementation. + * + * Requirements: 1.1, 1.2, 2.1, 2.2, 2.3, 3.1, 3.2, 3.3, 4.1, 4.2, 4.3, 4.4, + * 5.1, 6.1, 6.2, 6.3, 7.1, 7.2, 7.3 */ #include #include #include #include #include +#include #include #include #include @@ -52,9 +58,14 @@ #include "xce/errnox.h" #include "commit.h" +#include "controller.capnp.h" /* OPCode_jobsub (task 4.1 gate cases) */ +#include "ha.h" /* ha_state_t, ha_dispatch_gate fixture */ #include "persist.h" #include "repl.h" +/* ha_dispatch_gate — defined in ha.c (same pattern as test_ha_repl_ack.c) */ +int ha_dispatch_gate(ha_state_t *ha, int opcode); + /* ── Fixture ────────────────────────────────────────────────────────────── * Static: persist_t embeds large path buffers, and statics survive Unity's * longjmp on assertion failure (pattern from test_repl_majority.c). */ @@ -784,6 +795,272 @@ static void test_membership_change_returns_without_blocking(void) printf("test_membership_change_returns_without_blocking: PASS\n"); } +/* ════════════════════════════════════════════════════════════════════════ + * Task 4.1 — write-admission quorum accounting. + * (Req 3.1, 3.2, 3.3, 6.1, 6.2; design §Quorum Accounting) + * + * repl_active_standby_count is the registry-truth helper ha_has_write_quorum + * adopts in task 4.2: it counts only slots with ep != NULL && !in_catch_up. + * The catch-up exclusion AT THE GATE is asserted through the helper (tests + * 22-25): wiring ha_dispatch_gate to the registry requires the + * ha_state_t.repl back-pointer that task 4.2 adds to ha.h — out of this + * task's boundary. The ha_dispatch_gate cases (tests 26-28) pin the + * standby_endpoints fallback behavior that task 4.2 must preserve when the + * back-pointer is NULL (legacy contexts) and the stateless re-evaluation / + * single-member contracts (6.2, n_peers=1 always true). + * ════════════════════════════════════════════════════════════════════════ */ + +/* Standalone registry for count tests — static: survives Unity's longjmp */ +static replicator_t g_reg; + +/* Gate fixture — static for the same reason; standby_endpoints zarray is + * heap-backed per the test_ha_connection_reuse.c pattern */ +static ha_state_t g_gate_ha; + +static ha_config_t g_gate_cfg = { + .heartbeat_interval_ms = 3000, + .heartbeat_timeout_ms = 10000, + .election_timeout_ms = 2000, + .probe_connect_timeout_ms = 500, + .max_promotion_retries = 5, + .election_jitter_max_ms = 0, + .election_priority_step_ms = 500, + .committing_timeout_ms = 10000, +}; + +/* Arrange a CTRL_PRIMARY ha_state_t with n_endpoints connected standby + * endpoints (what raft_cb_become_leader / on_standby_connected populate). */ +static void gate_setup(int n_peers, int n_endpoints) +{ + int i; + + TEST_ASSERT_EQUAL_INT(0, ha_init(&g_gate_ha, &g_gate_cfg, 0)); + g_gate_ha.n_peers = n_peers; + atomic_store_explicit(&g_gate_ha.role, (int32_t)CTRL_PRIMARY, + memory_order_release); + g_gate_ha.standby_endpoints = calloc(1, sizeof(zarray_t)); + TEST_ASSERT_NOT_NULL(g_gate_ha.standby_endpoints); + zarray_init(g_gate_ha.standby_endpoints, 4, NULL); + for (i = 0; i < n_endpoints; i++) { + zarray_store(g_gate_ha.standby_endpoints, &g_eps[i]); + } +} + +static void gate_teardown(void) +{ + if (g_gate_ha.standby_endpoints) { + zarray_fini(g_gate_ha.standby_endpoints); + free(g_gate_ha.standby_endpoints); + g_gate_ha.standby_endpoints = NULL; + } + ha_fini(&g_gate_ha); +} + +/* ── Test 21: empty / NULL registry counts zero active standbys ─────────── + * Disconnected (never-connected) members never count toward write + * admission. (Req 3.3) */ +static void test_active_standby_count_empty_registry_is_zero(void) +{ + int count_empty, count_null; + + /* Arrange: freshly initialized registry, no standbys */ + TEST_ASSERT_EQUAL_INT(0, repl_init(&g_reg)); + + /* Act */ + count_empty = repl_active_standby_count(&g_reg); + count_null = repl_active_standby_count(NULL); + + /* Assert: zero connected members → zero active count */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, count_empty, + "empty registry must count zero active standbys"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, count_null, + "NULL registry must count zero, not crash"); + + repl_fini(&g_reg); + printf("test_active_standby_count_empty_registry_is_zero: PASS\n"); +} + +/* ── Test 22: count includes ONLY ep != NULL && !in_catch_up slots ──────── + * (Req 3.1, 3.2, 3.3) */ +static void test_active_standby_count_excludes_catchup_and_disconnected(void) +{ + /* Arrange: 3 connected standbys */ + TEST_ASSERT_EQUAL_INT(0, repl_init(&g_reg)); + TEST_ASSERT_EQUAL_INT(0, repl_add_standby(&g_reg, &g_eps[0], "sb0", 7000)); + TEST_ASSERT_EQUAL_INT(1, repl_add_standby(&g_reg, &g_eps[1], "sb1", 7001)); + TEST_ASSERT_EQUAL_INT(2, repl_add_standby(&g_reg, &g_eps[2], "sb2", 7002)); + TEST_ASSERT_EQUAL_INT_MESSAGE(3, repl_active_standby_count(&g_reg), + "all connected, none catching up → count 3"); + + /* Act + Assert: catch-up entry excludes a standby (3.2)... */ + repl_start_catchup(&g_reg, &g_eps[1], 0); + TEST_ASSERT_EQUAL_INT_MESSAGE(2, repl_active_standby_count(&g_reg), + "in_catch_up standby must be excluded"); + + /* ...and disconnect excludes another (3.3); the freed slot keeps its + * disconnect bookkeeping but ep == NULL must not count */ + repl_remove_standby(&g_reg, &g_eps[2]); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, repl_active_standby_count(&g_reg), + "disconnected standby must be excluded"); + + repl_fini(&g_reg); + printf("test_active_standby_count_excludes_catchup_and_disconnected: PASS\n"); +} + +/* ── Test 23: n_peers=3, only connected standby in catch-up → NO quorum ─── + * Gate decision (active >= n_peers/2) must be false: the catch-up standby + * cannot receive new log entries, so the write quorum does not exist. + * This is the case current ha_has_write_quorum gets WRONG — it counts + * standby_endpoints->count (1) including the catch-up member. (Req 3.2, 6.1) */ +static void test_active_count_catchup_only_standby_denies_write_quorum(void) +{ + int n_peers = 3; + int active; + + /* Arrange: one connected standby, in catch-up */ + TEST_ASSERT_EQUAL_INT(0, repl_init(&g_reg)); + TEST_ASSERT_EQUAL_INT(0, repl_add_standby(&g_reg, &g_eps[0], "sb0", 7000)); + repl_start_catchup(&g_reg, &g_eps[0], 0); + + /* Act */ + active = repl_active_standby_count(&g_reg); + + /* Assert: zero active → quorum (active >= n_peers/2 = 1) unmet */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, active, + "catch-up-only standby must yield zero active standbys"); + TEST_ASSERT_TRUE_MESSAGE(active < n_peers / 2, + "write quorum must be DENIED when the only standby is in catch-up"); + + repl_fini(&g_reg); + printf("test_active_count_catchup_only_standby_denies_write_quorum: PASS\n"); +} + +/* ── Test 24: n_peers=3, one ACTIVE standby → write quorum met ──────────── + * (Req 3.1) */ +static void test_active_count_one_active_standby_grants_write_quorum(void) +{ + int n_peers = 3; + int active; + + /* Arrange: one connected, non-catch-up standby */ + TEST_ASSERT_EQUAL_INT(0, repl_init(&g_reg)); + TEST_ASSERT_EQUAL_INT(0, repl_add_standby(&g_reg, &g_eps[0], "sb0", 7000)); + + /* Act */ + active = repl_active_standby_count(&g_reg); + + /* Assert: one active standby satisfies active >= n_peers/2 = 1 */ + TEST_ASSERT_EQUAL_INT_MESSAGE(1, active, + "one connected active standby must count as 1"); + TEST_ASSERT_TRUE_MESSAGE(active >= n_peers / 2, + "write quorum must be GRANTED with one active standby (n_peers=3)"); + + repl_fini(&g_reg); + printf("test_active_count_one_active_standby_grants_write_quorum: PASS\n"); +} + +/* ── Test 25: stateless re-evaluation — count recovers with NO reset call ─ + * Catch-up completion and reconnect (registry restored) must each restore + * the count purely from registry state; there is no quorum-reset API. + * (Req 6.2) */ +static void test_active_count_recovers_after_catchup_and_reconnect(void) +{ + /* Arrange: one standby, pushed into catch-up → count 0 */ + TEST_ASSERT_EQUAL_INT(0, repl_init(&g_reg)); + TEST_ASSERT_EQUAL_INT(0, repl_add_standby(&g_reg, &g_eps[0], "sb0", 7000)); + repl_start_catchup(&g_reg, &g_eps[0], 0); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, repl_active_standby_count(&g_reg), + "catch-up standby must not count"); + + /* Act + Assert: catch-up completion restores the count... */ + repl_complete_catchup(&g_reg, &g_eps[0]); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, repl_active_standby_count(&g_reg), + "catch-up completion must restore the active count with no reset call"); + + /* ...disconnect drops it... */ + repl_remove_standby(&g_reg, &g_eps[0]); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, repl_active_standby_count(&g_reg), + "disconnect must drop the active count"); + + /* ...and reconnect (registry restored) recovers it — stateless */ + TEST_ASSERT_TRUE_MESSAGE( + repl_add_standby(&g_reg, &g_eps[0], "sb0", 7000) >= 0, + "reconnect re-add must succeed"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, repl_active_standby_count(&g_reg), + "reconnect must restore the active count with no reset call"); + + repl_fini(&g_reg); + printf("test_active_count_recovers_after_catchup_and_reconnect: PASS\n"); +} + +/* ── Test 26: n_peers=1 → write quorum ALWAYS true (self is majority) ───── + * Gate-level: single-member primary admits writes with no standbys at all. + * Must stay green before AND after task 4.2 (quorum condition unchanged). */ +static void test_gate_single_member_always_admits_writes(void) +{ + int rc; + + /* Arrange: single-member primary, zero standby endpoints */ + gate_setup(1, 0); + + /* Act */ + rc = ha_dispatch_gate(&g_gate_ha, OPCode_jobsub); + + /* Assert: self-majority — writes always admitted */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, + "n_peers=1 primary must always admit state-changing ops"); + + gate_teardown(); + printf("test_gate_single_member_always_admits_writes: PASS\n"); +} + +/* ── Test 27: n_peers=3, ZERO connected standbys → write rejected ───────── + * Gate-level fast rejection with the retriable not-ready error. (Req 6.1, 3.3) */ +static void test_gate_rejects_writes_with_zero_connected_standbys(void) +{ + int rc; + + /* Arrange: 3-member primary, no connected standbys */ + gate_setup(3, 0); + + /* Act */ + rc = ha_dispatch_gate(&g_gate_ha, OPCode_jobsub); + + /* Assert: cluster cannot currently commit */ + TEST_ASSERT_EQUAL_INT_MESSAGE(-XCE_ERR_NOT_READY, rc, + "zero connected standbys must reject writes with NOT_READY"); + + gate_teardown(); + printf("test_gate_rejects_writes_with_zero_connected_standbys: PASS\n"); +} + +/* ── Test 28: gate re-accepts after reconnection with NO reset call ─────── + * The gate is stateless per request: once the standby endpoint registry is + * restored, the very next dispatch is admitted — no operator intervention, + * no reset API. (Req 6.2) */ +static void test_gate_reaccepts_after_reconnect_without_reset(void) +{ + int rc_lost, rc_restored; + + /* Arrange: 3-member primary that has lost all standbys */ + gate_setup(3, 0); + rc_lost = ha_dispatch_gate(&g_gate_ha, OPCode_jobsub); + + /* Act: a standby reconnects (endpoint registry restored) — nothing else */ + zarray_store(g_gate_ha.standby_endpoints, &g_eps[0]); + rc_restored = ha_dispatch_gate(&g_gate_ha, OPCode_jobsub); + + /* Assert: rejected while below quorum... */ + TEST_ASSERT_EQUAL_INT_MESSAGE(-XCE_ERR_NOT_READY, rc_lost, + "writes must be rejected while below quorum"); + /* ...admitted immediately after restoration — stateless re-evaluation */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc_restored, + "gate must re-accept immediately after reconnect, no reset call"); + + gate_teardown(); + printf("test_gate_reaccepts_after_reconnect_without_reset: PASS\n"); +} + /* ── Test group entry point ──────────────────────────────────────────────── */ void test_commit_engine(void) @@ -811,5 +1088,14 @@ void test_commit_engine(void) 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); + /* Task 4.1 — write-admission quorum accounting */ + RUN_TEST(test_active_standby_count_empty_registry_is_zero); + RUN_TEST(test_active_standby_count_excludes_catchup_and_disconnected); + RUN_TEST(test_active_count_catchup_only_standby_denies_write_quorum); + RUN_TEST(test_active_count_one_active_standby_grants_write_quorum); + RUN_TEST(test_active_count_recovers_after_catchup_and_reconnect); + RUN_TEST(test_gate_single_member_always_admits_writes); + RUN_TEST(test_gate_rejects_writes_with_zero_connected_standbys); + RUN_TEST(test_gate_reaccepts_after_reconnect_without_reset); UNITY_END(); } diff --git a/src/include/repl.h b/src/include/repl.h index 3c76b9b0..f11e0962 100644 --- a/src/include/repl.h +++ b/src/include/repl.h @@ -51,6 +51,12 @@ int repl_add_standby(replicator_t *r, repl_ep_t ep, const char *host, uint32_t void repl_remove_standby(replicator_t *r, repl_ep_t ep); void repl_expire_disconnect_slots(replicator_t *r, time_t now_s, uint32_t snap_interval_s); +/* Write-admission quorum accounting (ha-commit-engine Req 3.1-3.3). + * Counts only standbys able to receive new log entries: slots with + * ep != NULL && !in_catch_up, evaluated under r->lock. + * NULL registry counts zero. Consumed by ha_has_write_quorum (ha.c). */ +int repl_active_standby_count(replicator_t *r); + /* Replication send (stub in P2-1; wired in P2-4) */ int repl_broadcast(replicator_t *r, const wal_entry_hdr_t *hdr, const void *payload, uint32_t payload_len);