diff --git a/.wolf/buglog.json b/.wolf/buglog.json index d95b835..8c758cf 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -12562,5 +12562,50 @@ ], "occurrences": 11, "last_seen": "2026-07-05T07:45:00.000Z" + }, + { + "id": "bug-1417", + "timestamp": "2026-07-07T09:30:00.000Z", + "error_message": "[xce-test-ha --chaotic] FAIL round 4/10: election stall after killing ctrl-1 \u2014 ctrl-2 PreVote wins but real election times out despite ctrl-3 granting RequestVote", + "file": "src/controller/ha.c", + "root_cause": "vote_reply_dispatch passes local votes_idx (compressed, excludes self) directly to raft_on_vote_reply which expects ABSOLUTE peer index (includes self). For self_idx=1 (ctrl-2), ctrl-3's vote (absolute=2, local=1) lands in vote_replies[1]=ctrl-2's own self-slot \u2192 treated as duplicate \u2192 vote silently dropped. With ctrl-1 dead in chaotic mode, ctrl-2 only has self-vote (1 < quorum 2) \u2192 eternal timeout. Self_idx=0 and self_idx=2 are partially/fully immune due to index alignment luck. ha_repl_ack_abs_peer_idx() already exists (ha.c:3779) with correct mapping and passing tests \u2014 vote_reply_dispatch just doesn't call it.", + "fix": [ + "FIXED (src/controller/ha.c vote_reply_dispatch, ~line 5233): raft_on_vote_reply now receives ha_repl_ack_abs_peer_idx(ha, 1, idx) instead of the raw compressed local idx. ha_repl_ack_abs_peer_idx (ha.c:3779, already existed + unit-tested for the heartbeat-ACK path) performs the exact inverse of raft_cb_send_vote_request send-side remap: votes_idx < self_idx ? votes_idx : votes_idx + 1. ha_record_vote_reply (immediately preceding line) is UNCHANGED -- it correctly wants the compressed idx for ctx->votes[]. Confirmed pre_vote_reply_dispatch needs NO equivalent fix: raft_cb_send_pre_vote_request (ha.c:301) already stores the ABSOLUTE peer_idx in vctx->peer_idx for the PreVote path (explicit code comment there), matching raft_on_pre_vote_reply/pre_vote_replies[] absolute-index expectation -- only the real-vote send path (raft_cb_send_vote_request, ha.c:175) stores compressed. RED->GREEN: new unit test test_bug1417_middle_index_real_vote_from_peer2_not_dropped (test_ha_vote_send.c) drives a REAL 3-node election (self_idx=1) through raft_start_election then a genuine peer-2 grant via vote_reply_msg_handler_for_test (the actual decode/dispatch path, not the xce_vote_reply_hook bypass every other real-vote test in this file uses) -- reproduced Expected 2 Was 1 pre-fix, GREEN post-fix (votes_granted=2, role=RAFT_LEADER). Verified: full 79-group controller_unit_test sweep 0 new failures; 10-round --chaotic xce-test-ha soak PASS (committed_divergence=0, committed_job_loss=0, 18 jobs). Soak surfaced an UNRELATED, pre-existing --chaotic harness bug (run-ha-soak.sh run_chaotic_rounds: DOWN variable is not cleared on a skip-this-round no-primary-to-kill path, so a subsequent round calls start_node on the same already-running node without killing it first -- 6 stacked ctrl-2 processes observed piling up over rounds 5-10) -- confirmed via git diff this script was untouched by this fix, logged separately, NOT fixed here (out of bug-1417 boundary)." + ], + "tags": [ + "vote", + "raft", + "self_idx", + "index-mapping", + "chaotic-failover", + "off-by-one", + "fixed", + "ha", + "ha_repl_ack_abs_peer_idx", + "election" + ], + "related_bugs": [ + "bug-828" + ], + "occurrences": 1, + "last_seen": "2026-07-07T10:20:00.000Z" + }, + { + "id": "bug-1418", + "timestamp": "2026-07-07T10:20:00.000Z", + "error_message": "[xce-test-ha --chaotic] rounds 6-10 of a 10-round soak all skip (no primary to kill) after round 5 hit a slow-but-eventually-successful election; ctrl-2 restarted repeatedly without the prior instance ever being killed -- 6 stacked ctrl-2 xce_controller processes observed alive simultaneously by end of run", + "file": ".claude/skills/xce-test-ha/run-ha-soak.sh", + "root_cause": "run_chaotic_rounds's DOWN variable tracks the node killed THIS round so the NEXT round can restart it. On a round where primary_from_log finds no primary (log \"no primary to kill this round -- skipping\"), the loop continues without ever updating DOWN or killing anything -- but the NEXT round's \"if [ -n \"\" ]\" restart-previously-downed step still fires unconditionally on the stale DOWN value from several rounds back, calling start_node on a node that was never actually killed (or was already restarted in a prior skipped round), stacking a new xce_controller process on top of an already-running one every single skipped round. Discovered incidentally while verifying bug-1417's fix (10-round chaotic soak, unrelated to the vote-index fix itself -- confirmed via git diff that run-ha-soak.sh was untouched this session).", + "fix": "NOT FIXED (out of scope for the bug-1417 investigation that surfaced it). Candidate fix: only treat a node as DOWN (eligible for the next round's restart) if this round genuinely killed it; on a skip-round, DOWN should stay whatever it was (already handled -- the bug is that the skip round should also skip minus the restart-if-still-actually-down check), or more robust: before start_node(\"\"), pgrep for an existing xce_controller --myname and skip the restart if one is already running.", + "tags": [ + "xce-test-ha", + "chaotic-mode", + "harness-bug", + "restart-bookkeeping", + "process-leak" + ], + "related_bugs": [], + "occurrences": 1, + "last_seen": "2026-07-07T10:20:00.000Z" } -] +] \ No newline at end of file diff --git a/src/controller/ha.c b/src/controller/ha.c index c68a093..f3e8413 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -5230,7 +5230,24 @@ static void vote_reply_dispatch(ha_state_t *ha, ha_elect_ctx_t *ctx, int idx, * raft_cb_send_vote_request via the absolute→local mapping), which is exactly * the index ha_compute_safe_committed_lsn and the test hook use. */ ha_record_vote_reply(ha, ctx, idx, term, vote_granted, peer_tag, peer_lsn); - raft_on_vote_reply(&ha->raft, idx, vote_granted, term); + + /* bug-1417: raft_on_vote_reply's from_idx must be the ABSOLUTE cluster + * index (the space vote_replies[]/self_idx live in — raft_start_election + * pre-marks vote_replies[self_idx] and sends to every absolute peer != + * self_idx, raft.c:113-153), NOT the compressed self-excluded votes_idx + * `idx` carries for ctx->votes[] above. Passing the compressed idx + * straight through collides with vote_replies[self_idx] whenever a + * peer's compressed votes_idx numerically equals self_idx (true for any + * peer at or above self_idx in a >2-node cluster, e.g. self_idx=1's + * absolute peer 2 compresses to votes_idx=1) — raft_on_vote_reply's + * per-peer dedup guard then silently treats the genuine reply as an + * already-counted duplicate and drops it, so a middle-priority node can + * never reach quorum on that peer's vote alone. ha_repl_ack_abs_peer_idx + * (ha.c:3779) already implements the exact inverse of + * raft_cb_send_vote_request's votes_idx remap and is unit-tested; reuse + * it here (ep_is_vote=1) rather than duplicating the arithmetic. */ + raft_on_vote_reply(&ha->raft, ha_repl_ack_abs_peer_idx(ha, /*ep_is_vote=*/1, idx), + vote_granted, term); } /* diff --git a/src/controller/testsuite/test_ha_vote_send.c b/src/controller/testsuite/test_ha_vote_send.c index d8610c9..1155dbc 100644 --- a/src/controller/testsuite/test_ha_vote_send.c +++ b/src/controller/testsuite/test_ha_vote_send.c @@ -557,6 +557,103 @@ static void test_vote_reply_handler_no_uaf(void) printf("test_vote_reply_handler_no_uaf: PASS\n"); } +/* ── bug-1417: middle-index (self_idx=1) real-vote index-mapping regression ── + * + * raft_cb_send_vote_request (ha.c:142) stores the COMPRESSED local votes_idx + * (self-excluded) in vctx->peer_idx: votes_idx = peer_idx < self_idx ? + * peer_idx : peer_idx - 1. vote_reply_dispatch (ha.c:5208) then passes that + * SAME compressed idx straight to raft_on_vote_reply, which indexes + * s->vote_replies[] in ABSOLUTE space (the space raft_start_election's + * self-mark and per-peer send loop both use, raft.c:113-153). + * + * For self_idx=1 (a 3-node cluster's middle node), the peer at absolute + * index 2 maps to compressed votes_idx=1 -- which COLLIDES with + * vote_replies[self_idx=1], the slot raft_start_election already marked as + * counted for the self-vote. raft_on_vote_reply's per-peer dedup guard + * (raft.c:477) then treats the genuine reply from peer 2 as a duplicate and + * silently drops it -- vote_replies[1] was already 1, votes_granted stays + * at 1 (self only), quorum (2) is never reached, and the election times out + * even though peer 2 genuinely granted. + * + * Peer 0 is unaffected (compressed votes_idx=0 == absolute 0, no collision), + * which is why sequential/all-peers-alive elections never surfaced this -- + * only a 3-node cluster's middle-priority node losing exactly its + * higher-priority peer (index 0) and depending on its lower peer's (index 2) + * vote exposes the collision. + */ + +static void make_three_node_self1(ha_state_t *ha) +{ + ha->raft.n_peers = 3; + ha->raft.self_idx = ha->my_priority; /* my_priority == 1 for this test */ + ha->raft.role = RAFT_FOLLOWER; + ha->n_peers = 3; +} + +static void test_bug1417_middle_index_real_vote_from_peer2_not_dropped(void) +{ + reset_reply_mock(); + xce_vote_reply_hook = NULL; /* drive the REAL raft_on_vote_reply path, not the hook */ + + ha_state_t ha; + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_vote_cfg, /*my_priority=*/1)); + make_three_node_self1(&ha); + + /* Start a real election: term++, self-vote, vote_replies[self_idx=1]=1 + * pre-marked (raft.c:122-123), CANDIDATE role, one on_send_vote_request + * call per non-self absolute peer (0 and 2) -- c==NULL so each send is a + * safe no-op (raft_cb_send_vote_request returns 1 immediately), exactly + * the same test-mode pattern test_ha_prevote.c uses for + * ha_start_prevote_async(&ha, NULL). */ + int rc = raft_start_election(&ha.raft); + TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_OK, rc, + "3-node multi-node election must return RAFT_OK (CANDIDATE transition)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_CANDIDATE, (int)ha.raft.role, + "raft role must be CANDIDATE after starting a 3-node election"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.votes_granted, + "self-vote must be counted (1) before any peer reply"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.vote_replies[1], + "self_idx's own vote_replies slot must be pre-marked"); + + ha_elect_ctx_t *ctx = alloc_ctx_for_vote(2); /* n_other = n_peers - 1 = 2 */ + pthread_mutex_init(&ctx->lock, NULL); + + uint64_t term = atomic_load_explicit(&ha.term, memory_order_acquire); + + /* Peer at ABSOLUTE index 2 grants. raft_cb_send_vote_request's mapping + * (peer_idx=2 >= self_idx=1) computes votes_idx = 2 - 1 = 1 -- this IS + * the value production code stores in vctx->peer_idx for this peer, so + * the test reproduces the exact wire-level index the real send path + * would have used. */ + endpoint_t ep2; + make_vote_reply_ep(&ep2, &ha, ctx, /*peer_idx (compressed votes_idx)=*/1); + + io_packet_t *pkt2 = vote_reply_build_pkt_for_test(term, /*granted=*/1, 0ULL, 0ULL); + TEST_ASSERT_NOT_NULL_MESSAGE(pkt2, "vote_reply_build_pkt_for_test must succeed"); + + vote_reply_msg_handler_for_test(&ep2, pkt2); + + /* This is the bug: without ha_repl_ack_abs_peer_idx remapping compressed + * votes_idx=1 back to absolute peer_idx=2 before calling + * raft_on_vote_reply, the reply lands in vote_replies[1] -- the + * self-vote's own slot, already 1 -- and is silently discarded as a + * duplicate. votes_granted stays at 1 (self only), never reaching the + * 2-vote quorum, even though peer 2 genuinely granted. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(2, ha.raft.votes_granted, + "bug-1417: peer 2's genuine grant must be counted (votes_granted==2), " + "not silently dropped into the self-vote's slot"); + TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_LEADER, (int)ha.raft.role, + "bug-1417: with self-vote + peer 2's grant, quorum (2) is reached and " + "the node must become LEADER"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.election_decided, + "election must be decided (won) once quorum is reached"); + + free(ep2.data); + free_ctx_for_vote(ctx); + ha_fini(&ha); + printf("test_bug1417_middle_index_real_vote_from_peer2_not_dropped: PASS\n"); +} + /* ── Suite entry point ───────────────────────────────────────────────────── */ void test_ha_vote_send(void) @@ -570,4 +667,6 @@ void test_ha_vote_send(void) RUN_TEST(test_vote_reply_handler_denied_vote); RUN_TEST(test_vote_reply_handler_term_fencing); RUN_TEST(test_vote_reply_handler_no_uaf); + /* bug-1417 */ + RUN_TEST(test_bug1417_middle_index_real_vote_from_peer2_not_dropped); }