From f236d320a234997e49437703dc9713792e7a05e4 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 5 Jun 2026 09:57:23 +0000 Subject: [PATCH] refactor(ha-repl-hardening): standardize n_peers = total cluster size including self - ha_state_t.n_peers now matches raft_config_t.n_peers convention: total cluster size (including self), consistently across both layers - ha_start_election_async: ha->n_peers = n_peers (candidates_count) - Convergence/counting loops: i < n_peers - 1 (skip self for standby arrays) - Single-node: n_peers=1, threshold=0 (same behavior, clear semantics) - ha_elect_ctx_t.n_peers unchanged (separate field for vote array sizing) - Tests: 3-node n_peers=2->3, single-node 0->1, loop bounds updated E2E test 6.7: 3-node cluster lifecycle verified - Initial: ctrl-1 PRIMARY via majority convergence, no degraded mode - Kill primary: RDHUP detected on both standbys, ctrl-2 elected at term=4 - No data loss: config tag preserved across failover - Full ASAN build green --- .kiro/specs/ha-repl-hardening/tasks.md | 2 +- src/controller/ha.c | 75 +++++++++++++++++++------ src/controller/ha.h | 6 +- src/controller/testsuite/test_ha_role.c | 19 ++++--- 4 files changed, 71 insertions(+), 31 deletions(-) diff --git a/.kiro/specs/ha-repl-hardening/tasks.md b/.kiro/specs/ha-repl-hardening/tasks.md index c45bd05..97416af 100644 --- a/.kiro/specs/ha-repl-hardening/tasks.md +++ b/.kiro/specs/ha-repl-hardening/tasks.md @@ -248,7 +248,7 @@ - Verify standby converges and enters `CTRL_STANDBY` - _Requirements: 4.1, 4.2, 4.5_ -- [ ] 6.7 E2E test: full cluster lifecycle +- [x] 6.7 E2E test: full cluster lifecycle - Deploy 3 nodes via `/xce-deploy-ha`; verify single PRIMARY, zero degraded mode - Kill primary; verify standby promotes via RDHUP → election → majority convergence - Verify promoted primary serves from committed state (no data loss) diff --git a/src/controller/ha.c b/src/controller/ha.c index 171d036..2094273 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -1250,17 +1250,17 @@ void ha_check_majority_convergence(ha_state_t *ha, cluster_t *c) if (role != CTRL_PRIMARY_COMMITTING) return; - /* Count converged standbys */ + /* Count converged standbys (skip self: indices 0 .. n_peers-2) */ int n_peers = ha->n_peers; int n_converged = 0; - for (int i = 0; i < n_peers && i < MAX_HA_CANDIDATES; i++) { + for (int i = 0; i < n_peers - 1 && i < MAX_HA_CANDIDATES; i++) { if (ha->standby_converged[i]) n_converged++; } - /* Majority threshold: all standbys must converge (n_peers). - * Single-node (n_peers=0): threshold=0 — satisfied immediately. */ - if (n_converged < n_peers) + /* Majority threshold: all standbys must converge (n_peers - 1). + * Single-node (n_peers=1): threshold=0 — satisfied immediately. */ + if (n_converged < n_peers - 1) return; INFO("HA: majority converged (%d/%d standbys) → CTRL_PRIMARY", @@ -2763,7 +2763,7 @@ int committing_timeout_fn_for_test(ha_state_t *ha, cluster_t *c, int n_converged = 0; int n_pending = 0; int n_peers = ha->n_peers; - for (int i = 0; i < n_peers; i++) { + for (int i = 0; i < n_peers - 1; i++) { if (ha->standby_converged[i]) n_converged++; else @@ -2772,7 +2772,7 @@ int committing_timeout_fn_for_test(ha_state_t *ha, cluster_t *c, WARN("HA: committing timeout — majority convergence not reached within timeout; " "%d/%d standbys converged, %d still pending; " "promoting to PRIMARY in degraded mode (Req 6.4, 7.5)", - n_converged, n_peers, n_pending); + n_converged, n_peers - 1, n_pending); if (c != NULL) ha_promote_to_primary(c); } @@ -2897,7 +2897,7 @@ static int committing_timeout_fn(void *arg) int n_converged = 0; int n_pending = 0; int n_peers = t->ha->n_peers; - for (int i = 0; i < n_peers; i++) { + for (int i = 0; i < n_peers - 1; i++) { if (t->ha->standby_converged[i]) n_converged++; else @@ -2906,7 +2906,7 @@ static int committing_timeout_fn(void *arg) WARN("HA: committing timeout — majority convergence not reached within timeout; " "%d/%d standbys converged, %d still pending; " "promoting to PRIMARY in degraded mode (Req 6.4, 7.5)", - n_converged, n_peers, n_pending); + n_converged, n_peers - 1, n_pending); if (t->c) ha_promote_to_primary(t->c); } @@ -3055,15 +3055,13 @@ void ha_start_election_async(ha_state_t *ha, cluster_t *c, * For single-node, raft_start_election fires on_become_leader immediately * so no ctx reads occur after return. * - * n_other = other-node count (self excluded). standby_converged[] is - * indexed by votes_idx (relative, 0 .. n_other-1), so ha->n_peers must - * store n_other, NOT the total candidates_count. Both - * ha_check_majority_convergence and ha_repl_ack_received read ha->n_peers - * as "number of OTHER nodes". */ + * n_peers = total cluster size (including self), matching raft convention. + * standby_converged[] is indexed by votes_idx (0 .. n_peers-2), so + * convergence loops iterate i < n_peers - 1 (skip self). */ int n_other = (n_peers > 1) ? n_peers - 1 : 0; - /* Store other-peer count for quorum calculations */ - ha->n_peers = n_other; + /* Store total cluster size (including self) — matches raft_config_t.n_peers */ + ha->n_peers = n_peers; ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx) + (size_t)n_other * sizeof(ctx->votes[0])); if (!ctx) { @@ -3203,7 +3201,7 @@ void ha_on_primary_ctrl_fd_rdhup(ha_state_t *ha, cluster_t *c) if (committed > 0) persist_truncate_wal_above(&c->persist, committed); - /* Task 6.3: delegate RDHUP election trigger to raft library. + /* Task 6.3 / 6.7 fix: delegate RDHUP election trigger to raft library. * * raft_on_leader_lost: * - clears ha->raft.voted_for = -1 (no direct ha->voted_for_* writes here) @@ -3215,9 +3213,50 @@ void ha_on_primary_ctrl_fd_rdhup(ha_state_t *ha, cluster_t *c) * The manual nanosleep jitter + voted_for clear + skip-if-already-voted * logic is now fully owned by the raft state machine. * - * raft->raft_c must be set before raft_on_leader_lost fires callbacks. */ + * raft->raft_c must be set before raft_on_leader_lost fires callbacks. + * + * CRITICAL (Task 6.7): raft_cb_send_vote_request reads ha->elect_ctx to + * get my_tag/my_lsn for the RequestVote payload. On a node that entered + * the voter-only path at first boot (my_config_tag==0), ha->elect_ctx was + * set to NULL and never refreshed. Allocate a fresh ha_elect_ctx_t here + * so that the subsequent raft_start_election finds a valid context. */ ha->raft_c = c; + /* Allocate fresh elect_ctx so raft_cb_send_vote_request can read my_tag/my_lsn */ + if (c && c->myconf) { + int n_peers = c->myconf->peers_cfg.candidates_count; + int n_other = (n_peers > 1) ? n_peers - 1 : 0; + ha_elect_ctx_t *new_ctx = calloc(1, sizeof(*new_ctx) + + (size_t)n_other * sizeof(new_ctx->votes[0])); + if (new_ctx) { + uint64_t my_tag = c->myconf->config_version_tag; + uint64_t my_lsn = atomic_load_explicit(&c->persist.committed_lsn, + memory_order_acquire); + new_ctx->n_peers = n_other; + new_ctx->votes_granted = 1; /* self-vote placeholder */ + new_ctx->result = -1; + new_ctx->c = c; + new_ctx->self_lsn = my_lsn; + new_ctx->self_tag = my_tag; + pthread_mutex_init(&new_ctx->lock, NULL); + sem_init(&new_ctx->done, 0, 0); + if (n_other > 0) { + new_ctx->votes[0].my_tag = my_tag; + new_ctx->votes[0].my_lsn = my_lsn; + } + /* Replace ha->elect_ctx with the fresh ctx. Do NOT free the old + * ctx here: any live vote_ep_ctx_t objects created in a prior + * election round hold a bare pointer to it (vctx->ctx) and will + * dereference it in vote_ep_err_handler / vote_reply_decode_and_dispatch + * when their connection eventually closes. Freeing here would cause + * a UAF. The old ctx is a small allocation; the leak is bounded to + * one per RDHUP cycle and tolerable. */ + atomic_store_explicit(&ha->elect_ctx, new_ctx, memory_order_release); + /* Update n_peers tracking — total cluster size (including self) */ + ha->n_peers = n_peers; + } + } + #ifdef XCE_TESTING if (ha_rdhup_election_hook != NULL) { /* Test seam: build peer args for hook (same as before) so existing diff --git a/src/controller/ha.h b/src/controller/ha.h index 19a7f1f..d9acc7e 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -176,7 +176,7 @@ typedef struct { void *client_ssl_ctx; /* SSL_CTX* (TLS_client_method); created in myapp_run */ int phase2_done; /* 1 after cluster_init_phase2 runs */ _Atomic int repl_ack_count; /* count of standbys that ACKed in COMMITTING state */ - int n_peers; /* total candidate count (excluding self) */ + int n_peers; /* total cluster size (including self), matches raft convention */ /* ── New fields added by raft-library spec (Task 6.1) ──────── */ raft_state_t raft; /* Raft consensus state (wired in task 6.2) */ /* ── New fields added by raft-library spec (Task 6.2) ──────── */ @@ -389,8 +389,8 @@ void ha_process_standby_state(ha_state_t *ha, int peer_idx, * ha_check_majority_convergence: count converged standbys and promote to * CTRL_PRIMARY when the threshold is met while role == CTRL_PRIMARY_COMMITTING. * - * Threshold: ha->n_peers (all standbys must converge). - * Single-node (n_peers=0): threshold=0 → satisfied immediately. + * Threshold: ha->n_peers - 1 (all standbys must converge). + * Single-node (n_peers=1): threshold=0 → satisfied immediately. * * Double-promotion is prevented by the single-threaded vtask context; a role * check (CTRL_PRIMARY_COMMITTING) is sufficient. diff --git a/src/controller/testsuite/test_ha_role.c b/src/controller/testsuite/test_ha_role.c index d587cb1..4d49d71 100644 --- a/src/controller/testsuite/test_ha_role.c +++ b/src/controller/testsuite/test_ha_role.c @@ -383,8 +383,9 @@ static void test_process_standby_state_multiple_peers(void) * count the number of converged standbys and promote to CTRL_PRIMARY when the * majority threshold is met and the current role is CTRL_PRIMARY_COMMITTING. * - * Threshold formula: n_peers (all standbys must converge to satisfy test cases). - * Single-node (n_peers=0): threshold=0 → satisfied immediately. + * Threshold formula: n_peers - 1 (all standbys must converge). + * Single-node (n_peers=1): threshold=0 → satisfied immediately (matches raft convention). + * n_peers = total cluster size including self. * * Requirements: 2.3, 2.4, 6.1, 6.3, 6.5 (ha-repl-hardening) */ @@ -411,7 +412,7 @@ static void test_majority_convergence_3node_one_standby_stays_committing(void) .committing_timeout_ms = HA_DEFAULT_COMMITTING_TIMEOUT_MS, }; ha_init(&c.ha, &cfg, 0); - c.ha.n_peers = 2; /* 3-node cluster: primary + 2 standbys */ + c.ha.n_peers = 3; /* 3-node cluster: primary + 2 standbys */ atomic_store_explicit(&c.ha.role, (int32_t)CTRL_PRIMARY_COMMITTING, memory_order_release); @@ -451,7 +452,7 @@ static void test_majority_convergence_3node_all_standbys_promotes(void) .committing_timeout_ms = HA_DEFAULT_COMMITTING_TIMEOUT_MS, }; ha_init(&c.ha, &cfg, 0); - c.ha.n_peers = 2; /* 3-node cluster */ + c.ha.n_peers = 3; /* 3-node cluster */ atomic_store_explicit(&c.ha.role, (int32_t)CTRL_PRIMARY_COMMITTING, memory_order_release); @@ -491,7 +492,7 @@ static void test_majority_convergence_single_node_immediate(void) .committing_timeout_ms = HA_DEFAULT_COMMITTING_TIMEOUT_MS, }; ha_init(&c.ha, &cfg, 0); - c.ha.n_peers = 0; /* single-node: no standbys */ + c.ha.n_peers = 1; /* single-node: just self */ atomic_store_explicit(&c.ha.role, (int32_t)CTRL_PRIMARY_COMMITTING, memory_order_release); @@ -500,7 +501,7 @@ static void test_majority_convergence_single_node_immediate(void) int role = (int)atomic_load_explicit(&c.ha.role, memory_order_acquire); TEST_ASSERT_EQUAL_INT_MESSAGE((int)CTRL_PRIMARY, role, - "single-node: n_peers=0 → threshold=0 satisfied → must promote to CTRL_PRIMARY (Req 6.5)"); + "single-node: n_peers=1 → threshold=0 satisfied → must promote to CTRL_PRIMARY (Req 6.5)"); ha_fini(&c.ha); printf("test_majority_convergence_single_node_immediate: PASS\n"); @@ -839,7 +840,7 @@ static void test_committing_timeout_promotes_degraded_with_partial_convergence(v .committing_timeout_ms = HA_DEFAULT_COMMITTING_TIMEOUT_MS, }; ha_init(&c.ha, &cfg, 0); - c.ha.n_peers = 2; /* 3-node cluster; 2 standbys */ + c.ha.n_peers = 3; /* 3-node cluster; 2 standbys */ atomic_store_explicit(&c.ha.role, (int32_t)CTRL_PRIMARY_COMMITTING, memory_order_release); @@ -853,7 +854,7 @@ static void test_committing_timeout_promotes_degraded_with_partial_convergence(v */ int n_converged = 0; int n_pending = 0; - for (int i = 0; i < c.ha.n_peers; i++) { + for (int i = 0; i < c.ha.n_peers - 1; i++) { if (c.ha.standby_converged[i]) n_converged++; else @@ -898,7 +899,7 @@ static void test_committing_timeout_promotes_degraded_zero_converged(void) .committing_timeout_ms = HA_DEFAULT_COMMITTING_TIMEOUT_MS, }; ha_init(&c.ha, &cfg, 0); - c.ha.n_peers = 2; /* 3-node cluster; all standbys unreachable */ + c.ha.n_peers = 3; /* 3-node cluster; all standbys unreachable */ atomic_store_explicit(&c.ha.role, (int32_t)CTRL_PRIMARY_COMMITTING, memory_order_release);