fix(ha-term-safety): bug-828 FIXED — remove premature pre-election WAL truncate (committed-job-loss on clean failover)

ROOT CAUSE (confirmed via live instrumentation — NOT a durability violation):
ha_enter_real_election truncated the WAL to last_known_committed_lsn BEFORE the
vote. That watermark lags the true commit point (the primary committed LSN 2-5 on
a durable majority then died before a heartbeat carried committed=5 to the
standby), so the cutoff was 1; persist_truncate_wal_above -> clamp_durable_lsn
lowered the standby's durable_lsn 5->1, physically destroying durably-committed
entries before the election even computed the safe point. The candidate then
advertised floor=1, safe_committed_lsn=1, and the post-election truncate kept only
LSN 1. This directly violated the documented rule: 'Never use last_known_committed
_lsn for truncation — it may be stale.'

FIX: delete the premature truncate. The candidate keeps its full durable log; the
ONLY truncates are now the authoritative votes-derived post-election
persist_truncate_wal_above(safe_committed_lsn) (raft_cb_become_leader) + the
library on_truncate_from (term,lsn) reconcile — both before any commit under the
new leader.

SAFE (reviewer-verified): a divergent uncommitted tail can only exist at a
stale/lower term (single-writer); the library's term-primary (lastLogTerm,
lastLogIndex) freshness gate denies such a candidate — correct now that task 3.4
fixed the term axis (the prerequisite). No divergent entry can win-and-commit; no
bug-339 election-loop. 177 unit tests + a RED-proven regression test.

LIVE: clean failover now preserves committed jobs — walcompare committed-prefix
identical across all 3 members, both jobs present on disk, across multiple runs.
(Residual harness EXIT 1 is the SEPARATE pre-existing libxce client bug-854:
client can't reach the live primary when the dead node is first in the masters
list — a client-discovery defect, orthogonal to server-side data safety.)
This commit is contained in:
Rongsong Shen 2026-06-23 04:06:18 +00:00
parent ff869e4fc0
commit b77d412256
2 changed files with 91 additions and 6 deletions

View file

@ -464,6 +464,71 @@ static void test_repl_write_rejects_checkpoint(void)
printf("test_repl_write_rejects_checkpoint: PASS\n");
}
/* ================================================================== */
/* bug-828 final slice: a catch-up'd standby durable at LSN 5 must */
/* advertise its REAL durable tip (5) through a failover — the */
/* pre-election truncate must NOT discard durably-committed entries */
/* based on a STALE-LOW locally-known committed watermark. */
/* ================================================================== */
/* A standby streams catch-up entries up to LSN 5 (all fdatasync'd durable),
* so persist_durable_floor == 5. This is the LSN the candidate advertises
* (ha_candidate_advertise_lsn == persist_durable_floor) and the voter floor
* (ha_vote_self_freshness over persist_durable_floor) at the failover vote,
* which feeds safe-commit-from-votes.
*
* The clean-failover job-loss bug was that ha_enter_real_election truncated
* the WAL above the standby's STALE locally-known last_known_committed_lsn
* (e.g. 1 — the primary committed 2..5 but died before a heartbeat carried
* committed=5 to this standby) BEFORE the vote. persist_truncate_wal_above
* clamps durable_lsn down to the cutoff, so the standby then advertised 1,
* safe-commit computed 1, and the AUTHORITATIVE post-election truncate kept
* only LSN 1 — the committed jobs 2..5 (durable on this very node) were lost.
*
* This test pins the fix at the persist seam: after catch-up to 5 the durable
* floor is 5, and a truncate at the genuine safe-commit point (5, derived from
* the un-clamped voter floors) PRESERVES all five entries. A truncate at the
* stale-low watermark (1) — the removed pre-election step — would instead
* destroy LSN 2..5; this test asserts the durable committed prefix survives. */
static void test_repl_catchup_durable_floor_survives_failover(void)
{
char tmpdir[256];
persist_t* p = make_test_persist_in(tmpdir, sizeof(tmpdir));
/* Stream the committed prefix [1..5] as a standby does on catch-up — each
* persist_wal_write_replicated fdatasyncs inline (persist-before-ACK). */
for (uint64_t lsn = 1; lsn <= 5; lsn++) {
int rc = persist_wal_write_replicated(p, lsn, WAL_JOB_UPDATE,
"committed-job",
(uint32_t)strlen("committed-job"));
TEST_ASSERT_EQUAL_INT(0, rc);
}
/* All five are fdatasync'd durable, so the advertised floor is 5 — the
* value the failover vote must carry (RED before the partial fix: 1). */
TEST_ASSERT_EQUAL_UINT64_MESSAGE(5ULL, persist_durable_floor(p),
"catch-up'd standby must advertise its real fsync'd durable tip (5)");
/* The genuine safe-commit point (from the un-clamped voter floors) is 5;
* truncating above it is a no-op and preserves the committed prefix. This
* is the ONLY truncate the fixed failover path runs (post-election). */
TEST_ASSERT_EQUAL_INT(0, persist_truncate_wal_above(p, 5));
TEST_ASSERT_EQUAL_UINT64_MESSAGE(5ULL, persist_durable_floor(p),
"safe-commit truncate at the real tip must preserve all committed entries");
TEST_ASSERT_EQUAL_UINT64(6, p->current_lsn);
/* Recovery over the same WAL replays the full committed prefix [1..5]. */
destroy_test_persist(p);
persist_t* p2 = reopen_test_persist(tmpdir);
TEST_ASSERT_TRUE(persist_recover(p2) >= 0);
TEST_ASSERT_EQUAL_UINT64_MESSAGE(6, p2->current_lsn,
"the durable committed prefix [1..5] must survive a restart");
TEST_ASSERT_EQUAL_UINT64(5ULL, persist_durable_floor(p2));
destroy_test_persist(p2);
printf("test_repl_catchup_durable_floor_survives_failover: PASS\n");
}
/* ------------------------------------------------------------------ */
/* Suite entry point (registered in controller_test.c) */
/* ------------------------------------------------------------------ */
@ -482,4 +547,7 @@ void test_repl_apply(void)
RUN_TEST(test_repl_write_dedup_idempotent);
RUN_TEST(test_repl_write_rejects_forward_gap);
RUN_TEST(test_repl_write_rejects_checkpoint);
/* bug-828 final: durable committed prefix survives the failover vote */
RUN_TEST(test_repl_catchup_durable_floor_survives_failover);
}