From 3218d9ffafb75ef2feed938f8b8ea18ad99b1e02 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 14 Jun 2026 06:09:01 +0000 Subject: [PATCH] test(ha-convergence-recovery): RED quorum transition logging tests + seam (task 5.1) --- .kiro/specs/ha-convergence-recovery/tasks.md | 3 +- src/controller/ha.h | 62 +++++ src/controller/ha_convergence.c | 25 ++ .../testsuite/test_convergence_verification.c | 233 +++++++++++++++++- 4 files changed, 321 insertions(+), 2 deletions(-) diff --git a/.kiro/specs/ha-convergence-recovery/tasks.md b/.kiro/specs/ha-convergence-recovery/tasks.md index 67a7ae8..e24881a 100644 --- a/.kiro/specs/ha-convergence-recovery/tasks.md +++ b/.kiro/specs/ha-convergence-recovery/tasks.md @@ -60,7 +60,7 @@ - _Requirements: 5.1, 5.2, 5.3, 7.1_ - [ ] 5. Quorum transition observability -- [ ] 5.1 Write RED tests for once-per-transition quorum logging +- [x] 5.1 Write RED tests for once-per-transition quorum logging - Tests in `test_convergence_verification.c` (log-capture or counter-hook pattern): dropping standbys below majority logs the loss WARN exactly once with connected/required counts; repeated dispatch-gate rejections while quorum stays lost add no further transition logs; restoring a standby logs the restore WARN once; a second loss/restore cycle logs again (edge detector, not a latch) - Observable completion: new cases FAIL (red) — no transition logging exists (ha.c:1507-1525 anchor) - _Requirements: 8.1, 8.2, 8.3_ @@ -104,3 +104,4 @@ - **CLOEXEC**: controller sockets are already `FD_CLOEXEC` at creation (`socket_create` netio.c:219, `socket_accept` netio.c:308) — the re-exec'd image inherits no live sockets; no netio.c change was needed. - **Disk space**: heavy nix builds can fill the root overlay with stale `/tmp/nix-shell.*` AND `/tmp/xce_*` test dirs (reviewers reclaimed ~16GB nix-shell + ~570GB stale xce test dirs). If a build/test fails on "No space left on device", clear stale `/tmp/nix-shell.*` and `/tmp/xce_*` and retry — not a code failure. - **Task 3.2 — delaymgr one-shot helper + once-guard test strength**: `ha_rejoin_reconcile` lives in `ha_convergence.c` (alongside `ha_self_restart`/demotion task, unit-linkable) and defers truncate→WARN→restart to a delaymgr one-shot, reusing the 2.2 pattern. The test helper `dc_run_pending_task` was fixed to DEQUEUE (`ordq_get`+free) mirroring the real delaymgr worker (delay.c:65-90) — the prior peek-without-dequeue caused a test-only heap-UAF (re-running an already-freed one-shot); production never re-runs a one-shot, so the body-frees-own-ctx pattern is safe (bug-343). **Follow-up (non-blocking, for task 6.1 or 7): add a once-guard test that calls `ha_rejoin_reconcile` TWICE before draining the delaymgr (tip unchanged) and asserts exactly one task scheduled** — the current second-call test runs truncation synchronously between calls, so call 2 exits at the 2.3 early-return and exercises only the structural guard, not the defensive CAS (CAS is present + correct, needed for a duplicate CtrlHelloReply arriving before the deferred truncation runs). +- **Task 5.1 done (RED) + 5.2 notes**: counter-hook seam `ha_quorum_transition_log_hook(int lost,int connected,int required)` is the observable (no log-capture harness exists); `_Atomic int quorum_lost` added to ha_state_t; no-op stub `ha_log_quorum_transition` in ha_convergence.c. **5.2 must**: (a) report `connected = repl_active_standby_count(repl)` and `required = n_peers/2` to match the `ha_has_write_quorum` BODY (ha.c:~1488-1498) — its header comment says "ceil(n_peers/2)+1" but that's a pre-existing stale comment; do NOT change the quorum math, just mirror the body so the 5.1 tests pass; (b) re-grep the promotion standby-registration call site inside `raft_cb_become_leader` (rewritten by ha-single-leader-role) — RDHUP removal (~ha.c:3160), `on_standby_connected` (~ha.c:3764), and the gate-rejection backstop (~ha.c:1511) are verified present; (c) invoke the hook once per WARN emitted (CAS edge-detect on `quorum_lost`). diff --git a/src/controller/ha.h b/src/controller/ha.h index 5ba7bf7..2d3c0ad 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -211,6 +211,15 @@ typedef struct { * Slot identity (the repl registry slot * index) is stable across schedule/run. * 0 at init (cleared by ha_init memset). */ + /* ── New field added by ha-convergence-recovery spec (Task 5.x) ──────── */ + _Atomic int quorum_lost; /* Quorum-transition edge detector (Req + * 8.1-8.3): 0 = quorum held (or unknown + * at init), 1 = quorum lost. Flipped by + * ha_log_quorum_transition via CAS so the + * loss/restore WARN is emitted exactly + * once per edge (no log storms across + * repeated dispatch-gate rejections). + * 0 at init (cleared by ha_init memset). */ } ha_state_t; /* ── Public API ────────────────────────────────────────────── */ @@ -377,6 +386,35 @@ void cluster_apply_ctrlhello_reply(cluster_t *c, uint64_t committed_lsn, */ int ha_walack_on_ack(cluster_t *c, void *ep, int peer_idx, uint64_t acked_lsn); +/* + * ha_log_quorum_transition: emit an operator-visible WARN on each write-quorum + * loss/restore EDGE (ha-convergence-recovery task 5.x, Req 8.1, 8.2, 8.3). + * + * Evaluates the current write-quorum state and compares it against the + * ha_state_t.quorum_lost edge-detector flag. On a TRANSITION (and only on a + * transition), it atomically flips the flag via CAS and emits exactly one WARN: + * - loss: "HA quorum lost: connected=%d required=%d" (Req 8.1) + * - restore: "HA quorum restored: connected=%d required=%d" (Req 8.2) + * The flag makes the logging an edge detector, NOT a latch: repeated calls while + * the state is unchanged emit nothing (Req 8.3 — no log storms across repeated + * dispatch-gate rejections), but a later loss/restore cycle logs again. The CAS + * guarantees once-per-transition under concurrency. + * + * Call sites (task 5.2): the standby_endpoints mutation points (promotion + * registration, RDHUP removal, on_standby_connected) plus the ha_dispatch_gate + * quorum-rejection branch as a backstop. Observability only — never gates an + * operation outcome. + * + * Defined in ha_convergence.c (a unit-linkable TU) so the quorum-transition + * tests can drive it directly against a real persist/repl fixture. + * + * RED scaffold (task 5.1): the current definition is a NO-OP — it performs no + * edge detection, flips no flag, and emits no WARN — so the quorum-transition + * tests FAIL until task 5.2 lands the real edge-detection logic and wires the + * call sites. + */ +void ha_log_quorum_transition(ha_state_t *ha); + /* ── Redirect helpers used by join.c ──────────────────────── */ @@ -986,6 +1024,30 @@ void raft_cb_become_follower_with_cluster_for_test(cluster_t *c, uint64_t term); */ int ha_walack_continuation_for_test(cluster_t *c, void *ep, int peer_idx, uint64_t acked_lsn); + +/* + * ha_quorum_transition_log_hook: test-only emission-count seam for + * ha_log_quorum_transition (ha-convergence-recovery task 5.x, Req 8.1-8.3). + * + * There is NO log-capture harness in this testsuite, so the quorum-transition + * tests use a counter/edge-observable instead (the task permits "log-capture OR + * counter-hook pattern"). When non-NULL, ha_log_quorum_transition invokes this + * hook EXACTLY ONCE on each transition it WARNs about, passing the new state and + * the connected/required counts that go into the WARN line. Tests count + * invocations to assert "logged once per transition" (Req 8.3) and inspect the + * arguments to assert the loss line carries the right counts (Req 8.1). + * + * Signature: void hook(int lost, int connected, int required) + * lost — 1 on a quorum-LOSS edge, 0 on a quorum-RESTORE edge. + * connected — connected standby count reported in the WARN line. + * required — required standby count (quorum threshold) in the WARN line. + * + * RED scaffold (task 5.1): ha_log_quorum_transition is a no-op stub that never + * invokes this hook, so the transition tests' emission-count assertions FAIL + * until task 5.2 implements the real edge detection. + */ +extern void (*ha_quorum_transition_log_hook)(int lost, int connected, + int required); #endif #endif /* HA_H */ diff --git a/src/controller/ha_convergence.c b/src/controller/ha_convergence.c index 379faf4..aee6773 100644 --- a/src/controller/ha_convergence.c +++ b/src/controller/ha_convergence.c @@ -56,6 +56,13 @@ * Tests set these before exercising the restart/fatal contract. */ int (*ha_restart_hook)(cluster_t *c) = NULL; void (*ha_fatal_hook)(const char *msg) = NULL; + +/* Quorum-transition emission-count seam (task 5.x, Req 8.1-8.3) — defined here, + * declared in ha.h. ha_log_quorum_transition invokes this once per WARN it + * emits so the counter-hook tests can assert once-per-transition + counts. + * NULL until a test installs it. */ +void (*ha_quorum_transition_log_hook)(int lost, int connected, + int required) = NULL; #endif /* @@ -375,6 +382,24 @@ void cluster_apply_ctrlhello_reply(cluster_t *c, uint64_t committed_lsn, (void)ha_rejoin_reconcile(c, committed_lsn); } +/* + * ha_log_quorum_transition — RED scaffold (ha-convergence-recovery task 5.1). + * + * Task 5.2 replaces this body with the real edge detector: evaluate the current + * write-quorum state (ha_has_write_quorum, ha.c), compare against ha->quorum_lost, + * and on a transition CAS-flip the flag and emit exactly one WARN with the + * connected/required counts (Req 8.1, 8.2), invoking ha_quorum_transition_log_hook + * once under XCE_TESTING. Repeated calls with no state change emit nothing + * (Req 8.3); a later loss/restore cycle logs again (edge detector, not a latch). + * + * This RED stub does NOTHING: no edge detection, no flag flip, no WARN, no hook + * invocation — so the task 5.1 quorum-transition tests FAIL until 5.2 lands. + */ +void ha_log_quorum_transition(ha_state_t *ha) +{ + (void)ha; /* RED scaffold — task 5.2 implements the edge detection. */ +} + #ifdef XCE_TESTING /* ─────────────────────────────────────────────────────────────────────────── * ha_walack_continuation_for_test — task 4.2 GREEN. diff --git a/src/controller/testsuite/test_convergence_verification.c b/src/controller/testsuite/test_convergence_verification.c index 5c2fb49..3573109 100644 --- a/src/controller/testsuite/test_convergence_verification.c +++ b/src/controller/testsuite/test_convergence_verification.c @@ -60,7 +60,8 @@ * ha_repl_stream_wal_hook, HA_REPL_* */ #include "ordq.h" /* ordq_item_t (drive the scheduled task) */ #include "persist.h" /* persist_t */ -#include "repl.h" /* repl_add_standby / repl_set_commit_engine / standby slots */ +#include "repl.h" /* repl_add_standby / repl_remove_standby / + * repl_active_standby_count / standby slots */ #include "xce/cluster_def.h" /* full cluster_t layout */ #ifdef XCE_TESTING @@ -463,6 +464,229 @@ static void test_heartbeat_decision_path_unchanged(void) printf("test_heartbeat_decision_path_unchanged: checked\n"); } +/* ── Quorum transition logging (task 5.1, Req 8.1-8.3) ─────────────────────── + * + * Observable pattern: there is NO log-capture harness in this testsuite, so + * these tests use the emission-count seam ha_quorum_transition_log_hook (the + * task permits "log-capture OR counter-hook pattern"). The real + * ha_log_quorum_transition (task 5.2) invokes the hook exactly once on each + * loss/restore EDGE it WARNs about, passing the new state and the + * connected/required counts that go into the WARN line. We count invocations to + * assert "logged once per transition" and inspect the args to assert the loss + * line carries the right counts. + * + * RED status against the current tree: ha_log_quorum_transition is a no-op RED + * scaffold (ha_convergence.c) — it performs no edge detection, flips no flag, + * and never invokes the hook. Every assertion below that expects an emission + * (or an edge-flag flip) therefore FAILS until task 5.2 lands the real edge + * detector and wires the call sites. + * + * Quorum arithmetic for the fixture (ha_has_write_quorum, ha.c): + * n_peers=3, ha->repl set → connected = repl_active_standby_count(repl); + * quorum held iff connected >= n_peers/2 = 1. So 1 active standby = quorum, + * 0 active standbys = quorum lost. The required count in the WARN line is the + * threshold n_peers/2 = 1. + */ + +/* Emission-count hook: records every transition WARN ha_log_quorum_transition + * emits, capturing the latest (lost, connected, required) triple. */ +static atomic_int g_qt_calls; +static atomic_int g_qt_loss_calls; +static atomic_int g_qt_restore_calls; +static atomic_int g_qt_last_lost; +static atomic_int g_qt_last_connected; +static atomic_int g_qt_last_required; + +static void cv_quorum_log_hook(int lost, int connected, int required) +{ + atomic_fetch_add(&g_qt_calls, 1); + if (lost) + atomic_fetch_add(&g_qt_loss_calls, 1); + else + atomic_fetch_add(&g_qt_restore_calls, 1); + atomic_store(&g_qt_last_lost, lost); + atomic_store(&g_qt_last_connected, connected); + atomic_store(&g_qt_last_required, required); +} + +static void cv_quorum_reset_counters(void) +{ + atomic_store(&g_qt_calls, 0); + atomic_store(&g_qt_loss_calls, 0); + atomic_store(&g_qt_restore_calls, 0); + atomic_store(&g_qt_last_lost, -1); + atomic_store(&g_qt_last_connected, -1); + atomic_store(&g_qt_last_required, -1); +} + +/* Build a quorum fixture: n_peers=3, registry-truth quorum accounting wired + * (ha.repl = &persist.repl) and exactly one active standby so the cluster STARTS + * with write quorum held (quorum_lost == 0). Installs the emission-count hook. */ +static void cv_quorum_setup(const char *suffix) +{ + cv_setup(suffix, /*n_peers=*/3, /*n_standbys=*/1, + /*appended_lsn=*/0, /*n_vcpus=*/8); + /* Wire registry-truth quorum accounting (ha-commit-engine path): with + * ha.repl set, ha_has_write_quorum counts repl_active_standby_count. */ + g_cv.ha.repl = &g_cv.persist.repl; + g_cv.ha.role = CTRL_PRIMARY; /* quorum gate is a CTRL_PRIMARY concern */ + + cv_quorum_reset_counters(); + ha_quorum_transition_log_hook = cv_quorum_log_hook; + + /* Fixture precondition: one active standby → connected=1 >= required=1 → + * quorum held at the start of every quorum test. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(1, + repl_active_standby_count(&g_cv.persist.repl), + "quorum fixture precondition: exactly one active standby (quorum held)"); +} + +static void cv_quorum_teardown(void) +{ + ha_quorum_transition_log_hook = NULL; + cv_teardown(); +} + +/* 8.1: dropping the only standby below majority logs the loss WARN EXACTLY ONCE, + * carrying the connected/required counts; the edge flag flips to lost. + * RED: the no-op stub never detects the edge → 0 emissions, flag stays 0. */ +static void test_quorum_loss_logged_once_with_counts(void) +{ + cv_quorum_setup("qloss"); + + /* Act: remove the only standby → connected drops 1→0 → quorum lost. */ + repl_remove_standby(&g_cv.persist.repl, &g_cv_eps[0]); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, + repl_active_standby_count(&g_cv.persist.repl), + "fixture: the cluster must have zero active standbys after removal"); + + ha_log_quorum_transition(&g_cv.ha); + + /* Assert (Req 8.1): exactly one loss WARN, no restore WARN. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_loss_calls), + "dropping below majority must log the quorum-loss WARN exactly once " + "(Req 8.1)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, atomic_load(&g_qt_restore_calls), + "a quorum-loss edge must not emit a restore WARN (Req 8.1)"); + + /* Assert (Req 8.1): the WARN carries the connected/required counts. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, atomic_load(&g_qt_last_connected), + "the quorum-loss WARN must report connected=0 (Req 8.1)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_last_required), + "the quorum-loss WARN must report required=1 (n_peers/2 threshold) " + "(Req 8.1)"); + + /* The edge-detector flag must now read 'lost'. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_cv.ha.quorum_lost), + "a quorum-loss edge must set the ha_state_t.quorum_lost flag (Req 8.1)"); + + cv_quorum_teardown(); + printf("test_quorum_loss_logged_once_with_counts: checked\n"); +} + +/* 8.3: while quorum stays lost, repeated dispatch-gate rejections (modeled as + * repeated ha_log_quorum_transition calls — the gate backstop calls it on every + * rejected op) add NO further transition logs. + * RED: the no-op stub never logs the first loss either, so the "exactly one loss + * across many calls" assertion fails (0 != 1). */ +static void test_quorum_loss_no_repeat_log_while_lost(void) +{ + cv_quorum_setup("qstorm"); + + /* Cross the loss edge once. */ + repl_remove_standby(&g_cv.persist.repl, &g_cv_eps[0]); + ha_log_quorum_transition(&g_cv.ha); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_loss_calls), + "setup: the first call after the loss edge must log once (Req 8.3 setup)"); + + /* Act: simulate many rejected operations re-invoking the backstop while + * quorum stays lost (no membership change between calls). */ + for (int i = 0; i < 5; i++) + ha_log_quorum_transition(&g_cv.ha); + + /* Assert (Req 8.3): still exactly one loss WARN — no log storm. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_loss_calls), + "repeated quorum-transition calls while quorum stays lost must NOT log " + "again — once per transition, no storms (Req 8.3)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, atomic_load(&g_qt_restore_calls), + "no restore WARN may appear while quorum remains lost (Req 8.3)"); + + cv_quorum_teardown(); + printf("test_quorum_loss_no_repeat_log_while_lost: checked\n"); +} + +/* 8.2: restoring a standby above majority logs the restore WARN ONCE; the edge + * flag flips back to held. + * RED: the no-op stub neither sets nor clears the flag and never emits. */ +static void test_quorum_restore_logged_once(void) +{ + cv_quorum_setup("qrestore"); + + /* Drive into the lost state first. */ + repl_remove_standby(&g_cv.persist.repl, &g_cv_eps[0]); + ha_log_quorum_transition(&g_cv.ha); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_loss_calls), + "setup: must be in the lost state with one loss WARN (Req 8.2 setup)"); + + /* Act: re-register a standby → connected 0→1 → quorum restored. */ + TEST_ASSERT_TRUE( + repl_add_standby(&g_cv.persist.repl, &g_cv_eps[0], "cvsb0", 7600) >= 0); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, + repl_active_standby_count(&g_cv.persist.repl), + "fixture: the re-registered standby must restore the active count to 1"); + + ha_log_quorum_transition(&g_cv.ha); + + /* Assert (Req 8.2): exactly one restore WARN, loss count unchanged. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_restore_calls), + "restoring quorum must log the quorum-restore WARN exactly once " + "(Req 8.2)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_loss_calls), + "the restore edge must not re-emit a loss WARN (Req 8.2)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, atomic_load(&g_cv.ha.quorum_lost), + "a quorum-restore edge must clear the ha_state_t.quorum_lost flag " + "(Req 8.2)"); + + cv_quorum_teardown(); + printf("test_quorum_restore_logged_once: checked\n"); +} + +/* 8.1+8.2 edge detector (not a latch): a SECOND loss/restore cycle logs again. + * RED: the no-op stub never logs any cycle. */ +static void test_quorum_second_cycle_logs_again(void) +{ + cv_quorum_setup("qcycle"); + + /* Cycle 1: lose then restore. */ + repl_remove_standby(&g_cv.persist.repl, &g_cv_eps[0]); + ha_log_quorum_transition(&g_cv.ha); + TEST_ASSERT_TRUE( + repl_add_standby(&g_cv.persist.repl, &g_cv_eps[0], "cvsb0", 7600) >= 0); + ha_log_quorum_transition(&g_cv.ha); + + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_loss_calls), + "cycle 1: exactly one loss WARN (edge-detector setup)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_qt_restore_calls), + "cycle 1: exactly one restore WARN (edge-detector setup)"); + + /* Cycle 2: lose then restore AGAIN — the detector is not a one-shot latch. */ + repl_remove_standby(&g_cv.persist.repl, &g_cv_eps[0]); + ha_log_quorum_transition(&g_cv.ha); + TEST_ASSERT_TRUE( + repl_add_standby(&g_cv.persist.repl, &g_cv_eps[0], "cvsb0", 7600) >= 0); + ha_log_quorum_transition(&g_cv.ha); + + /* Assert: each transition logged again (totals = 2 loss + 2 restore). */ + TEST_ASSERT_EQUAL_INT_MESSAGE(2, atomic_load(&g_qt_loss_calls), + "a SECOND quorum-loss edge must log again — the transition logger is an " + "edge detector, not a one-shot latch (Req 8.1)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(2, atomic_load(&g_qt_restore_calls), + "a SECOND quorum-restore edge must log again (Req 8.2)"); + + cv_quorum_teardown(); + printf("test_quorum_second_cycle_logs_again: checked\n"); +} + #endif /* XCE_TESTING */ /* ── Suite entry point ──────────────────────────────────────────────────── */ @@ -477,6 +701,13 @@ void test_convergence_verification(void) RUN_TEST(test_walack_reschedules_after_continuation_completes); RUN_TEST(test_walack_at_parity_completes_catchup_and_clears_in_catch_up); RUN_TEST(test_heartbeat_decision_path_unchanged); + + printf("\n>> Quorum transition logging tests (RED, task 5.1)\n\n"); + + RUN_TEST(test_quorum_loss_logged_once_with_counts); + RUN_TEST(test_quorum_loss_no_repeat_log_while_lost); + RUN_TEST(test_quorum_restore_logged_once); + RUN_TEST(test_quorum_second_cycle_logs_again); #else printf("test_convergence_verification: SKIP (non-test build)\n"); #endif