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

feat(raft-apply-decoupling): route DIVERGENT verdict through existing fence (task 5.2)

Browse source 
Add bool apply_divergent to the existing recover_policy_inputs_t;
persist_recover_decision() unconditionally short-circuits to the
existing RECOVER_DECISION_FENCE_UNRECOVERABLE when true, reusing the
same sticky recovery_unrecoverable flag and
persist_recover_loud_fence_if_unrecoverable() -- no new fence
mechanism, no ha.c changes needed (the existing promotion gate
already reads the flag independently), no new self-termination path.

Fixes a real bug found during review: the fence's operator-facing
ERROR() log unconditionally asserted a "no healthy peer reachable...
durable_floor_lsn < hwm" clause that was only truthful for the
pre-existing structural-fence trigger. For the new apply_divergent
trigger this produced literally false claims in test output
("100 < 100", "2 < 0"). Fixed by branching the log call itself on
apply_divergent so it states only the verified reason.
This commit is contained in:
Rongsong Shen 2026-07-04 12:22:09 +00:00
parent c005d40741
commit 099ee1a216
4 changed files with 417 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

100
src/controller/persist.c
View file

@ -3311,7 +3311,7 @@ int persist_recover(persist_t* p)
};
p->apply_verdict = persist_verify_apply_correctness(
p, &apply_exp, jobmgr_count_before_replay);
if (p->apply_verdict == APPLY_DIVERGENT)
if (p->apply_verdict == APPLY_DIVERGENT) {
ERROR("persist_recover: APPLY_DIVERGENT — materialized state does "
"not match the committed log tip (expected last_applied_lsn=%"
PRIu64 " new_job_count=%" PRIu64 "); a hidden apply defect is "
@ -3320,7 +3320,32 @@ int persist_recover(persist_t* p)
apply_exp.expected_new_job_count,
(verdict == RECOVER_TRUNCATED) ? "RECOVER_TRUNCATED"
: "RECOVER_CLEAN");
else
/* raft-apply-decoupling task 5.2 (Req 8.3/8.4): route the DIVERGENT
* verdict through the EXISTING non-promotable loud-fence mechanism
* (persist_recover_loud_fence_if_unrecoverable + the sticky
* recovery_unrecoverable flag) instead of a new fence path.
* apply_divergent=true fences UNCONDITIONALLY (see
* persist_recover_decision), so the peer/HWM fields below are
* inert placeholders here -- they are not yet known at this
* boot-time replay call site (unlike the structural verdict's own
* fence check, which stays deferred to promotion time in ha.c,
* where live peer/HWM context exists). verdict/recovered_tip are
* passed through only for accurate, consistent logging. The member
* stays ALIVE, non-promotable, and keeps retrying catch-up -- it
* never exits (controller-never-self-terminate invariant,
* CLAUDE.md). */
recover_policy_inputs_t apply_fence_in = {
.verdict = verdict,
.durable_below_hwm = false,
.healthy_peer_reachable = false,
.catch_up_succeeded = false,
.durable_floor_lsn = recovered_tip,
.cluster_hwm_lsn = 0,
.apply_divergent = true,
};
persist_recover_loud_fence_if_unrecoverable(p, &apply_fence_in);
} else
INFO("persist_recover: APPLY_CLEAN — materialized state matches the "
"committed log tip (last_applied_lsn=%" PRIu64 ")",
apply_cursor);
@ -3366,6 +3391,20 @@ recover_decision_t persist_recover_decision(const recover_policy_inputs_t* in)
if (in == NULL)
return RECOVER_DECISION_ELIGIBLE; /* defensive: nothing to fence */
/* raft-apply-decoupling task 5.2 (Req 8.3/8.4): an APPLY_DIVERGENT
* post-replay verdict is a NEW INPUT to this same decision table, not a
* new fence mechanism (design.md "Restart Replay & Verification"). It
* fences UNCONDITIONALLY -- every other field below is irrelevant once
* this is true. Unlike a structurally truncated/behind log (rows 2/3/5),
* where a healthy peer's missing entries can repair the gap and legitimately
* downgrade the outcome to a plain FENCE-then-catch-up, an apply-time
* defect means the LOCAL apply path itself produced the wrong
* materialized state from bytes it already had -- streaming more entries
* from a peer does not prove that path is now trustworthy, so this is
* never downgraded by peer reachability or catch-up success. */
if (in->apply_divergent)
return RECOVER_DECISION_FENCE_UNRECOVERABLE;
bool truncated = (in->verdict == RECOVER_TRUNCATED);
/* Rows 1 & 6: a structurally CLEAN log that is NOT behind the cluster
@ -3430,22 +3469,47 @@ persist_recover_loud_fence_if_unrecoverable(persist_t* p,
if (decision != RECOVER_DECISION_FENCE_UNRECOVERABLE)
return decision;
/* Loud, operator-visible alert: log the truncation boundary
* (durable_floor_lsn) and the cluster high-water mark so an operator sees
* exactly where the local log stops short of the cluster's committed state.
* The member STAYS ALIVE, stays non-promotable, and keeps retrying catch-up
* (Req 2.4) — it NEVER self-exits. */
const char* why = (in->verdict == RECOVER_TRUNCATED)
? "recovery-truncated log (structural corruption)"
: "log behind the cluster committed high-water mark";
ERROR("persist_recover: UNRECOVERABLE — %s and no healthy peer reachable to "
"catch up; truncation boundary durable_floor_lsn=%" PRIu64
" < cluster committed high-water hwm=%" PRIu64 ". Staying ALIVE and "
"PERMANENTLY non-promotable (refusing to serve divergent state) and "
"RETRYING catch-up indefinitely — will self-heal when a healthy peer "
"returns; operator attention may be needed "
"(ha-commit-durability Req 2.4)",
why, in->durable_floor_lsn, in->cluster_hwm_lsn);
/* Loud, operator-visible alert. The two triggers reaching this terminal
* are NOT logged with the same clause (round-2 fix, raft-apply-
* decoupling task 5.2 remediation):
*
* - apply_divergent: fences UNCONDITIONALLY (persist_recover_decision),
* evaluated at boot-replay time BEFORE peer/HWM context exists (see
* the call site in persist_recover()). `healthy_peer_reachable` and
* `cluster_hwm_lsn` are inert placeholders there, not real facts —
* asserting "no healthy peer reachable" or a durable_floor_lsn < hwm
* comparison for this trigger would report unevaluated-or-false
* information to the operator (round-1 review: the placeholders can
* make the comparison numerically impossible, e.g. "2 < 0"). This
* branch names the real reason (apply-correctness divergence) only.
* - structural (truncated / behind-HWM): reaches this terminal only
* via the pre-existing "no healthy peer reachable" branch in
* persist_recover_decision, so the peer-reachability and
* durable_floor_lsn/hwm comparison ARE guaranteed true here — that
* clause is unchanged from before this task.
*/
if (in->apply_divergent) {
ERROR("persist_recover: UNRECOVERABLE — post-replay apply-correctness "
"verification is DIVERGENT (materialized state does not match "
"the committed log tip, raft-apply-decoupling Req 8.3); a local "
"apply defect cannot be proven fixed by peer reachability or "
"catch-up alone. Staying ALIVE and PERMANENTLY non-promotable "
"(refusing to serve divergent state) and RETRYING catch-up "
"indefinitely — operator attention is needed to diagnose the "
"apply path (raft-apply-decoupling Req 8.3/8.4)");
} else {
const char* why = (in->verdict == RECOVER_TRUNCATED)
? "recovery-truncated log (structural corruption)"
: "log behind the cluster committed high-water mark";
ERROR("persist_recover: UNRECOVERABLE — %s and no healthy peer reachable to "
"catch up; truncation boundary durable_floor_lsn=%" PRIu64
" < cluster committed high-water hwm=%" PRIu64 ". Staying ALIVE and "
"PERMANENTLY non-promotable (refusing to serve divergent state) and "
"RETRYING catch-up indefinitely — will self-heal when a healthy peer "
"returns; operator attention may be needed "
"(ha-commit-durability Req 2.4)",
why, in->durable_floor_lsn, in->cluster_hwm_lsn);
}
/* Set the sticky non-promotable flag so the promotion gate refuses
* leadership as belt-and-suspenders until catch-up clears it (Req 2.4/2.6).

310
src/controller/testsuite/test_apply_correctness_verification.c
View file

@ -508,10 +508,320 @@ static void test_verify_apply_correctness_clean_when_no_cluster(void)
printf("test_verify_apply_correctness_clean_when_no_cluster: PASS\n");
}
/* ── raft-apply-decoupling task 5.2 (Req 8.3/8.4): route an APPLY_DIVERGENT
* verdict through the EXISTING non-promotable loud-fence mechanism
* (persist_recover_decision / persist_recover_loud_fence_if_unrecoverable
* / the recovery_unrecoverable sticky flag) rather than a new fence path.
* ────────────────────────────────────────────────────────────────────── */
/*
* An APPLY_DIVERGENT verdict is a NEW INPUT to the EXISTING Boot Policy
* Decision Table (persist_recover_decision), not a new decision path. Prove
* apply_divergent fences UNCONDITIONALLY: even when every OTHER input looks
* perfectly healthy (structurally CLEAN, at/above the cluster HWM, a healthy
* peer reachable, catch-up already succeeded — exactly the combination that
* otherwise decides ELIGIBLE), apply_divergent=true alone must still force
* RECOVER_DECISION_FENCE_UNRECOVERABLE. Unlike a structurally truncated or
* behind log, a healthy peer's missing entries do not prove a LOCAL apply
* defect is fixed, so this input is never downgraded to a plain FENCE the
* way truncated-with-a-peer is (rows 2/3/5) — it is not something catch-up
* alone resolves.
*/
static void test_apply_divergent_decision_forces_unconditional_fence(void)
{
recover_policy_inputs_t healthy_but_divergent = {
.verdict = RECOVER_CLEAN,
.durable_below_hwm = false,
.healthy_peer_reachable = true,
.catch_up_succeeded = true,
.durable_floor_lsn = 42,
.cluster_hwm_lsn = 42,
.apply_divergent = true,
};
TEST_ASSERT_EQUAL_INT_MESSAGE(
RECOVER_DECISION_FENCE_UNRECOVERABLE,
persist_recover_decision(&healthy_but_divergent),
"an APPLY_DIVERGENT verdict must fence UNCONDITIONALLY, even when "
"every structural/HWM/peer/catch-up input looks eligible");
/* Sanity: flipping apply_divergent back to false with the SAME other
* fields reverts to the pre-existing ELIGIBLE outcome -- confirms this
* is an ADDITIVE input, not a change to the pre-5.2 decision table for
* every other combination. */
recover_policy_inputs_t healthy_and_clean = healthy_but_divergent;
healthy_and_clean.apply_divergent = false;
TEST_ASSERT_EQUAL_INT_MESSAGE(
RECOVER_DECISION_ELIGIBLE,
persist_recover_decision(&healthy_and_clean),
"sanity: without apply_divergent this exact combination is "
"unchanged from the pre-5.2 ELIGIBLE outcome");
printf("test_apply_divergent_decision_forces_unconditional_fence: PASS\n");
}
/*
* The ACTION helper (persist_recover_loud_fence_if_unrecoverable) must react
* to apply_divergent exactly like the pre-existing structural-corruption
* terminal: log LOUDLY, set the SAME sticky recovery_unrecoverable flag (no
* second/parallel flag), and RETURN -- the test simply continuing past the
* call is the proof the process stayed alive (no abort()/exit() anywhere on
* this path), mirroring test_boot_policy_row4_truncated_no_peer_loud_fence's
* verification style (test_commit_durability.c) -- the existing template
* for this exact assertion shape.
*/
static void test_apply_divergent_loud_fence_sets_sticky_flag_and_stays_alive(void)
{
persist_t p;
memset(&p, 0, sizeof(p));
recover_policy_inputs_t in = {
.verdict = RECOVER_CLEAN,
.durable_below_hwm = false,
.healthy_peer_reachable = true,
.catch_up_succeeded = true,
.durable_floor_lsn = 100,
.cluster_hwm_lsn = 100,
.apply_divergent = true,
};
TEST_ASSERT_FALSE_MESSAGE(
persist_recovery_unrecoverable(&p),
"sanity: the sticky flag starts clear on a fresh persist_t");
/* Logs LOUDLY, sets the sticky flag, and RETURNS -- reaching the
* assertions below IS the proof the process stayed alive. */
recover_decision_t d = persist_recover_loud_fence_if_unrecoverable(&p, &in);
TEST_ASSERT_EQUAL_INT_MESSAGE(
RECOVER_DECISION_FENCE_UNRECOVERABLE, d,
"an APPLY_DIVERGENT input must decide the loud, non-terminating "
"fence -- not process exit");
TEST_ASSERT_TRUE_MESSAGE(
persist_recovery_unrecoverable(&p),
"an APPLY_DIVERGENT verdict must set the SAME sticky "
"recovery_unrecoverable flag the structural-corruption case uses -- "
"not a second, parallel fence flag");
printf("test_apply_divergent_loud_fence_sets_sticky_flag_and_stays_alive: "
"PASS\n");
}
/*
* End-to-end: a REAL materialization mismatch produced by an actual
* persist_recover() replay (not the pure functions tested above) must ITSELF
* set the sticky non-promotable fence, with no test-only shortcut faking the
* verdict.
*
* Forces a genuine mismatch: replay_wal's new_job_submits_out counts every
* RAW WAL_JOB_SUBMIT header it scans (persist.c), unconditionally -- it does
* NOT know whether apply_committed_entry()'s restore_job() actually created a
* new job or silently no-op'd on a duplicate jobid (restore_job is
* idempotent, Req 7.1). Appending the identical jobid twice therefore drives
* new_job_submits=2 while the live jobmgr table only grows by 1 -- a real,
* reproducible APPLY_DIVERGENT with nothing about
* persist_verify_apply_correctness() itself mocked or shortcut.
*/
static void test_real_replay_divergence_sets_unrecoverable_fence(void)
{
char tmp[256];
strcpy(tmp, "/tmp/xce_tacv_divergent_XXXXXX");
TEST_ASSERT_NOT_NULL(mkdtemp(tmp));
/* Phase 1: append the SAME jobid twice as two separate WAL_JOB_SUBMIT
* entries -- a real replay counts both, but restore_job() only
* materializes the job once. */
{
persist_t* p = tacv_make_fresh_persist(tmp);
TEST_ASSERT_NOT_NULL(p);
for (int i = 0; i < 2; i++) {
xce_job_t jdef;
memset(&jdef, 0, sizeof(jdef));
jdef.jobid = 72001;
jdef.state = JOB_STAT_PEND;
job_submit_data_t submit;
memset(&submit, 0, sizeof(submit));
submit.queue = (char*)"rq1";
submit.user = (char*)"tester";
jdef.submit = &submit;
uint8_t* payload = NULL;
size_t plen = 0;
TEST_ASSERT_EQUAL_INT(0, tacv_encode_job(&jdef, &payload, &plen));
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(p, WAL_JOB_SUBMIT, payload, (uint32_t)plen));
free(payload);
}
TEST_ASSERT_EQUAL_INT(0, persist_flush(p));
tacv_teardown_persist(p);
}
/* Phase 2: fresh persist_t + fresh cluster_t, recover from disk. The
* duplicate-jobid replay must produce a genuine APPLY_DIVERGENT verdict,
* which persist_recover() must now route through the existing
* non-promotable loud-fence mechanism (task 5.2). */
{
persist_t* p2 = tacv_make_fresh_persist(tmp);
TEST_ASSERT_NOT_NULL(p2);
tacv_build_cluster();
p2->cluster = &g_tacv_c;
TEST_ASSERT_FALSE_MESSAGE(
persist_recovery_unrecoverable(p2),
"sanity: the sticky flag starts clear before recovery runs");
/* persist_recover() itself must RETURN normally (0) -- no
* abort()/exit() anywhere on this path (controller-never-self-
* terminate invariant). Reaching the assertions below is itself
* part of the proof the process stayed alive. */
TEST_ASSERT_EQUAL_INT(0, persist_recover(p2));
TEST_ASSERT_EQUAL_INT_MESSAGE(APPLY_DIVERGENT,
persist_apply_verdict(p2),
"fixture sanity: the duplicate-jobid replay must genuinely "
"diverge (new_job_submits=2, jobmgr only grew by 1)");
TEST_ASSERT_TRUE_MESSAGE(
persist_recovery_unrecoverable(p2),
"a real APPLY_DIVERGENT verdict from persist_recover() must set "
"the sticky non-promotable fence via the existing loud-fence "
"mechanism (Req 8.4)");
tacv_teardown_cluster();
tacv_teardown_persist(p2);
}
char cmd[512];
snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp);
if (system(cmd) != 0)
fprintf(stderr, "cleanup warning: rm -rf %s failed\n", tmp);
printf("test_real_replay_divergence_sets_unrecoverable_fence: PASS\n");
}
/*
* raft-apply-decoupling task 5.2 round 2 (Req 8.3 remediation): the
* apply_divergent ERROR log must NOT append the shared structural-fence
* boilerplate clause ("...and no healthy peer reachable to catch up;
* truncation boundary durable_floor_lsn=X < cluster committed high-water
* hwm=Y...") -- round-1 review found that clause is only guaranteed true for
* the pre-existing structural-fence rows (reached exclusively via the
* "no healthy peer" branch in persist_recover_decision). apply_divergent
* fences UNCONDITIONALLY, evaluated at boot-replay time before peer/HWM
* context exists, so `healthy_peer_reachable`/`cluster_hwm_lsn` are inert
* placeholders there -- emitting that clause reports unevaluated-or-false
* facts to the operator (round-1 repro: "durable_floor_lsn=2 < ... hwm=0",
* a numerically impossible claim).
*
* This test picks fixture inputs that make the false clause easy to catch if
* regressed -- a peer that WAS reachable (true) and durable_floor_lsn (100)
* NOT below cluster_hwm_lsn (100), the exact combination
* test_apply_divergent_loud_fence_sets_sticky_flag_and_stays_alive uses --
* and asserts on the REAL ERROR() text.
*
* Log capture mechanism: this test binary (controller_unit_test) never calls
* mylog_init()/tlog_init() -- that only happens in the real daemon main()s
* (controller/main.c, scheduler/main.c) and the xce_test client tool's
* main(), never in controller_test.c. So tlog's internal root log handle
* stays NULL for this whole process, and every ERROR()/INFO() call falls
* through tlog's early-print path, which writes the fully formatted line
* directly to STDOUT_FILENO via a raw write() (vendored tinylog, tlog.c).
* Redirecting STDOUT_FILENO for the duration of the call under test
* therefore captures the EXACT text an operator would see on their
* terminal/log -- nothing about persist_recover_loud_fence_if_unrecoverable
* or ERROR() itself is mocked or shortcut. There is no pre-existing
* log-content-assertion helper elsewhere in this test file or its siblings
* (test_ha_startup.c/test_ha_role_lifecycle.c/test_commit_barrier_rollout.c
* only redirect a *forked child's* fds to /dev/null to silence noise, never
* capture-and-inspect content in-process) -- this is a new, self-contained,
* one-test-local mechanism, not a shared harness change.
*/
static void test_apply_divergent_loud_fence_log_omits_false_peer_hwm_clause(void)
{
persist_t p;
memset(&p, 0, sizeof(p));
recover_policy_inputs_t in = {
.verdict = RECOVER_CLEAN,
.durable_below_hwm = false,
.healthy_peer_reachable = true,
.catch_up_succeeded = true,
.durable_floor_lsn = 100,
.cluster_hwm_lsn = 100,
.apply_divergent = true,
};
char tmp[] = "/tmp/xce_tacv_logcap_XXXXXX";
int fd = mkstemp(tmp);
TEST_ASSERT_TRUE_MESSAGE(fd >= 0,
"fixture: mkstemp for stdout capture must succeed");
fflush(stdout);
int saved_stdout = dup(STDOUT_FILENO);
TEST_ASSERT_TRUE_MESSAGE(saved_stdout >= 0,
"fixture: dup(STDOUT_FILENO) must succeed");
TEST_ASSERT_TRUE_MESSAGE(dup2(fd, STDOUT_FILENO) >= 0,
"fixture: dup2 redirect of STDOUT_FILENO must succeed");
recover_decision_t d = persist_recover_loud_fence_if_unrecoverable(&p, &in);
fflush(stdout);
dup2(saved_stdout, STDOUT_FILENO);
close(saved_stdout);
TEST_ASSERT_EQUAL_INT_MESSAGE(RECOVER_DECISION_FENCE_UNRECOVERABLE, d,
"sanity: fixture inputs must still decide the unrecoverable fence");
off_t len = lseek(fd, 0, SEEK_CUR);
TEST_ASSERT_TRUE_MESSAGE(len > 0,
"fixture: the ERROR() call must have produced captured output");
char* buf = calloc(1, (size_t)len + 1);
TEST_ASSERT_NOT_NULL(buf);
TEST_ASSERT_TRUE_MESSAGE(lseek(fd, 0, SEEK_SET) == 0,
"fixture: rewind of the capture file must succeed");
ssize_t nread = read(fd, buf, (size_t)len);
TEST_ASSERT_TRUE_MESSAGE(nread == len,
"fixture: reading back the full captured buffer must succeed");
close(fd);
unlink(tmp);
TEST_ASSERT_NULL_MESSAGE(strstr(buf, "no healthy peer reachable"),
"the apply_divergent fence log must not claim no healthy peer was "
"reachable -- healthy_peer_reachable is an unevaluated placeholder "
"for this trigger, and was actually true in this fixture");
TEST_ASSERT_NULL_MESSAGE(
strstr(buf, "durable_floor_lsn=100 < cluster committed high-water "
"hwm=100"),
"the apply_divergent fence log must not assert a false/unevaluated "
"durable_floor_lsn < hwm comparison");
/* Positive: the log must still be loud, operator-visible, and name the
* real reason accurately (Req 8.3's "loud, operator-visible" mandate is
* a requirement to satisfy, not just a clause to strip). */
TEST_ASSERT_NOT_NULL_MESSAGE(strstr(buf, "UNRECOVERABLE"),
"the fence log must still be loud/operator-visible");
TEST_ASSERT_NOT_NULL_MESSAGE(
strstr(buf, "apply-correctness verification is DIVERGENT"),
"the fence log must still accurately name the apply-divergence "
"reason");
free(buf);
printf("test_apply_divergent_loud_fence_log_omits_false_peer_hwm_clause: "
"PASS\n");
}
void test_apply_correctness_verification(void)
{
RUN_TEST(test_clean_replay_produces_apply_clean_alongside_crc_verdict);
RUN_TEST(test_checkpoint_tail_with_no_snapshot_is_apply_clean);
RUN_TEST(test_verify_apply_correctness_detects_job_count_mismatch);
RUN_TEST(test_verify_apply_correctness_clean_when_no_cluster);
RUN_TEST(test_apply_divergent_decision_forces_unconditional_fence);
RUN_TEST(test_apply_divergent_loud_fence_sets_sticky_flag_and_stays_alive);
RUN_TEST(test_real_replay_divergence_sets_unrecoverable_fence);
RUN_TEST(test_apply_divergent_loud_fence_log_omits_false_peer_hwm_clause);
}