From 74784b2b1810c15149ebb5313d848bd8b57851af Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 30 Jun 2026 13:29:15 +0000 Subject: [PATCH] fix(raft-ha-core-safety): randomize per-campaign election timeout to [T,2T] (task 5.7) The per-campaign election watchdog was armed with a FIXED election_timeout_ms (raft_start_election -> on_schedule_timeout -> raft_cb_schedule_timeout). Under cold-connection churn a just-restarted survivor opens a fresh TCP+SSL endpoint per vote round, so its single needed grant can land a hair AFTER a constant deadline on EVERY campaign -- a resonance: the campaign times out, the term re-escalates, and the now-stale grant is dropped (DropStaleResponse). Forever. Terms climb unbounded; the freshest survivor never tallies quorum. This is the final round-4 chaotic-soak election-liveness stall. Fix (Ongaro thesis 3.4): re-randomize the watchdog per campaign to the canonical [T, 2T] interval in the HA adapter, so some campaign's deadline falls AFTER its grant RTT and the grant lands in-window. Done in raft_cb_schedule_timeout so the pure (sim-tested) raft library stays deterministic. Independent of election_jitter_max_ms (which tunes only the sub-second startup/stagger jitter) so it spreads even when jmax is small/0; never below the configured floor so genuine-loss failover is not sped up past election_timeout_ms. Preserves Election Safety (no change to term/vote/DropStaleResponse logic). new ha_jittered_election_delay() helper + 2 unit tests (RED verified: stubbing jitter to 0 fails test_5_7_campaign_timeout_is_randomized with the resonance message; PIN guards floor + modulo-by-zero). Suite: 24 Tests / 0 Failures. --- src/controller/ha.c | 24 +++++ src/controller/ha.h | 20 +++++ .../testsuite/test_ha_core_safety.c | 87 +++++++++++++++++++ 3 files changed, 131 insertions(+) diff --git a/src/controller/ha.c b/src/controller/ha.c index 82f8867..95b2bd8 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -449,6 +449,14 @@ static void raft_cb_schedule_timeout(void *ctx, uint32_t delay_ms) t->c = c; t->ctx = NULL; /* raft path does not use ha_elect_ctx_t */ + /* task 5.7: randomize the per-campaign watchdog to [delay_ms, 2*delay_ms] + * (Ongaro §3.4). The library passes a FIXED election_timeout_ms; arming the + * watchdog at exactly that value lets a constant cold-connection grant RTT + * land just past every deadline forever (resonance → unbounded term climb). + * Re-randomizing here breaks the resonance without touching the pure + * (sim-tested) library. */ + delay_ms = ha_jittered_election_delay(ha, delay_ms); + int rc = delay_task_ms(&c->delaymgr, (int)delay_ms, 1, raft_election_timeout_fn, t); if (rc < 0) { @@ -6671,6 +6679,22 @@ uint32_t ha_election_stagger_ms(ha_state_t *ha) return delay; } +/* ha_jittered_election_delay: see ha.h. Canonical Raft randomized election + * timeout — [base, 2*base], re-drawn each campaign — applied to the per-campaign + * watchdog so a constant cold-connection grant RTT cannot resonate just past a + * fixed deadline forever (task 5.7). Independent of election_jitter_max_ms and + * never below the configured floor. */ +uint32_t ha_jittered_election_delay(ha_state_t *ha, uint32_t base) +{ + if (!ha || base == 0) + return base; + if (ha->jitter_seed == 0) + ha_seed_jitter(&ha->jitter_seed, ha->my_priority); + uint32_t extra = (uint32_t)(xorshift64_next(&ha->jitter_seed) + % ((uint64_t)base + 1)); + return base + extra; +} + /* failover_stagger_ctx_t: carries ha/c into the delaymgr-scheduled PreVote start. * Freed by ha_failover_stagger_fn (one-shot). */ typedef struct { diff --git a/src/controller/ha.h b/src/controller/ha.h index 2d11a8e..e377818 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -1402,6 +1402,26 @@ void ha_begin_failover(ha_state_t *ha, cluster_t *c); */ uint32_t ha_election_stagger_ms(ha_state_t *ha); +/* + * ha_jittered_election_delay: the per-CAMPAIGN election-timeout watchdog delay, + * randomized to the canonical Raft interval [base, 2*base] (Ongaro §3.4) and + * re-drawn on every campaign. + * + * task 5.7: the campaign watchdog (raft_start_election → on_schedule_timeout → + * raft_cb_schedule_timeout) was previously armed with a FIXED election_timeout_ms. + * Under cold-connection churn a survivor's single needed grant can land a hair + * AFTER a constant deadline on EVERY campaign — a resonance that times out, + * re-escalates the term, and then drops the now-stale grant (DropStaleResponse) + * forever (unbounded term climb; the final round-4 chaotic-soak stall). + * Randomizing the watchdog per campaign breaks the resonance: some campaign's + * deadline falls AFTER its grant RTT, the grant lands in-window, quorum is + * reached. Independent of election_jitter_max_ms (which tunes only the + * sub-second startup/stagger jitter) so it still spreads when jmax is small/0. + * Never returns below `base`, so genuine-loss failover is never sped up past the + * configured election_timeout_ms floor. base==0 returns 0 (no modulo-by-zero). + */ +uint32_t ha_jittered_election_delay(ha_state_t *ha, uint32_t base); + /* * ha_dispatch_repl_action_for_standby: evaluate the replication decision for * one standby and execute the resulting action. diff --git a/src/controller/testsuite/test_ha_core_safety.c b/src/controller/testsuite/test_ha_core_safety.c index 711c6c5..2a09b57 100644 --- a/src/controller/testsuite/test_ha_core_safety.c +++ b/src/controller/testsuite/test_ha_core_safety.c @@ -1811,6 +1811,90 @@ static void test_5_6_genuine_loss_still_fails_over_when_not_electing(void) printf("test_5_6_genuine_loss_still_fails_over_when_not_electing: PASS (pin)\n"); } +/* ═══════════════════════════════════════════════════════════════════════════ + * TASK 5.7 — the per-campaign election watchdog must be RANDOMIZED [T, 2T] + * + * Surfaced after 5.6 (research.md §"Task 5.6 fix VERIFIED"): with RDHUP no + * longer preempting an in-progress election, round 4 STILL stalls. The freshest + * survivor (ctrl-2) wins PreVote, enters the real election, and ctrl-3 GRANTS + * it — but every grant arrives as vote_reply_stale_drop (reply term < ctrl-2's + * current term). Root cause: raft_start_election arms the campaign watchdog with + * a FIXED election_timeout_ms (raft.c on_schedule_timeout). Under cold-connection + * churn (a just-restarted survivor opens a fresh TCP+SSL endpoint per vote round) + * the single needed grant lands a hair AFTER that constant deadline on EVERY + * campaign — a resonance: the campaign times out, the term re-escalates, and the + * now-stale grant is dropped (DropStaleResponse). Forever. Terms climb 25→30+. + * + * Fix (Ongaro §3.4): re-randomize the watchdog per campaign to [T, 2T] so some + * campaign's deadline falls AFTER its grant RTT and the grant lands in-window. + * + * (CORE RED) test_5_7_campaign_timeout_is_randomized: the watchdog delay must + * VARY across campaigns and stay within [T, 2T]. A fixed timeout yields + * one distinct value (RED) — it cannot break the resonance. + * + * (PIN) test_5_7_floor_preserved_and_zero_safe: the randomized delay never + * drops BELOW the configured floor (genuine-loss failover is never sped up + * past election_timeout_ms), and base==0 is modulo-safe. + * + * Requirements: 6.1, 6.2, 6.3 (Election liveness under sustained failover churn). + * Preserves Election Safety (no change to term/vote/DropStaleResponse logic). + * ═══════════════════════════════════════════════════════════════════════════ */ + +/* CORE RED — the campaign watchdog must be re-randomized to [T, 2T] each call. */ +static void test_5_7_campaign_timeout_is_randomized(void) +{ + ha_state_t ha; + hcs5_make_standby(&ha); + ha.my_priority = 1; + ha.jitter_seed = 0; /* force a deterministic re-seed */ + + const uint32_t base = 5000; /* HA_DEFAULT_ELECTION_TIMEOUT_MS */ + uint32_t samples[32]; + for (int i = 0; i < 32; i++) { + samples[i] = ha_jittered_election_delay(&ha, base); + TEST_ASSERT_TRUE_MESSAGE(samples[i] >= base && samples[i] <= 2u * base, + "campaign watchdog must lie in the canonical Raft interval [T, 2T]"); + } + + int distinct = 0; + for (int i = 0; i < 32; i++) { + int seen = 0; + for (int j = 0; j < i; j++) + if (samples[j] == samples[i]) { seen = 1; break; } + if (!seen) distinct++; + } + TEST_ASSERT_TRUE_MESSAGE(distinct > 1, + "RED: the per-campaign election timeout must be RE-RANDOMIZED each " + "campaign. A fixed watchdog resonates with a constant cold-connection " + "grant RTT — the grant lands just past every deadline, the term climbs " + "unbounded, and every grant is dropped as stale (task 5.7)."); + + ha_fini(&ha); + printf("test_5_7_campaign_timeout_is_randomized: done\n"); +} + +/* PIN — never below the configured floor; base==0 is modulo-safe. */ +static void test_5_7_floor_preserved_and_zero_safe(void) +{ + ha_state_t ha; + hcs5_make_standby(&ha); + ha.my_priority = 1; + + /* base==0 must not divide/modulo by zero — returns 0. */ + TEST_ASSERT_EQUAL_UINT32_MESSAGE(0u, ha_jittered_election_delay(&ha, 0), + "PIN: base==0 must be modulo-safe and return 0"); + + /* The randomized delay never drops below the configured floor, so genuine + * single-survivor failover is never sped up past election_timeout_ms. */ + for (int i = 0; i < 64; i++) + TEST_ASSERT_TRUE_MESSAGE(ha_jittered_election_delay(&ha, 2000) >= 2000u, + "PIN: the randomized campaign timeout must never fall BELOW the " + "configured election_timeout_ms floor"); + + ha_fini(&ha); + printf("test_5_7_floor_preserved_and_zero_safe: PASS (pin)\n"); +} + /* ═══════════════════════════════════════════════════════════════════════════ * SCAFFOLDING (PASS) — prove each shared seam is usable by later phases. * ═══════════════════════════════════════════════════════════════════════════ */ @@ -1982,6 +2066,9 @@ void test_ha_core_safety(void) * PIN — genuine loss (STANDBY, not electing) still fails over. */ RUN_TEST(test_5_6_rdhup_does_not_restart_election_in_progress); RUN_TEST(test_5_6_genuine_loss_still_fails_over_when_not_electing); + + RUN_TEST(test_5_7_campaign_timeout_is_randomized); + RUN_TEST(test_5_7_floor_preserved_and_zero_safe); /* Scaffolding: each shared seam is usable by 2.3 / 4.1 / 5.1. */ RUN_TEST(test_seam_wal_construct_and_probe); RUN_TEST(test_seam_divergence_is_observable);