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.
This commit is contained in:
Rongsong Shen 2026-06-11 16:15:21 +00:00
parent 631a08939b
commit b2598823c0
6 changed files with 322 additions and 3 deletions

View file

@ -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.

View file

@ -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)

View file

@ -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 |

View file

@ -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;
}

View file

@ -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 <errno.h>
#include <pthread.h>
#include <stdatomic.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unity/unity.h>
@ -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();
}

View file

@ -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);