feat(raft-apply-decoupling): post-replay apply-correctness verification (task 5.1)
Add apply_verdict_t (APPLY_CLEAN/APPLY_DIVERGENT) computed alongside the existing structural (CRC) recover_verdict_t. Dual-check design: an entity-count check (WAL_JOB_SUBMIT entries scanned vs live jobmgr growth, genuinely independent of apply_committed_entry()'s return code) and a position check against a new last_applied_lsn field (forward-looking infrastructure for task 6's live pump). Fixes a real bug found during review: the position check's expectation was computed from the physical WAL-scan tip (current_lsn - 1), which advances past checkpoint markers and unknown entry types that never reach apply_committed_entry(). A crash between persist_checkpoint()'s marker-fsync step and its later snapshot-persist step leaves a trailing checkpoint marker on next boot, which falsely reported APPLY_DIVERGENT on a perfectly clean recovery -- reachable today, not gated behind a future task. Fixed by reading last_applied_lsn directly, which only advances for entries actually dispatched through the apply seam. Known accepted gap: entity-count check covers jobs only, not workers (which can genuinely be created via WAL_WORKER_CTRL) -- documented for future extension.
This commit is contained in:
parent
665b29f5b0
commit
c005d40741
6 changed files with 800 additions and 3 deletions
|
|
@ -612,6 +612,12 @@ int persist_init(persist_t* p, persist_config_t* cfg, cluster_t* c)
|
|||
* on an unrecoverable boot; cleared on catch-up (never self-exits). */
|
||||
atomic_store(&p->recovery_unrecoverable, false);
|
||||
|
||||
/* Post-replay apply-correctness verdict defaults to CLEAN (raft-apply-
|
||||
* decoupling task 5.1, Req 8.2): a fresh store has nothing to diverge
|
||||
* from. persist_recover overwrites both after every replay. */
|
||||
p->apply_verdict = APPLY_CLEAN;
|
||||
atomic_store(&p->last_applied_lsn, 0);
|
||||
|
||||
/* Durable-advance wake hook starts unregistered (ha-commit-durability 2.2):
|
||||
* the commit engine registers it in commit_engine_init. Until then every
|
||||
* flusher durable advance is a no-op wake. */
|
||||
|
|
@ -2811,13 +2817,24 @@ int apply_committed_entry(persist_t* p, uint32_t wal_type, uint64_t lsn,
|
|||
* about the STRUCTURE of the local log, not about being behind the cluster: a
|
||||
* clean log that simply ends below the cluster high-water is still CLEAN.
|
||||
*
|
||||
* new_job_submits_out (raft-apply-decoupling task 5.1, when non-NULL) is set
|
||||
* to the number of WAL_JOB_SUBMIT entries this call actually dispatched via
|
||||
* apply_committed_entry() — i.e. entries above `from_lsn` that reached
|
||||
* dispatch, not merely present in the log. This is the log-derived half of
|
||||
* the post-replay apply-correctness expectation (see apply_expectation_t);
|
||||
* it is a pure scan tally, independent of apply_committed_entry()'s own
|
||||
* return code.
|
||||
*
|
||||
* Returns number of entries replayed (>= 0) or negative errno on I/O error. */
|
||||
static int replay_wal(persist_t* p, uint64_t from_lsn,
|
||||
recover_verdict_t* verdict_out)
|
||||
recover_verdict_t* verdict_out,
|
||||
uint64_t* new_job_submits_out)
|
||||
{
|
||||
/* Default to CLEAN; only a structural corruption flips it to TRUNCATED. */
|
||||
if (verdict_out)
|
||||
*verdict_out = RECOVER_CLEAN;
|
||||
if (new_job_submits_out)
|
||||
*new_job_submits_out = 0;
|
||||
|
||||
/* --- Collect and sort segment filenames by start LSN --- */
|
||||
DIR* d = opendir(p->wal_path);
|
||||
|
|
@ -3028,6 +3045,27 @@ static int replay_wal(persist_t* p, uint64_t from_lsn,
|
|||
hdr.payload_len);
|
||||
free(payload);
|
||||
|
||||
/* raft-apply-decoupling task 5.1: track the replay-derived
|
||||
* LOGICAL apply cursor separately from current_lsn (the physical
|
||||
* WAL position owned by this loop, bug-778) — last_applied_lsn is
|
||||
* the "highest entry actually dispatched via
|
||||
* apply_committed_entry()" position the post-replay verification
|
||||
* compares against. Segments are scanned in ascending LSN order
|
||||
* (sorted above) and entries within a segment are sequential, so
|
||||
* a plain store (no monotonic max needed) is correct here. */
|
||||
atomic_store_explicit(&p->last_applied_lsn, hdr.lsn,
|
||||
memory_order_relaxed);
|
||||
|
||||
/* Log-derived half of the post-replay apply-correctness
|
||||
* expectation (Req 8.1): every WAL_JOB_SUBMIT that reaches this
|
||||
* point is, by construction, above the loaded snapshot's
|
||||
* checkpoint_lsn (the from_lsn skip above already filtered out
|
||||
* anything the snapshot covers) — so it names a job that must now
|
||||
* exist in the live job table that did not exist before this
|
||||
* replay pass began. */
|
||||
if (hdr.type == WAL_JOB_SUBMIT && new_job_submits_out)
|
||||
(*new_job_submits_out)++;
|
||||
|
||||
/* Advance current_lsn to one past this entry */
|
||||
if (hdr.lsn + 1 > p->current_lsn)
|
||||
p->current_lsn = hdr.lsn + 1;
|
||||
|
|
@ -3047,6 +3085,85 @@ static int replay_wal(persist_t* p, uint64_t from_lsn,
|
|||
return replayed;
|
||||
}
|
||||
|
||||
/* raft-apply-decoupling task 5.1: cheap read-lock count of c->jobmgr's live
|
||||
* entries, mirroring collect_job_frames' iteration idiom (this file, above)
|
||||
* without collecting or filtering anything. Used only to derive the
|
||||
* post-replay materialized-entity expectation (never on a hot path). Safe on
|
||||
* a NULL cluster or jobmgr (a bare unit-test persist_t) — returns 0. */
|
||||
static uint64_t count_jobmgr_entries(cluster_t* c)
|
||||
{
|
||||
if (!c || !c->jobmgr)
|
||||
return 0;
|
||||
|
||||
lock(c->jobmgr, RTTI_LOCK_READ);
|
||||
|
||||
iterator_t it;
|
||||
int irc = iter_make(&it, c->jobmgr);
|
||||
if (irc < 0) {
|
||||
unlock(c->jobmgr);
|
||||
return 0; /* empty table */
|
||||
}
|
||||
|
||||
uint64_t n = 0;
|
||||
for (;;) {
|
||||
job_t* j = NULL;
|
||||
int nrc = iter_next(&it, (void**)&j);
|
||||
if (nrc < 0 || nrc == RTTI_ITER_NO_DATA)
|
||||
break;
|
||||
if (j == NULL)
|
||||
continue;
|
||||
n++;
|
||||
}
|
||||
|
||||
iter_destroy(&it);
|
||||
unlock(c->jobmgr);
|
||||
return n;
|
||||
}
|
||||
|
||||
/* persist_verify_apply_correctness — see persist.h for the full contract
|
||||
* doc (raft-apply-decoupling task 5.1, Req 8.1/8.2). Pure comparison: no
|
||||
* logging, no fence side effects. */
|
||||
apply_verdict_t persist_verify_apply_correctness(
|
||||
persist_t* p, const apply_expectation_t* exp,
|
||||
uint64_t jobmgr_count_before_replay)
|
||||
{
|
||||
if (!p || !exp)
|
||||
return APPLY_CLEAN; /* defensive: nothing to verify */
|
||||
|
||||
uint64_t actual_last_applied =
|
||||
atomic_load_explicit(&p->last_applied_lsn, memory_order_acquire);
|
||||
if (actual_last_applied != exp->expected_last_applied_lsn)
|
||||
return APPLY_DIVERGENT;
|
||||
|
||||
/* Entity-materialization half of the check: skip gracefully when there
|
||||
* is no live cluster to query — mirrors apply_committed_entry()'s own
|
||||
* "nothing to dispatch into" convention for a bare unit-test persist_t.
|
||||
* Only the position check above applies in that case. */
|
||||
if (!p->cluster || !p->cluster->jobmgr)
|
||||
return APPLY_CLEAN;
|
||||
|
||||
uint64_t jobmgr_count_after = count_jobmgr_entries(p->cluster);
|
||||
if (jobmgr_count_after < jobmgr_count_before_replay)
|
||||
return APPLY_DIVERGENT; /* table shrank — impossible on a clean replay */
|
||||
|
||||
uint64_t actual_new_jobs = jobmgr_count_after - jobmgr_count_before_replay;
|
||||
if (actual_new_jobs != exp->expected_new_job_count)
|
||||
return APPLY_DIVERGENT;
|
||||
|
||||
return APPLY_CLEAN;
|
||||
}
|
||||
|
||||
apply_verdict_t persist_apply_verdict(persist_t* p)
|
||||
{
|
||||
return p ? p->apply_verdict : APPLY_CLEAN;
|
||||
}
|
||||
|
||||
uint64_t persist_last_applied_lsn(persist_t* p)
|
||||
{
|
||||
return p ? atomic_load_explicit(&p->last_applied_lsn, memory_order_acquire)
|
||||
: 0;
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
/* Public: persist_recover */
|
||||
/* ------------------------------------------------------------------ */
|
||||
|
|
@ -3104,6 +3221,20 @@ int persist_recover(persist_t* p)
|
|||
}
|
||||
}
|
||||
|
||||
/* raft-apply-decoupling task 5.1 (Req 8.1): seed the replay-derived
|
||||
* apply cursor from the loaded baseline (checkpoint_lsn stays 0 when no
|
||||
* snapshot was found) so that, on a CLEAN replay with no new entries
|
||||
* beyond the snapshot, the cursor already sits at the recovered tip —
|
||||
* mirroring how current_lsn is seeded by load_snapshot. Step 3's
|
||||
* replay_wal advances it further for every entry actually dispatched
|
||||
* above this baseline. Also capture the live job table's size BEFORE
|
||||
* replay runs: the post-replay verification below needs this baseline to
|
||||
* isolate the jobs replay itself materializes from anything the snapshot
|
||||
* already restored. */
|
||||
atomic_store_explicit(&p->last_applied_lsn, p->checkpoint_lsn,
|
||||
memory_order_release);
|
||||
uint64_t jobmgr_count_before_replay = count_jobmgr_entries(p->cluster);
|
||||
|
||||
/* --- Step 3: replay WAL entries after checkpoint_lsn --- */
|
||||
/* Recovery-integrity verdict (ha-commit-durability task 3.1, Req 2.1):
|
||||
* replay distinguishes a clean log end from a corruption-truncated end.
|
||||
|
|
@ -3113,7 +3244,8 @@ int persist_recover(persist_t* p)
|
|||
* CLEAN — the sentinel established the committed position; replay only adds
|
||||
* entries above it. */
|
||||
recover_verdict_t verdict = RECOVER_CLEAN;
|
||||
int replayed = replay_wal(p, p->checkpoint_lsn, &verdict);
|
||||
uint64_t new_job_submits = 0;
|
||||
int replayed = replay_wal(p, p->checkpoint_lsn, &verdict, &new_job_submits);
|
||||
if (replayed < 0)
|
||||
return replayed;
|
||||
|
||||
|
|
@ -3148,6 +3280,51 @@ int persist_recover(persist_t* p)
|
|||
uint64_t recovered_tip = (p->current_lsn > 0) ? p->current_lsn - 1 : 0;
|
||||
atomic_store_explicit(&p->durable_lsn, recovered_tip, memory_order_release);
|
||||
|
||||
/* raft-apply-decoupling task 5.1 (Req 8.1/8.2): post-replay apply-
|
||||
* correctness verification, reported ALONGSIDE the structural
|
||||
* recovery_verdict set above — neither implies the other. See
|
||||
* persist_verify_apply_correctness's doc comment for exactly what is
|
||||
* compared. Routing an APPLY_DIVERGENT verdict into the existing
|
||||
* non-promotable loud-fence mechanism is task 5.2; this only classifies
|
||||
* and logs.
|
||||
*
|
||||
* Round-2 fix: the expected position MUST be the LOGICAL apply cursor
|
||||
* replay_wal() already maintains in p->last_applied_lsn — NOT
|
||||
* recovered_tip (current_lsn - 1, the PHYSICAL WAL-scan tip used above
|
||||
* for durable_lsn). current_lsn advances for every WAL entry replay_wal
|
||||
* scans, including WAL_CHECKPOINT markers and unknown future types,
|
||||
* neither of which is ever routed through apply_committed_entry() (see
|
||||
* the skip branches in replay_wal). last_applied_lsn only advances when
|
||||
* a real dispatch happens. If the highest-LSN entry above checkpoint_lsn
|
||||
* is a trailing WAL_CHECKPOINT marker — the exact state left behind by a
|
||||
* crash between persist_write_checkpoint_marker() (step 1, durably
|
||||
* fsynced) and write_snapshot() (step 3b, persists the advanced
|
||||
* checkpoint_lsn) — recovered_tip over-counts past it while
|
||||
* last_applied_lsn correctly does not, and comparing against
|
||||
* recovered_tip previously produced a false APPLY_DIVERGENT for a
|
||||
* perfectly clean recovery (violates Req 8.2). */
|
||||
uint64_t apply_cursor =
|
||||
atomic_load_explicit(&p->last_applied_lsn, memory_order_acquire);
|
||||
apply_expectation_t apply_exp = {
|
||||
.expected_last_applied_lsn = apply_cursor,
|
||||
.expected_new_job_count = new_job_submits,
|
||||
};
|
||||
p->apply_verdict = persist_verify_apply_correctness(
|
||||
p, &apply_exp, jobmgr_count_before_replay);
|
||||
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 "
|
||||
"present (ha-commit-durability recover_verdict=%s)",
|
||||
apply_exp.expected_last_applied_lsn,
|
||||
apply_exp.expected_new_job_count,
|
||||
(verdict == RECOVER_TRUNCATED) ? "RECOVER_TRUNCATED"
|
||||
: "RECOVER_CLEAN");
|
||||
else
|
||||
INFO("persist_recover: APPLY_CLEAN — materialized state matches the "
|
||||
"committed log tip (last_applied_lsn=%" PRIu64 ")",
|
||||
apply_cursor);
|
||||
|
||||
/* --- Step 4: retention pass (TTL check) — no-op in P0 (no entities) --- */
|
||||
|
||||
return 0;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue