fix(controller): run the become-leader tail on a task class that may block (bug-1433)
A winning RequestVoteReply is dispatched through peer_ctrl_classify, which
returned VTASK_LIVENESS for all peer-control traffic -- the never-blocks,
never-admission-gated class. The reply runs raft_cb_become_leader, whose
tail persist_apply_pump_run_sync() does bounded blocking WAL pread()s (it
cannot be made async without reopening the bug-1043-family job loss on a
freshly promoted primary). The fdatasync half of bug-1433 was removed in
40f510fb; this pread was the tracked residual.
peer_ctrl_classify now returns VTASK_LIVE_BLOCK while the node is
CTRL_ELECTING -- its peer-control traffic is then vote/PreVote replies,
election work like inbound RequestVote, which already uses LIVE_BLOCK.
As PRIMARY/STANDBY the traffic is WAL/config ACKs and stays VTASK_LIVENESS,
so a quorum of acks is never admission-gated (R4.5); a candidate has no
commit to advance.
New test_peer_ctrl_classify_vote_replies_while_electing (RED before).
Controller suite 1240/0.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
33c4f28c6a
commit
255b0faad1
GPG key ID:
80533536B0EB651C
2 changed files with 68 additions and 11 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue