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.
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.
- 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
- 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
