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.
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.)
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.
On the standby replicated-apply path, ha_apply_single_entry wrote the entry via
persist_wal_write_replicated (which correctly set the self-computed CRC chain
link p->last_crc), then called apply_wal_entry with a SYNTHETIC header whose
crc=0. apply_wal_entry's `p->last_crc = hdr->crc` clobbered the chain link with
0, so every subsequent replicated write stamped prev_crc=0 on disk. The on-disk
CRC chain broke at the second replicated entry — invisible while live (standbys
serve in-memory state) but fatal on COLD RESTART: replay hit the break, returned
RECOVER_TRUNCATED, recovered only the config prefix, and lost all committed jobs
(joblist=0). The sticky truncated verdict then drove a promotion-fence
availability wedge (bug-777) when every survivor was truncated.
Fix — single ownership of the WAL header + CRC chain:
- apply_wal_entry is now LOGICAL entity-restore only: signature
(p, type, lsn, payload, payload_len); no header. It advances the current_lsn
high-water (monotonic) but NEVER touches p->last_crc.
- The physical layer is the sole owner of the CRC chain: persist_wal_write_-
replicated on the live path, replay_wal/persist_recover on recovery.
- ha_apply_single_entry drops the fabricated header and passes entry fields
directly. The persist_recover replay caller is updated to the new signature.
Verified: new RED->GREEN test test_replicated_apply_chain_survives_cold_restart
(drives the real ha_apply_wal_catchup_entry path, cold-replays -> RECOVER_CLEAN,
all N recovered); controller suite 0 failures; 3-node rolling-SIGKILL soak on a
real ext2/3 fs now passes 5/5 (was wedge+loss at round 3); full cold restart
recovers all 6 committed jobs (was 0) with CLEAN verdicts and a normal election.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A standby applied replicated WAL entries via apply_wal_entry (the replay/
in-memory path) which restores entities + advances applied_lsn but NEVER writes
the entry to the standby's WAL segment. The standby then ACKed (feeding the
primary's commit majority) without persisting — a Raft durability violation:
entries counted toward a commit could be lost on standby restart, and standby
WAL files stayed empty (contradicting steering: log[i] = WAL segments for every
node). This is why ctrl-2/ctrl-3 received + acked the streamed own-term NOOP yet
kept empty seg-*.wal.
Fix: new persist_wal_write_replicated(p, lsn, type, payload, len) writes the
entry to the local WAL at the PRIMARY's lsn (no lsn-reassignment), with a
self-consistent intra-segment CRC chain (the streamed entry carries no term/crc;
each node chains independently and recovery validates intra-segment), fdatasyncs
for durability, advances current_lsn = max(current_lsn, lsn+1), rotates segments,
and does NOT re-broadcast. Wired into ha_apply_single_entry as
persist(durable) -> apply(in-memory) -> ACK; on persist failure: no apply, no ACK
(a standby never ACKs an entry it didn't persist). Covers single + batch
catch-up; OPCode_replEntry has no separate apply path (allow-list only), so all
live WAL apply now persists. Dedup guard (lsn+1<=current_lsn) makes re-streams
idempotent while still ACKing.
TDD: standby write -> drop persist_t -> reopen + persist_recover replays it
(single, 3-entry crc chain, post-dedup); checkpoint/oversized rejected. Full
controller suite green under ASAN; persist_wal_async / recovery / truncation /
streamer / wire untouched.
WAL payloads:
- Job state transitions (START/FINISH/FAIL/STOP/RESUME/UPDATE etc):
wal_jobstate_t {uint64_t jobid, uint32_t state} — restores exact state flags
- Queue state (QUEUE_CTRL): wal_queuehdr_t {uint32_t status} + name string —
re-applies admin open/close/active/inactive after config queues are built
- Worker CTRL: existing [uint32_t detail][capnp xce_worker_t] unchanged
New recovery functions:
- restore_job_state(c, jobid, state): looks up job, overwrites state flags
- restore_queue_state(c, name, status): looks up queue, overwrites status
apply_wal_entry: dispatches all WAL types including job state and queue
test_repl_apply.c: weak stubs for restore_job_state/restore_queue_state
so unit test binary links without job.c/queue.c
Upgrade apply_wal_entry() from P0 static no-op stub to the real public
standby replay path:
- Reject oversized payloads (> MAX_WAL_PAYLOAD_BYTES) with -EMSGSIZE
- Skip WAL_CHECKPOINT (100) entries silently — local-only markers, never
replicated to standbys; current_lsn is NOT advanced on skip
- Skip unknown types (> WAL_ENTRY_TYPE_MAX) with WARN log and rc=0 for
forward compatibility with WAL written by newer controller versions
- Advance current_lsn to hdr->lsn+1 and update last_crc under p->lock
for all accepted known entries
- Gate restore_job()/worker_from_capnp() calls on p->cluster != NULL so
unit tests work without a full cluster wired in
- Add NULL guard returning -EINVAL for missing p or hdr
Add public declaration in persist.h with forward-declare stubs for
restore_job/worker_from_capnp in persist.c (avoids heavy cluster.h chain).
Test suite: 5 new Unity tests in test_repl_apply.c covering all branches.
Weak stubs for restore_job/worker_from_capnp in the test file satisfy the
linker without linking job.c/worker.c's heavy dependency graph.
All 5 new tests pass; full controller suite green.
