wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

feat(ha-single-leader-role): legacy election path symmetry — direct CTRL_PRIMARY via shared sequence (task 3.1)

Browse source
This commit is contained in:
Rongsong Shen 2026-06-13 10:16:14 +00:00
parent c089fb69f3
commit b7b951b9e7
4 changed files with 167 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

4
.kiro/specs/ha-single-leader-role/tasks.md
View file

@ -31,7 +31,7 @@
- _Requirements: 1.1, 1.3, 1.4, 1.5, 2.1, 2.2, 2.3_
- [ ] 3. Legacy XCE_TESTING election path symmetry
- [ ] 3.1 Update ha_elect_evaluate_and_signal and its direct tests (RED → GREEN in one pass)
- [x] 3.1 Update ha_elect_evaluate_and_signal and its direct tests (RED → GREEN in one pass)
- Retarget existing legacy-path expectations first (test_ha_start_election_async.c, the evaluation cases in test_ha_role.c): quorum win → role `CTRL_PRIMARY` directly, no watchdog armed — run to confirm RED against current code
- In the CTRL_PRIMARY branch of `ha_elect_evaluate_and_signal` (ha.c:2247-2300 anchor): replace the COMMITTING store (2248-2252) with the direct `CTRL_PRIMARY` store, delete the `repl_ack_count` reset (2255) and the watchdog-arm block (2275-2299); keep the config-push/ctrlhello XCE_TESTING hooks; route promotion side effects through the same shared sequence (noop+gate tail guarded on persist/engine availability, as in production)
- Observable completion: retargeted legacy-path tests PASS; lifecycle test for symmetry PASSES (both entry points produce identical role/hook/heartbeat/no-watchdog state)
@ -84,4 +84,6 @@
- **(2.2) Expected category-(a) failures open from task 2.2 until retargeted**: after the raft path enters `CTRL_PRIMARY` directly, three single-node tests still assert `CTRL_PRIMARY_COMMITTING (4)` and FAIL (`Expected 4 Was 0`): `test_single_node_shortcircuit_evaluates_immediately` + `test_single_node_not_primary_directly` (test_ha_start_election_async.c) → retargeted by **task 3.1**; `test_nonblocking_single_node_election_enters_committing` (test_ha_startup.c) → retargeted by **task 5.1**. Single-node election reaches `raft_cb_become_leader` via the `raft_start_election` shortcircuit firing `on_become_leader`.
- **(2.2) ha.c shared sequence**: raft path stores `CTRL_PRIMARY` directly with NO `ha_do_promote` (term synced at entry — avoids double-increment); `ha_promote_to_primary` keeps `ha_do_promote` (owns its term). Both call static `ha_register_persist_hooks(c)` (BEFORE noop append) and static `ha_arm_own_term_gate(c)` (`persist_wal_async(WAL_NOOP)` → `noop_lsn = current_lsn-1` under p->lock → `commit_submit(RETAIN)` → `commit_set_writable_lsn`); append-failure degrades safe (gate unset, promotion completes); non-blocking. `committing_timeout_fn` + enum retained until tasks 4.x/5.2.
- **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.
- **(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`).

58
src/controller/ha.c
View file

@ -2346,16 +2346,21 @@ apply_role:
ha->phase2_done = 1;
}
/* Enter COMMITTING: block job traffic until majority ack (Req 4.1, 8.1) */
atomic_store_explicit(&ha->role, (int32_t)CTRL_PRIMARY_COMMITTING,
/* ha-single-leader-role task 3.1 (Req 1.1, 1.5): enter CTRL_PRIMARY
* DIRECTLY — symmetric with raft_cb_become_leader's rewrite. There
* is no transitional COMMITTING state and no committing-timeout
* watchdog; new-leader safety comes from the own-term WAL_NOOP +
* writable_lsn gate armed below (ha_arm_own_term_gate), not from a
* fourth role value. The legacy XCE_TESTING election path OWNS its
* term (no raft library in test mode); it historically did NOT
* increment the term in this branch, so we preserve that — no
* ha_do_promote, no new term store (research.md §4 finding 2). */
atomic_store_explicit(&ha->role, (int32_t)CTRL_PRIMARY,
memory_order_release);
INFO("HA FSM: → CTRL_PRIMARY_COMMITTING (term=%" PRIu64
INFO("HA FSM: → CTRL_PRIMARY (term=%" PRIu64
", first_boot=%d)", atomic_load_explicit(&ha->term, memory_order_acquire),
is_first_boot);
/* Reset ack counter for this committing round (Req 5.2) */
atomic_store(&ha->repl_ack_count, 0);
/* Push config to standbys with lower config_tag (Req 7.1) */
#ifdef XCE_TESTING
if (ha_push_config_hook != NULL)
@ -2374,30 +2379,25 @@ apply_role:
ha_send_ctrlhello_to_all_standbys(c);
#endif
/* Arm committing timeout watchdog (degraded-mode fallback, Req 8.4) */
/* ha-single-leader-role task 3.1: route promotion side effects
* through the SAME shared sequence raft_cb_become_leader runs so the
* test-driven (legacy) and raft-driven promotion paths cannot diverge
* (design §Legacy Path Symmetry). Guarded on a real cluster_t — the
* helpers dereference &c->hooksys / &c->persist / &c->commit, and the
* unit-test evaluation path passes c == NULL. Identical role / hook /
* heartbeat / no-watchdog / own-term-gate state results either road. */
if (c) {
/* Cancel any prior committing timeout */
committing_timeout_ctx_t *prev = (committing_timeout_ctx_t *)
atomic_exchange(&ha->committing_timeout_ctx, NULL);
if (prev) {
atomic_store(&prev->cancelled, 1);
}
committing_timeout_ctx_t *ct = calloc(1, sizeof(*ct));
if (ct) {
atomic_init(&ct->cancelled, 0);
ct->ha = ha;
ct->c = c;
atomic_store(&ha->committing_timeout_ctx, ct);
int rc = delay_task_ms(&c->delaymgr,
(int)ha->config.committing_timeout_ms,
1, committing_timeout_fn, ct);
if (rc < 0) {
WARN("ha_elect_evaluate_and_signal: delay_task_ms "
"for committing timeout failed: %d", rc);
atomic_store(&ha->committing_timeout_ctx, NULL);
free(ct);
}
}
/* Register the persistence hooks BEFORE the own-term no-op append,
* so the persistence seam is live before any racing client write.
* Idempotent via the hooks_registered guard. */
ha_register_persist_hooks(c);
/* Periodic heartbeat to standbys (idempotency guard prevents
* double-arming if already started). */
ha_start_primary_heartbeat(c);
/* Own-term WAL_NOOP + writable_lsn gate (Req 1.4, 2.1, 2.2) —
* non-blocking; append failure degrades safe (gate unset,
* promotion still completes). */
ha_arm_own_term_gate(c);
}
} else {
/* STANDBY result */

111
src/controller/testsuite/test_ha_role_lifecycle.c
View file

@ -28,6 +28,7 @@
#include <fcntl.h>
#include <inttypes.h>
#include <pthread.h>
#include <semaphore.h>
#include <signal.h>
#include <stdatomic.h>
#include <stdint.h>
@ -1053,6 +1054,113 @@ static void test_standalone_promotion_gate_opens_immediately(void)
#endif
}
/* ── Case 8: LEGACY/RAFT PROMOTION SYMMETRY (ha-single-leader-role task 3.1,
* Req 1.1, 1.5; design §Legacy Path Symmetry). Both entry points to PRIMARY
* must produce IDENTICAL observable state so test-driven and raft-driven
* promotion cannot diverge:
*
* (1) raft road — raft_cb_become_leader_for_test (the XCE_TESTING shim, which
* forces ha->raft_c == NULL): only the c-independent state is observable
* on this seam (role == CTRL_PRIMARY). The hook/gate tail is c!=NULL-
* guarded and skipped here — that half of the contract is pinned on the
* real-cluster road below (matching cases 3/4/5/7, which drive the SAME
* shared helpers ha_register_persist_hooks / ha_arm_own_term_gate).
*
* (2) legacy road — ha_elect_evaluate_and_signal on the real g_lc cluster
* (ctx->c = &g_lc, single-node quorum win): drives the full shared
* sequence. Must land role == CTRL_PRIMARY, hooks_registered == 1,
* writable_lsn == noop_lsn (own-term gate armed), and arm NO delaymgr
* watchdog — byte-for-byte the state ha_promote_to_primary produces.
*
* Pin: the legacy real-cluster path produces role/hook/gate/no-watchdog state
* identical to the raft road's role and to ha_promote_to_primary's full state.
* RED before task 3.1: the legacy branch stored CTRL_PRIMARY_COMMITTING, reset
* repl_ack_count, and armed the committing watchdog — diverging from the raft
* road on every axis. */
static void test_legacy_and_raft_promotion_are_symmetric(void)
{
#ifdef XCE_TESTING
/* ── Road 1: raft shim (c == NULL) — role contract only ──────────────── */
ha_state_t ha_raft;
ha_config_t cfg = lifecycle_ha_cfg();
TEST_ASSERT_EQUAL_INT(0, ha_init(&ha_raft, &cfg, 0));
ha_raft.phase2_done = 1;
atomic_store_explicit(&ha_raft.role, (int32_t)CTRL_ELECTING,
memory_order_release);
raft_cb_become_leader_for_test(&ha_raft, /*term=*/7);
ctrl_role_t raft_role =
(ctrl_role_t)atomic_load_explicit(&ha_raft.role, memory_order_acquire);
TEST_ASSERT_EQUAL_INT_MESSAGE((int)CTRL_PRIMARY, (int)raft_role,
"raft road must land DIRECTLY in CTRL_PRIMARY (Req 1.1)");
ha_fini(&ha_raft);
/* ── Road 2: legacy ha_elect_evaluate_and_signal on a real cluster ───── */
/* persist_wal_async appends at current_lsn then post-increments, and
* ha_arm_own_term_gate derives noop_lsn = current_lsn - 1; the fixture
* primes current_lsn = appended_lsn + 1, so passing appended_lsn =
* noop_lsn - 1 lands the own-term no-op at exactly lsn = noop_lsn
* (same idiom as case 5). */
const uint64_t noop_lsn = 9;
(void)lifecycle_cluster_setup("sym", /*n_peers=*/1,
/*n_standbys=*/0,
/*appended_lsn=*/noop_lsn - 1,
/*n_vcpus=*/8);
int before = delaymgr_task_count(&g_lc.delaymgr);
TEST_ASSERT_EQUAL_INT_MESSAGE(0, g_lc.ha.hooks_registered,
"fixture precondition: hooks must be unregistered before promotion");
/* Single-node quorum win: n_peers==0 shortcircuit (votes[] empty),
* votes_granted=1 satisfies quorum=1. ctx->c = &g_lc so the shared
* promotion sequence runs against the real cluster. */
ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx));
TEST_ASSERT_NOT_NULL_MESSAGE(ctx, "ctx calloc failed");
ctx->n_peers = 0;
ctx->votes_granted = 1;
ctx->n_replied = 0;
ctx->result = -1;
ctx->done_posted = 0;
ctx->c = &g_lc;
pthread_mutex_init(&ctx->lock, NULL);
sem_init(&ctx->done, 0, 0);
ctrl_role_t legacy_result = ha_elect_evaluate_and_signal(&g_lc.ha, ctx);
/* Symmetry axis 1: the legacy road reaches PRIMARY (matches the raft road). */
TEST_ASSERT_EQUAL_INT_MESSAGE((int)CTRL_PRIMARY, (int)legacy_result,
"legacy evaluate must RESULT in CTRL_PRIMARY (Req 1.1)");
ctrl_role_t legacy_role =
(ctrl_role_t)atomic_load_explicit(&g_lc.ha.role, memory_order_acquire);
TEST_ASSERT_EQUAL_INT_MESSAGE((int)raft_role, (int)legacy_role,
"legacy and raft promotion must land in the SAME role (CTRL_PRIMARY)");
/* Symmetry axis 2: persist hooks registered by the shared sequence. */
TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_lc.ha.hooks_registered,
"legacy road must register persist hooks via the shared sequence (Req 2.1)");
/* Symmetry axis 3: own-term gate armed at the no-op LSN. */
TEST_ASSERT_EQUAL_UINT64_MESSAGE(noop_lsn, g_lc.commit.writable_lsn,
"legacy road must arm the own-term gate at noop_lsn (Req 1.4, 2.1) — "
"identical to the raft road's shared tail");
/* Symmetry axis 4: NO committing-timeout watchdog armed (Req 1.5). */
int after = delaymgr_task_count(&g_lc.delaymgr);
TEST_ASSERT_EQUAL_INT_MESSAGE(before, after,
"legacy road must arm NO delaymgr watchdog (Req 1.5) — symmetric with "
"the raft road; the committing-timeout arm is deleted from this path");
pthread_mutex_destroy(&ctx->lock);
sem_destroy(&ctx->done);
free(ctx);
lifecycle_cluster_teardown();
printf("test_legacy_and_raft_promotion_are_symmetric: PASS "
"(role=%d both roads; hooks=1; writable_lsn=%" PRIu64
"; watchdog tasks before=%d after=%d)\n",
(int)legacy_role, g_lc.commit.writable_lsn, before, after);
#else
printf("test_legacy_and_raft_promotion_are_symmetric: SKIP (non-test build)\n");
#endif
}
/* ------------------------------------------------------------------ */
/* Entry point (registered in controller_test.c / meson.build) */
/* ------------------------------------------------------------------ */
@ -1073,4 +1181,7 @@ void test_ha_role_lifecycle(void)
RUN_TEST(test_promotion_appends_noop_and_arms_own_term_gate); /* case 5 */
RUN_TEST(test_pre_own_term_wait_has_no_third_path); /* case 6 */
RUN_TEST(test_standalone_promotion_gate_opens_immediately); /* case 7 */
/* Task 3.1 — legacy/raft promotion symmetry (Req 1.1, 1.5) */
RUN_TEST(test_legacy_and_raft_promotion_are_symmetric); /* case 8 */
}

40
src/controller/testsuite/test_ha_start_election_async.c
View file

@ -4,9 +4,11 @@
*
* Strategy:
* - n_peers=1 shortcircuit: no iomgr needed; election completes synchronously
* via ha_elect_evaluate_and_signal immediately after ha_start_election_async
* returns (role flips to CTRL_PRIMARY_COMMITTING; task 4.4 committing timeout
* will later promote to CTRL_PRIMARY for degraded/single-node mode).
* immediately after ha_start_election_async returns. The single-node
* shortcircuit delegates to raft_start_election, which fires on_become_leader
* synchronously → raft_cb_become_leader (ha-single-leader-role task 2.2),
* so the role lands DIRECTLY in CTRL_PRIMARY — there is no transitional
* CTRL_PRIMARY_COMMITTING and no committing-timeout watchdog (Req 1.1, 1.5).
* - Function returns in < 5ms (observable via clock_gettime).
* - n_peers=1 shortcircuit stores n_peers in ha->n_peers.
*
@ -101,7 +103,7 @@ static void test_single_node_stores_n_peers(void)
printf("test_single_node_stores_n_peers: PASS\n");
}
/* ── Test 3: single-node shortcircuit enters CTRL_PRIMARY_COMMITTING ──────── */
/* ── Test 3: single-node shortcircuit enters CTRL_PRIMARY directly ───────── */
static void test_single_node_shortcircuit_evaluates_immediately(void)
{
@ -111,22 +113,24 @@ static void test_single_node_shortcircuit_evaluates_immediately(void)
/* Start in CTRL_ELECTING state so role transition is visible */
atomic_store_explicit(&ha.role, (int32_t)CTRL_ELECTING, memory_order_release);
/* n_peers=1: must call ha_elect_evaluate_and_signal immediately.
* Fix 1 (CRITICAL): single-node shortcircuit must enter CTRL_PRIMARY_COMMITTING,
* not CTRL_PRIMARY. Task 4.4 committing timeout will promote to CTRL_PRIMARY
* once the committing phase completes (Req 8.4 / degraded-single-node path). */
/* n_peers=1: the shortcircuit fires raft_start_election → on_become_leader
* (raft_cb_become_leader) synchronously. ha-single-leader-role (Req 1.1):
* a winning node lands DIRECTLY in CTRL_PRIMARY — there is no transitional
* CTRL_PRIMARY_COMMITTING. New-leader safety comes from the own-term
* WAL_NOOP + writable_lsn gate, not from a fourth role value. */
ha_start_election_async(&ha, NULL, 1, NULL, 100ULL, 50ULL, "ctrl-a");
ctrl_role_t role = (ctrl_role_t)atomic_load_explicit(&ha.role, memory_order_acquire);
TEST_ASSERT_EQUAL_INT_MESSAGE((int)CTRL_PRIMARY_COMMITTING, (int)role,
"single-node shortcircuit must enter CTRL_PRIMARY_COMMITTING (not CTRL_PRIMARY)");
TEST_ASSERT_EQUAL_INT_MESSAGE((int)CTRL_PRIMARY, (int)role,
"single-node shortcircuit must enter CTRL_PRIMARY directly (Req 1.1) — "
"no transitional CTRL_PRIMARY_COMMITTING");
ha_fini(&ha);
printf("test_single_node_shortcircuit_evaluates_immediately: PASS (role=%d == CTRL_PRIMARY_COMMITTING)\n",
printf("test_single_node_shortcircuit_evaluates_immediately: PASS (role=%d == CTRL_PRIMARY)\n",
(int)role);
}
/* ── Test 6: single-node shortcircuit does NOT go directly to CTRL_PRIMARY ── */
/* ── Test 6: single-node shortcircuit lands directly in CTRL_PRIMARY ──────── */
static void test_single_node_not_primary_directly(void)
{
@ -135,13 +139,17 @@ static void test_single_node_not_primary_directly(void)
ha_start_election_async(&ha, NULL, 1, NULL, 0ULL, 0ULL, "ctrl-a");
/* ha-single-leader-role (Req 1.1): the winning node lands in CTRL_PRIMARY
* immediately — the old "must go through COMMITTING first" contract is gone.
* Promotion arms NO committing-timeout watchdog (Req 1.5); the own-term gate
* (WAL_NOOP + writable_lsn) provides new-leader safety instead. */
ctrl_role_t role = (ctrl_role_t)atomic_load_explicit(&ha.role, memory_order_acquire);
TEST_ASSERT_NOT_EQUAL_MESSAGE((int)CTRL_PRIMARY, (int)role,
"single-node shortcircuit must NOT jump directly to CTRL_PRIMARY; "
"must go through CTRL_PRIMARY_COMMITTING first");
TEST_ASSERT_EQUAL_INT_MESSAGE((int)CTRL_PRIMARY, (int)role,
"single-node shortcircuit must land DIRECTLY in CTRL_PRIMARY (Req 1.1) — "
"no transitional CTRL_PRIMARY_COMMITTING, no committing watchdog (Req 1.5)");
ha_fini(&ha);
printf("test_single_node_not_primary_directly: PASS (role=%d)\n", (int)role);
printf("test_single_node_not_primary_directly: PASS (role=%d == CTRL_PRIMARY)\n", (int)role);
}
/* ── Test 4: ha->n_peers set for multi-peer count too ───────────────────── */