From 71ce41f486ae1d86e0824b36a5859b73a2f2fb90 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 13 Jun 2026 12:59:58 +0000 Subject: [PATCH] feat(ha-single-leader-role): delete dispatch-gate COMMITTING branch + collapse role predicates to PRIMARY (task 4.2) --- .kiro/specs/ha-single-leader-role/tasks.md | 4 +- src/controller/ha.c | 59 ++++++-------------- src/controller/testsuite/test_ha_vote_gate.c | 26 +++++---- 3 files changed, 35 insertions(+), 54 deletions(-) diff --git a/.kiro/specs/ha-single-leader-role/tasks.md b/.kiro/specs/ha-single-leader-role/tasks.md index f3f86508..ab91af86 100644 --- a/.kiro/specs/ha-single-leader-role/tasks.md +++ b/.kiro/specs/ha-single-leader-role/tasks.md @@ -45,7 +45,7 @@ - Observable completion: controller builds; `grep -rn "committing_timeout_fn\|ha_check_majority_convergence\|ha_repl_ack_received" src/controller/*.c src/controller/*.h` returns zero matches; test suite green - _Requirements: 1.5, 2.3_ -- [ ] 4.2 Delete the dispatch-gate COMMITTING branch and collapse role predicates +- [x] 4.2 Delete the dispatch-gate COMMITTING branch and collapse role predicates - Delete ONLY the `case CTRL_PRIMARY_COMMITTING:` block from `ha_dispatch_gate` (ha.c:1594-1612 pre-rollout anchor — re-grep; the block follows the ELECTING case); the CTRL_PRIMARY branch — including barrier-rollout's no-quorum fact allowlist (`OPCode_notify/update/setpreason/clrpreason`) — is preserved byte-for-byte - Collapse compound role checks to PRIMARY-only: `raft_cb_vote_eligible` (ha.c:210-211), `handle_request_vote` gates (ha.c:1984, 2004), `vote_ep_err_handler` (ha.c:3159), `peer_ctrl_msg_handler` case labels (ha.c:2789), `primary_heartbeat_fn` re-arm/run condition (ha.c:3956 — heartbeat re-arm condition drops COMMITTING); update the stale comments at each site plus ha.c:1940, 1971, 2057-2060, 2625-2627, 3044-3045 and the `ha_is_primary`/`ha_should_redirect` doc comments (no code change in those two functions) - Update gate tests: COMMITTING-specific cases in `test_ha_vote_gate.c` and gate fixtures retargeted (elected node denies same-term votes as PRIMARY — denial parity pinned) @@ -86,6 +86,8 @@ - **Unity runner gotcha**: the aggregated controller runner prints the PREVIOUS passing test's `file:line` as a stale prefix on a FAIL line — attribute failures by test NAME, not the printed file:line prefix. - **(3.1) Legacy path routes through BARE helpers, not `ha_promote_to_primary`**: `ha_elect_evaluate_and_signal`'s PRIMARY branch calls `ha_register_persist_hooks` + `ha_start_primary_heartbeat` + `ha_arm_own_term_gate` directly (matching the raft path), NOT `ha_promote_to_primary` — because the latter calls `ha_do_promote` (term increment) and the legacy branch historically has NO term store/increment (research §4 finding 2). This is a deliberate deviation from design §283's loose wording ("route through ha_promote_to_primary / ha_register_persist_hooks"); the bare-helper route preserves term semantics and is the correct reading. - **(4.1) No shims — DELETE, don't stub** (design §Transitional-State Deletions / §Design Synthesis: "rewritten or DELETED, not stubbed"). Machinery-subject test cases (convergence checker, committing watchdog, ack-driven promotion, ack counting) were DELETED, not shimmed: test_ha_role.c (5 majority_convergence + 2 committing_timeout_promotes_degraded), test_ha_rdhup_integration.c (rdhup_committing_to_primary_on_ack, rdhup_3node_one_ack_promotes), test_ha_connection_reuse.c (dispatcher_committing_handles_repl_ack, promoted_ep_repl_ack_promotes_to_primary). Kept state-entry cases had their deleted-fn CALLS excised (not shimmed). `test_ha_connection_reuse.c` was overlooked by the spec's §3 census disposition for machinery cases — confirmed in census (line 97) but its ack cases needed DELETE, not UPDATE. +- **(4.2) Residual stale COMMITTING comments in ha.c** (~lines 443, 826, 1240): documentary prose on the legacy raft_cb_become_leader/convergence path — NOT live branches. Scrub in 5.2/6.x (they fail the task-6.2 repo-wide grep). After 4.2, ZERO live code branches on `CTRL_PRIMARY_COMMITTING` in ha.c; the enum value persists in ha.h:64 until 5.2. +- **(4.2) Test-leak / suite-timeout facts**: the controller test target sets `ASAN_OPTIONS=detect_leaks=0` (controller/meson.build:163), so a pre-existing suite-wide LSan leak (~153KB, present at baseline 678b7f8, this spec leaks slightly LESS) is suppressed under the meson gate — LSan exit=1 only on a RAW direct-binary run, not a regression. The controller suite runs ~268s against a 300s meson timeout — borderline; the final task-6.2 full regression may TIMEOUT under machine load — re-run if so (it is not a failure). - **(4.1) Residual for task 6.2 final grep**: 3 DOCUMENTARY PROSE comments in `test_ha_role_lifecycle.c` (~lines 760/815/825) still mention `ha_repl_ack_received`/`committing_timeout_fn` by name (no live code). The 4.1 gate (`src/controller/*.c|*.h`) excludes testsuite so this passed; the task-6.2 repo-wide grep WILL catch them — scrub these comments in 5.x/6.x. Also a non-tracked editor backup `src/controller/.ha.c.~undo-tree~` holds stale refs (irrelevant; not a build input — do not commit). - **(3.1) Expected-failure set after task 3.1 (8 category-(a), all owned downstream)**: task 4.1 DELETES `test_ha_evaluate_committing.c` (3 fails: test_winning_quorum_enters_committing, test_committing_resets_repl_ack_count, test_election_timeout_fn_fires_evaluate); task 5.1 retargets `test_nonblocking_single_node_election_enters_committing` (test_ha_startup.c), `test_3node_win_calls_both_pushes` (test_ha_repl_push.c), and 3 RDHUP cases (test_ha_rdhup_integration.c: rdhup_full_path_reaches_committing, rdhup_committing_to_primary_on_ack, rdhup_3node_one_ack_promotes). `committing_timeout_fn` now has a benign `-Wunused-function` warning (no arming site left) until task 4.1 deletes it. - **Lifecycle fixture (test_ha_role_lifecycle.c)**: `lifecycle_cluster_setup` installs `repl_broadcast_send_hook = lc_drop_send_hook` (drop-only, reset to NULL in teardown) — required once promotion broadcasts the WAL_NOOP, else fake `int` endpoints cause an ASAN overflow. Gate cases prime `current_lsn = noop_lsn` so the post-increment append lands the noop at `noop_lsn` (production derives `noop_lsn = current_lsn-1`). diff --git a/src/controller/ha.c b/src/controller/ha.c index d9a13c0a..8bde5326 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -198,7 +198,7 @@ static int raft_cb_send_vote_request(void *ctx, int peer_idx, uint64_t term) * * from_idx is the absolute candidate index of the requester. * Applies the same checks as handle_request_vote: - * - role gate: PRIMARY or COMMITTING → deny + * - role gate: PRIMARY → deny * - fresh heartbeat on STANDBY → deny * - in_catch_up → deny * - freshness: candidate config_tag/lsn must be ≥ ours @@ -212,7 +212,7 @@ static int raft_cb_vote_eligible(void *ctx, int from_idx, uint64_t term) /* Role gate */ ctrl_role_t role = (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire); - if (role == CTRL_PRIMARY || role == CTRL_PRIMARY_COMMITTING) + if (role == CTRL_PRIMARY) return 0; /* Fresh heartbeat on STANDBY → primary is alive, deny */ @@ -1534,26 +1534,6 @@ int ha_dispatch_gate(ha_state_t *ha, int opcode) return 0; return -XCE_ERR_NOT_READY; - case CTRL_PRIMARY_COMMITTING: - /* Req 5.1, 5.2, 8.1: Primary won election but has not yet received - * majority replication ack. Allow only replication-related and - * administrative opcodes; deny job dispatch and queue management - * with XCE_ERR_NOT_READY. */ - switch (opcode) { - case OPCode_heartbeat: - case OPCode_replConfig: - case OPCode_replConfigPush: - case OPCode_upgradeConfigAck: - case OPCode_hello: - case OPCode_walAck: - case OPCode_replEntry: - case OPCode_replAck: - case OPCode_replCatchUpDone: - return 0; - default: - return -XCE_ERR_NOT_READY; - } - case CTRL_STANDBY: { /* Allow ops that standbys legitimately handle */ switch (opcode) { @@ -1957,7 +1937,7 @@ static int ha_heartbeat_is_fresh(ha_state_t *ha) * * Decision order (fail-fast): * 0. role gate (Req 6.1, 6.2): - * - CTRL_PRIMARY or CTRL_PRIMARY_COMMITTING → deny + * - CTRL_PRIMARY → deny * - CTRL_STANDBY with fresh heartbeat → deny * 1. in_catch_up → deny (Req 2.5) * 2. req term < my term → deny (Req 2.2) @@ -1988,20 +1968,19 @@ int handle_request_vote(ha_state_t *ha, * Raft §5.1: "If RPC request or response contains term T > currentTerm: * set currentTerm = T, convert to follower." * - * Without this ordering, a node in CTRL_PRIMARY_COMMITTING at term=N - * would deny a vote request at term=N+1 without ever seeing the higher - * term. Both nodes then independently promote to PRIMARY via their - * committing timeout → split-brain. */ + * Without this ordering, a CTRL_PRIMARY node at term=N would deny a vote + * request at term=N+1 without ever seeing the higher term, leaving two + * nodes both believing they lead → split-brain. */ uint64_t my_term = atomic_load_explicit(&ha->term, memory_order_acquire); if (term > my_term) { atomic_store_explicit(&ha->term, term, memory_order_release); ha->voted_for_term = 0; memset(ha->voted_for_name, 0, sizeof(ha->voted_for_name)); - /* Step down from leader/committing: a higher-term election is in - * progress and we must not block it with the role gate below. */ + /* Step down from leader: a higher-term election is in progress and we + * must not block it with the role gate below. */ ctrl_role_t cur_role = (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire); - if (cur_role == CTRL_PRIMARY || cur_role == CTRL_PRIMARY_COMMITTING) { + if (cur_role == CTRL_PRIMARY) { ha_do_demote(ha); INFO("HA: stepping down (term %" PRIu64 " → %" PRIu64 ") — higher-term election in progress", my_term, term); @@ -2017,12 +1996,12 @@ int handle_request_vote(ha_state_t *ha, /* Req 6.1, 6.2: role-based deny gate. * Evaluated AFTER the term catch-up above: if we just stepped down due to * a higher term, role is now CTRL_STANDBY and we fall through to voting. - * PRIMARY and PRIMARY_COMMITTING at the current term still deny votes - * (protecting the live primary from same-term challengers). */ + * PRIMARY at the current term still denies votes (protecting the live + * primary from same-term challengers). */ ctrl_role_t my_role = (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire); - if (my_role == CTRL_PRIMARY || my_role == CTRL_PRIMARY_COMMITTING) - return 0; /* deny — I am primary (or committing); protect against challengers */ + if (my_role == CTRL_PRIMARY) + return 0; /* deny — I am primary; protect against same-term challengers */ if (my_role == CTRL_STANDBY && ha->primary_ctrl_fd >= 0 && ha_heartbeat_is_fresh(ha)) return 0; /* deny — my primary is alive (active primary connection) */ @@ -2507,7 +2486,7 @@ static void on_vote_connected(endpoint_t *ep) #endif /* Set state-aware dispatcher — routes by ha->role at message-arrival time. - * Never reassigned: ELECTING → vote reply; COMMITTING/PRIMARY → repl ack. */ + * Never reassigned: ELECTING → vote reply; PRIMARY → repl ack. */ ep->msg_handler = peer_ctrl_msg_handler; } @@ -2642,7 +2621,7 @@ done: /* * repl_ack_decode_and_dispatch: decode an inbound packet on a promoted vote - * endpoint while role == CTRL_PRIMARY_COMMITTING or CTRL_PRIMARY. + * endpoint while role == CTRL_PRIMARY. * * - OPCode_replCatchUpDone → INFO no-op (ack-counting removed in task 4.1) * - OPCode_reply with stateReportMsg → ha_process_standby_state (Task 3.1) @@ -2820,7 +2799,6 @@ static int repl_ack_decode_and_dispatch(endpoint_t *ep, vote_ep_ctx_t *vctx) * peer_ctrl_msg_handler: state-aware iomgr handler set once in on_vote_connected. * Routes by ha->role at dispatch time — no explicit handler reassignment needed. * CTRL_ELECTING → vote_reply_decode_and_dispatch - * CTRL_PRIMARY_COMMITTING / * CTRL_PRIMARY → repl_ack_decode_and_dispatch * any other role → drop packet (WARN) */ @@ -2836,7 +2814,6 @@ static int peer_ctrl_msg_handler(void *ep_raw) case CTRL_ELECTING: return vote_reply_decode_and_dispatch(ep, vctx); - case CTRL_PRIMARY_COMMITTING: case CTRL_PRIMARY: return repl_ack_decode_and_dispatch(ep, vctx); @@ -3068,7 +3045,7 @@ void vote_ep_ctx_set_cluster_for_test(endpoint_t *ep, cluster_t *c) /* * raft_cb_become_leader_for_test: drive the leader promotion path without * a live cluster. Passes ha as ctx; sets ha->raft_c = NULL to suppress - * production-only calls (phase2, push_config, ctrlhello, committing timer). + * production-only calls (phase2, push_config, ctrlhello). */ void raft_cb_become_leader_for_test(ha_state_t *ha, uint64_t term) { @@ -3105,7 +3082,7 @@ static int vote_ep_err_handler(void *ep_raw) ctrl_role_t role = (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire); - if (role == CTRL_PRIMARY_COMMITTING || role == CTRL_PRIMARY) { + if (role == CTRL_PRIMARY) { /* Promoted connection dropped — remove from standby_endpoints */ if (ha->standby_endpoints) { pthread_mutex_lock(&ha->lock); @@ -3902,7 +3879,7 @@ static int primary_heartbeat_fn(void *arg) ctrl_role_t role = (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire); - if (role != CTRL_PRIMARY && role != CTRL_PRIMARY_COMMITTING) + if (role != CTRL_PRIMARY) return 0; /* demoted — stop sending NOPs */ /* 1. Heartbeat all connected standbys — idle-triggered NOP (Req 3.1-3.4). diff --git a/src/controller/testsuite/test_ha_vote_gate.c b/src/controller/testsuite/test_ha_vote_gate.c index 5e7768f0..b1a7c9bd 100644 --- a/src/controller/testsuite/test_ha_vote_gate.c +++ b/src/controller/testsuite/test_ha_vote_gate.c @@ -3,7 +3,7 @@ * * Tests verify (Req 6.1, 6.2, 6.4): * Req 6.1: CTRL_PRIMARY → deny all vote requests (return 0) - * Req 6.1: CTRL_PRIMARY_COMMITTING → deny all vote requests (return 0) + * Req 6.1: CTRL_PRIMARY + fresher same-term challenger → still deny (return 0) * Req 6.2: CTRL_STANDBY + fresh heartbeat → deny (return 0) * Req 6.4: CTRL_STANDBY + stale heartbeat → evaluate Raft gates (not denied by role gate) * Req 6.4: CTRL_ELECTING → evaluate Raft gates (not denied by role gate) @@ -88,16 +88,18 @@ static void test_primary_role_denies_vote_request(void) printf("test_primary_role_denies_vote_request: PASS\n"); } -/* ── Req 6.1: CTRL_PRIMARY_COMMITTING → deny ────────────────────────────── */ +/* ── Req 6.1: CTRL_PRIMARY denies a fresher same-term challenger ─────────── */ -static void test_primary_committing_role_denies_vote_request(void) +static void test_primary_role_denies_fresher_same_term_challenger(void) { - /* When the receiving node is CTRL_PRIMARY_COMMITTING (waiting for quorum - * replication ack before opening traffic), it must also deny all vote - * requests. A challenger must not displace a committing primary. */ + /* The vote-deny contract that a transitional COMMITTING role used to carry + * now lives entirely on CTRL_PRIMARY (single leader role). An elected + * primary must deny a same-term challenger even when the challenger looks + * FRESHER (higher config_tag / LSN): the role gate fires before any + * freshness evaluation, protecting the live primary from displacement. */ ha_state_t ha; ha_init(&ha, &g_gate_cfg, 0); - atomic_store_explicit(&ha.role, (int32_t)CTRL_PRIMARY_COMMITTING, + atomic_store_explicit(&ha.role, (int32_t)CTRL_PRIMARY, memory_order_release); atomic_store_explicit(&ha.term, 3ULL, memory_order_release); ha.voted_for_term = 0; @@ -105,17 +107,17 @@ static void test_primary_committing_role_denies_vote_request(void) int granted = handle_request_vote(&ha, 3ULL, /* same term */ "ctrl-c", - 200ULL, /* candidate is fresh */ + 200ULL, /* candidate is fresher */ 100ULL, 100ULL, /* my config_tag */ 50ULL, 0); TEST_ASSERT_EQUAL_INT_MESSAGE(0, granted, - "CTRL_PRIMARY_COMMITTING must deny all vote requests (Req 6.1)"); + "CTRL_PRIMARY must deny a fresher same-term challenger (Req 6.1)"); ha_fini(&ha); - printf("test_primary_committing_role_denies_vote_request: PASS\n"); + printf("test_primary_role_denies_fresher_same_term_challenger: PASS\n"); } /* ── Req 6.2: CTRL_STANDBY + fresh heartbeat → deny ────────────────────── */ @@ -234,8 +236,8 @@ void test_ha_vote_gate(void) /* Req 6.1 — PRIMARY denies */ RUN_TEST(test_primary_role_denies_vote_request); - /* Req 6.1 — PRIMARY_COMMITTING denies */ - RUN_TEST(test_primary_committing_role_denies_vote_request); + /* Req 6.1 — PRIMARY denies a fresher same-term challenger */ + RUN_TEST(test_primary_role_denies_fresher_same_term_challenger); /* Req 6.2 — STANDBY + fresh heartbeat denies */ RUN_TEST(test_standby_fresh_heartbeat_denies_vote_request);