diff --git a/src/controller/ha.c b/src/controller/ha.c index 603c7a50..47e206fd 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -2012,19 +2012,18 @@ static void raft_cb_become_leader(void *ctx, uint64_t term) * vtask" -- that is TRUE only for the single-node self-grant and RDHUP- * failover election-start paths (ha_dispatch_election_start). It is FALSE * for the common real multi-node case: a winning RequestVoteReply is - * dispatched via peer_ctrl_classify, which hardcodes VTASK_LIVENESS (never - * admission-gated, documented as never expected to block -- see the + * dispatched via peer_ctrl_classify, which (before bug-1433) always said + * VTASK_LIVENESS (never admission-gated, never expected to block -- see the * bug-1198 hazard box above peer_ctrl_classify's definition, which closed * this EXACT class of bug for HA_REPL_SEND_SNAPSHOT's persist_checkpoint). * persist_apply_pump_run_sync's pread()s of recently-written WAL segments * are lower practical risk than a bare fdatasync (almost always page-cache * hot, since the entries were just appended) but are still, strictly, - * blocking I/O on a task class the design says never blocks. This residual - * gap is tracked (see buglog.json) rather than fixed inline here: closing - * it properly means either reclassifying the winning-vote-reply case in - * peer_ctrl_classify or dispatching promotion's tail onto a fresh - * VTASK_LIVE_BLOCK vtask, both larger changes needing their own dedicated - * soak-validation pass. Do NOT remove this call to "fix" the classification + * blocking I/O on a task class the design says never blocks. bug-1433 + * (fixed): peer_ctrl_classify now classifies peer-control traffic + * received while CTRL_ELECTING -- the winning vote reply included -- as + * VTASK_LIVE_BLOCK, so this tail runs on a task class permitted bounded + * blocking. Do NOT remove this call to "fix" the classification * gap -- it is what closes the bug-1043-family apply-lag/job-loss bug; * removing it reopens that regression. The backlog itself stays inherently * small (only entries committed since the last heartbeat-driven kick). */ @@ -7101,8 +7100,9 @@ static int peer_ctrl_vtask_entry(vtask_t *task) * * (The vote-GRANT term fsync — VTASK_LIVE_BLOCK, R4.6 — is handled on the * INBOUND RequestVote handler, not on these reply/ack endpoints; and the long - * config-push/snapshot ops are offloaded to the SlowLane in tasks 3.2/3.3. So - * everything this handler dispatches is liveness work today.) + * config-push/snapshot ops are offloaded to the SlowLane in tasks 3.2/3.3. + * Vote replies received while CTRL_ELECTING are the exception, classified + * VTASK_LIVE_BLOCK below -- bug-1433.) * * ┌─ FLAG-ON HAZARD CLOSED (raft-drive-async finding-2; bug-1198) ───────────┐ * │ LIVENESS is correct only once the snapshot send runs OFF the pool. The │ @@ -7126,11 +7126,31 @@ static int peer_ctrl_vtask_entry(vtask_t *task) */ static vtask_class_t peer_ctrl_classify(ha_state_t *ha, io_packet_t *pkt) { - (void)ha; (void)pkt; + + /* bug-1433: while this node is a candidate, its peer-control traffic is + * vote / PreVote replies, and the winning one runs raft_cb_become_leader, + * whose tail (persist_apply_pump_run_sync) does bounded blocking WAL + * pread()s -- not VTASK_LIVENESS work. Classify it VTASK_LIVE_BLOCK, the + * class inbound RequestVote (the vote-grant fsync) already uses. Once + * PRIMARY (or STANDBY) the traffic is WAL/config ACKs, which stay + * VTASK_LIVENESS so a quorum of acks is never admission-gated (R4.5): + * a candidate has no commit to advance, so nothing is lost there. */ + if (ha != NULL + && (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire) + == CTRL_ELECTING) + return VTASK_LIVE_BLOCK; + return VTASK_LIVENESS; } +#ifdef XCE_TESTING +vtask_class_t ha_peer_ctrl_classify_for_test(ha_state_t *ha, io_packet_t *pkt) +{ + return peer_ctrl_classify(ha, pkt); +} +#endif + /* ── bug-1197: deferred vote-ep teardown ────────────────────────────────────── * * When g_drive_async_peerctrl_dispatch_enabled is ON, raft_cb_become_follower diff --git a/src/controller/testsuite/test_ha_dispatch_integration.c b/src/controller/testsuite/test_ha_dispatch_integration.c index 948f54ef..e0eada8f 100644 --- a/src/controller/testsuite/test_ha_dispatch_integration.c +++ b/src/controller/testsuite/test_ha_dispatch_integration.c @@ -592,6 +592,42 @@ static void test_p3_cross_endpoint_not_serialized(void) /* ── Suite entry point ──────────────────────────────────────────────────── */ +/* bug-1433: a winning vote reply runs raft_cb_become_leader, whose tail + * persist_apply_pump_run_sync() does blocking WAL pread()s -- so while this + * node is a candidate (CTRL_ELECTING) its peer-control messages (vote + * replies) must be VTASK_LIVE_BLOCK, the class inbound RequestVote already + * uses, not the never-blocks VTASK_LIVENESS. As PRIMARY/STANDBY they are + * WAL/config ACKs and must stay VTASK_LIVENESS (never admission-gated) so + * commit can always advance. */ +extern vtask_class_t ha_peer_ctrl_classify_for_test(ha_state_t *, io_packet_t *); + +static void test_peer_ctrl_classify_vote_replies_while_electing(void) +{ + cluster_t c; + io_packet_t pkt; + + memset(&c, 0, sizeof(c)); + memset(&pkt, 0, sizeof(pkt)); + ha_init(&c.ha, &g_cfg, 0); + + atomic_store(&c.ha.role, CTRL_ELECTING); + TEST_ASSERT_EQUAL_INT_MESSAGE(VTASK_LIVE_BLOCK, + ha_peer_ctrl_classify_for_test(&c.ha, &pkt), + "bug-1433: vote replies while electing may run the become-leader " + "tail's blocking reads -- VTASK_LIVE_BLOCK"); + + atomic_store(&c.ha.role, CTRL_PRIMARY); + TEST_ASSERT_EQUAL_INT_MESSAGE(VTASK_LIVENESS, + ha_peer_ctrl_classify_for_test(&c.ha, &pkt), + "ACKs to a primary must never be admission-gated"); + + atomic_store(&c.ha.role, CTRL_STANDBY); + TEST_ASSERT_EQUAL_INT(VTASK_LIVENESS, + ha_peer_ctrl_classify_for_test(&c.ha, &pkt)); + + ha_fini(&c.ha); +} + void test_ha_dispatch_integration(void) { UnitySetTestFile(__FILE__); @@ -600,6 +636,7 @@ void test_ha_dispatch_integration(void) RUN_TEST(test_p1_configpush_transfer_does_not_block_cross_endpoint); /* P2 — VTASK_LIVENESS ack runs under *_BLOCK pool saturation */ RUN_TEST(test_p2_ack_runs_under_pool_saturation); + RUN_TEST(test_peer_ctrl_classify_vote_replies_while_electing); /* P3 — cross-endpoint peer control is not serialized */ RUN_TEST(test_p3_cross_endpoint_not_serialized); }