lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(ha-term-safety): bug-828 partial — revive dead safe-commit-from-votes + advertise real durable tip

Browse source 
Clean failover was truncating committed jobs because the post-election
safe-commit-from-votes computation was dead: vote_reply_dispatch decoded each
granting voter's advertised peerLastLogLsn but discarded it, so ha_compute_safe
_committed_lsn saw only the self-vote (the 'from 1 granted voters' fingerprint)
and truncated to self_lsn.

Part 1: vote_reply_dispatch now records the granting voter's (lsn,tag,grant) into
ctx->votes[] via a record-only ha_record_vote_reply seam (no double-promotion;
raft_on_vote_reply stays the sole production promotion driver) — so safe-commit
reflects the true voted majority.
Part 2: the candidate/voter advertise the real DURABLE (fsync'd) tip; persist_wal
_write_replicated advances durable_lsn after its fail-loud inline fdatasync.

Reviewer-verified SAFE (advertises only durable positions; safe-commit bounded by
the durable majority, preserving more committed entries, never marking a
non-majority-durable LSN safe). 177 unit tests; new test_ha_vote_gate + election
tests green.

PARTIAL: does NOT yet make the live clean-failover pass. A deeper slice of
bug-828/795 remains — a catch-up'd standby still under-advertises its durable
position in the live run (the documented architectural catch-up/durability gap),
so a job-less node can still win and truncate. Tracked for a dedicated effort.
This commit is contained in:
Rongsong Shen 2026-06-23 03:13:02 +00:00
parent 14c41cb298
commit 91b9a445f0
6 changed files with 313 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

115
src/controller/ha.c
View file

@ -3957,19 +3957,35 @@ apply_role:
* - votes_granted reaches quorum (fast path — no need to wait for all replies), OR
* - n_replied reaches n_peers (all peers accounted for, win or lose)
*/
void handle_request_vote_reply(ha_state_t *ha,
ha_elect_ctx_t *ctx,
int peer_idx,
uint64_t term,
int vote_granted,
uint64_t peer_config_tag,
uint64_t peer_lsn)
/*
* ha_record_vote_reply: record a decoded vote reply into ctx->votes[peer_idx]
* WITHOUT signalling the legacy promotion path (bug-828).
*
* Records the granting flag, peer config_tag, and peer LSN under ctx->lock with
* the same idempotency guard handle_request_vote_reply uses, and updates the
* vote tally + freshest-responder tracking. It does NOT call
* ha_elect_evaluate_and_signal — so it never posts ctx->done and never runs the
* legacy apply_role promotion side-effects.
*
* This is the seam the PRODUCTION vote_reply_dispatch path calls so the granting
* voter's advertised LSN reaches ha_compute_safe_committed_lsn (via ctx->votes[])
* while the raft library remains the SOLE production promotion driver
* (raft_on_vote_reply → on_become_leader). Routing the record through here (not
* handle_request_vote_reply) avoids a second, racing promotion signal.
*
* Returns non-zero if this reply newly reached the legacy quorum/all-replied
* threshold (so handle_request_vote_reply knows to signal); 0 otherwise.
* Must be called WITHOUT ctx->lock held (it takes the lock internally).
*/
static int ha_record_vote_reply_locked(ha_elect_ctx_t *ctx,
int peer_idx,
int vote_granted,
uint64_t peer_config_tag,
uint64_t peer_lsn)
{
(void)term; /* fencing: callers bump ha->term if peer's term > ours, before calling here */
/* Guard against out-of-range index */
if (peer_idx < 0 || peer_idx >= ctx->n_peers)
return;
return 0;
pthread_mutex_lock(&ctx->lock);
@ -3978,7 +3994,7 @@ void handle_request_vote_reply(ha_state_t *ha,
/* Idempotency guard: ignore duplicate replies */
if (entry->replied) {
pthread_mutex_unlock(&ctx->lock);
return;
return 0;
}
/* Record reply fields */
@ -4000,18 +4016,63 @@ void handle_request_vote_reply(ha_state_t *ha,
ctx->max_resp_lsn = peer_lsn;
}
/* Decide whether to signal the waiting thread.
* Quorum = ceil((n_peers + 1) / 2) where n_peers+1 is total cluster size.
* n_peers is the peer count excluding self; self-vote is already in votes_granted.
* ha_elect_evaluate_and_signal is called under ctx->lock; its done_posted guard
* prevents double sem_post if two threads reach quorum simultaneously. */
/* Compute whether the legacy quorum/all-replied threshold is now reached.
* Quorum = ceil((n_peers + 1) / 2) where n_peers+1 is total cluster size. */
int total_nodes = ctx->n_peers + 1;
int quorum = (total_nodes + 1) / 2;
if (ctx->votes_granted >= quorum || ctx->n_replied >= ctx->n_peers)
ha_elect_evaluate_and_signal(ha, ctx);
int reached = (ctx->votes_granted >= quorum || ctx->n_replied >= ctx->n_peers);
pthread_mutex_unlock(&ctx->lock);
return reached;
}
/* Public record-only seam (bug-828): records the reply but never signals the
* legacy promotion path. Used by the production vote_reply_dispatch path. */
void ha_record_vote_reply(ha_state_t *ha,
ha_elect_ctx_t *ctx,
int peer_idx,
uint64_t term,
int vote_granted,
uint64_t peer_config_tag,
uint64_t peer_lsn)
{
(void)ha; /* fencing: caller bumps ha->term before calling here */
(void)term;
if (!ctx)
return;
(void)ha_record_vote_reply_locked(ctx, peer_idx, vote_granted,
peer_config_tag, peer_lsn);
}
void handle_request_vote_reply(ha_state_t *ha,
ha_elect_ctx_t *ctx,
int peer_idx,
uint64_t term,
int vote_granted,
uint64_t peer_config_tag,
uint64_t peer_lsn)
{
(void)term; /* fencing: callers bump ha->term if peer's term > ours, before calling here */
if (peer_idx < 0 || peer_idx >= ctx->n_peers)
return;
/* Record the reply (shared with the production record-only seam) and learn
* whether the legacy quorum/all-replied threshold is now reached. */
int reached = ha_record_vote_reply_locked(ctx, peer_idx, vote_granted,
peer_config_tag, peer_lsn);
/* Decide whether to signal the waiting thread.
* ha_elect_evaluate_and_signal takes ctx->lock; its done_posted guard
* prevents double sem_post if two threads reach quorum simultaneously.
* Skip if the index was out of range / a duplicate reply (reached==0 and
* nothing recorded) — the duplicate case is already idempotent in
* ha_record_vote_reply_locked, which returns 0 without re-counting. */
if (reached) {
pthread_mutex_lock(&ctx->lock);
ha_elect_evaluate_and_signal(ha, ctx);
pthread_mutex_unlock(&ctx->lock);
}
}
/* ── Task 2.2: iomgr-native vote send path ──────────────────────────────────
@ -4182,7 +4243,21 @@ static void vote_reply_dispatch(ha_state_t *ha, ha_elect_ctx_t *ctx, int idx,
return;
}
#endif
(void)ctx; (void)peer_tag; (void)peer_lsn; /* consumed by raft library via callbacks */
/* bug-828 (PRIMARY fix): route the decoded reply into the election context
* BEFORE driving the raft library. Without this, the granting voter's
* advertised peer_lsn was discarded — ctx->votes[idx].vote_granted stayed 0,
* so a clean failover's ha_compute_safe_committed_lsn (read in
* raft_cb_become_leader / on_become_leader) saw only the self-vote
* (n_granted=1) and truncated committed entries down to self_lsn.
*
* Record-only: ha_record_vote_reply records ctx->votes[idx] under ctx->lock
* but does NOT post ctx->done / run the legacy apply_role promotion path.
* raft_on_vote_reply remains the SOLE production promotion driver (it owns
* the quorum tally and fires on_become_leader) — so there is no double
* promotion. `idx` is vctx->peer_idx, the local ctx->votes[] index (set in
* 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);
}