forked from lei.chen/myxce
feat(raft-adapter-conformance): route post-connect vote/PreVote failures to production tally
Splits vote_ep_err_handler into a real-vote handler and a new pre_vote_ep_err_handler, each registered at its existing (already separate) connection-setup call site. Extracts the identical CTRL_PRIMARY-role cleanup into a shared vote_ep_handle_promoted_disconnect helper, behavior unchanged. Each handler's election-phase branch now routes a post-connect failure (RDHUP/SSL-error/remote-close after connect, before any reply) through the same production tally the connect-time failure branches already use (vote_reply_dispatch / pre_vote_reply_dispatch, bug-1423/1424's fix), instead of the legacy handle_request_vote_reply that never updated the tally. Closes the one failure window bug-1423 didn't cover. 1060 -> 1061 tests, 0 failures. PreVote-specific test coverage is task 3.2's scope (pre_vote_reply_dispatch has no hook seam analogous to xce_vote_reply_hook; verified only by code symmetry + full-suite regression here).
This commit is contained in:
parent
65afa04aaf
commit
fce73b5b58
3 changed files with 160 additions and 33 deletions
|
|
@ -32,7 +32,7 @@
|
|||
- _Depends: 2.1, 2.2_
|
||||
|
||||
- [ ] 3. Core: vote and pre-vote endpoint failure routing
|
||||
- [ ] 3.1 (P) Split the shared vote-endpoint error handler and route post-connect failures to the production tally
|
||||
- [x] 3.1 (P) Split the shared vote-endpoint error handler and route post-connect failures to the production tally
|
||||
- Split `vote_ep_err_handler` into a real-vote handler (registered at its existing connection-setup call site) and a `pre_vote_ep_err_handler` (registered at its existing, separate PreVote connection-setup call site)
|
||||
- Extract the identical `CTRL_PRIMARY`-role cleanup branch (standby-endpoint deregistration, `repl_remove_standby`, quorum-transition logging) into one shared helper called by both, unchanged in behavior
|
||||
- Change each handler's election-phase branch to route a post-connect failure through the same production tally path already used for connect-time failures (`vote_reply_dispatch` for the real-vote handler, `pre_vote_reply_dispatch` for the pre-vote handler) instead of the legacy `handle_request_vote_reply`
|
||||
|
|
|
|||
|
|
@ -504,6 +504,10 @@ int ha_schedule_phase2_retry(cluster_t *c)
|
|||
static void on_vote_connected(endpoint_t *ep);
|
||||
static void on_pre_vote_connected(endpoint_t *ep);
|
||||
static int vote_ep_err_handler(void *ep_raw);
|
||||
/* raft-adapter-conformance task 3.1: PreVote's own on_err handler, registered
|
||||
* at raft_cb_send_pre_vote_request's endpoint-setup call site (mirrors the
|
||||
* real-vote vote_ep_err_handler forward-declared above). */
|
||||
static int pre_vote_ep_err_handler(void *ep_raw);
|
||||
/* bug-1423: forward-declared so raft_cb_send_vote_request's connect/monitor
|
||||
* failure branches can route a synthetic "no vote" reply through the SAME
|
||||
* production path a real reply uses (see vote_reply_dispatch's own comment,
|
||||
|
|
@ -824,7 +828,7 @@ static int raft_cb_send_pre_vote_request(void *ctx, int peer_idx,
|
|||
}
|
||||
|
||||
endpoint_set_on_connected(ep, on_pre_vote_connected);
|
||||
endpoint_set_err_handler(ep, vote_ep_err_handler);
|
||||
endpoint_set_err_handler(ep, pre_vote_ep_err_handler);
|
||||
endpoint_set_on_fini(ep, vote_ep_on_fini);
|
||||
|
||||
int rc = endpoint_connect(ep, peer->hostname, peer->port,
|
||||
|
|
@ -7555,11 +7559,54 @@ void raft_cb_become_leader_with_cluster_for_test(cluster_t *c, uint64_t term)
|
|||
}
|
||||
#endif /* XCE_TESTING */
|
||||
|
||||
/* ── Task 2.4: vote_ep_err_handler ─────────────────────────────────────────
|
||||
/* ── Task 2.4 / raft-adapter-conformance task 3.1: vote endpoint err handlers ──
|
||||
*
|
||||
* Called by iomgr when the outgoing vote endpoint encounters an error
|
||||
* (connect failure, SSL error, remote close before reply).
|
||||
* Counts the peer as denied: grant=0, tag=0, lsn=0.
|
||||
* vote_ep_handle_promoted_disconnect: shared CTRL_PRIMARY-role cleanup for a
|
||||
* dropped promoted connection (standby-endpoint deregistration, WAL
|
||||
* replicator deregistration, quorum-transition logging). Extracted
|
||||
* byte-for-byte from the formerly-single vote_ep_err_handler so both the
|
||||
* real-vote and PreVote on_err handlers apply the identical cleanup — this
|
||||
* branch's behavior is unchanged by the split.
|
||||
*/
|
||||
static void vote_ep_handle_promoted_disconnect(ha_state_t *ha, endpoint_t *ep,
|
||||
vote_ep_ctx_t *vctx)
|
||||
{
|
||||
/* Promoted connection dropped — remove from standby_endpoints */
|
||||
if (ha->standby_endpoints) {
|
||||
pthread_mutex_lock(&ha->lock);
|
||||
for (int i = 0; i < ha->standby_endpoints->count; i++) {
|
||||
if (zarray_get(ha->standby_endpoints, i) == ep) {
|
||||
zarray_del(ha->standby_endpoints, i);
|
||||
break;
|
||||
}
|
||||
}
|
||||
pthread_mutex_unlock(&ha->lock);
|
||||
}
|
||||
/* Task 2.2: deregister from WAL replicator.
|
||||
* Req 1.2: when a standby disconnects, the HA layer shall deregister
|
||||
* the endpoint from the replicator via repl_remove_standby.
|
||||
* vctx->c may be NULL in test mode — guard accordingly. */
|
||||
if (vctx->c)
|
||||
repl_remove_standby(&vctx->c->persist.repl, ep);
|
||||
|
||||
/* Quorum-transition observability (Req 8.1): a standby just dropped — the
|
||||
* write-quorum count changed. Record the loss edge once (silent if
|
||||
* quorum is still held after the removal). */
|
||||
ha_log_quorum_transition(ha);
|
||||
}
|
||||
|
||||
/*
|
||||
* vote_ep_err_handler: on_err callback for outgoing REAL-VOTE endpoints
|
||||
* (registered in raft_cb_send_vote_request). Called by iomgr when the
|
||||
* endpoint encounters an error (connect failure, SSL error, remote close
|
||||
* before reply).
|
||||
*
|
||||
* Election-phase branch counts a post-connect failure through
|
||||
* vote_reply_dispatch — the SAME production tally path a connect-time
|
||||
* failure (raft_cb_send_vote_request's own rc<0 branches, bug-1423) or a
|
||||
* delivered denial (vote_reply_msg_handler) already uses — instead of the
|
||||
* legacy handle_request_vote_reply, which bypasses raft_on_vote_reply
|
||||
* (raft-adapter-conformance Requirement 4.2).
|
||||
*/
|
||||
static int vote_ep_err_handler(void *ep_raw)
|
||||
{
|
||||
|
|
@ -7572,34 +7619,50 @@ static int vote_ep_err_handler(void *ep_raw)
|
|||
memory_order_acquire);
|
||||
|
||||
if (role == CTRL_PRIMARY) {
|
||||
/* Promoted connection dropped — remove from standby_endpoints */
|
||||
if (ha->standby_endpoints) {
|
||||
pthread_mutex_lock(&ha->lock);
|
||||
for (int i = 0; i < ha->standby_endpoints->count; i++) {
|
||||
if (zarray_get(ha->standby_endpoints, i) == ep) {
|
||||
zarray_del(ha->standby_endpoints, i);
|
||||
break;
|
||||
}
|
||||
}
|
||||
pthread_mutex_unlock(&ha->lock);
|
||||
}
|
||||
/* Task 2.2: deregister from WAL replicator.
|
||||
* Req 1.2: when a standby disconnects, the HA layer shall deregister
|
||||
* the endpoint from the replicator via repl_remove_standby.
|
||||
* vctx->c may be NULL in test mode — guard accordingly. */
|
||||
if (vctx->c)
|
||||
repl_remove_standby(&vctx->c->persist.repl, ep);
|
||||
|
||||
/* Quorum-transition observability (Req 8.1): a standby just dropped — the
|
||||
* write-quorum count changed. Record the loss edge once (silent if
|
||||
* quorum is still held after the removal). */
|
||||
ha_log_quorum_transition(ha);
|
||||
vote_ep_handle_promoted_disconnect(ha, ep, vctx);
|
||||
} else {
|
||||
/* Election phase: count as denied vote */
|
||||
handle_request_vote_reply(ha, vctx->ctx, vctx->peer_idx,
|
||||
vctx->my_term,
|
||||
0, /* vote_granted = false */
|
||||
0, 0);
|
||||
/* Election phase: count as a denied vote through the production
|
||||
* tally, same path a connect-time failure or a delivered denial
|
||||
* uses (raft-adapter-conformance Requirement 4.2). */
|
||||
vote_reply_dispatch(ha, vctx->ctx, vctx->peer_idx, vctx->my_term,
|
||||
/*vote_granted=*/0, 0, 0);
|
||||
}
|
||||
|
||||
/* Do NOT free vctx here — peer_ctrl_msg_handler may still reference ep->data.
|
||||
* vctx is freed in ha_election_cleanup() when all per-peer contexts release. */
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* pre_vote_ep_err_handler: on_err callback for outgoing PREVOTE endpoints
|
||||
* (registered in raft_cb_send_pre_vote_request). Mirrors vote_ep_err_handler
|
||||
* above for the PreVote endpoint kind — the two kinds share vote_ep_ctx_t but
|
||||
* are already registered at distinct call sites, so the split (rather than a
|
||||
* new discriminant field on vote_ep_ctx_t) mirrors an existing seam.
|
||||
*
|
||||
* Election-phase branch counts a post-connect failure through
|
||||
* pre_vote_reply_dispatch — the SAME production tally path a connect-time
|
||||
* failure (raft_cb_send_pre_vote_request's own rc<0 branches, bug-1423 PreVote
|
||||
* twin) already uses — instead of the legacy handle_request_vote_reply
|
||||
* (raft-adapter-conformance Requirement 4.3).
|
||||
*/
|
||||
static int pre_vote_ep_err_handler(void *ep_raw)
|
||||
{
|
||||
endpoint_t *ep = (endpoint_t *)ep_raw;
|
||||
vote_ep_ctx_t *vctx = (vote_ep_ctx_t *)ep->data;
|
||||
if (!vctx) return 0;
|
||||
|
||||
ha_state_t *ha = vctx->ha;
|
||||
ctrl_role_t role = (ctrl_role_t)atomic_load_explicit(&ha->role,
|
||||
memory_order_acquire);
|
||||
|
||||
if (role == CTRL_PRIMARY) {
|
||||
vote_ep_handle_promoted_disconnect(ha, ep, vctx);
|
||||
} else {
|
||||
/* Election phase: count as a not-granted PreVote reply through the
|
||||
* production tally, same path a connect-time failure uses
|
||||
* (raft-adapter-conformance Requirement 4.3). */
|
||||
pre_vote_reply_dispatch(ha, vctx->c, vctx->peer_idx, /*granted=*/0);
|
||||
}
|
||||
|
||||
/* Do NOT free vctx here — peer_ctrl_msg_handler may still reference ep->data.
|
||||
|
|
|
|||
|
|
@ -654,6 +654,68 @@ static void test_bug1417_middle_index_real_vote_from_peer2_not_dropped(void)
|
|||
printf("test_bug1417_middle_index_real_vote_from_peer2_not_dropped: PASS\n");
|
||||
}
|
||||
|
||||
/* ── raft-adapter-conformance task 3.1: post-connect vote failure routing ──
|
||||
*
|
||||
* vote_ep_err_handler's election-phase branch (the iomgr err_handler fired
|
||||
* when an already-connected outgoing vote endpoint fails/closes before a
|
||||
* reply arrives) must count that outcome through vote_reply_dispatch -- the
|
||||
* SAME production tally path a connect-time failure (raft_cb_send_vote_request's
|
||||
* own rc<0 branches, bug-1423) or a delivered denial (vote_reply_msg_handler)
|
||||
* uses -- not the legacy handle_request_vote_reply, which bypasses
|
||||
* raft_on_vote_reply entirely and, critically for this test, never reaches the
|
||||
* xce_vote_reply_hook seam vote_reply_dispatch checks.
|
||||
*
|
||||
* Test seam: xce_vote_reply_hook (checked ONLY inside vote_reply_dispatch) lets
|
||||
* us observe, without a live iomgr/raft election, whether the post-connect
|
||||
* failure reached the production tally path. Pre-fix, this hook is never
|
||||
* invoked by vote_ep_err_handler's election branch -- proving the bug.
|
||||
*/
|
||||
|
||||
extern int vote_ep_err_handler_for_test(endpoint_t *ep);
|
||||
|
||||
static void test_post_connect_vote_failure_routes_through_production_tally(void)
|
||||
{
|
||||
reset_reply_mock();
|
||||
|
||||
ha_state_t ha;
|
||||
TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_vote_cfg, 0));
|
||||
/* Election phase, not CTRL_PRIMARY -- exercises the "else" branch. */
|
||||
atomic_store_explicit(&ha.role, CTRL_ELECTING, memory_order_release);
|
||||
atomic_store_explicit(&ha.term, 4ULL, memory_order_release);
|
||||
|
||||
ha_elect_ctx_t *ctx = alloc_ctx_for_vote(2);
|
||||
pthread_mutex_init(&ctx->lock, NULL);
|
||||
|
||||
endpoint_t ep;
|
||||
make_endpoint(&ep);
|
||||
|
||||
/* Build a real-vote endpoint context as if raft_cb_send_vote_request had
|
||||
* already connected it (on_vote_connected fired, request sent) -- then
|
||||
* the connection fails before any reply arrives: iomgr invokes the
|
||||
* registered err_handler on this same ep. */
|
||||
vote_ep_ctx_build_for_test(&ep, &ha, ctx, /*peer_idx=*/0,
|
||||
4ULL, 100ULL, 50ULL, "ctrl-a");
|
||||
|
||||
int rc = vote_ep_err_handler_for_test(&ep);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_reply_hook_calls,
|
||||
"post-connect vote failure must reach vote_reply_dispatch's tally "
|
||||
"(xce_vote_reply_hook), not the legacy handle_request_vote_reply path "
|
||||
"which never invokes this hook");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, g_reply_granted,
|
||||
"a post-connect failure must be counted as a denied vote (granted=0)");
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(4ULL, g_reply_term,
|
||||
"the synthesized denial must carry the candidate's own term");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, g_reply_peer_idx,
|
||||
"peer_idx must match the failed endpoint's vote_ep_ctx_t peer_idx");
|
||||
|
||||
free(ep.data);
|
||||
free_ctx_for_vote(ctx);
|
||||
ha_fini(&ha);
|
||||
printf("test_post_connect_vote_failure_routes_through_production_tally: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Suite entry point ───────────────────────────────────────────────────── */
|
||||
|
||||
void test_ha_vote_send(void)
|
||||
|
|
@ -670,4 +732,6 @@ void test_ha_vote_send(void)
|
|||
RUN_TEST(test_vote_reply_handler_no_uaf);
|
||||
/* bug-1417 */
|
||||
RUN_TEST(test_bug1417_middle_index_real_vote_from_peer2_not_dropped);
|
||||
/* raft-adapter-conformance task 3.1 */
|
||||
RUN_TEST(test_post_connect_vote_failure_routes_through_production_tally);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue