Regression from 63fd3d2a (bug-1870), found by the --config-upgrade HA soak
(force-elect: "submit on {ctrl-2,ctrl-1} failed", ~640 standby NACKs;
bisected: 725301dd passes, 88c4310e fails).
load_snapshot() seeded current_lsn = checkpoint_lsn, but current_lsn is
the NEXT lsn to write and the snapshot covers 1..checkpoint_lsn. The
off-by-one was always there, hidden because every checkpoint also wrote a
WAL_CHECKPOINT marker ABOVE the checkpoint, so the WAL scan moved
current_lsn past it. With the marker gone, a standby whose WAL held
nothing above its config-marker checkpoint (LSN 11) recovered with
current_lsn=11: its raft layer counted entry 11 as held (ae_already_have,
not written), and persist_wal_write_replicated() then refused entry 12 as
a gap (ae_append_persist_fail) on every resend -- stuck at 11 forever.
Both of load_snapshot's commit points now set current_lsn =
checkpoint_lsn + 1, matching the apply-side catch-up path (lsn + 1).
New test_recover_from_snapshot_without_wal_resumes_after_it (RED before:
current_lsn 3, not 4). Two snapshot-entity tests asserted the old
current_lsn == checkpoint_lsn and now expect +1. Controller suite 1245/0.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
persist_checkpoint() and persist_checkpoint_marker_triggered() wrote a
local-only WAL_CHECKPOINT marker that consumed an LSN of the replicated log
(current_lsn++) but was never replicated. A checkpoint taken by one member
only -- the primary's periodic max-wal-records checkpoint, or the one before
a snapshot send -- left a hole no standby had: the leader's next entry
carried prev_log_index = the marker's LSN, logOk failed on every standby
forever, commits timed out and writes stalled ("could not submit 110 jobs"
in the --force-snapshot soak; reproduced on the pre-bug-1867 baseline). Even
the config-marker checkpoint, taken on every member, stamped the marker with
each member's own current_lsn at apply time, which need not agree.
Nothing reads the marker except to skip it, so checkpoints simply stop
writing it. The recovery, streamer and bug-1475 log_len skip paths stay, for
WALs written by older binaries; persist_test_write_legacy_checkpoint_marker()
(XCE_TESTING only) reproduces such a WAL for the tests that cover them.
- new test_bug1870_leader_only_checkpoint_leaves_no_log_hole (RED before):
a leader-only checkpoint takes no LSN and a standby that never
checkpointed catches up past it via the real streamer/receive path.
- test_checkpoint_writes_marker_to_wal inverted to ..._writes_no_marker_...
- bug-1475 x2 and the apply-correctness trailing-marker test now build
their legacy marker explicitly.
Controller suite 1234/0. HA soaks: --force-snapshot PASS (113 jobs, real
snapshot install -- also live-verifies bug-1867), --config-upgrade-wal-trim
PASS, --sequential 6 rounds PASS (SIMVERIFY 7 files, 0 mismatches).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Only 21 of 86 test_*.c group functions called their own nested
UNITY_BEGIN()/UNITY_END(). Each nested call reset Unity's global
NumberOfTests/TestFailures counters, so controller_test.c's final
`return UNITY_END();` -- the value meson test actually gates on --
only ever reflected whichever small window of tests ran since the
LAST nested reset, silently discarding pass/fail evidence for
everything before it (measured: 16 of 972 total tests in one run).
The same nested-BEGIN calls also caused PASS/FAIL banners to show
the wrong source file for tests in files that don't reset it
themselves (Unity.TestFile just carries over from whichever group
last called UNITY_BEGIN()).
Fix: replace every nested UNITY_BEGIN() with UnitySetTestFile(__FILE__)
(sets file attribution only, no counter reset) and drop the matching
UNITY_END(); add the same UnitySetTestFile(__FILE__) as the first
statement to the other group functions that had neither. The 2
genuinely standalone binaries (test_job_array_sched_push.c,
test_job_array_apiserver_push.c -- each has its own int main(), not
registered via controller_test.c's TEST() macro) are untouched.
Verified: single consolidated summary now reads
"972 Tests 0 Failures 0 Ignored OK", exactly matching the
ground-truth RUN_TEST call-site count across all 83 registered
group files (976 total minus 4 belonging to the 2 standalone
binaries). Every PASS/FAIL banner now names its own true source
file. Full xce-build.sh all regression pass clean (Ok: 9, Fail: 0).
No production code touched -- test-harness plumbing only.
persist_checkpoint() stamped checkpoint_lsn from p->current_lsn (physical
WAL append tip) instead of p->last_applied_lsn (the true "jobmgr reflects
up through here" cursor), uncoordinated with the async apply pump
introduced by raft-apply-decoupling. write_snapshot() serializes live
jobmgr state, so a checkpoint taken while apply lags the WAL tip mislabels
the snapshot: on restart last_applied_lsn is re-seeded from checkpoint_lsn
and replay resumes strictly above it, silently skipping committed-but-not-
yet-applied entries jobmgr never received. This is the actual mechanism
behind kill-mid-apply producing a permanent APPLY_DIVERGENT fence —
apply_committed_entry() is deterministic/idempotent and write_snapshot()
is crash-atomic, so the bug was a mislabeled watermark, not a structural
divergence requiring a safety/liveness tradeoff.
Fix stamps checkpoint_lsn from last_applied_lsn and holds apply_pump_lock
across the label capture + write_snapshot() so the label and the jobmgr
state it describes are captured atomically w.r.t. the live apply pump.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VgT3n4vTv4ZjCuCTmzed1F
- persist_write_checkpoint_marker(): writes WAL_CHECKPOINT header entry
with CRC, increments current_lsn; never updates last_replicated_crc
(checkpoint markers are local-only, never replicated)
- write_snapshot(): atomic tmp→rename write of P0 sentinel snapshot
("XCESNAP\0" + checkpoint_lsn); full Cap'n Proto body in Task 11
- trim_wal_segments(): removes seg-*.wal files whose start LSN is
strictly less than checkpoint_lsn
- rotate_snapshots(): collects .capnp files, sorts by name (LSN order),
deletes oldest beyond snap_keep_n
- persist_checkpoint(): EBUSY guard via atomic_exchange, sequences
marker→lsn record→snapshot→trim→rotate, clears flag on exit
- 5 TDD tests in test_persist_checkpoint.c covering: snapshot file
creation, checkpoint_lsn advance, WAL lsn advance, EBUSY guard,
and rotation keeping at most snap_keep_n files
- All 18 controller unit tests pass (0 failures)
