From 07de4581b0b77e3e7c7cebfdbf9125c44a8cc8da Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 14 Jun 2026 06:31:25 +0000 Subject: [PATCH] =?UTF-8?q?feat(ha-convergence-recovery):=20quorum=20trans?= =?UTF-8?q?ition=20logging=20=E2=80=94=20CAS=20edge-detect=20+=204=20call?= =?UTF-8?q?=20sites=20(task=205.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .kiro/specs/ha-convergence-recovery/tasks.md | 5 +- src/controller/ha.c | 20 ++++++ src/controller/ha_convergence.c | 71 +++++++++++++++++--- 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/.kiro/specs/ha-convergence-recovery/tasks.md b/.kiro/specs/ha-convergence-recovery/tasks.md index e24881a..0b87b0b 100644 --- a/.kiro/specs/ha-convergence-recovery/tasks.md +++ b/.kiro/specs/ha-convergence-recovery/tasks.md @@ -59,13 +59,13 @@ - Observable completion: all task 4.1 tests PASS; heartbeat decision loop diff-clean (fallback + snapshot initiator preserved) - _Requirements: 5.1, 5.2, 5.3, 7.1_ -- [ ] 5. Quorum transition observability +- [x] 5. Quorum transition observability - [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_ -- [ ] 5.2 Implement quorum transition logging to green +- [x] 5.2 Implement quorum transition logging to green - ha.h: `_Atomic int quorum_lost` on `ha_state_t`; ha.c: `ha_log_quorum_transition(ha)` — CAS edge detection around `ha_has_write_quorum`, WARN with connected/required counts on each edge only - Call sites: promotion standby registration (ha.c:531 anchor — NOTE: this anchor is inside `raft_cb_become_leader` (ha.c:445-617), promotion code owned and rewritten wholesale by ha-single-leader-role; re-anchor after that spec lands, or coordinate with it if this task is implemented first), RDHUP removal (ha.c:3160-3166 anchor), `on_standby_connected` (ha.c:3764+ anchor), and the `ha_dispatch_gate` quorum-rejection branch as backstop - Observable completion: all task 5.1 tests PASS; no log line is emitted per rejected operation @@ -105,3 +105,4 @@ - **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`). +- **Task 5.2 done (GREEN) — quorum-decision duplication note**: `ha_log_quorum_transition` (ha_convergence.c) re-derives `connected = repl_active_standby_count(repl)` / `required = n_peers/2` / single-node short-circuit as a FAITHFUL same-input mirror of the file-static `ha_has_write_quorum` (ha.c) — it cannot report a state inconsistent with the gate. Non-blocking maintainability risk: if `ha_has_write_quorum`'s decision ever changes, this mirror must change too. **Follow-up (optional cleanup): expose `ha_has_write_quorum` via a non-static accessor and call it from `ha_log_quorum_transition`** instead of duplicating. Call sites wired (post ha-single-leader-role rewrite): promotion ha.c:567, RDHUP removal ha.c:3180, on_standby_connected ha.c:3837, gate backstop ha.c:1521. diff --git a/src/controller/ha.c b/src/controller/ha.c index 5d0dfe7..2618909 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -561,6 +561,11 @@ static void raft_cb_become_leader(void *ctx, uint64_t term) } } + /* Quorum-transition observability (Req 8.1, 8.2): promotion registered the + * surviving vote endpoints as standbys — the write-quorum count just changed. + * Record any loss/restore edge once (no-op when quorum is held at promotion). */ + ha_log_quorum_transition(ha); + if (c) { /* Push config to standbys — read raw bytes from disk so standbys * can save the file. NULL bytes are passed only in test mode. */ @@ -1509,6 +1514,11 @@ int ha_dispatch_gate(ha_state_t *ha, int opcode) * are durable across failover. Read-only queries and replication * ops are always allowed. */ if (!ha_has_write_quorum(ha)) { + /* Quorum-transition observability backstop (Req 8.1, 8.3): record the + * loss edge if a membership-change site missed it. The edge detector + * makes this silent on every call after the first — repeated rejected + * operations do NOT produce a log storm. */ + ha_log_quorum_transition(ha); switch (opcode) { /* Replication ops — always allowed (they build majority) */ case OPCode_heartbeat: @@ -3163,6 +3173,11 @@ static int vote_ep_err_handler(void *ep_raw) * vctx->c may be NULL in test mode — guard accordingly. */ if (vctx->c) repl_remove_standby(&vctx->c->persist.repl, ep); + + /* Quorum-transition observability (Req 8.1): a standby just dropped — the + * write-quorum count changed. Record the loss edge once (silent if + * quorum is still held after the removal). */ + ha_log_quorum_transition(ha); } else { /* Election phase: count as denied vote */ handle_request_vote_reply(ha, vctx->ctx, vctx->peer_idx, @@ -3819,6 +3834,11 @@ static void on_standby_connected(endpoint_t *ep) sctx->peer_idx, rc); } + /* Quorum-transition observability (Req 8.2): a late-joiner standby connected — + * the write-quorum count changed. Record the restore edge once (silent if + * quorum was already held). */ + ha_log_quorum_transition(ha); + /* Set state-aware dispatcher for subsequent messages on this endpoint */ ep->msg_handler = peer_ctrl_msg_handler; } diff --git a/src/controller/ha_convergence.c b/src/controller/ha_convergence.c index aee6773..409cbb5 100644 --- a/src/controller/ha_convergence.c +++ b/src/controller/ha_convergence.c @@ -383,21 +383,72 @@ void cluster_apply_ctrlhello_reply(cluster_t *c, uint64_t committed_lsn, } /* - * ha_log_quorum_transition — RED scaffold (ha-convergence-recovery task 5.1). + * ha_log_quorum_transition — write-quorum loss/restore edge logger + * (ha-convergence-recovery task 5.2, Req 8.1, 8.2, 8.3). * - * 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). + * Evaluate the current write-quorum state and compare it against the + * ha_state_t.quorum_lost edge-detector flag. On a TRANSITION — and only on a + * transition — atomically flip the flag via CAS and emit exactly one WARN + * carrying the connected/required member counts: + * - loss: "HA quorum lost: connected=%d required=%d" (Req 8.1) + * - restore: "HA quorum restored: connected=%d required=%d" (Req 8.2) * - * 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. + * 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 the + * dispatch-gate backstop's per-rejection calls), but a later loss/restore cycle + * logs again. The CAS guarantees once-per-transition under concurrency: only the + * caller that actually flips the flag emits the WARN. + * + * Observability only — this never returns a status and never gates an operation + * outcome; the dispatch gate's existing -XCE_ERR_NOT_READY behavior is unchanged. + * + * connected/required mirror the ha_has_write_quorum BODY (ha.c): with the repl + * back-pointer wired, connected = repl_active_standby_count(repl) and the + * threshold required = n_peers/2 (integer division — for n_peers=3 → 1). When + * the back-pointer is NULL (legacy contexts), connected falls back to the + * standby_endpoints count, matching the gate's own fallback exactly. The quorum + * decision math is NOT changed here — this only reports what that body decides. */ void ha_log_quorum_transition(ha_state_t *ha) { - (void)ha; /* RED scaffold — task 5.2 implements the edge detection. */ + if (ha == NULL) + return; + + /* Single-node clusters always hold quorum (ha_has_write_quorum returns true + * for n_peers <= 1); there is no transition to observe. */ + if (ha->n_peers <= 1) + return; + + /* Mirror ha_has_write_quorum's connected-count derivation (ha.c body). */ + int connected; + if (ha->repl != NULL) + connected = repl_active_standby_count(ha->repl); + else + connected = ha->standby_endpoints ? ha->standby_endpoints->count : 0; + + int required = (int)(ha->n_peers / 2); + int lost_now = (connected < required) ? 1 : 0; + + /* Edge detection: only the caller that flips quorum_lost from its prior value + * to lost_now emits. CAS expected=!lost_now → desired=lost_now; if the flag + * already equals lost_now (no transition) the CAS fails and we return + * silently (Req 8.3). */ + int expected = lost_now ? 0 : 1; + if (!atomic_compare_exchange_strong_explicit( + &ha->quorum_lost, &expected, lost_now, + memory_order_acq_rel, memory_order_acquire)) + return; /* no edge (or another thread already logged it) — no WARN */ + + if (lost_now) + WARN("HA quorum lost: connected=%d required=%d", connected, required); + else + WARN("HA quorum restored: connected=%d required=%d", connected, required); + +#ifdef XCE_TESTING + /* Emission-count seam (Req 8.1-8.3 observable): invoked once per WARN. */ + if (ha_quorum_transition_log_hook != NULL) + ha_quorum_transition_log_hook(lost_now, connected, required); +#endif } #ifdef XCE_TESTING