Per design invariant (a controller exits only on crash or admin kill), replace the
self-abort recovery terminal (tasks 3.3/4.3/4.4) with a loud, permanent, non-promotable
fence that stays alive and keeps retrying catch-up (self-heals on peer return; no manual
admin recovery). Records the never-self-terminate invariant as project-wide guidance in
src/controller/CLAUDE.md + .kiro/steering/ha-async-io.md.
fsync_worker slept the full wal_sync_ms in one uninterruptible nanosleep()
before re-checking fsync_stop. persist_stop sets the flag and immediately
pthread_join()s, so the join blocked until the in-flight sleep returned —
up to a full wal_sync_ms per teardown. Tests set wal_sync_ms=5000 ("slow
fsync — no races"), so each persist_stop cost ~5s; across the ~7 suites
that use it the cumulative teardown time blew past the 300s controller-test
meson cap, killing the suite with SIGTERM.
Replace the nanosleep with pthread_cond_timedwait on a new fsync_mutex/
fsync_cond (added to persist_t, init in persist_init, destroyed in
persist_fini). persist_stop now sets fsync_stop and signals fsync_cond
under fsync_mutex before joining, so the worker wakes immediately. fdatasync
still runs under p->lock, released before re-acquiring fsync_mutex — no lock
nesting. The "commit: WAIT deadline expired" log lines were unrelated
intentional negative tests (50-300ms), not the cause.
Full controller suite now completes in ~10s (was >300s TIMEOUT); 0 failures.
Logged as bug-604.
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.
Root cause (found via live 3-node E2E + a verification test): lsn was 0-based
(hdr.lsn = current_lsn++ from current_lsn=0), and 0 was overloaded as a
sentinel, causing three cold-start defects on a freshly-promoted leader:
- the first WAL entry (own-term promotion NOOP at lsn 0) was structurally
uncommittable: commit_advance_locked only stores when cand>cur and
committed_lsn starts at 0, so lsn 0 can never advance it;
- commit_set_writable_lsn(noop_lsn=0) set writable_lsn=0, which wait_done
reads as 'gate off' — the own-term safety gate was a no-op on the first leader;
- bug-543 (WAL hole): ha_stream_wal_to_standby filters hdr.lsn>standby_lsn;
a fresh empty standby reports standby_lsn=0 (ambiguous 'empty' vs 'has lsn 0'),
so lsn 0 failed 0>0 and was dropped -> non-contiguous standby WAL.
Fix: a fresh persist store initializes current_lsn=1, reserving lsn 0 as the
none/empty sentinel. The first real entry is lsn 1 -> committable (0->1),
writable_lsn=1 -> gate effective, and standby_lsn=0 unambiguously means 'empty'
so catch-up streams from the first entry with no hole. persist_recover is
data-driven (current_lsn=max(hdr.lsn+1)), so existing on-disk WALs and non-fresh
recovery are unaffected; the change only sets the base for brand-new stores.
No wire/capnp change.
Verification: test_coldstart_lsn0 drives the real commit engine + real streamer
and proves H1 (first entry commits 0->1), H3 (gate effective + blocks when
committed<writable), H4 (empty-standby catch-up is contiguous [1,2], control
standby_lsn=1 gets only [2]). Full controller suite green under ASAN. Tests that
relied on the lsn-0 base were shifted (several strengthened); commit.c /
config_repl.c / ha.c / wire format untouched.
Owning specs: ha-commit-engine / ha-wal-replication / persist. Follow-up:
sweep stale lsn-0 comments (repl.c:429-436, persist.c:1055-1075/1240-1243/1296).
7 cases: two covered short-circuits, timeout with committed_lsn
unmoved, covering-ACK release, sync_timeout_ms fallback (bounded both
directions), n_peers=1 standalone, NOT_READY passthrough via cap
saturation. Watermark resolved to current_lsn - 1 (design's literal
phrasing unsatisfiable per persist.h:71 invariant) with off-by-one
killing power both ways. 15 tests / 7 RED; suite baseline zero.
Verified table implemented (STOP/RESUME/UPDATE/FAIL RETAIN per
supersession; WORKER detail switch; CHECKPOINT LOCAL); unknown type =
ERROR + XCE_TESTING abort + production RETAIN fallback. All three
hooks route through wal_hook_append_classified (zero bare async
appends); RETAIN registers with the engine lock-free; facts propagate
local append rc only. 8/8 group tests green; full suite 465/465.
cluster_t embeds commit_engine_t; init in cluster_init_phase2 after the
vtaskmgr guard (idempotence-guarded — phase2 can run twice), wiring
repl_set_commit_engine + ha.repl back-pointer before any standby can
register; fini after iomgr/vtask shutdown. persist_wal_sync_append
passes written_lsn; engine-wired path drops -ENODEV and the
committed_lsn store (engine owns advancement); LSN-0 first-entry
routes to legacy per the accepted interim. p2 -ENODEV pin updated to
the Req 6.3 contract. 34/34 + 6/6 green through honest delegation;
census exactly at the 3 validated baselines; production binary links.
Four bugs fixed:
1. persist.c: array job WAL loop aborted after index [0] when replication
returned -ENODEV (no standbys). Loop guard now uses the return value of
persist_wal_sync_append() which is 0 on local-disk success regardless of
replication status.
2. join.c: WAL-recovered worker slot (status=UNAVAIL) caused add_worker() to
return -EEXIST on reconnect. New EEXIST path in handle_worker_join() looks
up the existing slot by name, updates its endpoint, and clears UNAVAIL so
the normal hook/reply/on_worker_join flow proceeds.
3. xcectx.c: cbuf_get() in xce_hello() blocked indefinitely when the TCP
connection dropped because no error handler was registered. Added
handle_disconnect() which sets ctx->state=XCE_CTX_ERR and calls
cbuf_tryput(-ECONNRESET) to unblock the waiter, allowing
checking_connection() to detect the failure and rejoin.
4. cluster.c: persist_recover() was called before cluster_build_resources()
populated resmgr, causing restore_worker() → add_hostattrs_resvals() to
fail with EEXIST noise. Moved persist_recover() to after HOOK_AFTER_EVENT
so resmgr is fully populated before WAL replay.
All three persist hook functions (job/worker/queue) were ignoring the
situation argument, so they fired on both BEFORE and AFTER events.
BEFORE_EVENT hooks pass different va_list layouts than AFTER_EVENT:
- WORKER_JOIN BEFORE: (cluster_t*, char* name, int 0) vs AFTER: (cluster_t*, worker_t*)
- JOB_NEW BEFORE: (cluster_t*, xce_job_submit_t*) vs AFTER: (cluster_t*, xlist_t*)
Reading the wrong type from va_list caused a dereference crash.
Fix: return 0 immediately on HOOK_BEFORE_EVENT in all three hooks.
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
WAL hooks now write real Cap'n Proto payloads instead of stub uint32_t:
- WAL_JOB_SUBMIT: full xce_job_t serialized via encode_Job/write_Job
- WAL_WORKER_CTRL: [uint32_t detail][xce_worker_t capnp] via encode_Worker
- Other job events (START/FINISH/FAIL/STOP etc): uint64_t jobid only
restore_job(): deserializes Cap'n Proto xce_job_t, guards against duplicate
replay with key_find check, then calls create_job()
worker_from_capnp(): skips 4-byte detail prefix, deserializes xce_worker_t,
guards against duplicate replay, then calls restore_worker()
join.c: pass worker_t* (looked up by idx via vget) to WORKER_JOIN hook
so persist hook can serialize worker data without touching workermgr RTTI
Replace all fprintf(stderr,...) calls with INFO/WARN/ERROR/TRACE so
persist.c output appears in the controller's structured log rather than
mixing plain text on stderr with tlog-formatted output.
- Move repl.h from controller/ to include/ (shared header) to break
persist.h ↔ repl.h circular include
- Extract wal_entry_hdr_t + wal_entry_type into include/wal_types.h so
repl.h can include only what it needs without pulling in all of persist.h
- Add replicator_t repl field to persist_t (embedded by value, P2-4)
- Wire repl_init/repl_fini into persist_init/persist_fini lifecycle
- Wire repl_broadcast into persist_wal_async after each WAL write;
last_replicated_crc updated here (not in persist_write_checkpoint_marker)
- Wire repl_wait_ack into persist_wal_sync_append; returns 0 on local
WAL success; sync_rc_out carries -ENODEV/-ETIMEDOUT for degraded mode
- Add 4 TDD tests in test_persist_repl.c: structural repl field check,
async no-standbys succeeds, sync degraded mode rc, checkpoint CRC invariant
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.
Add terminal-state constants and GC retention predicates for the
checkpoint xzarray_thaw() GC pass:
- types.h: add JOB_STAT_TRANSIENT (0x400), XCE_WORKER_HIDDEN (0x00200000),
QUEUE_STAT_HIDDEN (0x0100)
- persist.c: implement job_is_terminal(), job_should_retain(),
worker_should_retain() — placed here so the unit test binary
(which links only persist.c) can access them without RTTI deps
- persist.h: declare the three GC predicates; include xce/types.h
- worker.c: set dead_since = time(NULL) in mark_worker_unavail()
when dead_since is not already set
- cluster.c: add TODO P0-8b marker on zarray_del endpoint cleanup
- meson.build: register test_gc_retention.c; raise test timeout 60→120s
- testsuite/test_gc_retention.c: 19 tests covering job_is_terminal,
job_should_retain (TRANSIENT TTL, terminal TTL, non-terminal always
retained), and worker_should_retain (HIDDEN/UNAVAIL/healthy/zero
dead_since); all pass (63 total, 0 failures)
Implements persist_job_hook_fun, persist_queue_hook_fun, and
persist_worker_hook_fun — AFTER_EVENT hooks that write WAL entries
whenever state mutations occur in the controller.
- detail_to_wal_type() maps all 15 HOOK_DETAIL_* values to WAL types;
unknown details return 0 and are silently skipped
- HOOK_DETAIL_JOB_NEW uses the sync replication path (WAL_JOB_SUBMIT);
all others use persist_wal_async
- P0 payload: 4-byte detail value; full Cap'n Proto serialization in Task 15
- Hook signature matches hook_fn_t exactly:
int fn(int type, int situation, int detail, void* cls, va_list ap)
- Added <stdarg.h> to persist.h for va_list; added hook.h and
xce/cluster_def.h includes to persist.c
- 18 new unit tests in test_persist_hooks.c covering all 15 detail→WAL
mappings, skip-on-unknown, and sequential LSN advancement
- 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)
- In persist_wal_async(), check rotate_segment() return value and propagate -EIO on failure (MEDIUM)
- Change last_crc and last_replicated_crc fields from uint64_t to uint32_t to match CRC32 width (LOW)
Both fixes address code review issues in commit b0a5fbb.
- Expose persist_t struct concretely in persist.h (opaque pattern deferred)
- persist_set_workdir() derives wal/snap paths from workdir+name
- persist_init() creates WAL/snap directories, rejects NFS mounts
- persist_wal_async() writes WAL entries with chained CRC32 via zlib
- Dedicated fsync pthread fires every wal_sync_ms milliseconds
- persist_flush() forces fdatasync for tests and clean shutdown
- WAL_CHECKPOINT type rejected with -EINVAL (never written via async path)
- Stubs: persist_recover, persist_wal_sync_append, persist_checkpoint
- Add fsync_thread_running flag to guard persist_stop when thread not started
- Add zlib_dep to controller and unit test meson targets
- 3 new TDD tests: init_creates_directories, wal_async_writes_entry,
checkpoint_type_rejected — all GREEN
