diff --git a/src/docs/persistence-and-ha-design.md b/src/docs/persistence-and-ha-design.md index 6f471046..9019786c 100644 --- a/src/docs/persistence-and-ha-design.md +++ b/src/docs/persistence-and-ha-design.md @@ -39,7 +39,7 @@ The controller has no database code. It uses `kvmgr` (in-memory hash tables) for | Location | Code | Action | |----------|------|--------| -| `worker.c:367-391` | `restore_worker()` | **Keep** — currently unused but will be the persistence restore entry point (Part 2) | +| `worker.c:367-391` | `restore_worker()` | **Keep** — currently unused but will be the persistence restore entry point (Part 2). Note: `restore_worker()` currently takes different parameters than a deserialized worker context will provide. The signature will need to be updated to accept a `worker_t*` from Cap'n Proto deserialization, or a shim function must be added. | ### 1.3 Empty Function in `cluster.c` @@ -101,8 +101,18 @@ typedef struct { uint32_t type; /* wal_entry_type enum */ uint32_t payload_len; /* Bytes following this header */ uint32_t crc; /* CRC32 of (header + payload) */ - uint32_t prev_crc; /* Previous record's CRC (chained) */ + uint32_t prev_crc; /* Previous record's CRC (chained); WAL_CRC_NONE (0xFFFFFFFF) for first record in segment */ } wal_entry_hdr_t; +static_assert(sizeof(wal_entry_hdr_t) == 40, "wal_entry_hdr_t must be 40 bytes for 8-byte alignment and on-disk format stability"); + +/* Sentinel value for prev_crc: no predecessor (first record in segment). + * 0xFFFFFFFF is used instead of 0 because CRC32 can legitimately produce 0, + * which would be indistinguishable from the sentinel. */ +#define WAL_CRC_NONE 0xFFFFFFFF + +/* Bytes covered by CRC in the header — all fields except crc and prev_crc. + * Used for both on-disk WAL validation and replication CRC verification. */ +#define WAL_HDR_CRC_SIZE 32 /* sizeof(wal_entry_hdr_t) - 2*sizeof(uint32_t) */ ``` ### 2.3 WAL Entry Types @@ -134,18 +144,51 @@ enum wal_entry_type { **Mapping to existing hook detail enums:** -| Hook Detail (`cluster_def.h`) | WAL Entry Type | Replication | -|-------------------------------|----------------|-------------| -| `HOOK_DETAIL_JOB_NEW` | `WAL_JOB_SUBMIT` | **SYNC** | -| `HOOK_DETAIL_JOB_START` | `WAL_JOB_START` | ASYNC | -| `HOOK_DETAIL_JOB_EXIT` | `WAL_JOB_FINISH` | ASYNC | -| `HOOK_DETAIL_JOB_FAIL` | `WAL_JOB_FAIL` | ASYNC | -| `HOOK_DETAIL_JOB_UPDATE` | `WAL_JOB_UPDATE` | ASYNC | -| `HOOK_DETAIL_JOB_CKPT` | `WAL_JOB_CKPT` | ASYNC | -| `HOOK_DETAIL_JOB_RUSAGE_UPDATE` | `WAL_JOB_RUSAGE` | ASYNC | -| `HOOK_DETAIL_SETPREASON` | `WAL_JOB_PREASON` | ASYNC | -| `HOOK_DETAIL_QUEUE_UPDATE` | `WAL_QUEUE_CTRL` | ASYNC | -| `HOOK_DETAIL_WORKER_UPDATE` | `WAL_WORKER_CTRL` | ASYNC | +| Hook Detail (`cluster_def.h`) | WAL Entry Type | Replication | Notes | +|-------------------------------|----------------|-------------|-------| +| `HOOK_DETAIL_JOB_NEW` | `WAL_JOB_SUBMIT` | **SYNC** | | +| `HOOK_DETAIL_JOB_START` | `WAL_JOB_START` | ASYNC | | +| `HOOK_DETAIL_JOB_EXIT` | `WAL_JOB_FINISH` | ASYNC | | +| `HOOK_DETAIL_JOB_FAIL` | `WAL_JOB_FAIL` | ASYNC | | +| `HOOK_DETAIL_JOB_STOP` | `WAL_JOB_STOP` | ASYNC | Not in current enum — must be added (see footnote 1) | +| `HOOK_DETAIL_JOB_RESUME` | `WAL_JOB_RESUME` | ASYNC | Not in current enum — must be added (see footnote 2) | +| `HOOK_DETAIL_JOB_UPDATE` | `WAL_JOB_UPDATE` | ASYNC | | +| `HOOK_DETAIL_JOB_CKPT` | `WAL_JOB_CKPT` | ASYNC | | +| `HOOK_DETAIL_JOB_RUSAGE_UPDATE` | `WAL_JOB_RUSAGE` | ASYNC | | +| `HOOK_DETAIL_SETPREASON` | `WAL_JOB_PREASON` | ASYNC | Both map to same type — see footnote 3 | +| `HOOK_DETAIL_CLRPREASON` | `WAL_JOB_PREASON` | ASYNC | Both map to same type — see footnote 3 | +| `HOOK_DETAIL_QUEUE_UPDATE` | `WAL_QUEUE_CTRL` | ASYNC | | +| `HOOK_DETAIL_WORKER_JOIN` | `WAL_WORKER_CTRL` | ASYNC | | +| `HOOK_DETAIL_WORKER_UPDATE` | `WAL_WORKER_CTRL` | ASYNC | | +| `HOOK_DETAIL_WORKER_DEAD` | `WAL_WORKER_CTRL` | ASYNC | | + +Footnotes: + +1. **`HOOK_DETAIL_JOB_STOP`**: Does not exist in the current `cluster_def.h` enum. It must be added as a prerequisite for P0-6. Job stop currently goes through the same code path as job exit (`HOOK_DETAIL_JOB_EXIT`). If stop and exit should map to different WAL types, a new hook detail must be added and the `on_job_stop()` function must call hooks with `HOOK_DETAIL_JOB_STOP` instead of `HOOK_DETAIL_JOB_EXIT`. + +2. **`HOOK_DETAIL_JOB_RESUME`**: Does not exist in the current `cluster_def.h` enum. It must be added as a prerequisite for P0-6. The existing `on_job_resume()` function in `job.c` must be updated to call hooks with this new detail. + +3. **Pending reason deduplication**: Both `HOOK_DETAIL_SETPREASON` and `HOOK_DETAIL_CLRPREASON` map to `WAL_JOB_PREASON` without payload differentiation. This is acceptable because pending reasons are transient — after a controller restart or standby promotion, the scheduler re-evaluates pending jobs and publishes fresh pending reasons when scheduling resumes. On recovery, jobs in `JOB_STAT_PEND` state are assigned a default pending reason ("Job is waiting to be scheduled") until the scheduler updates them. The critical invariant is that the job's pend/run/exit state is preserved, not the specific pending reason text. + +**WAL payload serialization format**: Each WAL entry's payload is serialized as Cap'n Proto using the `controller.capnp` schemas. The mapping from entry type to schema is: + +| WAL Entry Type | Cap'n Proto Schema | Key fields serialized | +|----------------|--------------------|-----------------------| +| `WAL_JOB_SUBMIT` | `Job` | All fields: `jobid`, `name`, `queue`, `num_proc`, `command`, `environment`, `submit_time`, `stat`, resource limits | +| `WAL_JOB_START` | `Job` (partial) | `jobid`, `stat`, `start_time`, `exec_host` | +| `WAL_JOB_FINISH` | `Job` (partial) | `jobid`, `stat`, `end_time`, `exit_code` | +| `WAL_JOB_FAIL` | `Job` (partial) | `jobid`, `stat`, `end_time`, `fail_reason` | +| `WAL_JOB_STOP` | `Job` (partial) | `jobid`, `stat` | +| `WAL_JOB_RESUME` | `Job` (partial) | `jobid`, `stat` | +| `WAL_JOB_UPDATE` | `Job` (partial) | `jobid`, changed fields only | +| `WAL_JOB_CKPT` | `Job` (partial) | `jobid`, `ckpt_time` | +| `WAL_JOB_RUSAGE` | `Job` (partial) | `jobid`, `rusage` fields | +| `WAL_JOB_PREASON` | `Job` (partial) | `jobid`, `preason` | +| `WAL_QUEUE_CTRL` | `QueueConf` | All fields for the affected queue | +| `WAL_WORKER_CTRL` | `Worker` | All fields for the affected worker | +| `WAL_CHECKPOINT` | (none) | No payload — `payload_len = 0` | + +"Partial" payloads serialize only changed fields using Cap'n Proto's standard optional field encoding — unset fields are omitted. `apply_wal_entry()` applies partial payloads by merging into the existing in-memory object (not replacing it). **Merge semantics (CRITICAL)**: For partial payloads, `apply_wal_entry()` sets **only the fields present in the Cap'n Proto payload**. Fields that are not set (absent/optional in the Cap'n Proto message) are **not touched** in the in-memory object — they retain their previous value. This is naturally supported by Cap'n Proto's optional field encoding: a field that was never set in the message has `has() == false`, and the merge logic skips it. Implementers must NOT use a "replace entire struct" approach, which would zero out absent fields and corrupt in-memory state. The merge rule is: `if (payload.has()) in_memory->Field = payload.get();` for each optional field. ### 2.4 WAL Segment Management @@ -158,23 +201,50 @@ wal_path/ - Segments are pre-allocated to 64 MB (configurable) - When a segment is full, rotate to the next -- `fsync()` at configurable interval (`wal_sync_ms`, default 10ms) — batched, not per-write -- Segments before the latest snapshot are deleted during recovery -- Chained CRC detects corruption: if `prev_crc` doesn't match previous record's CRC, stop replay at last valid entry +- `fsync()` at configurable interval (`wal_sync_ms`, default 10ms) — batched, not per-write. If `fsync()` fails, `persist_wal_sync_append()` and `persist_wal_async()` return an error. For sync replication, the failed write is not sent to standbys and the client receives an error reply. For async replication, the error is logged and the controller continues operating — the WAL entry is in the OS page cache but not on disk, so it may be lost on crash. Monitoring should alert on persistent fsync failures. + > **Implementation note**: `delaymgr` uses `sleep(precision)` with `precision=2` seconds (see `cluster.c:604`), giving 2-second minimum granularity — far too coarse for `wal_sync_ms=10`. The WAL fsync timer must use a **dedicated pthread** with `nanosleep()` or a `timerfd_create(CLOCK_MONOTONIC)` loop, not `delaymgr`. The periodic checkpoint timer (P0-3) can still use `delaymgr` since `snap_interval_s` is measured in seconds. +- **WAL trim guard**: WAL segments are only deleted up to `min(acked_lsn)` across all connected standbys (`standby_conn_t.acked_lsn` — updated on each `OP_replAck` received). This prevents trimming segments that a lagging standby still needs for catch-up. When a standby reconnects after a gap, it reports its last applied LSN, and the primary streams missing entries from that point. If the standby's LSN is older than the primary's oldest surviving WAL segment, a full snapshot transfer is required (see Part 6 standby catch-up). Each standby heartbeat includes the standby's `applied_lsn` so the primary always knows the safe trim point. **Empty-standby fallback**: When no standbys are connected (`replicator.nstandbys == 0`), use the primary's own `checkpoint_lsn` as the trim point — WAL segments before the latest confirmed checkpoint can be safely deleted. **Standby disconnect trim delay**: When a standby disconnects, the primary should delay WAL trimming by one checkpoint interval before falling back to `checkpoint_lsn`-based trimming. This gives the standby a window to reconnect and resume catch-up without requiring a full snapshot transfer. The implementation uses per-slot tracking in `replicator_t.last_disconnected_lsn[]` and `replicator_t.last_disconnect_time[]` (see Appendix A.3). When a standby disconnects, its `acked_lsn` and current time are recorded in its slot. The trim point is computed as `min(min_over_disconnected_slots(last_disconnected_lsn[i]), checkpoint_lsn)` for one checkpoint interval after each disconnect, then falls back to `checkpoint_lsn` per slot. +- **WAL growth bound during standby disconnect**: The worst-case WAL accumulation when no standbys are connected is: `wal_growth_MB ≈ snap_wal_size_mb + (submit_rate_per_sec * avg_WAL_submit_KB * snap_interval_s * 2 / 1024)`. The `* 2` accounts for the one-interval trim delay before `checkpoint_lsn`-based trimming takes effect. At default settings (`snap_interval_s = 60`, `snap_wal_size_mb = 64`) with 10 submits/sec and ~1 KB average `WAL_JOB_SUBMIT` payload, worst-case growth is ≈65 MB — well within a single segment. At pathological rates (1000 submits/sec, 4 KB average payload), worst-case growth is ≈535 MB, requiring ~9 segments. **Mitigation**: If WAL disk usage exceeds `wal_max_size_mb` (default: 2 GB), log an `ERROR` and trigger an immediate checkpoint to advance `checkpoint_lsn` and enable trimming. The one-interval trim delay is skipped when `replicator.nstandbys == 0` and WAL usage exceeds `wal_max_size_mb / 2` — in this scenario, preserving disk space takes priority over the reconnection catch-up window. +- Segments before the latest snapshot are deleted during recovery, subject to the trim guard above +- Chained CRC detects corruption: if `prev_crc` doesn't match previous record's CRC, stop replay at last valid entry. The first record in a WAL segment uses `prev_crc = WAL_CRC_NONE` (defined as `0xFFFFFFFF`, not 0 — CRC32 can legitimately produce 0 for certain inputs, which would be indistinguishable from the "no predecessor" sentinel if 0 were used). Recovery validates by checking that each record's `prev_crc` matches the previous record's `crc` field, starting from `WAL_CRC_NONE` for the first record. +- **Truncated segment recovery**: If a crash occurs mid-write, the last WAL segment may contain a partial record. The recovery algorithm handles two cases: + 1. **Valid header CRC, truncated payload**: Read the full 40-byte header, validate `hdr_crc` against `WAL_HDR_CRC_SIZE` bytes. If the header CRC is valid but `payload_len` exceeds the remaining bytes in the segment, truncate at the previous valid record. Do NOT use the declared `payload_len` to advance the read position — a partially-written `payload_len` could be corrupted, causing the scanner to jump past valid records. + 2. **Invalid header CRC (corrupted header)**: Enter 8-byte linear scan mode — skip forward 8 bytes at a time, checking for a valid `hdr_crc` and matching `prev_crc` at each alignment boundary. This ensures that a corrupted header cannot cause the scanner to skip past valid data. + In both cases, recovery stops when it reaches `seg_offset` (the last `fsync` point) without finding a valid record. Records beyond `seg_offset` may be partially written and are not trusted. + +**Data at rest**: WAL segments and snapshot files contain job submission payloads (which may include environment variables, command lines, and user-specified data). If the storage directory is on a shared or compromised filesystem, this data is accessible. XCE relies on OS-level filesystem permissions (`wal_path`/`snap_path` directories should be owner-read-only, mode 0700) and physical host security rather than encryption at rest. If encryption at rest is required, it should be provided by the underlying filesystem (e.g., LUKS, encrypted EBS volumes) rather than by the controller — adding application-level encryption would double the encryption cost with filesystem-level encryption already in place for most production deployments. + +**Data retention and scrubbing**: Sensitive data (environment variables, job command lines) persists in WAL files until those segments are trimmed (after checkpoint), and in snapshot files until the next checkpoint replaces them. The retention window is bounded by `finished_retain_s` (default 1h) for job payloads: once a job is GC'd at checkpoint, its data no longer appears in new snapshots, and its WAL segment is trimmed after the checkpoint that removed it. Old WAL segments and snapshot files are deleted with `unlink()` — no explicit overwrite/scrubbing of file data is performed. If the storage device requires explicit data destruction (e.g., SSDs with wear leveling where unlink does not guarantee physical erasure), this must be handled at the storage layer (encrypted volumes with key rotation, or secure erase tooling). XCE does not implement application-level data scrubbing of WAL or snapshot files. + +**WAL payload logging policy**: The persistence and replication modules must **never log WAL payload content**. Only entry metadata (type, LSN, term, payload_len, CRC) may appear in log messages. This is a security hardening invariant: `WAL_JOB_SUBMIT` payloads contain job environment variables which frequently hold API keys, database passwords, and other credentials. All logging call sites in `persist.c` and `repl.c` that reference WAL entries must log only `hdr->type`, `hdr->lsn`, `hdr->term`, `hdr->payload_len`, and CRC values — never the `payload` pointer or its contents. Code review must verify this as a gating check before merge. The CRC mismatch error path already follows this pattern: `ERROR("replication CRC mismatch: expected %u, got %u, lsn %lu", hdr.crc, computed_crc, hdr.lsn)`. + +**Filesystem type requirement**: `wal_path` and `snap_path` must be on a local filesystem (ext4, xfs, etc.), not on NFS or other network filesystems. The `fsync()` durability guarantee depends on the OS writing to stable storage — on NFS, `fsync()` may only flush to the server's cache, not to persistent storage, undermining the WAL's crash-recovery guarantee. Run a startup check using `statfs()` and reject `f_type` values matching `NFS_SUPER_MAGIC` (0x6969) or `SMB_SUPER_MAGIC` (0x517B) — log `ERROR` and abort if detected. ### 2.5 Recovery Sequence On startup, `cluster_init()` calls `persist_recover()`: 1. **Find latest snapshot**: Scan `snap_path/` for the snapshot with the highest LSN -2. **Load snapshot**: Deserialize Cap'n Proto data, rebuild kvmgr instances +2. **Load snapshot**: Read the snapshot file into memory + 2a. **Verify snapshot checksums**: (i) Recompute CRC32 over the header fields (lsn, timestamp, term, version, nJobs, nQueues, nWorkers) and compare against `SnapshotHeader.headerChecksum`. If mismatch, the header has been tampered or corrupted — log an error and try the next-oldest snapshot. (ii) Recompute CRC32 over the Cap'n Proto payload and compare against `SnapshotHeader.checksum`. If mismatch, log an error and try the next-oldest snapshot. If **all** snapshots fail verification and the WAL contains entries that predate any valid snapshot (i.e., WAL segments have already been trimmed), the controller **must abort with a fatal error** — silently booting with empty state would silently discard live job/worker state and is data loss. The fatal error message must include: the number of corrupt snapshots found, the LSN range of surviving WAL segments, and the instruction `"Contact support or restore from backup before restarting"`. Do not fall back to empty state in this scenario. The empty-state fallback is only valid on first boot (see below). **Snapshot version check**: After verifying both checksums, check `SnapshotHeader.version`. If the version is higher than the current controller's `WAL_FORMAT_VERSION` (defined in `persist.h` as `1`), log a fatal error and abort — a snapshot written by a newer controller cannot be safely loaded by an older one. This catches version mismatches earlier than encountering unknown WAL entry types during replay. + 2b. **Deserialize**: Deserialize Cap'n Proto data, rebuild kvmgr instances. Set `capnp::ReaderOptions.traversalLimitInWords` proportional to the snapshot file size (`file_size / 8` words) to prevent O(n²) traversal amplification from a crafted snapshot — this is the same hardening practice used for WAL payload deserialization (Part 6.4). If the snapshot file is larger than `MAX_SNAPSHOT_BYTES` (default: 256 MB — 4× the expected 64 MB for 10K jobs × 1 KB + overhead), log an `ERROR` and reject the snapshot. - Jobs → `makeJob(c->jobmgr, ...)` (reuse existing `create_job()`) - Workers → `restore_worker(c, host)` (already exists in `worker.c:367`) - Queues → `create_queue(c, qconf)` (already exists) - Resources → `makeResource(c->resmgr, ...)` (already exists) - Configs → `add_config(c, key, val)` (already exists) -3. **Replay WAL**: Starting from the snapshot's LSN, read and apply WAL entries forward +3. **Replay WAL**: Starting from the snapshot's LSN, read and apply WAL entries forward via `apply_wal_entry()`. Hooks are **not yet registered** during recovery — persistence hooks are registered only after `persist_recover()` completes in `cluster_init()`, so no bypass flag is needed. This avoids re-persisting (WAL write during recovery) and prevents replication sends during replay. For `WAL_JOB_SUBMIT` entries, replay calls `restore_job(c, jobid, payload)` — a dedicated function that inserts the job at its original jobid without allocating a new one. Calling `job_submit()` here would assign a new jobid, breaking replay by creating duplicate or wrong-ID jobs. 4. **Trim WAL**: Delete WAL segments fully covered by the loaded snapshot +5. **Retention pass**: After replay, run the retention/GC check on all entities (equivalent to a thaw pass) to clean up any entities whose TTL has expired during the controller's downtime. This handles the case where a controller was down for longer than `finished_retain_s` or `worker_retain_s` — expired entities are cleaned immediately on recovery. +6. **Restore election term**: Set `ha.term = max(snapshot_header.term, max_term_from_replayed_wal)`. This is a security-critical invariant: a controller must never start with a term lower than any term it has previously used, as this would undermine split-brain fencing. On first boot (empty state), `ha.term` is initialized to 0. + +> **Forward compatibility warning — unknown WAL entry types during recovery**: `apply_wal_entry()` logs a `WARN` and returns 0 (skips) unrecognized `wal_entry_type` values. This allows a controller running an older version to recover from WAL segments written by a newer version — the old controller boots, skips entries it cannot interpret, and continues operating. However, the state changes intended by those skipped entries are **silently lost**. This is a downgrade risk: if a newer controller writes entry types the old controller does not understand, and the old controller is started on that WAL, the cluster state will be incomplete (missing features, incorrect job/worker state for data written by the newer controller). Deployments should never roll back to an older controller version after a newer version has written persistent state. If rollback is required, the only safe procedure is: drain all jobs, stop all controllers, delete WAL and snapshot directories on all candidates, then start the old controller in empty-state-first-boot mode. +> +> **WAL entry type version guard**: To convert silent data loss into a loud failure, `persist_recover()` tracks the maximum `wal_entry_type` value encountered during WAL replay. If any entry has `type > WAL_ENTRY_TYPE_MAX` (defined in `persist.h` as the highest known entry type in the current version — initially `WAL_CHECKPOINT = 100`), `persist_recover()` logs a `FATAL` error and aborts: `"WAL contains entry type %u beyond current version's maximum %u — controller version is too old to safely recover; upgrade to a newer version or drain and reset WAL"`. This mirrors the snapshot version check pattern (step 2a) and prevents an older controller from silently booting with incomplete state. Operators who genuinely need to force-boot against a newer WAL (e.g., emergency recovery) can use the `--force-wal-version` startup flag, which downgrades the abort to a `WARN` per unknown entry type. + +**Empty state (first boot after upgrade)**: If both `snap_path/` and `wal_path/` are empty (no snapshots, no WAL segments), `persist_recover()` returns 0 (success) with no state loaded — equivalent to today's in-memory-only boot. This ensures backward compatibility: existing deployments upgrading from in-memory-only operation need no migration steps. The controller simply starts with empty state and begins accumulating WAL entries. + +**Valid snapshot, empty WAL (clean shutdown)**: If `snap_path/` contains a valid snapshot file but `wal_path/` has no segments, `persist_recover()` loads the snapshot with no WAL replay. This is the expected state after a clean shutdown where the last checkpoint has already trimmed all WAL segments. No error or warning is generated. ### 2.6 Configuration @@ -219,7 +289,17 @@ The original design considered `fork()` + COW for checkpoint snapshots. This is - kvmgr's internal `my_rwlock_t` is not fork-safe. The child must not take any lock, but serializing kvmgr state likely requires traversal that touches locked structures. - Linux memory overcommit: if the parent modifies many pages during the child's serialization, the OS may OOM-kill the child (or parent). -### 3.2 Design: Application-Level COW via Dual zarray +### 3.2 Why Not Online Compaction or Snapshot-and-Continue? + +Two alternatives to the dual-zarray COW approach were considered: + +**Online compaction** (e.g., log-structured merge, shadow paging with page-level COW): requires per-page dirty tracking or a WAL-driven snapshot mechanism. XCE's kvmgr uses pointer-based structs (`job_t*`, `worker_t*`) — page-level COW would require every hot pointer to be remapped across page boundaries, adding complexity the codebase does not support today. + +**Snapshot-and-continue** (serializing while holding a read lock the whole time): viable but blocks mutations for the duration of serialization. For 10K jobs at ~1KB each, serialization takes ~10–100ms. Blocking all job submits for 100ms at checkpoint time is worse than the brief freeze/thaw window (≤100μs for 10K entries — see Part 3.8). + +**Decision: dual-zarray application-level COW.** The "stop-the-world" cost is only at freeze (O(1) pointer swap, ~ns) and thaw (O(shadow_caps) pointer copies, ~100μs). Serialization of the shadow arrays happens with no locks held, so mutations continue unblocked. This gives checkpoint-without-blocking at the cost of ~2× peak memory during COW. At XCE's scale (10K jobs × ~1KB = ~10 MB), this is negligible. + +### 3.3 Design: Application-Level COW via Dual zarray XCE's kvmgr uses a two-level structure: `kvtab` (hash table for key→index lookup) + `data` (xzarray — dense pointer array for iteration and index-based access). This structure makes application-level COW practical. @@ -237,24 +317,68 @@ COW operation: 4. After serialization: merge shadow back into data, apply GC ``` -### 3.3 xzarray COW Extensions +### 3.4 xzarray COW Extensions -Add two fields to `xzarray_t`: +**`xzarray_t` is file-private in `zarray.c`**: the struct definition is internal to the RTTI type system (line 353–369), and external code only accesses it through the `typeinfo_t xzarray_type` interface. Rather than expose the struct, COW state is stored in `xzarray_private_t` (the RTTI private section, allocated by `makeInstance()` alongside each xzarray instance): ```c +/* In zarray.c — extend xzarray_private_t */ typedef struct { - int count; - size_t caps; - size_t delta; - int pos; - xzarray_item_t* data; /* Active array (COW writes go here) */ - xzarray_item_t* shadow; /* Frozen snapshot (NULL when not in COW) */ - size_t shadow_caps; /* Shadow array capacity */ - destroy_fn_t killer; -} xzarray_t; + mylock_t lock; + xzarray_item_t* shadow; /* Frozen snapshot (NULL when not in COW) */ + size_t shadow_caps; /* Shadow array capacity */ +} xzarray_private_t; ``` -When `shadow != NULL`, the array is in COW mode. The existing `xzarray_item_t` is unchanged: +New freeze/thaw operations are exposed by extending the RTTI `intf_varray_t` interface. The rtti library is maintained in-house (`/nix/store/...-rtti-0.0.1/`), so adding new function pointers is a controlled change — version bumped to 0.1.0 (see Appendix A.9 for the full rtti enhancement proposal). + +Add two new function pointer types and slots to the `intf_varray_t` interface in `rtti/typeinfo.h`: + +```c +/* retain_fn_t is defined in rtti/typeinfo.h (see Appendix A.9) */ +typedef int (*vfreeze_fn_t)(meta_obj_t* obj); +typedef int (*vthaw_fn_t)(meta_obj_t* obj, retain_fn_t should_retain, void* arg); + +typedef struct { + vstore_fn_t vstore; + vget_fn_t vget; + vget_vec_fn_t vget_vec; + vput_fn_t vput; + vput_vec_fn_t vput_vec; + vclear_fn_t vclear; + vfreeze_fn_t vfreeze; /* NEW — freeze array for COW snapshot */ + vthaw_fn_t vthaw; /* NEW — thaw + merge + GC after snapshot */ +} intf_varray_t; +``` + +These map to `xzarray_freeze()` and `xzarray_thaw()` implementations in `zarray.c`, which access shadow state through `RTTI_OBJ_TO_PRIV()`. + +```c +static int xzarray_freeze(meta_obj_t* obj) { + xzarray_private_t* priv = RTTI_OBJ_TO_PRIV(obj); + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + + priv->shadow = a->data; + priv->shadow_caps = a->caps; + + a->data = calloc(a->caps, sizeof(xzarray_item_t)); + if (a->data == NULL) { + /* Rollback: restore original data from shadow. + * a->count still holds the pre-freeze value (not modified above), + * so no restoration needed for count. */ + a->data = priv->shadow; /* restore active pointer */ + a->pos = 0; + priv->shadow = NULL; + priv->shadow_caps = 0; + return -ENOMEM; + } + a->count = 0; + a->pos = priv->shadow_caps; /* start past shadow to prevent vstore index collisions */ + return 0; +} +``` + +When `shadow != NULL` (i.e., `priv->shadow != NULL`), the array is in COW mode. The existing `xzarray_item_t` is unchanged: ```c typedef struct { @@ -263,24 +387,35 @@ typedef struct { } xzarray_item_t; ``` -### 3.4 Modified zarray Operations During COW +### 3.5 Modified zarray Operations During COW **Critical constraint**: During COW mode, `vclear` is never called (see Part 4). This means `inuse=0` in the active array always means "slot not yet touched" — there is no tombstone ambiguity. +**Runtime enforcement**: `xzarray_clear()` must assert `priv->shadow == NULL` at entry. If `vclear` is invoked while COW is active, the array's invariant is violated (a hole in the active array would cause `vget` to fall through to stale shadow data). The assertion converts a silent data corruption bug into a loud crash during development: + +```c +static void xzarray_clear(meta_obj_t* obj) { + xzarray_private_t* priv = RTTI_OBJ_TO_PRIV(obj); + assert(priv->shadow == NULL && "xzarray_clear called during COW — deferred cleanup required"); + /* ... existing clear logic ... */ +} +``` + #### vget — Two-Level Lookup ```c static int xzarray_get(meta_obj_t* obj, int idx, void** data) { - xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + xzarray_private_t* priv = RTTI_OBJ_TO_PRIV(obj); /* COW mode: check active first, fall through to shadow */ - if (a->shadow != NULL) { + if (priv->shadow != NULL) { if (idx < a->caps && a->data[idx].inuse) { *data = a->data[idx].data; return 0; } - if (idx < a->shadow_caps && a->shadow[idx].inuse) { - *data = a->shadow[idx].data; + if (idx < priv->shadow_caps && priv->shadow[idx].inuse) { + *data = priv->shadow[idx].data; return 0; } return -ENOENT; @@ -319,28 +454,92 @@ static int xzarray_put_vec(meta_obj_t* obj, int count, int* idxs, void** data) { Since indices are stable (hash table maps key→same index always), a `vput(42, new_data)` writes to `active[42]`. The hash table still says key→42. Reads check `active[42]` first and find the updated value. The shadow's `shadow[42]` is silently masked. -#### vstore — Always Goes to Active, Unchanged +**Lock protocol design**: The rtti `intf_lock_t` provides a thin abstraction — `lock()` / `unlock()` function pointers that each type implements. Whether vget acquires a read lock internally is an implementation decision, not an rtti contract. The current design uses the kvmgr `my_rwlock_t` as the outer coordination lock: callers acquire the kvmgr READ lock before vget and release it after they are done with the returned pointer. The xzarray `mylock_t` mutex is used for individual array mutations (vstore, vput, vclear). This two-level locking (kvmgr rwlock for container-level coordination, xzarray mutex for array-level mutations) is the established pattern — see `handle_jobsubmit()` in `job.c:455-525` for the reference implementation. vget does not acquire the xzarray mutex because the caller already holds the kvmgr READ lock, which prevents concurrent freeze/thaw from modifying the array structure. -New entries (e.g., new job submit) get new indices that don't exist in shadow. `vstore` logic is unchanged. +**Memory ordering during COW serialization (CRITICAL — must be implemented)**: During the no-locks-held serialization phase (Part 3.8), `vput()` writes to `active[idx]` while the serialization thread reads `shadow[]` (which is immutable). These two accesses are to different memory regions, so there is no data race on the shadow array. However, concurrent `vput()` and `vget()` on the active array require careful memory ordering. `vget()` reads `active[idx].inuse` then `active[idx].data` as separate fields — a `vput()` that writes `.data` before setting `.inuse = 1` could cause `vget()` to see `inuse = 1` with a stale `.data` pointer if the compiler reorders the stores or the CPU reorders their visibility. + +**Why x86-64 alone is NOT sufficient**: The x86 memory model prevents the CPU from reordering stores to the same location, but `.data` and `.inuse` are **different fields**. The C compiler is free to reorder stores to different fields of a struct regardless of target architecture — the `volatile` qualifier on the struct would prevent this but imposes performance costs on all accesses. Without `volatile`, the compiler may emit the store to `.inuse` before the store to `.data`, and a concurrent `vget()` would see `inuse = 1` with an uninitialized or stale `.data` pointer — a use-after-free. + +**Required implementation**: Use C11 atomics for the `.inuse` field to enforce release-acquire ordering between `vput()` (writer) and `vget()` (reader): +- `vput()` writes `.data` first, then stores `.inuse = 1` using `atomic_store_explicit((_Atomic uint32_t*)&a->data[idx].inuse, 1, memory_order_release)`. +- `vget()` reads `.inuse` using `atomic_load_explicit((_Atomic uint32_t*)&a->data[idx].inuse, memory_order_acquire)`, and only accesses `.data` if `.inuse == 1`. +- The release-acquire pair ensures `.data` is visible to `vget()` when it sees `.inuse == 1`. + +This is required on ALL architectures, not just ARM — compiler reordering is sufficient to cause the race even on x86-64. The kvmgr READ lock in `vget()` callers provides happens-before between `vput()` and `vget()` on the **same vcpu thread** (serialized by lock/unlock), but does not prevent the compiler from reordering stores within a single `vput()` call. The atomic store on `.inuse` is the only reliable barrier against both compiler and CPU reordering. + +#### vstore — Must Skip Shadow Indices + +**Design invariant**: After `xzarray_freeze()`, the new active array is empty (`count=0`, `pos=shadow_caps`). The `vstore` scan during COW mode must use a **linear scan bound to `[shadow_caps, a->caps)`** — it must NOT wrap around to index 0 when reaching `a->caps`. If all slots in `[shadow_caps, a->caps)` are occupied, `vstore` grows the array via `xzarray_ensure_items()` and assigns from the new capacity. This prevents index reuse below `shadow_caps`, which would cause silent data corruption when `vget` falls through to the stale shadow entry. + +**Why circular scan is unsafe during COW**: The existing `xzarray_store()` at `zarray.c:456-490` wraps from `idx >= a->caps` to `idx = 0`. With `pos = shadow_caps`, after enough `vstore` calls fill `[shadow_caps, a->caps)`, the next store wraps to index 0 — but index `i < shadow_caps` may have an `inuse=0` slot which `vstore` reclaims. The kvtab now maps `new_key → i`, while `shadow[i]` still holds the old entity's pointer. A `vget(i)` finds `active[i].inuse == 0` (recently assigned but not yet visible to a concurrent reader) or `active[i]` holds the new entity while `shadow[i]` holds stale data — either case produces wrong results. + +**Fix**: Add a COW-mode flag check in `xzarray_store()`: when `priv->shadow != NULL`, scan only the range `[pos, a->caps)` without wrapping. If no free slot is found, grow the array: + +After thaw, `pos` is reset to the normal low-water scan (or left at the current high-water mark — the existing vstore scan-forward logic handles this correctly). + +#### vstore — Linear Scan During COW (No Wrap) + +```c +static int xzarray_store(meta_obj_t* obj, void* data) { + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + xzarray_private_t* priv = RTTI_OBJ_TO_PRIV(obj); + int idx; + + if (priv->shadow != NULL) { + /* COW mode: linear scan from pos without wrapping. + * Must not reuse indices < shadow_caps — those belong to shadow. + * inuse must be read with atomic_load_explicit(acquire) to pair + * with vput's atomic_store_explicit(release) — both vstore and + * vput hold the kvmgr READ lock concurrently, so the atomic + * ordering (not the lock) provides inter-reader visibility. */ + for (idx = a->pos; idx < (int)a->caps; idx++) { + if (atomic_load_explicit((_Atomic uint32_t*)&a->data[idx].inuse, + memory_order_acquire) == 0) { + a->data[idx].data = data; + atomic_store_explicit((_Atomic uint32_t*)&a->data[idx].inuse, + 1, memory_order_release); + a->count++; + a->pos = idx + 1; + return idx; + } + } + /* No free slot in [shadow_caps, a->caps) — grow array. + * Overflow check: must not attempt growth if caps + delta overflows. */ + int old_caps = a->caps; + if (a->caps > SIZE_MAX - a->delta) return -ENOMEM; + xzarray_ensure_items(a, a->caps + a->delta); + a->data[old_caps].data = data; + atomic_store_explicit((_Atomic uint32_t*)&a->data[old_caps].inuse, + 1, memory_order_release); + a->count++; + a->pos = old_caps + 1; + return old_caps; + } + + /* Normal path: unchanged circular scan */ + /* ... existing xzarray_store() code ... */ +} +``` #### Iteration — Merge Walk of Both Arrays ```c static int xzarray_iter_next(meta_obj_t* obj, void** data) { - xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + xzarray_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - if (a->shadow != NULL) { + if (priv->shadow != NULL) { /* COW mode: walk both arrays, active takes precedence */ - size_t max_caps = (a->caps > a->shadow_caps) ? a->caps : a->shadow_caps; - for (int i = (int)(long)iter->data; i < max_caps; i++) { + size_t max_caps = (a->caps > priv->shadow_caps) ? a->caps : priv->shadow_caps; + for (size_t i = (size_t)iter->data; i < max_caps; i++) { if (i < a->caps && a->data[i].inuse) { - *data = a->data[i].data; /* active wins */ - iter->data = (void*)(long)(i + 1); + *data = a->data[i].data; /* active wins — shadow[i] intentionally skipped */ + iter->data = (void*)(i + 1); return RTTI_ITER_OK; } - if (i < a->shadow_caps && a->shadow[i].inuse) { - *data = a->shadow[i].data; /* shadow (not in active) */ - iter->data = (void*)(long)(i + 1); + if (i < priv->shadow_caps && priv->shadow[i].inuse) { + *data = priv->shadow[i].data; /* shadow only (no active entry at i) */ + iter->data = (void*)(i + 1); return RTTI_ITER_OK; } } @@ -352,55 +551,111 @@ static int xzarray_iter_next(meta_obj_t* obj, void** data) { } ``` -### 3.5 Freeze — Start COW Phase +**Serialization must iterate shadow only**: During the no-locks-held serialization phase (Part 3.8), the iterator must only walk the shadow array (`priv->shadow[0..shadow_caps-1]`), since shadow is immutable and active[] may be concurrently modified by vput. The `xzarray_item_t` struct has a bitfield (`inuse : 1`) and a pointer (`data`) — these are not atomically written, so reading active[] without locks is a data race. New entries written to active[] during serialization are covered by the WAL and will be included in the next checkpoint's shadow. The serialization code should use a shadow-only iterator (bound: `0..shadow_caps-1`), not the merge-walk iterator above. The merge-walk iterator is for normal-mode queries where callers hold the kvmgr READ lock. **The kvmgr READ lock acquisition provides the necessary memory ordering to see vput's release-ordered stores to `active[].inuse`. Iterating without the READ lock is a data race.** + +**COW invariant — new keys may hash below shadow_caps**: New entities added during COW mode (via vstore) are allocated at indices `>= shadow_caps` (the linear scan starts at `pos = shadow_caps`). However, a `vput` for a new hash table key can map to any index, including indices below `shadow_caps`. This is safe because: (1) vput writes to `active[i]` at the same index, (2) vget checks active first and finds the new entry, (3) thaw merges shadow and detects `active[i].inuse == 1`, taking the active-wins branch and freeing the stale shadow entry. The COW vstore linear scan only prevents index *reuse* from the free list below `shadow_caps` — it does not prevent new hash-to-index mappings in that range via vput. + +### 3.6 Freeze — Start COW Phase ```c -void xzarray_freeze(xzarray_t* a) { +static int xzarray_freeze(meta_obj_t* obj) { + xzarray_private_t* priv = RTTI_OBJ_TO_PRIV(obj); + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + /* Current data becomes shadow (frozen, immutable) */ - a->shadow = a->data; - a->shadow_caps = a->caps; + priv->shadow = a->data; + priv->shadow_caps = a->caps; /* Allocate new active with same capacity, all inuse=0 */ a->data = calloc(a->caps, sizeof(xzarray_item_t)); - a->count = 0; - a->pos = 0; - /* caps stays the same */ + if (a->data == NULL) { + /* Rollback: restore original data pointer from shadow. + * a->count still holds the pre-freeze value (not modified above), + * so no restoration needed for count. */ + a->data = priv->shadow; + a->pos = 0; + priv->shadow = NULL; + priv->shadow_caps = 0; + return -ENOMEM; + } + a->count = 0; /* calloc zeroes the allocation; count reflects zero live entries */ + a->pos = priv->shadow_caps; /* start past shadow to prevent vstore index collisions */ + return 0; } ``` This is a **pointer swap** — O(1), no data copied. The write lock is held only for the duration of the swap (~nanoseconds). -### 3.6 Thaw — End COW Phase with Merge and GC +### 3.7 Thaw — End COW Phase with Merge and GC ```c -void xzarray_thaw(xzarray_t* a, retain_fn_t should_retain, void* arg) { - for (int i = 0; i < a->shadow_caps; i++) { - if (!a->shadow[i].inuse) continue; /* empty slot, skip */ +/* xzarray_thaw — merge shadow back into active, apply optional GC. + * should_retain: callback to decide entity survival. NULL = keep all, + * no GC (used when aborting a checkpoint — merge shadow back unchanged). + * + * TWO-PASS DESIGN (required for correct GC): + * Pass 1: Merge all shadow entries into active[] (pointer copies only, + * no retention decisions, no frees). This ensures all entities + * are visible in active[] before any GC check runs. + * Pass 2: Run should_retain() on merged active[] entries, freeing those + * that fail retention. This is correct because job_has_dependents() + * can now see ALL entities (including ex-shadow ones) in active[]. + * + * Single-pass designs where retention checks happen during merge are + * INCORRECT: when should_retain(shadow[i]) calls job_has_dependents(), + * dependents at shadow[j] (j > i) have not yet been merged and are + * invisible in active[]. This causes premature GC of jobs with dependents. */ +static int xzarray_thaw(meta_obj_t* obj, retain_fn_t should_retain, void* arg) { + xzarray_private_t* priv = RTTI_OBJ_TO_PRIV(obj); + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + + /* Pass 1: Merge all shadow entries into active. No retention checks. */ + for (size_t i = 0; i < priv->shadow_caps; i++) { + if (!priv->shadow[i].inuse) continue; /* empty slot, skip */ if (i < a->caps && a->data[i].inuse) { /* Active was written during COW — free shadow's stale object */ - if (a->killer) a->killer(a->shadow[i].data); - } else if (should_retain != NULL - && !should_retain(arg, a->shadow[i].data)) { - /* Entity failed retention check — GC at checkpoint */ - if (a->killer) a->killer(a->shadow[i].data); - /* Caller must also remove kvtab entry for this index */ + if (a->killer) a->killer(priv->shadow[i].data); } else { - /* Entity still alive — copy pointer to active */ + /* Copy shadow pointer to active (no retention check yet) */ xzarray_ensure_items(a, i + 1); - a->data[i] = a->shadow[i]; /* struct copy (pointer, inuse) */ - /* Note: a->data[i].inuse is already 1 from shadow copy */ + a->data[i] = priv->shadow[i]; /* struct copy (pointer, inuse) */ + a->count++; } } - /* Free shadow */ - free(a->shadow); - a->shadow = NULL; - a->shadow_caps = 0; + /* Free shadow — all live pointers are now in active[] */ + free(priv->shadow); + priv->shadow = NULL; + priv->shadow_caps = 0; + + /* Pass 2: Run retention/GC checks on the fully-merged active[]. + * All entities are now visible, so job_has_dependents() finds dependents + * regardless of their original position in the shadow array. */ + if (should_retain != NULL) { + for (size_t i = 0; i < (size_t)a->caps; i++) { + if (!a->data[i].inuse) continue; + if (!should_retain(arg, a->data[i].data)) { + if (a->killer) a->killer(a->data[i].data); + a->data[i].inuse = 0; + a->data[i].data = NULL; + a->count--; + /* Caller must also remove kvtab entry for this index */ + } + } + } + + /* Reset scan position for normal vstore operation. + * After merge, some indices < shadow_caps have inuse=0 (GC'd entities) + * and new entries at indices >= shadow_caps have inuse=1 (added during COW). + * Starting from 0 ensures vstore reuses freed slots instead of growing + * the array monotonically across checkpoints. */ + a->pos = 0; + return 0; } ``` -### 3.7 Checkpoint Flow +### 3.8 Checkpoint Flow ``` persist_checkpoint() fires on timer thread @@ -416,6 +671,28 @@ persist_checkpoint() fires on timer thread Serialize all shadows → Cap'n Proto → disk (no locks held, controller serving requests normally, reads go through COW two-level lookup transparently) + + **CRITICAL: kvtab access during serialization**: The serialization phase + reads shadow data without locks (shadow is immutable after freeze), but + MUST NOT access the kvmgr hash table (kvtab) during this phase. The kvtab + is shared mutable state — concurrent vstore calls can insert new keys while + serialization is iterating the hash table, creating a data race on the + hash chain pointers. Instead, serialization enumerates shadow entries by + walking the shadow zarray indices [0..shadow_caps) directly, without + consulting kvtab. Each shadow slot at index i contains an inuse flag and + a data pointer; serialization reads shadow[i].inuse and shadow[i].data + for each slot — these are immutable (no lock needed). The kvtab key + lookup is only needed during the freeze phase (under write lock) to build + a snapshot of key→index mappings, which is stored alongside the shadow + array for serialization use: + + **Serialization index snapshot (built during freeze, used during serialize)**: + During `xzarray_freeze()`, after the pointer swap but before releasing the + write lock, the checkpoint code captures key→index mappings from kvtab for + each live entity. This snapshot is stored in a side array + (`persist_t.key_snapshot`) indexed by shadow slot. After thaw, the snapshot + is freed. This avoids any kvtab access during the no-locks-held + serialization phase. │ ▼ After serialization confirmed on disk: @@ -427,16 +704,41 @@ persist_checkpoint() fires on timer thread - Free shadow kvtab_gc(): ← remove hash entries for GC'd indices rwlock_wunlock() + + **CRITICAL: kvtab_gc must run inside the same write-lock section as thaw.** + `kvtab_gc_keys()` MUST be called before releasing the kvmgr write lock. Freed + zarray indices (where `inuse` was set to 0 by thaw's GC pass) become + available for reuse by the next `vstore()` call. If `kvtab_gc_keys()` runs + after the write lock is released, a concurrent `vstore()` (holding only a + READ lock) can allocate the freed index for a new entity before `kvtab_gc` + removes the stale key→index mapping, causing the old key to resolve to the + new entity's data — a hash→zarray inconsistency. The checkpoint flow above + (freeze → serialize → thaw → kvtab_gc → unlock) is correct because all four + operations run under the same write lock. │ ▼ Delete WAL segments covered by this checkpoint ``` +**Cross-kvmgr freeze ordering and referential consistency**: The checkpoint flow freezes kvmgrs sequentially (jobs → workers → queues → resources → configs), releasing each kvmgr's write lock after its freeze. This means the shadow snapshots for different kvmgrs capture state at slightly different LSN points. A job serialized in the jobmgr snapshot may reference a worker (via `exec_host`) that was added after the workermgr snapshot was taken. This is acceptable because WAL replay after snapshot loading corrects all inconsistencies — all mutations between the earliest and latest freeze points are captured in the WAL entries that follow the snapshot. On recovery, `persist_recover()` loads the snapshot and then replays WAL entries in strict LSN order, which restores full consistency. The checkpoint_lsn recorded per-kvmgr (used for WAL trimming) is the maximum LSN across all kvmgrs at the time of each freeze, ensuring no WAL entry needed for cross-kvmgr consistency is trimmed prematurely. +``` + +**Non-kvmgr zarrays**: `cluster_t` also contains standalone `zarray_t` fields (endpoints, configs, sharedinst) that are not managed by kvmgr. These are handled differently during checkpoint: +- `c->endpoints`: Not serialized in the snapshot. Endpoints are transient network connections that are rebuilt on recovery (workers/schedulers reconnect after restart). The deferred endpoint cleanup mechanism (Part 4.9) handles cleanup during normal operation, not checkpoint. +- `c->configs`: Serialized under a separate write lock (`lock(c->configmgr, RTTI_LOCK_READ)` for reads, `RTTI_LOCK_WRITE` for freeze/thaw). Configs are read-only during normal operation and rare writes (admin config changes). Config freezes are serialized as a separate Cap'n Proto array in the snapshot. +- `c->sharedinst`: Not serialized. Shared instances are reconstructed from config on recovery. + +**Shadow lifetime and shutdown safety**: `xzarray_thaw()` calls `free(a->shadow)` and sets `a->shadow = NULL`. The serialization thread reads shadow without any lock (shadow is "immutable after freeze"). This is safe **only if thaw never runs while the serialization thread is still reading**. The normal checkpoint flow guarantees this: thaw is called only after serialization confirms the snapshot on disk, and both happen on the same checkpoint timer thread. However, the **shutdown path** must also observe this constraint: if `cluster_fini()` or a shutdown signal fires while a checkpoint is in progress, it must wait for the serialization thread to complete before calling `xzarray_thaw()`. Implement with a `persist_t.checkpoint_in_progress` atomic flag **and** a `pthread_cond_t checkpoint_done` condition variable. **The condition variable must use a dedicated `pthread_mutex_t checkpoint_mutex` — NOT `persist.lock` (which is a `mylock_t`).** `pthread_cond_timedwait()` requires a `pthread_mutex_t*`, and `mylock_t` may not be a `pthread_mutex_t` (it could be a spinlock or custom wrapper, which would cause undefined behavior with condition variables). Additionally, `persist.lock` is held by vcpu threads during `persist_wal_sync_append()` (which may block on `repl_wait_ack()`), so `cluster_fini()` attempting to acquire `persist.lock` for the timed wait would deadlock against a vcpu thread holding it. The dedicated `checkpoint_mutex` avoids both issues: it is always a `pthread_mutex_t` and is never held during replication waits. On shutdown: `cluster_fini()` acquires `checkpoint_mutex`, checks `checkpoint_in_progress`. If set, it calls `pthread_cond_timedwait(&persist.checkpoint_done, &checkpoint_mutex, &deadline)` where `deadline` is `now + 5 seconds` (configurable). The checkpoint thread signals `checkpoint_done` after completing serialization and thaw (or after canceling). If the deadline expires, `cluster_fini()` cancels the in-progress checkpoint (sets a cancel flag) and skips thaw — incomplete checkpoint state is cleaned by the next restart's `persist_recover()`. This avoids busy-waiting on the atomic flag and provides bounded shutdown latency. + +**Crash safety during checkpoint**: If the controller crashes mid-checkpoint (during shadow serialization or mid-thaw), the previous snapshot on disk remains valid. On recovery, `persist_recover()` loads the last valid snapshot and replays WAL entries from that snapshot's LSN forward — the incomplete checkpoint is simply ignored. WAL segments are only deleted after the new snapshot is fully written and `fsync`'d to disk. This ensures atomicity: a checkpoint either completes fully (new snapshot + WAL trim) or is treated as if it never happened. + +**Mid-thaw crash and COW in-memory state**: If the controller crashes mid-thaw (some shadow entries merged back to `active[]`, some not yet merged, `priv->shadow != NULL`), the COW in-memory state is lost on restart — it was never persisted. `persist_recover()` starts from the last completed snapshot and replays WAL entries from that point. Mutations that were applied to `active[]` during the COW phase but **not yet written to WAL** (e.g., a `vput` that occurred after the freeze but before the WAL write via the hook) are also lost. This is consistent with the WAL durability model: only mutations that have been written to WAL and fsynced are durable. The COW mechanism is a transient optimization for non-blocking checkpoint; it provides no additional durability guarantee beyond the WAL. Implementers must NOT attempt to recover COW state from WAL — on restart, `xzarray_freeze()` runs anew from the fully recovered state. + **Lock hold times:** - Freeze: ~nanoseconds (pointer swap) -- Thaw + GC: O(shadow_caps) pointer copies + retention checks. For 10K entries, ~50-100 microseconds. +- Thaw + GC: O(shadow_caps) pointer copies + retention checks. For 10K entries, ~50-100 microseconds. Note: if `should_retain` calls `job_has_dependents()` for job entities, the retention check becomes O(n_terminal_jobs × n_total_jobs) in the worst case because `job_has_dependents()` scans all jobs per call. At XCE's current scale (typically <1K finished jobs in a retention window), this is fast. For deployments with >10K jobs, consider building a reverse dependency index to make `job_has_dependents()` O(k) where k is the number of dependents per job, reducing GC to O(shadow_caps + n_terminal_jobs × k). -### 3.8 Lock Hierarchy for COW +### 3.9 Lock Hierarchy for COW XCE uses a two-lock hierarchy in kvmgr: @@ -446,7 +748,7 @@ kvmgr rwlock (outer) — protects kvtab + serializes freeze/thaw ``` - **kvmgr `my_rwlock_t`**: The outer lock. Taken as read-lock for normal lookups (`xkvmgr_find`). Taken as write-lock for additions (`xkvmgr_add`) and for COW freeze/thaw coordination. -- **xzarray `mylock_t`**: The inner lock. Automatically taken inside `xzarray_store`, `xzarray_get`, `xzarray_put_vec`, `xzarray_clear`. Not taken explicitly by COW code. +- **xzarray `mylock_t`**: The inner lock. **Not taken automatically inside `xzarray_put_vec()`** — the current `src/lib/zarray.c` implementation of `xzarray_put_vec()` does not call `mylock_lock()` internally; the caller is responsible. P0-2 must verify the actual mutex ownership contract and document it explicitly. During COW, `vput` calls happen under the kvmgr read lock, which serializes them against freeze/thaw — if this is the intended protocol, the inner mutex may be redundant for COW operations. If `xzarray_put_vec()` is expected to be called without the kvmgr lock, it must acquire the inner mutex itself. **COW lock protocol:** @@ -462,34 +764,45 @@ serialize shadows: thaw + GC: rwlock_wlock(kvmgr) ← serialize: block lookups during merge xzarray_thaw(data, retain) ← merges shadow→active, frees GC'd entities - xzarray mutex taken internally per vput + (kvmgr write lock held; inner mutex not needed) kvtab_gc_keys(kvmgr) ← prune stale hash entries rwlock_wunlock(kvmgr) ``` **Per-object locks are NOT nested inside kvmgr lock.** The kvmgr lock serializes container-level operations (key→index lookup, COW coordination). Per-object locks (`job_t.lock`, `worker_t.lock`) are taken after releasing the kvmgr lock — the lookup returns a referenced pointer, and the caller locks the object separately. +**Why there is no lock race during serialization + thaw**: The concern is that concurrent `vput` calls during the "no locks held" serialization phase might interleave with `xzarray_thaw()`. This is safe because: +1. During serialization (no locks): `vput` writes to `active[]` under the kvmgr READ lock. These writes are to `active[i].data` — they never touch `shadow[]`. +2. During thaw (WRITE lock held): `xzarray_thaw()` merges `shadow[]` → `active[]`. The write lock blocks all concurrent `vput` (read-lock) calls. No interleave is possible. +3. Thaw's merge logic: for each `shadow[i]`, if `active[i].inuse == 1` (set by a concurrent vput during serialization), thaw takes the "active wins" branch and frees the stale shadow pointer. This is the intended behavior — active is newer. + This matches the existing pattern in `handle_jobsubmit()` (`job.c:455-525`): `lock(c->jobmgr, RTTI_LOCK_READ)` for lookup, release, then per-object lock for modification. -### 3.9 Checkpoint Timer +### 3.10 Checkpoint Timer The checkpoint thread should use the existing `delaymgr` timer system (already used for heartbeat checks in `cluster.c`) rather than a raw `pthread_create`. This ensures the checkpoint timer integrates with the controller's timer infrastructure: ```c /* In persist_start() — register periodic checkpoint timer */ -delay_id_t ckpt_timer = delay_add(c->delaymgr, - persist_checkpoint_timer_fn, - (void*)c, - config->snap_interval_s * 1000, /* ms */ - true /* recurring */); +/* delay_task signature: delay_task(delaymgr_t*, int seconds, int repeat, fn, void*) + * repeat=-1 means recurring (matches cluster.c:756 heartbeat pattern) */ +int ckpt_timer = delay_task(&(c->delaymgr), + (int)config->snap_interval_s, + -1, /* recurring */ + (delay_task_entry_fn_t)persist_checkpoint_timer_fn, + (void*)c); ``` +**Checkpoint overlap handling**: If a timer-driven checkpoint fires while a previous checkpoint is still in progress (checked via `persist_t.checkpoint_in_progress` atomic flag), the timer callback logs an `INFO` message ("checkpoint already in progress, skipping") and returns — the next timer interval will trigger a new check. WAL trimming only occurs after a successful checkpoint, so skipped timers result in continued WAL growth until the next successful checkpoint completes. The size-based inline trigger (Part 3.11, via `checkpoint_requested` atomic flag) provides a safety valve for rapid WAL growth between timer intervals. + +**Checkpoint timer for inline-triggered checkpoints**: The checkpoint timer thread also checks `persist_t.checkpoint_requested` (set by the inline trigger in Part 3.11). When this flag is observed, the timer thread runs the checkpoint immediately regardless of the regular interval. Use a secondary `delay_task` with a 1-2 second interval to poll `checkpoint_requested`, or combine the check into the main timer callback (if the delaymgr supports sub-interval wakeups via flag check). + Benefits: - No raw `pthread_create`/`pthread_join` management for the timer thread - `delaymgr` already handles timer cancellation in `cluster_fini()` - Consistent with how heartbeat timers are managed -### 3.10 Snapshot File Format +### 3.11 Snapshot File Format Serialized from shadow arrays while no locks are held: @@ -500,6 +813,7 @@ Serialized from shadow arrays while no locks are held: │ - timestamp │ │ - term │ │ - nJobs, nQueues, nWorkers │ +│ - checksum: CRC32(body) │ ├─────────────────────────┤ │ Job array (Capnp) │ ├─────────────────────────┤ @@ -513,12 +827,18 @@ Serialized from shadow arrays while no locks are held: └─────────────────────────┘ ``` -### 3.11 Checkpoint Triggers +**Snapshot CRC scope**: The `SnapshotHeader.checksum` (CRC32) covers the Cap'n Proto–serialized body (Job array, Queue array, Worker array, Resource array, Config array) **excluding** the `SnapshotHeader` struct itself. The header is verified separately: the reader validates `nJobs`, `nQueues`, `nWorkers` against the actual array lengths to detect header corruption. This split design avoids the chicken-and-egg problem where the CRC needed for body verification is itself part of the data being verified. - Time-based: every `snap_interval_s` (default 60s) - Size-based: when WAL exceeds `snap_wal_size_mb` (default 64 MB) - Shutdown: final checkpoint in `cluster_run()` before exiting the main loop +**Inline size check in WAL append path**: The timer-driven size check alone creates a window where WAL can grow unbounded between timer ticks (e.g., 10K submits/sec × ~200 bytes = 2MB/sec, filling 64MB in ~32s, which may fall between two 60s timer ticks). Both `persist_wal_async()` and `persist_wal_sync_append()` must check cumulative WAL size after writing and trigger an inline checkpoint if `current_segment_bytes > snap_wal_size_mb`. + +**Inline checkpoint mechanism — atomic flag, not direct freeze**: The calling thread (which holds the kvmgr READ lock from the hook call chain) **cannot** directly call `persist_checkpoint()`, because `persist_checkpoint()` acquires the kvmgr WRITE lock — and a thread holding a READ lock that attempts to acquire a WRITE lock on the same POSIX rwlock results in undefined behavior (deadlock per POSIX). Instead, the inline trigger sets an atomic flag: `atomic_store(&persist->checkpoint_requested, 1)`. The checkpoint timer thread (running via `delaymgr`) checks this flag on each wake (every `snap_interval_s`, or sooner if the flag is set — use a shorter poll interval like 1-2 seconds when the flag is set). When `checkpoint_requested` is observed, the timer thread calls `persist_checkpoint()` (it holds no kvmgr lock, so acquiring the WRITE lock is safe). After the checkpoint completes, the timer thread clears the flag. This approach: (1) avoids the rwlock self-deadlock, (2) ensures checkpoint always runs on a thread that does not hold the kvmgr READ lock, and (3) bounds the WAL growth to `snap_wal_size_mb + max_wal_per_poll_interval` (at 2-second poll, ~4MB additional WAL at 2MB/sec). If an inline checkpoint is already in progress (checked via `persist_t.checkpoint_in_progress` atomic flag), the WAL append skips setting `checkpoint_requested` — the in-progress checkpoint will trim the WAL when it completes. + +**WAL growth during an in-progress checkpoint**: When `checkpoint_in_progress` is set, the inline trigger is suppressed. WAL growth during this window is bounded by the checkpoint duration times the submission rate. For pathological cases (very large shadow state, slow serialization disk), the WAL can grow significantly. **Mitigation**: if cumulative WAL size exceeds `2 × snap_wal_size_mb` while `checkpoint_in_progress` is set, log a `WARN("WAL size %.1f MB exceeds 2x threshold during in-progress checkpoint")`. If WAL size exceeds `wal_max_size_mb` (default 2 GB), trigger an emergency checkpoint even if one is in progress — the existing checkpoint must be completing soon (serialization is lock-free, so a stuck checkpoint indicates an I/O problem). Operators should size `snap_wal_size_mb` based on expected peak throughput and worst-case checkpoint duration. A rule of thumb: `snap_wal_size_mb ≥ peak_submit_rate_KB/s × worst_case_checkpoint_seconds / 1024`. + --- ## Part 4: Deferred Entity Cleanup at Checkpoint @@ -527,31 +847,42 @@ Serialized from shadow arrays while no locks are held: **No entity is ever deleted during normal operation.** All entities follow the same lifecycle: +**User-visible behavior**: This is intentionally different from eager deletion. Finished jobs and unavailable workers remain visible in API queries (filtered by their terminal status) for their configured retention period — this is the desired behavior. Users need to see recently completed jobs to check exit status and history. Abruptly removing entities on state transition (e.g., a job disappearing the moment it finishes) would confuse users and break operational workflows that depend on querying recent job results. The retention periods (`finished_retain_s`, `worker_retain_s`) control how long this historical data remains visible. + ``` alive state ──▶ terminal state ──▶ [TTL/retention expires] ──▶ cleaned at next checkpoint ``` -This eliminates `vclear` and `del_key` from the hot path, which: -1. Simplifies COW (no tombstones, no three-state flags — see Part 3) -2. Simplifies WAL (no delete entry types — replay is purely additive) -3. Makes cleanup predictable and batched — no random delete spikes +This eliminates `vclear` from the hot path. The `del_key` callback in kvmgr is still registered as NULL (the existing design in `kvmgr.c:286`), but the deferred cleanup mechanism replaces it: instead of `xkvmgr_del()` being called at state transition time, expired entries are removed during checkpoint GC via a `kvtab_gc()` scan that iterates over all kvmgr entries, calls `should_retain()` on each, and removes those that fail retention. This is safe because: +1. Checkpoint GC holds the kvmgr write lock (serializing all other access) +2. kvtab removal is O(n) but n = total entries (10K jobs, etc.) — fast enough for the infrequent checkpoint path +3. Eliminates the need for `del_key` entirely — the existing NULL registration is the intended design, with deletion deferred to checkpoint GC instead of the hot path. + +**Design invariant**: `del_key` is deliberately NULL. The deferred GC at checkpoint handles entity removal via `kvtab_gc()`, not through hot-path key deletion. The RTTI interface's `del_key = NULL` is correct — no change is needed. + +This simplifies COW (no tombstones, no three-state flags — see Part 3) + + ### 4.2 Terminal States by Entity Type | Entity | Terminal States | Trigger | Cleanup Criteria | |--------|----------------|---------|-------------------| -| Job | `JOB_DONE`, `JOB_EXITED`, `JOB_FAILED` | Job finishes naturally | No dependents AND `finish_time + finished_retain_s < now` | -| Worker | `WORKER_UNAVAILABLE` | Worker daemon dies / disconnects | `disconnect_time + worker_retain_s < now` | -| Worker | `WORKER_HIDDEN` | Admin explicit remove | Immediate cleanup at next checkpoint (no TTL) | -| Queue | `QUEUE_HIDDEN` | Admin deletes queue | Immediate cleanup at next checkpoint | +| Job | `JOB_STAT_DONE`, `JOB_STAT_EXIT`, `JOB_STAT_PERR` | Job finishes naturally | No dependents AND `finish_time + finished_retain_s < now` | +| Job | `JOB_STAT_TRANSIENT` (0x400) | Primary failover — job was RUNNING on old primary but no state update in standby's WAL | Worker reconnection (sets actual state) or operator intervention if worker never reconnects | +| Worker | `WORKER_UNAVAILABLE` | Worker daemon dies / disconnects | `dead_since + worker_retain_s < now` | +| Worker | `WORKER_HIDDEN` | Admin explicit remove (off-hours maintenance only) | Immediate cleanup at next checkpoint (no TTL) | +| Queue | `QUEUE_HIDDEN` | Admin deletes queue (off-hours maintenance only) | Immediate cleanup at next checkpoint | | Config | `CONFIG_HIDDEN` | Admin deletes config key | Immediate cleanup at next checkpoint | | Resource | _(never deleted)_ | N/A | Resources persist permanently | +**Important**: In production clusters, deleting queues or removing hosts while the cluster is running can cause job loss and is not acceptable. `WORKER_HIDDEN` and `QUEUE_HIDDEN` are intended for off-hours maintenance windows only, when no jobs are running on the affected resources. The deferred cleanup design exists so that removals during maintenance can be batched and completed efficiently at the next checkpoint. + ### 4.3 How Terminal States Work Entities in terminal states remain `inuse=1` in the zarray and still have their kvtab entries. Business logic ignores them through **status-based filtering** that already exists: -- Scheduler skips `JOB_DONE`/`JOB_EXITED`/`JOB_FAILED` jobs (already happens) +- Scheduler skips `JOB_STAT_DONE`/`JOB_STAT_EXIT`/`JOB_STAT_PERR` jobs (already happens) - Scheduler skips `WORKER_UNAVAILABLE` workers (already happens) - API queries filter by status (already happens) @@ -559,26 +890,71 @@ The new states `WORKER_HIDDEN`, `QUEUE_HIDDEN`, `CONFIG_HIDDEN` follow the same ### 4.4 New HostState / EntityState Values -Add to the Cap'n Proto schema: +Worker status uses bitmask flags (e.g., `XCE_WORKER_UNAVAIL = 0x00010000`), not a state enum. Add new bitmask flags and a QueueStatus hidden value: -```capnp -# In HostState enum — new terminal states for deferred cleanup -enum HostState { - # ... existing states ... - unavail @X; # Worker daemon died / disconnected (replaces immediate removal) - hidden @Y; # Admin explicitly removed — invisible but not yet cleaned up -} +```c +/* In types.h — new bitmask flags for deferred cleanup (XCE_WORKER_UNAVAIL already exists at 0x00010000) */ +#define XCE_WORKER_HIDDEN (0x00200000) /* Admin explicitly removed — invisible but not yet cleaned up */ -# QueueStatus — add hidden state -enum QueueStatus { - # ... existing states ... - hidden @X; # Admin deleted — invisible but not yet cleaned up -} +/* Job status — add transient bitmask for HA failover. + * NOTE: 0x200 is already JOB_STAT_WAIT (types.h:351) — do NOT reuse it. + * JOB_STAT_TRANSIENT uses 0x400 which is currently free. */ +#define JOB_STAT_TRANSIENT (0x400) /* Job state uncertain after failover — awaiting worker re-join */ + +/* Queue status — add hidden bitmask */ +#define QUEUE_STAT_HIDDEN (0x0100) /* Admin deleted — invisible but not yet cleaned up */ ``` +In the Cap'n Proto schema, `Worker.status` is `UInt32` (bitmask-compatible). `QueueStatus` may also need a `hidden` value added depending on its representation. + +`XCE_WORKER_UNAVAIL` already exists in `types.h` as `0x00010000`. The new additions are `XCE_WORKER_HIDDEN`, `JOB_STAT_TRANSIENT`, and `QUEUE_STAT_HIDDEN` (or equivalent). + ### 4.5 Retention Policy at Checkpoint -The `should_retain` callback passed to `xzarray_thaw()` implements entity-specific retention: +The `should_retain` callback passed to `xzarray_thaw()` implements entity-specific retention. + +**Supporting functions** (both implemented as part of P0-13): + +```c +/* job_is_terminal — returns true if job is in a terminal state */ +bool job_is_terminal(const job_t* j); +/* Implementation: return (j->stat & (JOB_STAT_EXIT | JOB_STAT_DONE | JOB_STAT_PERR)) != 0; */ + +/* job_has_dependents — returns true if any other job depends on this jobid. + * Dependency sources: + * - Array jobs: if j is an array master, any array element with stat != terminal + * - Job dependencies: any pending job with pending_ctrl referencing j->jobid + * (i.e., held because this job must finish first) + * Called only on terminal jobs (job_is_terminal() == true) during checkpoint. + * Caller holds kvmgr write lock (during thaw), so this function must not + * re-acquire any kvmgr lock. + * + * CRITICAL: During thaw, shadow entries are merged into active one at a time. + * When should_retain() is called for shadow[i], entries at higher indices + * (shadow[j] where j > i) have NOT yet been merged into active[]. If + * job_has_dependents() scans only active[], it will miss dependents that + * reside in not-yet-merged shadow slots, causing premature GC of jobs that + * have dependents. To avoid this, xzarray_thaw() MUST use a two-pass + * approach: (1) merge ALL shadow entries into active[] (pointer copies for + * untouched entries, skip entries where active[i].inuse == 1 and free shadow[i].data), + * then (2) run should_retain() checks on the fully-merged active[] array. + * This ensures that all entities are visible in active[] before any retention + * decision is made. The two-pass approach is safe because merge (step 1) only + * copies pointers and sets inuse flags — it does not call should_retain() or + * free any memory. + * + * Returns false if the jobid is not found (treated as no dependents). + * + * LOCKING REQUIREMENT: should_retain() MUST be called inside the kvmgr WRITE + * lock section (during thaw). It MUST NOT be called off the critical path + * (e.g., in a background GC thread) because concurrent kvtab lookups via + * xkvmgr_find() (holding only a READ lock) would encounter freed entities + * still referenced by hash entries until kvtab_gc runs. The window between + * entity free and kvtab cleanup must be atomic with respect to READ-lock + * lookups. The checkpoint flow (freeze → serialize → thaw/GC → kvtab_gc → + * unlock) guarantees this atomicity by holding the WRITE lock throughout. */ +bool job_has_dependents(cluster_t* c, uint64_t jobid); +``` ```c /* Job retention: keep if it has dependents OR TTL not expired */ @@ -604,20 +980,22 @@ static bool worker_should_retain(void* arg, void* item) { cluster_t* c = (cluster_t*)arg; worker_t* w = (worker_t*)item; - if (w->state == WORKER_HIDDEN) + if ((w->host.status & XCE_WORKER_HIDDEN) != 0) return false; /* admin removed, cleanup immediately */ - if (w->state != WORKER_UNAVAILABLE) + if (!IS_WORKER_UNAVAIL(w->host.status)) return true; /* still connected, keep */ time_t now = time(NULL); - if (w->disconnect_time + c->persist.config.worker_retain_s > now) + if (w->host.dead_since + c->persist.config.worker_retain_s > now) return true; /* TTL not expired */ return false; /* safe to clean up */ } ``` +**CLOCK_MONOTONIC vs wall-clock time for retention**: `time(NULL)` returns wall-clock time, which is vulnerable to NTP step adjustments (backward clock jumps can cause premature GC of terminal entities whose TTL appears to have expired). For production deployments, either: (a) use `clock_gettime(CLOCK_MONOTONIC)` for retention comparisons and store a monotonic timestamp alongside `finish_time`/`dead_since`, or (b) document the sensitivity to NTP step changes and require slewing-only NTP configuration (e.g., `chrony` with the `smoothtime` directive, which never steps the clock backward). The heartbeat module already uses `CLOCK_MONOTONIC` (Part 5.5) for exactly this reason. Option (a) is preferred but requires storing a `struct timespec` or `time_t` from `CLOCK_MONOTONIC` per entity (one extra 8-byte field per job/worker). Option (b) is acceptable for deployments where NTP is controlled by the cluster administrator. + ### 4.6 kvtab Cleanup at Checkpoint When `xzarray_thaw` frees an entity (returns it from `should_retain` as `false`), the kvtab still has a key→index mapping for it. After thaw completes, walk the kvtab and prune stale entries: @@ -647,7 +1025,7 @@ After checkpoint cleanup, freed zarray slots become `inuse=0` with their kvtab e | WAL entry types | Need `WAL_*_DELETE` types | No delete types — WAL is purely additive | | WAL replay | Must handle insert-delete-reinsert correctly | Insert or update — always correct | | Delete spikes | Random (worker disconnect, admin action) | Bounded — only at checkpoint intervals | -| Memory overhead | Freed immediately | Entity retained for up to `snap_interval_s` (default 60s) | +| Memory overhead | Freed immediately | Jobs retained for up to `snap_interval_s`; workers retained for up to `worker_retain_s` (default 86400s = 24h). **Memory sizing**: a typical `worker_t` in XCE is ~500 bytes. Retaining 2000 disconnected workers for 24h consumes ~1 MB — negligible. Job retention at 1h (default `finished_retain_s = 3600`) with 100K finished jobs × ~1 KB/job = ~100 MB at the high end; operators with very high job throughput may reduce `finished_retain_s` to limit peak memory. | | zarray fragmentation | Holes appear unpredictably | Compacted at checkpoint during merge | | kvtab fragmentation | Hash chains shrink unpredictably | Pruned at checkpoint — predictable | @@ -659,7 +1037,7 @@ When a worker or scheduler disconnects: 1. **Immediate**: Close `fd` and free `SSL*` — these are OS resources that cannot be retained 2. **Immediate**: Set `ep->busy = 0` (unblock any pending vtask queue entries) -3. **Deferred**: Set `ep->state = ENDPOINT_DISCONNECTED` but do **NOT** call `zarray_del(&(c->endpoints), ep->idx)` — this was the old eager cleanup in `handle_endpoint_error()` (`cluster.c`) +3. **Deferred**: Set `ep->state = ENDPOINT_DISCONNECTED` but do **NOT** call `xzarray_clear()` on the slot — defer cleanup to checkpoint The endpoint slot is cleaned up at checkpoint via the same `should_retain` mechanism: @@ -672,13 +1050,17 @@ static bool endpoint_should_retain(void* arg, void* item) { } ``` -**Why not delete immediately?** The old code in `cluster.c` called `zarray_del()` in `handle_endpoint_error()`. This is the `xzarray_clear()` path — setting `inuse=0` and calling the killer. During COW mode, calling `vclear` would corrupt the active array (it would create a "hole" that doesn't exist in the shadow, causing `vget` two-level lookup to incorrectly fall through to stale shadow data). Deferring to checkpoint avoids this entirely. +**Why not delete immediately?** Calling `xzarray_clear()` during handle_endpoint_error() would set `inuse=0` on the active array during COW mode, creating a "hole" that doesn't exist in the shadow. This would cause `vget` two-level lookup to incorrectly fall through to stale shadow data. Deferring to checkpoint avoids this entirely. + +**Note on array type**: The current `c->endpoints` uses plain `zarray_t`. As part of P0-2, `zarray_t` will be merged into `xzarray_t` (this merge is already partially completed), which enables endpoints to participate in COW. **Until the merge is complete (P0-0a), the immediate-cleanup path is used**: `handle_endpoint_error()` calls `zarray_del()` as it does today. **After the merge, the deferred cleanup flow applies**: set `ep->state = ENDPOINT_DISCONNECTED`, close fd/SSL immediately, and clean up the slot at checkpoint via `should_retain`. The P0-8a step in the implementation phases enforces this ordering — it depends on P0-0a (zarray merge) being complete first. + +**Why immediate fd/SSL close is safe during COW**: The fd and SSL pointer are on the endpoint object itself (not on the zarray slot). Setting `ep->state = ENDPOINT_DISCONNECTED` and closing `ep->fd` + `ep->ssl` are mutations on the endpoint struct, not on the zarray metadata. The zarray slot remains `inuse=1` pointing to the endpoint — the deferred cleanup at thaw/merge will free the struct and set `inuse=0`. Since vget/vput check `inuse` on the active array first (which still has inuse=1), reads still find the disconnected endpoint, and business logic filters on `ep->state != ENDPOINT_DISCONNECTED`. **Connection to vtaskmgr**: When `endpoint_block_tasks(ep)` sets `ep->busy = 1`, and later `endpoint_unblock_tasks(ep)` sets `ep->busy = 0`, this per-endpoint serialization is maintained regardless of whether the endpoint is in COW mode. The `busy` flag is on the endpoint object itself, not on the zarray slot. ### 4.10 Existing Code Alignment: Worker Unavailable State -The `mark_worker_unavail()` function already exists in `worker.c:516-577`. It currently: +The `mark_worker_unavail()` function already exists in `worker.c:516-526`. It currently: 1. Sets the `XCE_WORKER_UNAVAIL` flag on the worker 2. Runs `AFTER_EVENT` hooks (which will trigger the WAL write for `WAL_WORKER_CTRL`) @@ -687,7 +1069,7 @@ The `mark_worker_unavail()` function already exists in `worker.c:516-577`. It cu This matches the deferred cleanup design: after `mark_worker_unavail()`, the worker is in `WORKER_UNAVAILABLE` state and will be garbage-collected at the next checkpoint when `worker_retain_s` expires. The new additions needed: -1. Set `w->disconnect_time = time(NULL)` in `mark_worker_unavail()` so the TTL check works +1. Set `w->host.dead_since = time(NULL)` in `mark_worker_unavail()` so the TTL check works 2. Add `WORKER_HIDDEN` state for admin-initiated removal (sets state, never deletes from zarray) 3. The `handle_worker_leave()` path currently calls `mark_worker_unavail(c, w)` and sets `w->ep = NULL` — this is correct and needs no change @@ -701,8 +1083,51 @@ Instead of Raft consensus (~5000 LOC), use LSF's simple priority-based model: - A fixed list of controller candidates is defined in cluster config, ordered by priority - The first (highest-priority) candidate that is alive becomes primary -- No voting, no term negotiation, no split-brain risk - "Am I the highest-priority alive candidate?" is the entire election algorithm +- **Split-brain risk**: Under a network partition, a standby may promote itself while the primary is still alive but unreachable. This is the same trade-off LSF makes — priority-based election cannot prevent split-brain without quorum. XCE accepts this risk with the following assumptions and mitigations: (1) controllers share a dedicated management network in the same LAN with low partition probability — primary and standby controllers are expected to be co-located in the same data center, so network partitions between them are abnormal events; (2) term-based fencing prevents stale standbys from accepting new writes; (3) workers always reconnect to the current primary after partition heals. If split-brain is unacceptable, a quorum-based protocol (Raft) would be required instead. + +**Split-brain impact analysis**: Under split-brain, the original primary continues serving its established API server connections. Since end users never connect directly to the controller — only through the API server — user requests continue flowing to the original primary. The promoted standby also accepts work if an API server connects to it, but in practice the API server maintains a long-lived TCP connection to the original primary and does not switch during a partition (this is confirmed by the current `RegisterToBackend()` implementation in `clib.go`, which uses `nmasters = 1` with a single long-lived connection and no reconnection logic). **However, if the partition drops the API server's TCP connection to the original primary, the API server has no reconnection path until P2.5-7 (libxce discovery for API server) is implemented.** After P2.5-7, the API server would use `xce_cluster_config_t` to reconnect to any candidate, which could redirect it to the promoted standby — in that case, user requests would flow to the promoted standby and writes would diverge from the original primary's WAL. + +**Worker movement during split-brain**: If the partition also severs the TCP connection between some workers and the original primary, those workers will reconnect (via libxce discovery) to the promoted standby. These workers then report their job status to the standby, which applies the updates and writes them to its local WAL. When the partition heals, the standby (lower term) must step down, and these workers must reconnect to the original primary. The critical reconciliation step is: **after partition heals, the restored primary must receive state updates from workers that were served by the standby during the partition.** This is self-healing — workers always re-notify their current job state on reconnection (existing behavior: `handle_worker_join` re-registers the worker, and the scheduler re-queries job state). No explicit state merge protocol is needed beyond the existing worker re-join flow, because workers are the source of truth for their own runtime state. + +The actual risk is **replication divergence and potential data loss**: the standby's WAL diverges from the primary's WAL during the partition. **Writes accepted by the promoting standby during a network partition are at risk of loss** — when the partition heals, term-based fencing ensures the lower-term controller steps down, and the diverged WAL entries from the standby are discarded during full re-sync from the primary (which has the authoritative higher-term WAL). This means any job submits ACKed by the standby during the partition are not durable. At XCE's scale (a few thousand entities), full re-sync is bounded by the partition duration and is handled by full re-sync rather than partial merge. If preserving standby-served writes during partitions is required, a quorum-based protocol (Raft) would be needed instead. + +**Deployment assumption: LAN-only**. XCE HA controllers all run on the same LAN (dedicated management network). The following table enumerates the specific failure modes that can cause LAN partitions, their expected recovery times, and the blast radius for split-brain data loss: + +| Failure Mode | Expected Duration | Blast Radius | Mitigation | +|--------------|-------------------|--------------|------------| +| Top-of-rack switch failure | 5–30 min (auto-failover or manual swap) | All controllers on that switch are isolated | Deploy controllers across separate switches/racks | +| Switch port failure (single) | 1–10 min (STP reconvergence) | One controller isolated; remaining 2 have quorum connectivity | Redundant uplinks per controller | +| Cable cut (copper/fiber) | 30 min – 4 hours (physical replacement) | Controllers on opposite sides of the cut | Diverse cable paths between controllers | +| NIC failure (single controller) | 5–15 min (operator restarts on backup NIC) | One controller down; no partition if remaining 2 can communicate | Dual-NIC bonding (active-passive) | +| Misconfigured VLAN or ACL | 10 min – 2 hours (operator detects and corrects) | Intermittent or partial isolation | Change management controls on switch config | +| Kernel NIC driver bug (hang) | 5–30 min (operator detect + reboot) | One controller appears dead but process is running | Health monitoring with NIC-level liveness checks | +| STP broadcast storm | 1–10 min (storm suppression) | All controllers may see intermittent connectivity | Storm control on management switches | + +**Split-brain window duration**: The window opens when the standby's heartbeat timer fires (default `heartbeat_timeout_ms = 10000ms`) and closes when the partition heals and the higher-term message reaches the old primary. **Data at risk**: all job submits ACKed by the standby during this window are discarded on partition healing. At typical submit rates (1–10 submits/second), the worst case for a 10-minute partition is ~600–6000 job submissions at risk of loss. + +**Quantitative risk model**: At XCE's expected submit rate of 1–10 jobs/second and a 99.9% LAN availability (8.76 hours downtime/year), the expected annual data loss is: `partition_probability × mean_partition_duration × submit_rate`. For a well-managed LAN: P(partition per year) ≈ 0.001 (conservative), mean duration ≈ 15 minutes, submit rate ≈ 5/sec → expected lost submits/year ≈ 4.5. Operators must evaluate whether this risk is acceptable for their workload. + +**Three-controller asymmetric partition scenario**: With 3 controllers (ctrl-1 primary, ctrl-2 standby P=1, ctrl-3 standby P=2), consider an asymmetric partition where ctrl-1 can reach ctrl-2 but not ctrl-3, and ctrl-2 can reach ctrl-1 but not ctrl-3. ctrl-3 detects heartbeat timeout from ctrl-1 and probes candidates with priority < 2. If ctrl-3 also cannot reach ctrl-2, it receives no ACK within `election_timeout_ms` and promotes itself to primary — creating two primaries (ctrl-1 and ctrl-3). This is the same split-brain risk already acknowledged in Part 5.1, with the same mitigation: term-based fencing resolves the conflict when the partition heals. The lower-term primary (determined by which controller incremented term first) steps down. In this specific scenario, ctrl-3 would have a lower term than ctrl-1 (assuming ctrl-1 was already primary before the partition), so ctrl-3 steps down when connectivity resumes. If the partition also allows an API server to connect to ctrl-3, any jobs submitted to ctrl-3 during the partition are at risk of loss (same as the two-controller case). This scenario illustrates why the `split_brain_risk_accepted: true` config key is mandatory — even single-LAN deployments can produce asymmetric partitions during switch failures or cable cuts (see the failure mode table above). + +This is fundamentally different from WAN or cross-DC deployments where partitions can persist indefinitely due to routing protocol convergence, BGP flap dampening, or ISP outages. For cross-site or multi-data-center HA requirements, the recommended approach is to deploy **separate XCE clusters** per site with cross-cluster job forwarding (out of scope), not a single replicated HA cluster. + +**Deployment requirements**: LSF-style priority election is appropriate under the following conditions: (1) controllers share a **dedicated management network** on the same LAN — cross-DC or WAN deployments have unacceptably high partition probability; (2) **≤3 controller candidates** — with more candidates, cascading promotions during instability become harder to reason about; (3) operators understand that job submits ACKed by a split-brain standby are **not durable** and are discarded on partition healing. If any of these conditions cannot be met, a quorum-based protocol (Raft) is required instead. + +**Required operator acknowledgment**: To prevent accidental deployment without understanding the split-brain risk, HA mode requires an explicit opt-in config key: + +```yaml +ha: + split_brain_risk_accepted: true # REQUIRED — HA refuses to start without this + candidates: [...] +``` + +`cluster_init()` must refuse to start HA mode if `split_brain_risk_accepted != true`, logging: `"HA split-brain risk has not been acknowledged — set ha.split_brain_risk_accepted: true in cluster.yaml after reading the HA deployment requirements"`. + +**Operator visibility for degraded mode**: When `sync_timeout_ms` expires and the primary falls back to local-only ACK, this is NOT silent: +1. Log at `ERROR` level (not `WARN`) — degraded replication is a durability-affecting event +2. Increment `ctrl_repl_degraded_count` metric (alongside `ctrl_split_brain_detected`) +3. The degraded state persists until the standby reconnects — subsequent submits in degraded mode each log at `ERROR` and increment the counter **Trade-off**: Non-preemptive — if a higher-priority candidate recovers, it does NOT preempt the current primary. This avoids disruptive failback. Admin can trigger manual failover. @@ -711,7 +1136,7 @@ Instead of Raft consensus (~5000 LOC), use LSF's simple priority-based model: The `MasterCandidate` struct already exists in `controller.capnp` with `name`, `host`, `port`. Priority is **list position** (index 0 = highest priority). ```yaml -# cluster.yaml +# cluster.yaml — human-editable config, processed by config tool controller: candidates: - name: ctrl-1 @@ -725,6 +1150,15 @@ controller: port: 7000 ``` +**Configuration pipeline**: The controller does **not** parse YAML directly. A separate configuration tool (CLI, out of scope for this design) validates `cluster.yaml` and produces a Cap'n Proto-encoded binary configuration file that the controller reads at startup. The validation tool checks: +- At least one candidate is defined +- No duplicate host:port pairs +- `ha.split_brain_risk_accepted: true` is set when HA mode is enabled +- `ha_cert_path`, `ha_key_path`, `ha_ca_cert_path` files exist and have correct permissions +- `ha_ca_cert_path` points to a valid PEM-encoded CA certificate + +The controller reads only the binary Cap'n Proto config file — it has no YAML parsing dependency, no text config parsing, and treats its config as a trusted, validated blob. This eliminates config injection risks and simplifies the controller's dependency tree. + ### 5.3 Role Management ```c @@ -735,11 +1169,25 @@ typedef struct { int my_priority; /* My index in candidates[] */ replicator_t repl; /* Replicate to standbys (primary only) */ uint64_t applied_lsn; /* Last applied WAL entry (standby only) */ - int primary_fd; /* Connection to current primary (standby only) */ + endpoint_t* primary_ep; /* Connection to current primary via iomgr SSL pipeline (standby only) */ char primary_host[XCE_MAXNAME_LEN]; uint32_t primary_port; - pthread_t monitor_tid; /* Monitors higher-priority candidates */ + struct timespec last_heartbeat; /* Last heartbeat from primary (CLOCK_MONOTONIC, standby) */ + int promotion_failed; /* Non-zero if last promotion attempt failed — retry on next watchdog tick */ + int promotion_retry_count; /* Number of consecutive failed promotion attempts */ + ha_config_t config; /* HA configuration (timeouts, intervals) */ } ha_state_t; + +typedef struct { + uint32_t heartbeat_interval_ms; /* Primary heartbeat interval (default: 3000). Minimum 2000ms — delay_task() takes integer seconds, so values below 2000ms truncate to 0 or 1 second. Enforce heartbeat_interval_ms >= 2000 in ha_init() to avoid delay_task() truncation. */ + uint32_t heartbeat_timeout_ms; /* Standby heartbeat timeout (default: 10000) */ + uint32_t election_timeout_ms; /* Shared deadline for all parallel election probes (default: 2000) */ + char ha_cert_path[XCE_MAXPATH_LEN]; /* Controller mTLS certificate (PEM) */ + char ha_key_path[XCE_MAXPATH_LEN]; /* Controller mTLS private key (PEM, mode 0600) */ + char ha_ca_cert_path[XCE_MAXPATH_LEN]; /* CA certificate for peer verification (PEM) */ + uint32_t ha_cert_warn_days; /* Warn if cert expires within this many days (default: 30) */ + uint32_t max_promotion_retries; /* Max consecutive failed promotion attempts before giving up (default: 5) */ +} ha_config_t; ``` Added to `cluster_t` as `ha_state_t ha;`. @@ -754,68 +1202,132 @@ Controller starts Find my_priority (my position in list) │ ▼ - ┌─────────────────────────────┐ - │ am_i_primary()? │ - │ │ - │ For each candidate with │ - │ priority < my_priority: │ - │ Try TCP connect │ - │ If any connects: NO │ - │ │ - │ If none connect: YES │ - └──────┬──────────┬───────────┘ + ┌──────────────────────────────────┐ + │ am_i_primary()? │ + │ │ + │ Launch one pthread per candidate │ + │ with priority < my_priority. │ + │ Each thread makes a synchronous │ + │ blocking TLS dial (no iomgr yet) │ + │ and sends OP_heartbeat probe. │ + │ │ + │ Single shared deadline: │ + │ election_timeout_ms (2000ms) │ + │ enforced via condvar/timedwait │ + │ │ + │ If ANY thread gets a live ACK │ + │ before deadline: → NO (standby) │ + │ Cancel remaining threads. │ + │ │ + │ If deadline expires, all probes │ + │ failed/timed out: → YES (primary)│ + └──────┬──────────────┬────────────┘ │ │ PRIMARY STANDBY │ │ ▼ ▼ - persist_init() connect to primary - persist_recover() receive WAL stream - cluster_init() replay WAL entries - persist_start() ha_monitor_thread() - cluster_start() standby_loop() + cluster_init() cluster_init() + persist_init() persist_init() + persist_recover() persist_recover() + persist_start() persist_start() + connect to primary + cluster_start() (via iomgr, now running) + ha_heartbeat_watch() + standby_loop() ``` +> **Promotion recovery**: A standby runs `persist_recover()` on startup (populating kvmgrs from its local snapshot + WAL) and then receives ongoing replication via `apply_wal_entry()` calls from `OP_replEntry`. By the time promotion occurs, in-memory state is already complete from both sources. It must **not** call `persist_recover()` again — that would replay the local WAL and double-apply every entry. Instead, the promotion path should: (1) validate CRC chain from the standby's own `persist_t.checkpoint_lsn` forward (already maintained by startup recovery + replication), (2) flush the local WAL fsync to ensure all replicated entries are on disk, (3) update the local WAL `term` to the new election term, (4) start replication to remaining standbys from the current LSN. If the promoted standby has gaps in its WAL (async entries not yet replicated before the old primary died), it requests the missing entries from the old primary's WAL on disk via catch-up (see Part 7 catch-up protocol). + +> **Startup ordering note**: `am_i_primary()` runs before `cluster_init()` (which starts iomgr/SSL/vtaskmgr). The election probe therefore launches one `pthread` per lower-priority candidate; each thread makes a synchronous blocking TLS dial using the same cert/key pair as the HA pipeline but without the event loop. All threads share a single `election_timeout_ms` deadline enforced via a `pthread_cond_timedwait`. +> +> **IMPORTANT — thread cancellation safety**: `pthread_cancel()` is **NOT** used to stop election threads. `pthread_cancel()` running through OpenSSL shutdown handlers in a thread mid-handshake can deadlock (cancellation points inside TLS negotiation are not safe). Instead, use a shared `atomic_int election_done` flag (from ``) with `atomic_store()` / `atomic_load()` operations: +> - The main thread sets `atomic_store(&election_done, 1)` when the first ACK or the deadline fires +> - Each probe thread checks `atomic_load(&election_done)` after each probe attempt and exits cleanly if set (closing its TLS connection and freeing resources) +> - The main thread calls `pthread_join()` on all threads (which return promptly since they check the flag) +> - If a thread is blocked during TLS dial, use a non-blocking `connect()` + `poll()` with a short per-attempt timeout so each thread can check `election_done` between probe retries +> - Total worst-case latch is bounded by one `poll()` timeout interval per thread +> +> After the election resolves, `cluster_init()` starts iomgr and all HA traffic uses the iomgr/SSL/endpoint pipeline. This avoids a circular dependency between election and iomgr startup, and parallel probing bounds worst-case startup latency to one `election_timeout_ms` regardless of cluster size. +> +> **Peer certificate validation in election probe**: The synchronous TLS dial in `am_i_primary()` must verify the peer's certificate CN and OU against the expected values from `cluster.yaml`. Specifically: the peer's cert must have `CN=ctrl-` and `OU=ha` (where `candidate_name` matches the entry in the candidates list). A probe that connects to an unexpected host (wrong CN or OU) must be rejected as if no response was received — this prevents an attacker from impersonating a lower-priority candidate to force the probing controller into standby mode. +> +> **`ha_init()` startup enforcement**: At startup, `ha_init()` validates: `heartbeat_interval_ms >= 1000` (values below 1000ms truncate to 0 when passed to `delay_task()`, which takes integer seconds — this disables the heartbeat watchdog entirely); `heartbeat_timeout_ms >= heartbeat_interval_ms`; `election_timeout_ms >= 500`; `ncandidates >= 1`. It also verifies `ha_key_path` permissions (must be mode 0600) and logs a `WARN` if the HA certificate expires within `ha_cert_warn_days` (default 30). **Duplicate candidate validation**: `ha_init()` scans `candidates[]` and rejects any config where two entries share the same `host` (case-insensitive `strcasecmp`) or the same `host:port` pair. Duplicate candidate names cause ambiguous election probes and mTLS CN matching — an entry appearing twice would receive two heartbeat monitors, trigger double replication connections, and confuse the priority ordering. The validation must log a `FATAL` error naming both duplicate entries and return `-EINVAL`. See `ha_config_t` in Part 5.3 for field documentation. + ### 5.5 Monitor Thread (Standby) -The standby's monitor thread watches all candidates with higher priority: +The standby uses the controller's iomgr/vtask heartbeat infrastructure to monitor higher-priority candidates. The primary sends periodic heartbeat messages (`OP_heartbeat`) to connected standbys at `ha_config.heartbeat_interval_ms` (default 3000ms), and standbys watch for heartbeat timeouts from higher-priority controllers at `ha_config.heartbeat_timeout_ms` (default 10000ms — approximately 3 missed heartbeats): ```c -static void* ha_monitor_thread(void* arg) +/* The monitor is not a separate thread with raw TCP probes. + Instead, it uses the existing iomgr/vtask/endpoint pipeline: + + 1. The primary sends OP_heartbeat to all connected standbys + at HA_HEARTBEAT_INTERVAL_MS (default 2000-5000ms). + 2. Each standby tracks last_heartbeat_time for the primary endpoint. + 3. If last_heartbeat_time + HA_HEARTBEAT_TIMEOUT_MS < now, + the primary is considered dead -> promote self. + + This provides application-level liveness (not just TCP connectivity) + and reuses the SSL/endpoint/vtask pipeline that all controller + connections already use. */ + +/* ha_check_heartbeat() called from delaymgr timer on standby. + * Note: delay_task() takes integer seconds. Register with interval: + * delay_task(&c->delaymgr, ha->config.heartbeat_interval_ms / 1000, -1, ha_check_heartbeat, c); + * heartbeat_interval_ms is in milliseconds; divide by 1000 for delay_task(). + * WARNING: Values below 2000ms truncate to 0 or 1 second via integer division. + * ha_init() enforces heartbeat_interval_ms >= 2000 to prevent truncation. + * Uses CLOCK_MONOTONIC — immune to NTP jumps and DST changes that would cause + * spurious promotions (clock jumped forward) or missed timeouts (clock jumped back). */ +static void ha_check_heartbeat(cluster_t* c) { - cluster_t* c = (cluster_t*)arg; ha_state_t* ha = &c->ha; - for (;;) { - sleep(HA_CHECK_INTERVAL); /* default 2-5 seconds */ + if (ha->role != CTRL_STANDBY || ha->primary_ep == NULL) + return; - /* Check if any higher-priority candidate is alive */ - for (int i = 0; i < ha->my_priority; i++) { - if (tcp_probe(candidates[i].host, candidates[i].port) == 0) { - /* Higher-priority candidate is alive — stay standby */ - goto next_check; - } + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + int64_t elapsed_ms = (now.tv_sec - ha->last_heartbeat.tv_sec) * 1000LL + + (now.tv_nsec - ha->last_heartbeat.tv_nsec) / 1000000LL; + if (elapsed_ms > (int64_t)ha->config.heartbeat_timeout_ms) { + /* Primary heartbeat timeout — promote self. + * If a previous promotion attempt failed (promotion_failed flag set), + * retry promotion unless the retry limit is exceeded. */ + if (ha->promotion_failed && + ha->promotion_retry_count >= ha->config.max_promotion_retries) { + ERROR("cannot promote after %d attempts — manual intervention required", + ha->promotion_retry_count); + return; } - - /* No higher-priority candidate is alive — promote self */ ha_promote_to_primary(c); - return NULL; - - next_check: - continue; } } ``` +**Heartbeat detection latency**: The standby watchdog timer uses `delay_task()` with integer-second granularity (`precision=2` seconds in cluster.c). This means the actual primary-failure detection latency is `heartbeat_timeout_ms + up to (heartbeat_interval_ms / 1000) * 1000` milliseconds — the timeout fires, plus up to one timer tick of jitter before the check runs. For the default `heartbeat_timeout_ms = 10000` and `heartbeat_interval_ms / 1000 = 3` seconds, worst-case detection is 13 seconds. For HPC workloads requiring sub-10-second failover, a dedicated timer thread with millisecond granularity (as used for the WAL fsync timer) should be used for the heartbeat watchdog instead of `delaymgr`. The P1-3 implementation step must use a dedicated `pthread` with `nanosleep()` or `timerfd_create(CLOCK_MONOTONIC)` for the heartbeat check, identical to the WAL fsync timer approach described in Part 2.4. Using `delaymgr` for the heartbeat watchdog is only acceptable for deployments that can tolerate 2+ second detection jitter. + ### 5.6 Promotion Sequence When a standby promotes to primary: 1. **Stop WAL replay** from old primary 2. **Increment term** (`ha.term++`) — fencing token for stale standbys -3. **Switch role** to `CTRL_PRIMARY` -4. **Run `persist_recover()`** on locally accumulated WAL (replay any unapplied entries) -5. **Initialize replicator** — establish connections to remaining standbys -6. **Start normal primary operation** — `cluster_start()`, accept connections +3. **Flush the local WAL** (`persist_flush()` — fsync the last WAL segment to ensure all replicated entries are on disk) and **verify WAL consistency** (validate CRC chain from the standby's own last snapshot LSN forward — standbys run `persist_recover()` on startup and maintain their own local WAL and snapshots, so the validation starting point is the standby's `persist_t.checkpoint_lsn`). Do **NOT** call `persist_recover()` again — the standby's kvmgrs are already populated from replication (via `apply_wal_entry()` during normal `OP_replEntry` processing) AND from the startup recovery pass. Calling `persist_recover()` a second time would double-apply entries. Only a WAL flush + CRC chain validation is needed. If the standby has a WAL gap (entries the old primary committed but had not yet replicated before crashing), the promotion path requests missing entries from the old primary's WAL on disk via catch-up (see Part 7 catch-up protocol). **WAL consistency guarantee**: the standby's local WAL is a strict prefix of the old primary's WAL — standbys only write entries received via `OP_replEntry` from the primary. No divergence is possible if the standby was continuously connected; the standby's WAL cannot contain entries the old primary did not originate. The only split-brain divergence scenario (standby accepted writes from clients during a partition) cannot occur because standbys do not accept writes — they only redirect to the primary. +4. **Switch role** to `CTRL_PRIMARY` — only after recovery succeeds. **If recovery fails, abort this promotion attempt and set `ha.promotion_failed = 1`**. The standby remains in `CTRL_STANDBY` role, but since the old primary is dead (that triggered this promotion), the next heartbeat watchdog check (`ha_check_heartbeat()`) will re-fire `ha_promote_to_primary()`. The `promotion_failed` flag gates retry: the standby retries promotion on each watchdog tick unless: (a) a new heartbeat is received from a higher-priority candidate (another controller won the race), or (b) a configurable `max_promotion_retries` (default: 5) is exceeded, at which point the standby logs `FATAL "cannot promote after %d attempts — manual intervention required"` and stops retrying. This prevents a permanent no-primary state while bounding the retry window. +5. **Register persistence hooks** — standby did not register persistence hooks during startup (hooks are registered only after `persist_recover()` completes, and only on the primary path; standbys register them here on promotion). **Idempotent registration guard**: `ha_promote_to_primary()` must check whether hooks are already registered before adding them (e.g., via a `persist->hooks_registered` flag). Without this guard, a promote→demote→re-promote cycle would accumulate duplicate hooks that fire N times per event, causing double WAL writes and replication sends. +6. **Initialize replicator** — establish connections to remaining standbys +7. **Gate the scheduler**: pause or flush the scheduler's run queue before accepting writes. The scheduler maintains allocation state derived from WAL-replayed job state; async-replicated events (job starts/finishes) may be stale immediately after promotion. The scheduler must re-query job state from the promoted primary before making scheduling decisions (see step 8). +8. **Start normal primary operation** — `cluster_start()`, accept connections +9. **Wait for workers and scheduler to reconnect** — the promoted standby's in-memory state may be stale for async-replicated events (job starts, finishes, rusage updates). This is self-healing: workers re-join via `handle_worker_join()` and re-report their current job state, and the scheduler re-queries job state when scheduling. The promoted primary is functionally equivalent to a freshly booted controller that has replayed its WAL — it has a consistent but potentially stale view of job lifecycle states, which is corrected as workers reconnect and update their status. + +This means async-replicated job lifecycle transitions (start, finish, fail, stop, resume, rusage) that were lost when the old primary crashed are not present in the promoted standby's WAL. The promoted standby's view of these jobs may be stale (e.g., showing a job as PEND when it is actually RUNNING). Workers correct this state when they reconnect and re-report their current job status. Until all workers reconnect, the scheduler should treat job state as uncertain and avoid scheduling decisions that depend on stale information. + +**Job state during the reconnection window**: Jobs that were in `JOB_STAT_RUN` on the old primary but have no matching update in the standby's WAL enter a **transient unknown state**. The promoted primary marks such jobs with a sentinel flag (e.g., `JOB_STAT_TRANSIENT`) to signal uncertainty to operators and the scheduler. The scheduler must not schedule new work onto workers that own transient jobs (their status is unknown — the job may have completed, crashed, or still be running). As workers reconnect via `handle_worker_join()`, each worker re-reports its current job status and the job transitions from `JOB_STAT_TRANSIENT` to its actual state (`JOB_STAT_RUN`, `JOB_STAT_EXIT`, `JOB_STAT_DONE`, etc.). This guarantees eventual consistency: no job state is permanently lost, only temporarily uncertain until the owning worker reconnects. If a worker never reconnects (hardware failure, network isolation), jobs on that worker remain transient until an operator intervenes — this is the same as today's behavior when a worker goes `WORKER_UNAVAILABLE`. + +**Stale scheduler state**: The scheduler maintains internal state derived from controller state (resource allocations, pending job counts per queue, scheduling constraints). This internal state is also stale after failover. The scheduler heals by re-initializing from scratch when it reconnects to the promoted primary via `xce_reconnect()`: on reconnect, the scheduler sends a full state-sync query to enumerate all PEND and RUN jobs, rebuilds its allocation table, and resumes scheduling from a clean slate. Admin-initiated changes (queue configuration changes, admin holds on jobs) that were async-replicated and lost during the partition are visible to the promoted standby through its WAL replay — they are ASYNC events and captured in the WAL; only job lifecycle events from workers are subject to stale state. The scheduler must therefore re-query job state but can trust queue and worker configuration from the promoted primary's WAL. + +**Reconciliation window definition**: The reconciliation window is the interval between promotion (step 2) and the point where all workers that were connected to the old primary have re-joined the promoted primary. **Scheduler behavior during the reconciliation window**: (1) New job submits are accepted normally — these have authoritative state. (2) Scheduling of PEND jobs onto reconnected workers proceeds normally. (3) Scheduling of PEND jobs onto **unreconnected** workers is blocked — the scheduler does not know whether those workers still have capacity (transient jobs may still be running). (4) The scheduler tracks a `reconciliation_progress` metric: `(workers_reconnected / workers_at_old_primary)`. (5) Once `reconciliation_progress >= 1.0` (all workers have reconnected), normal scheduling resumes on all workers. **Expected window duration**: 30–120 seconds for a healthy cluster (workers reconnect within one `worker_heartbeat_interval`, typically 15–30 seconds, plus one scheduling cycle for the scheduler to catch up). If workers are slow to reconnect (e.g., due to DNS or SSL re-handshake latency), the window extends. If a worker never reconnects, its jobs remain in `JOB_STAT_TRANSIENT` until the operator intervenes — these workers are excluded from `reconciliation_progress` after a configurable `worker_reconnect_timeout_s` (default: 300 seconds). ### 5.7 Term (Fencing Token) @@ -826,6 +1338,19 @@ The `term` field prevents split-brain: - A standby receiving a message with a lower term than its own ignores it - Workers/scheduler reconnect and verify term on join +**Partition heal — old primary returns**: When a partitioned primary recovers network connectivity, it resumes sending heartbeats and replication messages with its old (lower) term. The promoted primary (higher term) ignores these messages (term < own term rule above). The partitioned primary receives a heartbeat reply or replication message from the promoted primary carrying the higher term. On receiving any message with `term > ha.term`, the old primary must detect that it has been superseded and demote itself: set `ha.role = CTRL_STANDBY`, stop accepting writes, and reconnect to the promoted primary as a standby. The `handle_msg()` path for all incoming messages must check `hdr.term > ha.term` and call `ha_demote_to_standby()` when this condition is detected, then re-process the message in standby mode. This prevents the old primary from continuing to accept writes after the partition heals, eliminating the split-brain write window. + +**Hook unregistration on demotion**: `ha_demote_to_standby()` must unregister the persistence hooks that were registered during promotion (step 5 above). Use `xce_remove_hook()` (or equivalent) with the same function pointers and hook types registered in `ha_promote_to_primary()`. Without unregistration, a promote→demote→re-promote cycle accumulates duplicate hooks (despite the idempotent guard in step 5, the old hook entries still exist in the hook list). Unregistration must happen before the role switch to `CTRL_STANDBY` so that no in-flight requests trigger hooks after the role has changed. + +**In-flight request window during demotion (CRITICAL)**: Between the higher-term message arriving at `handle_msg()` and the demotion taking effect, vcpu threads may already be executing `handle_jobsubmit()` past the `AFTER_EVENT` hook. These in-flight requests have the following risks: +1. The persistence hook already called `persist_wal_sync_append()`, which is writing to the local WAL and may have already sent `repl_broadcast()`. Standbys following the higher-term primary will ignore the lower-term replication message. +2. After `ha_demote_to_standby()` sets `ha.role = CTRL_STANDBY`, the vcpu thread continues executing and eventually calls `xce_reply()`, ACKing the client for a job that is NOT replicated to the new primary's standbys. + +**Mitigation — role check in hot paths**: To bound this window, both the persistence hook and `repl_broadcast()` must check `ha.role` before proceeding: +- `persist_job_hook_fun()`: before calling `persist_wal_sync_append()`, check `c->ha.role == CTRL_PRIMARY`. If `ha.role != CTRL_PRIMARY`, return `-E_XCE_NOT_PRIMARY` without writing WAL or replicating. The caller (`handle_jobsubmit`) treats this as a failed submit and replies to the client with an error. +- `repl_broadcast()`: before sending to any standby, check the primary's `ha.role == CTRL_PRIMARY`. If the role has changed, skip the broadcast and return an error. +These checks use `atomic_load_explicit(&c->ha.role, memory_order_acquire)` — `ha.role` must be an atomic field (or at minimum guarded by a compiler barrier) so that the vcpu thread sees the demotion promptly. **The corresponding write in `ha_demote_to_standby()` must use `atomic_store_explicit(&c->ha.role, CTRL_STANDBY, memory_order_release)` — a plain write (`ha.role = CTRL_STANDBY`) does not pair with the acquire load and may not be visible to vcpu threads in time, especially on weakly-ordered architectures or with compiler optimizations that cache the value in a register. The release-acquire pair is essential for the in-flight window mitigation to work correctly.** Note: these checks are defense-in-depth — the primary mechanism is the `handle_msg()` term check. The in-flight window is bounded by the time between `ha_demote_to_standby()` setting the role and the vcpu thread reaching the next role check, which is at most one sync replication timeout cycle (`sync_timeout_ms`, default 50ms). Any client ACK sent during this window is acknowledged as a split-brain risk (see Part 5.1). + ### 5.8 Non-Preemptive Failover ``` @@ -843,9 +1368,24 @@ When ctrl-1 recovers: 1. It probes for candidates with priority < 0 → none exist 2. It becomes primary? **No** — it probes ctrl-2 and finds it alive 3. ctrl-1 stays standby and connects to ctrl-2 as primary -4. Admin can issue `xce hostcontrol --failover` to manually move primary back to ctrl-1 +4. Admin can trigger manual failover to restore optimal controller placement: -### 5.9 New Opcodes and Cap'n Proto Additions +**Manual failover protocol**: To move primary back to a higher-priority candidate (e.g., ctrl-1 after recovery), the operator sends `SIGUSR1` to the current primary (ctrl-2). The signal handler calls `ha_demote_to_standby()`, which: (a) unregisters persistence hooks (see Part 5.7 hook unregistration), (b) sets `ha.role = CTRL_STANDBY` using `atomic_store_explicit` with `memory_order_release`, (c) stops accepting new writes, (d) flushes and sync-replicates any pending WAL entries, (e) sends a `OP_steppingDown` message to all connected standbys with its current term and the election deadline. On receiving `OP_steppingDown`, the highest-priority connected standby (ctrl-1) immediately begins its election (calls `ha_promote_to_primary()`). The stepping-down controller then reconnects to the newly promoted primary as a standby. **During the transfer window (~1-2 election_timeout_ms), no controller is primary — job submits receive `EAGAIN` and must retry.** This is the same brief unavailability as an automatic failover. The `SIGUSR1` approach avoids needing a dedicated admin RPC or CLI command in the initial implementation; a future `xce hostcontrol --failover` CLI can wrap this by sending the signal remotely via SSH. + +**Demotion failure handling**: If `persist_flush()` fails during `ha_demote_to_standby()` (step d above), the controller has already set `ha.role = CTRL_STANDBY` and incremented term. The safest recovery is to **not rollback** the role change — instead, log a `FATAL("persist_flush failed during demotion — WAL may be incomplete. Manual investigation required.")`. The demoted controller will not accept new writes (role is STANDBY), and the WAL will be flushed again on the next checkpoint. A role rollback (setting `ha.role = CTRL_PRIMARY` on flush failure) would leave the cluster in a split-brain-eligible state where the original primary continues accepting writes while a new primary may already be promoting. Failing forward as STANDBY is safer: the cluster temporarily has no primary until another standby wins election, which is the same state as a normal failover. + +### 5.9 HA Connection Security (mTLS) + +All HA connections (primary→standby replication, standby→primary heartbeat, election probes) use the **same iomgr/SSL pipeline** as worker/scheduler connections, with mutual TLS peer authentication: + +- **Peer authentication**: HA peers authenticate each other using client certificates signed by the cluster CA. The same CA that signs worker/scheduler client certs signs controller-to-controller certs. A separate HA cert distinguished name (`CN=ctrl-`, `OU=ha`) allows the server to distinguish HA peers from workers/schedulers in the RBAC layer. +- **SSL context extension**: The existing `create_ssl_context(cert_file, key_file)` in `netio.c:890` creates server-side SSL contexts but does **not** accept a CA cert parameter for client-side peer verification. HA mTLS requires extending the signature to `create_ssl_context(cert_file, key_file, ca_cert_path)`. When `ca_cert_path` is non-NULL, the function calls `SSL_CTX_load_verify_locations()` and `SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ...)` for mTLS client-side verification. **The HA listeners (both election probe and replication accept) must use an SSL context created with `ca_cert_path` non-NULL, so that the TLS handshake itself rejects clients without a valid CA-signed certificate — before any application-layer CN check.** The CN check in `OP_replJoin` is then defense-in-depth only. When `ca_cert_path` is NULL, the function behaves as before (server-only context). This is a P1 prerequisite for both election probes and replication connections. **Note**: since there is no released version requiring backward compatibility, the 2-param signature is replaced in-place — all existing callers are updated to pass NULL for the new third parameter. +- **Certificate provisioning**: Controller certs are provisioned at deployment time alongside worker certs. This is an ops concern (cluster CA, cert renewal cadence) — the implementation only consumes a cert/key path from config. Add `ha_cert_path` and `ha_key_path` to `HAConfig` (i.e., nested under `ha:` in `cluster.yaml`, alongside `heartbeat_interval_ms`, `heartbeat_timeout_ms`, etc.) — **not** to `PersistConfig`. These are HA connection settings, not persistence settings; mixing them into `PersistConfig` would create a misleading YAML key hierarchy. Update P0-9 to include `ha_cert_path` and `ha_key_path` in the `HAConfig` capnp schema. **File permissions**: `ha_key_path` (the private key file) must be mode `0600` (owner-read-only). `ha_init()` must verify this at startup and refuse to start if the key file is world- or group-readable — a misconfigured key permission is a silent security risk that would be missed without a startup check. +- **Certificate rotation**: Controller certs must be renewed before expiry. `ha_init()` must log a `WARN` if the cert's `NotAfter` is within 30 days of the current time (configurable via `ha_cert_warn_days`, default 30). Cert rotation requires a rolling restart: update the cert file, restart each standby one at a time, then restart the primary (after standbys are online). The key file must be written with mode `0600` before the restart (e.g., `install -m 0600 `) — if the key file has incorrect permissions, `ha_init()` will refuse to start. The P4-4 operational runbook must document this rotation procedure. Hot rotation (without restart) is out of scope. +- **Election probe**: The initial `am_i_primary()` probe (before iomgr is started — see Part 5.4 startup sequencing note) launches one `pthread` per lower-priority candidate, each making a **synchronous blocking TLS dial** with the same cert/key pair. All threads share a single `election_timeout_ms` deadline; the first live response wins. These probes do not go through the iomgr event loop; after the election resolves, all subsequent HA traffic uses the iomgr/SSL pipeline for heartbeat and replication. +- **Deployment requirement**: HA requires a cluster CA and per-controller certificates. Clusters without a PKI infrastructure (no CA) cannot use HA — this is an explicit gate. Many HPC sites run without an internal CA; for these sites, HA is unavailable until a CA is provisioned. The P4-4 operational runbook must document: (a) how to set up a minimal cluster CA (e.g., using `openssl` or `cfssl`), (b) how to issue and distribute per-controller certificates, and (c) cert renewal cadence (recommend 1-year validity with 90-day renewal reminder). Sites that cannot operate a CA should treat HA as out of scope for their deployment. + +### 5.10 New Opcodes and Cap'n Proto Additions ```capnp /* In HostRole enum: */ @@ -879,14 +1419,28 @@ term @22 :UInt64 $C.mapname("term"); /* NEW — fencing token */ ### 6.2 Replicator Data Structure ```c -#define MAX_STANDBYS (N_CONTROLLER_MAX - 1) /* 15 */ +/* MAX_STANDBYS and MAX_HA_CANDIDATES defined in include/config.h (see Appendix A.10) */ + +**Constant reconciliation**: The existing `src/include/client.h` defines `N_MAX_MASTERS = 3` for the `xce_client_config_t.masters[]` array. `MAX_HA_CANDIDATES = 16` is defined in `include/config.h` for `standbys[]` and `xce_cluster_config_t.candidates[]`. These are different array sizes for different purposes: `N_MAX_MASTERS` is for the client-side connection config (max 3 controllers to try), while `MAX_HA_CANDIDATES` is a structural array size (max 16 candidates). The deployment ceiling advisory (≤3 controllers) aligns with `N_MAX_MASTERS = 3` which was already chosen for that purpose. The Go CGo bindings see both constants — they must be clearly separated in header exports. typedef struct { - int fd; /* TCP connection to this standby */ - uint64_t acked_lsn; /* Last LSN this standby has ACKed */ - int lagging; /* Flag: standby is behind threshold */ - char host[XCE_MAXNAME_LEN]; - uint32_t port; + endpoint_t* ep; /* Full endpoint (SSL, FSM, outq) — uses iomgr/vtask pipeline */ + uint64_t acked_lsn; /* Last LSN this standby has ACKed. ACCESS RULE: on 64-bit + platforms (LP64), acked_lsn is read/written atomically as a + single 8-byte value. On ILP32 or any platform where uint64_t + is not naturally atomic, this field MUST be accessed under + repl.ack_lock. All write paths (repl_on_ack, standby + disconnect tracking) and all read paths (repl_wait_ack + predicate, WAL trim guard min-over-standbys) must use the + same lock or use _Atomic uint64_t with acquire/release + ordering. The recommended implementation is: use C11 + _Atomic uint64_t for acked_lsn with relaxed loads for + advisory reads (lag monitoring, trim guard) and + release/acquire for the ack signal path. Alternatively, + gate ALL reads and writes through repl.ack_lock. */ + int lagging; /* Flag: standby is behind threshold */ + char host[XCE_MAXNAME_LEN]; + uint32_t port; } standby_conn_t; typedef struct { @@ -923,10 +1477,14 @@ Primary Standby-1 Standby-2 ### 6.4 Standby WAL Receiver -The standby runs a simple receive loop: +> **IMPLEMENTATION GUARD**: The pseudocode below is a **logical sketch** of WAL entry processing on the standby. The actual implementation **must use the endpoint/iomgr/vtask SSL pipeline** (see Part 6.5 and Part 6.6). **Do NOT implement standalone receive-loop threads with raw `read()`/`write()` on file descriptors.** All controller connections — including replication — go through the iomgr event loop. A raw-socket implementation would bypass SSL enforcement, per-endpoint message ordering, and backpressure; any such implementation must be rejected in code review. The `standby_recv_thread()` function below is pseudocode only — replace with `handle_msg()` dispatch for `OP_replEntry` in the iomgr/vtask pipeline. + +The standby processes replication entries through its iomgr event loop: ```c -/* On standby: receive WAL entries from primary */ +/* On standby: receive WAL entries from primary + * NOTE: This is PSEUDOCODE showing the logical processing steps. + * The actual implementation uses iomgr/vtask/endpoints — see Part 6.5. */ static void* standby_recv_thread(void* arg) { cluster_t* c = (cluster_t*)arg; @@ -936,31 +1494,115 @@ static void* standby_recv_thread(void* arg) wal_entry_hdr_t hdr; void* payload; - /* Read WAL entry from primary */ - if (read(ha->primary_fd, &hdr, sizeof(hdr)) != sizeof(hdr)) + /* Read WAL entry from primary (pseudocode — actual: iomgr recv via ha->primary_ep) */ + if (read(ha->primary_ep->fd, &hdr, sizeof(hdr)) != sizeof(hdr)) break; /* Primary connection lost */ + if (hdr.payload_len > MAX_WAL_PAYLOAD_BYTES) { + ERROR("replication: oversized payload %u > MAX_WAL_PAYLOAD_BYTES", hdr.payload_len); + break; /* Treat as protocol error — drop connection */ + } payload = malloc(hdr.payload_len); - if (read(ha->primary_fd, payload, hdr.payload_len) != hdr.payload_len) + if (read(ha->primary_ep->fd, payload, hdr.payload_len) != hdr.payload_len) break; - /* Validate term — reject stale entries */ + /* Validate term — reject stale or implausible entries */ if (hdr.term < ha->term) { free(payload); continue; } + if (hdr.term > ha->term + 1) { + /* Term is too high — a valid primary should only ever send entries + * with term == ha->term (steady state) or term == ha->term + 1 + * (after promotion of this standby or another candidate). Higher + * terms indicate a protocol error or tampering. */ + ERROR("replication: implausible term %lu (expected <= %lu) — " + "protocol error or tampering", hdr.term, ha->term + 1); + free(payload); + break; /* Tear down connection — primary may be compromised */ + } - /* Write to local WAL */ - persist_wal_async(&c->persist, hdr.type, payload, hdr.payload_len); + /* Validate LSN sequence — entries must arrive in strictly increasing order. + * A repeat means the primary re-sent without a term bump (unexpected). + * A backward LSN is always an error. + * A forward gap is expected for WAL_CHECKPOINT (type=100) entries, which are + * local-only markers not replicated to standbys — they create gaps in the + * LSN sequence from the standby's perspective. Therefore the check is + * strictly increasing (hdr.lsn > applied_lsn), NOT strictly consecutive + * (hdr.lsn == applied_lsn + 1). A missing-entry gap (network loss) is + * detected by CRC chain validation, not LSN sequence — and a true gap + * (more than one missing entry) indicates a network issue requiring + * reconnection and catch-up. A single-entry gap (typical for a skipped + * WAL_CHECKPOINT) is benign and must be accepted. + * + * Note: when applied_lsn == 0 (standby has not yet applied any entry), + * both guards are skipped — the first replicated entry is accepted with + * no LSN validation whatsoever. This is correct because (a) the first + * entry arrives via catch-up which establishes a known starting LSN, and + * (b) catch-up mode already performs CRC chain validation on the + * transferred entries. Any gap in the initial stream is caught by the + * catch-up protocol, not by the normal-mode LSN check. */ + if (ha->applied_lsn != 0 && hdr.lsn <= ha->applied_lsn) { + ERROR("replication: LSN sequence error: got %lu, expected > %lu", + hdr.lsn, ha->applied_lsn); + free(payload); + break; + } + if (ha->applied_lsn != 0 && hdr.lsn > ha->applied_lsn + 1) { + /* Multi-entry gap — possible network loss of entries. + * Single-entry gaps (hdr.lsn == applied_lsn + 2) are most likely a + * skipped WAL_CHECKPOINT marker. Log at WARN, accept the entry, + * and continue — CRC chain validation catches true corruption. */ + WARN("replication: LSN gap: expected %lu, got %lu (likely skipped WAL_CHECKPOINT)", + ha->applied_lsn + 1, hdr.lsn); + } - /* Apply to in-memory state */ + /* Validate CRC — detect corruption in replication path. + * WAL_HDR_CRC_SIZE is defined in persist.h (Part 2.2) — covers all + * header fields except crc and prev_crc. + * A static_assert ensures sizeof(wal_entry_hdr_t) == 40 so this + * computation remains correct regardless of field ordering. */ + uint32_t computed_crc = crc32(0, (void*)&hdr, WAL_HDR_CRC_SIZE); + computed_crc = crc32(computed_crc, payload, hdr.payload_len); + if (computed_crc != hdr.crc) { + /* Corrupted entry — log and skip */ + ERROR("replication CRC mismatch: expected %u, got %u, lsn %lu", + hdr.crc, computed_crc, hdr.lsn); + free(payload); + continue; + } + + /* Write to local WAL. + * WAL_JOB_SUBMIT (SYNC) entries must be fsynced before ACK so the + * primary's durability guarantee holds on the standby as well — if the + * standby crashes before fsync, the primary's ACK to the client was + * premature. All other entry types use async (fire-and-forget) writes. */ + if (hdr.type == WAL_JOB_SUBMIT) + persist_wal_sync_append(&c->persist, hdr.type, payload, hdr.payload_len); + else + persist_wal_async(&c->persist, hdr.type, payload, hdr.payload_len); + + /* Apply to in-memory state — MUST NOT fire AFTER_EVENT hooks. + * apply_wal_entry() mutates state directly (same as persist_recover()). + * If hooks fired here, the persistence hook would write another WAL + * entry and send another replication message — an infinite loop. + * Hooks are not registered on standbys (registered only on the + * primary path after persist_recover() completes), so no bypass + * flag is needed — the hook system is simply empty. + * + * Note: hdr.payload_len was already validated against MAX_WAL_PAYLOAD_BYTES + * above (line 1210) before malloc. apply_wal_entry() applies the same bound + * internally as defense-in-depth — it reads from the local WAL during + * recovery (also user-controllable if disk is compromised). */ apply_wal_entry(c, &hdr, payload); ha->applied_lsn = hdr.lsn; - /* Send ACK for sync-replicated entries */ - if (hdr.type == WAL_JOB_SUBMIT) { - send_repl_ack(ha->primary_fd, hdr.lsn); + /* Send ACK for sync-replicated entries — only when not in catch-up mode. + * During catch-up the standby is replaying historical entries; sending ACKs + * then would prematurely signal readiness to the primary (see Part 6.6). */ + if (hdr.type == WAL_JOB_SUBMIT && !ha->in_catch_up) { + send_repl_ack(ha->primary_ep, hdr.lsn); } free(payload); @@ -1018,6 +1660,53 @@ The primary initiates the replication connection as a client (using iomgr's outg The standby_recv_thread pseudocode above would instead be handled by the iomgr event loop: the standby's `handle_msg()` dispatches `OP_replEntry` to `vtask_handle_msg()`, which applies the WAL entry and sends `OP_replAck` back via the endpoint's outq. +### 6.6 OP_replAck Must Bypass vtaskmgr (ACK Deadlock Prevention) + +`OP_replAck` is the **only** opcode that must be handled directly in the io_worker read callback (`handle_msg()`) instead of being dispatched to vtaskmgr. All other opcodes go through vtaskmgr. + +**Why**: When sync replication is active, vtask threads executing `handle_jobsubmit()` block inside `repl_wait_ack()` waiting for an ACK. If `OP_replAck` were also dispatched via vtaskmgr, the incoming ACK message would sit in the vtask queue — but no vcpu thread would be free to process it (they are all blocked waiting for ACKs). This is a classic deadlock. + +**Implementation**: `handle_msg()` must check the opcode before calling `vtask_handle_msg()`: +```c +/* handle_msg is the io_worker read callback. It receives a protocol message, + * NOT a WAL entry header. The first field is the OPCode (see Part 5.10 for the + * opcode enum: OP_replEntry=23, OP_replAck=24, OP_redirect=25, OP_heartbeat=26). + * For OP_replEntry, the payload is a WAL entry (wal_entry_hdr_t + data). + * Do not confuse the protocol opcode with wal_entry_type values (Part 2.3). */ +void handle_msg(cluster_t* c, endpoint_t* ep, msg_hdr_t* hdr, void* payload) { + if (hdr->opcode == OP_replAck) { + /* Handle directly in io_worker — do NOT dispatch to vtaskmgr */ + repl_on_ack(&c->ha.repl, decoded_lsn); + return; + } + vtask_handle_msg(c, ep, hdr, payload); +} +``` + +`repl_on_ack()` is safe to call from io_worker: it only updates `acked_lsn` and calls `pthread_cond_signal()` under `repl.ack_lock`, with no vtaskmgr interaction. + +**Standby catch-up after reconnection**: When a standby reconnects to the primary (either after a network disruption or on initial startup), it must tell the primary its last `applied_lsn` so the primary can stream missing WAL entries. The catch-up protocol is: + +1. Standby sends `OP_replJoin` with its `applied_lsn` and `term` +2. **Primary authenticates the peer**: The primary verifies that the peer's mTLS certificate CN matches a known candidate in `cluster.yaml` (e.g., `CN=ctrl-`, `OU=ha`). An `OP_replJoin` from a peer with an unknown or unrecognized CN is rejected with an error reply — only known controller peers may join the replication group. +3. Primary checks if the standby's `applied_lsn` is still within available WAL segments (not yet trimmed by a checkpoint). If yes, stream entries from `applied_lsn + 1` forward via `OP_replEntry` messages on the existing SSL endpoint pipeline +4. If the standby's `applied_lsn` is too old — i.e., `applied_lsn < oldest_surviving_segment_start_lsn` (the WAL segments needed for catch-up have been trimmed) — the primary transfers the latest snapshot **via the same SSL endpoint pipeline** using a new `OP_replSnapshot` opcode. The snapshot is chunked at `MAX_WAL_PAYLOAD_BYTES` per message: the first chunk includes a snapshot header `{ snapshot_lsn, total_size, chunk_seq=0, final=false }`, subsequent chunks carry `{ chunk_seq, data }`, and the last chunk sets `final=true` and includes a CRC32 of the full snapshot payload. The standby reassembles chunks into a buffer, verifies the CRC32, then applies the snapshot to its kvmgrs. **Snapshot size validation on standby receive**: The `total_size` field in the first chunk must not exceed `MAX_SNAPSHOT_BYTES` (default: 256 MB). If `total_size > MAX_SNAPSHOT_BYTES`, the standby logs an `ERROR` and tears down the replication connection — a compromised or buggy primary could otherwise exhaust the standby's memory by sending an unbounded snapshot. This mirrors the local recovery path's snapshot size validation (Part 2.5 step 2b). After snapshot application, the primary streams WAL entries from `snapshot_lsn + 1` forward. No separate file transfer channel or `scp` is required — all data flows through the established SSL endpoint. +5. If the standby's `term` is higher than the primary's, the primary has stale data — the standby should not follow this primary (fencing) + +**Replication during catch-up**: While the primary is streaming historical WAL entries to a catching-up standby, it continues broadcasting new WAL entries to all other standbys. The catching-up standby's endpoint `sendq` buffers incoming live entries from the broadcast while replaying historical entries in order. **No separate catch-up queue is needed** — the existing endpoint `sendq` (already bounded by the iomgr flow-control mechanism) serves as both the catch-up buffer and the live-entry buffer. When the standby's `applied_lsn` reaches the primary's `committed_lsn` at the time of `OP_replJoin`, it transitions to normal replication mode and begins applying the buffered live entries from the `sendq`. The standby does not send `OP_replAck` for entries it receives during catch-up (to avoid prematurely signaling readiness) — `OP_replAck` is only sent once the standby has caught up and is processing live entries. + +**Catch-up receive ordering**: (1) `OP_replSnapshot` chunks arrive first and must be fully received and CRC-verified before any new WAL entries are applied to the kvmgrs. (2) The primary must NOT send live `OP_replEntry` messages to the catching-up standby until the snapshot transfer is complete — the standby sends `OP_replCatchupDone` (or equivalent signal) after successfully loading the snapshot. (3) After snapshot application, the primary streams WAL entries from `snapshot_lsn + 1` forward, and the standby applies them in strict LSN order. Only after reaching the primary's `committed_lsn` does the standby begin sending `OP_replAck` and transition to normal replication. This ensures the standby's state is self-consistent at each phase boundary: snapshot-loaded → historical-WAL-replayed → caught-up. + +**Snapshot catch-up flow control**: The `OP_replSnapshot` transfer sends the full snapshot in chunks of `MAX_WAL_PAYLOAD_BYTES` through the SSL endpoint pipeline. The primary sends chunks as fast as the endpoint pipeline allows, and the standby must receive, buffer, and reassemble all chunks before applying the snapshot. The existing iomgr backpressure mechanism (endpoint write FSM, outq buffering, SSL-level flow control via `SSL_write` partial writes) provides implicit flow control: if the standby's receive path is slow, the primary's endpoint outq fills, and subsequent `SSL_write` calls block or return partial writes, naturally throttling the sender. **No additional application-level flow control is needed** beyond the iomgr/SSL pipeline's built-in backpressure. However, operators should be aware that a snapshot transfer (up to `MAX_SNAPSHOT_BYTES` = 256 MB) temporarily increases the standby's memory usage for reassembly. The standby allocates a receive buffer of `total_size` bytes (validated against `MAX_SNAPSHOT_BYTES` at first chunk, per Part 6.6) before receiving subsequent chunks — this memory is freed after snapshot application. + +**Catch-up trigger**: When a standby reconnects (or initially joins), it includes its `applied_lsn` in the heartbeat to the primary. The primary compares this against its available WAL segments and streams missing entries from `applied_lsn + 1` forward. If the standby's `applied_lsn` is older than the primary's oldest surviving WAL segment, the primary initiates snapshot transfer (see `OP_replSnapshot` framing above) before streaming WAL entries. + +**`MAX_WAL_PAYLOAD_BYTES`**: The maximum payload size per WAL entry (and per `OP_replEntry` message) is `1 MB` (1,048,576 bytes). This bounds the memory allocation in the replication receive path (line `payload = malloc(hdr.payload_len)` above) to a safe maximum. The largest expected payload is a job submit with a large environment (~64 KB typical, ~512 KB pathological). The 1 MB cap provides 2× headroom over the pathological case while preventing unbounded allocation from a corrupted or malicious entry. Defined in `persist.h` as `MAX_WAL_PAYLOAD_BYTES` (see Appendix A.1). A `#ifndef` include guard allows test override. + +**Cap'n Proto traversal limit**: `apply_wal_entry()` must set a Cap'n Proto traversal limit when constructing the reader for WAL payloads. Set `capnp::ReaderOptions.traversalLimitInWords` proportional to `MAX_WAL_PAYLOAD_BYTES / 8` (the word count for the maximum payload). This prevents O(n²) or deeper traversal amplification from a crafted payload that is within the 1 MB byte limit but triggers excessive Cap'n Proto structural nesting. This is standard Cap'n Proto hardening practice and is a one-line change per deserialize call site. + +This catch-up mechanism is analogous to etcd's snapshot + WAL catch-up, simplified for XCE's single-writer model. The `OP_replJoin`, `OP_replSnapshot` opcodes and catch-up protocol will be added as part of P2-5 (`repl.c` — standby catch-up implementation; distinct from P2.5 which is the libxce smart discovery phase). + --- ## Part 7: Mixed Sync/Async Replication Protocol @@ -1036,28 +1725,35 @@ All other events (job start, finish, fail, rusage, queue/worker control) are sel ### 7.2 Sync Mode: Job Submit Flow +Sync replication sends WAL entries to **all** standbys in parallel, then waits for at least one ACK: + ``` -API Server Controller (Primary) Standby - │ │ │ - │ job submit request │ │ - │──────────────────────────▶│ │ - │ │ BEFORE_EVENT hook │ - │ │ job_submit() │ - │ │ AFTER_EVENT hook ──┐ │ - │ │ │ │ - │ │ persist_wal_sync() │ │ - │ │ WAL write + fsync│ │ - │ │ send(entry) ─────────▶│ - │ │ wait_ack(timeout)│ │ - │ │◀─── ACK(ack_lsn) ───────│ - │ │ │ │ - │ │ ← hook returns │ │ - │ │ │ - │ reply(jobid) │ │ - │◀──────────────────────────│ │ - │ │ │ +API Server Controller (Primary) Standby-1 Standby-2 + │ │ │ │ + │ job submit request │ │ │ + │──────────────────────────▶│ │ │ + │ │ BEFORE_EVENT hook │ │ + │ │ job_submit() │ │ + │ │ AFTER_EVENT hook ─┐ │ │ + │ │ │ │ │ + │ │ persist_wal_sync()│ │ │ + │ │ WAL write + fsync│ │ │ + │ │ send(entry) ──────────────────────▶│ + │ │ send(entry) ──────────────────▶│ │ + │ │ wait_any_ack() │ │ + │ │◀─── ACK(ack_lsn) ───────│ │ + │ │ │ │ │ + │ │ ← hook returns │ │ │ + │ │ │ │ + │ reply(jobid) │ │ │ + │◀──────────────────────────│ │ │ + │ │ │ │ ``` +**Parallel fan-out**: The primary sends the WAL entry to all connected standbys in parallel (fan-out, not sequential). It then waits for at least one ACK within the timeout. The first ACK received satisfies the durability guarantee — remaining standby ACKs are tracked for lag monitoring but do not block the client reply. This maximizes replication durability without adding latency proportional to the number of standbys. + +**Partial fan-out failure handling**: If `repl_broadcast()` encounters a send failure on one or more standby endpoints (e.g., `SSL_write` returns an error, or the endpoint's write FSM reports a broken connection), the broadcast continues sending to remaining standbys — a single failed send does not abort the broadcast. Failed standbys are marked for disconnection: `repl_remove_standby()` is called after the broadcast completes (not during, to avoid mutating the `standbys[]` array while iterating it). If no standby ACKs arrive within `sync_timeout_ms`, `persist_wal_sync_append()` returns `-ETIMEDOUT` (degraded mode, Part 7.4). If all standbys fail to send, the result is the same as having zero connected standbys: the entry is locally durable (WAL + fsync) but not replicated. + **Key change**: `xce_reply()` is moved **after** `AFTER_EVENT` hooks in `handle_jobsubmit()`. This allows the persistence hook to complete sync replication before the client is ACKed. ### 7.3 Async Mode: All Other Events @@ -1087,10 +1783,16 @@ When no standby ACKs within the timeout (`sync_timeout_ms`, default 50-100ms): 1. **Do NOT block the client indefinitely** — timeout is hard 2. **ACK the client based on local WAL durability alone** — the primary's local WAL is already fsynced -3. **Log a warning** — admin should investigate standby health +3. **Log a warning** — monitoring tooling should alert on degraded replication 4. **Self-heal later** — when the standby reconnects, it will catch up via WAL replay -This trades strict consistency for availability. In practice, the primary's local WAL provides durability as long as the primary doesn't lose its disk. +**Degraded mode is transient**: In a production cluster, replication degradation only occurs during cluster upgrade, cold boot (standbys not yet connected), or network partitions. A monitoring tool or operator should watch for sustained degraded-mode warnings and investigate immediately. The cluster health system (see Part 5.5 heartbeat monitoring) tracks which standbys are connected and their replication lag. Sustained degraded operation indicates a problem requiring intervention, not a normal operating state. + +**Degraded mode timeout**: Replication degradation must not persist indefinitely without operator awareness or intervention. Add a configurable `repl_degraded_timeout_s` to `persist_config_t` (default: 300 seconds). If no standby has acknowledged within this window, the primary: (1) logs an `ERROR` with the duration and the number of submits ACKed in degraded mode, (2) increments `ctrl_repl_degraded_timeout_count`, (3) transitions to **restricted mode**: new `WAL_JOB_SUBMIT` requests receive `E_XCE_REPL_DEGRADED` instead of a normal ACK. Restricted mode persists until at least one standby reconnects and acknowledges the current LSN. This prevents an unbounded accumulation of locally-durable-only submissions that would be lost if the primary itself fails. **Escape hatch**: Operators who accept the risk of sustained degraded operation (e.g., during a planned standby maintenance window) can set `repl_degraded_timeout_s = 0` to disable restricted mode. + +**Client-visible behavior during split-brain**: API server clients do not receive any explicit error or warning during a split-brain event — the controller continues to ACK job submits normally. However, job submits ACKed by the lower-term controller during a split-brain window are **silently lost** when the partition heals and the lower-term controller steps down (its diverged WAL is discarded). There is no application-level notification to the client. This is the acknowledged risk of the LSF-style election (see Part 5.1 split-brain impact analysis and the required `split_brain_risk_accepted: true` config key). Operators who require at-most-once or exactly-once job submission semantics must either use quorum-based HA (Raft) or implement client-side deduplication using the job's idempotency key. + +**Split-brain is unlikely in practice**: In a typical deployment, API servers maintain a persistent connection to a single controller. During a network partition, the API server stays connected to its original controller. Since API servers are not replicated and do not run their own election, they cannot become "split-brained" themselves — they simply become isolated from the primary if the primary fails. The split-brain risk applies to scenarios where multiple API servers connect to different controllers during a partition, which requires a configuration where API servers are load-balanced across controllers. The LSF-style election + redirect mechanism (Part 10.3) ensures that API servers connected to a standby are redirected to the current primary, minimizing the window where an API server could submit to a lower-term controller. ### 7.5 Sync Timeout Tuning @@ -1103,6 +1805,8 @@ This trades strict consistency for availability. In practice, the primary's loca **Recommended default**: 50ms. At ~1000 submits/sec, average inter-submit gap is 1ms. A 50ms timeout adds 5% to average submit latency while providing strong durability. If all standbys are healthy (RTT < 5ms), ACKs arrive well within the timeout and there's no waiting. +**Standby WAL sync analysis**: The standby calls `persist_wal_sync_append()` for every `WAL_JOB_SUBMIT` entry received via replication, which performs a local fsync before sending ACK. In the primary's 50ms sync timeout window, the standby must: (1) receive the entry via SSL/vtask, (2) write to local WAL, (3) fsync, (4) send ACK. If the standby's WAL is on a slow disk (HDD, shared storage), the fsync alone could take 10-30ms, consuming most of the 50ms budget. Operators should configure `sync_timeout_ms` based on the slowest standby's WAL disk latency — on HDD-backed standbys, consider raising the timeout to 100ms. The interaction between standby checkpoint (which holds the WAL lock) and sync replication fsync is additive; if both are on the same disk, the worst-case latency is `fsync_time + checkpoint_hold_time`. + --- ## Part 8: Hook-Driven Architecture @@ -1219,17 +1923,24 @@ xce_reply(ep, c->myhost, &reply); **Why this works**: The persistence hook runs inside `AFTER_EVENT`. For job submits, it does sync WAL + replication. Only after the hook returns (meaning the entry is durable) does the controller send the client ACK. This keeps all persistence logic inside hooks — no inline persistence code in `handle_jobsubmit()`. -**Lock safety analysis**: The current `handle_jobsubmit()` acquires `lock(c->jobmgr, RTTI_LOCK_READ)` for job lookup, then operates on the per-job object. No kvmgr write lock is held during the hook call or `xce_reply()`. The change to move `xce_reply()` after hooks is therefore safe: +**No reply reordering concern**: The controller processes requests from each API server connection one at a time — a request is fully handled (including hooks and `xce_reply()`) before the next request on the same connection is processed. SYNC replication adds latency to the `AFTER_EVENT` hook, which delays the `xce_reply()` for that specific request, but it does not cause out-of-order replies because the next request on that connection doesn't begin processing until the current one completes. Replies to *different* API servers may have different latencies, but each API server sees its own replies in request order. + +**Lock safety analysis**: The current `handle_jobsubmit()` acquires `lock(c->jobmgr, RTTI_LOCK_READ)` for job lookup, then operates on the per-job object. No kvmgr write lock is held during the hook call or `xce_reply()` for job submit. The change to move `xce_reply()` after hooks is therefore safe (note: `handle_worker_join()` in `join.c:58-69` is the exception — it holds the workermgr WRITE lock during `AFTER_EVENT` hooks, but this is acceptable because worker join uses async replication only): 1. `job_submit()` takes the jobmgr READ lock for lookup, then per-object lock for modification — no change here 2. `AFTER_EVENT` hooks (including `persist_wal_sync_append()`) acquire their own locks internally (`persist.lock` for WAL, `repl.lock` for replication) — no nesting with jobmgr 3. `xce_reply()` writes to the endpoint's outq — no kvmgr lock interaction -4. The only path that takes jobmgr WRITE lock during hooks is `handle_worker_join()` in `join.c:58-69` (via `add_worker()` + hook execution). This path runs on a separate vcpu from job submits and doesn't conflict with the reply ordering change +4. `handle_worker_join()` in `join.c:58-69` takes the workermgr WRITE lock for `add_worker()`. Worker join is always async-replicated (never blocks on `repl_wait_ack`), so holding the write lock during the hook call is safe — the hook only does a non-blocking WAL write and fire-and-forget replication send. The `xce_reply()` for worker join is also inside the write lock (at line 65), which risks blocking on endpoint outq backpressure under extreme conditions. `add_worker()` is infrequent enough that this is acceptable in practice, but if endpoint backpressure becomes a concern, `xce_reply()` should be moved after `unlock(c->workermgr)`. +> **Enforcement**: The persistence hook for `HOOK_DETAIL_WORKER_JOIN` must use `persist_wal_async()` — never `persist_wal_sync_append()`. To make this self-documenting and prevent a future refactor from accidentally switching to sync replication while the WRITE lock is held, add a `persist_wal_async_only()` wrapper that asserts `type != WAL_JOB_SUBMIT` at runtime (under `persist.lock`), or log a `CRITICAL` error and abort if a sync-type entry is attempted through the worker path. This is a belt-and-suspenders guard: the hook mapping in Part 2.3 already maps worker events to async-only WAL types, but the runtime assertion prevents regression. **Replication scope**: Sync replication (`repl_wait_ack`) only triggers for `WAL_JOB_SUBMIT` entries, which originate from API server connections. The controller only handles connections from workers, scheduler, and API servers — never end users. At 1–3 API server connections, the vcpu pool is not at risk of exhaustion from blocking `repl_wait_ack()` calls. ### 8.4 Registration in `cluster_init()` +Hook registration is **conditional on role** and happens **after** `persist_recover()` completes. This resolves two constraints: +- Part 2.5: "hooks not registered until after `persist_recover()` completes" — recovery replay must not trigger WAL writes +- Part 6.4: `apply_wal_entry()` on standbys must not fire hooks — standbys must never register persistence hooks + ```c /* In cluster_init(), after xce_install_hooks(): */ @@ -1237,16 +1948,29 @@ if (persist_config_enabled(&c->myconf->cluster_config->persist)) { rc = persist_init(&c->persist, &c->myconf->cluster_config->persist, c); if (rc < 0) return rc; + /* Step 1: Recover BEFORE registering hooks. + persist_recover() replays WAL via the same mutation functions + (job_submit, mark_worker_unavail, etc.) but hooks are not yet + registered, so no WAL writes occur during replay. */ rc = persist_recover(&c->persist, c); if (rc < 0) return rc; - /* Register persistence hooks */ - xce_add_hook(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, - (hook_fn_t)persist_job_hook_fun, NULL); - xce_add_hook(&(c->hooksys), XCE_CONTROLLER_HOOK_QUEUE, - (hook_fn_t)persist_queue_hook_fun, NULL); - xce_add_hook(&(c->hooksys), XCE_CONTROLLER_HOOK_WORKER, - (hook_fn_t)persist_worker_hook_fun, NULL); + /* Step 2: Register persistence hooks ONLY on primary path. + Standbys apply WAL entries via apply_wal_entry() — hooks are + not registered on standbys, so no infinite loop occurs. + Hook registration happens here for primary, or in + ha_promote_to_primary() step 5 when a standby promotes. */ + if (c->ha.role == CTRL_PRIMARY) { + xce_add_hook(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, + (hook_fn_t)persist_job_hook_fun, NULL); + xce_add_hook(&(c->hooksys), XCE_CONTROLLER_HOOK_QUEUE, + (hook_fn_t)persist_queue_hook_fun, NULL); + xce_add_hook(&(c->hooksys), XCE_CONTROLLER_HOOK_WORKER, + (hook_fn_t)persist_worker_hook_fun, NULL); + } + /* Standbys: no hooks registered here. apply_wal_entry() mutates state + directly — persistence hooks are registered only after cluster recovery + completes, and only on the primary path. */ } ``` @@ -1367,6 +2091,8 @@ int handle_join(cluster_t* c, endpoint_t* ep, xce_request_t* request) } ``` +**Redirect authentication**: All controller connections use SSL enforced by iomgr (`netio.c`), which means a redirect response can only come from a peer that has completed the SSL handshake with a valid certificate. A rogue endpoint that has not established an SSL connection cannot inject redirect responses. Additionally, the `RedirectInfo` contains only host/port of a known candidate from `cluster.yaml` — the receiving peer validates that the redirect target is in its candidate list before following it. + ### 9.5 Thundering Herd Mitigation on Failover When the primary dies, all workers connected to it get TCP RST simultaneously. If they all reconnect to the new primary at once, the listen backlog and accept loop could be overwhelmed. @@ -1388,7 +2114,7 @@ When a worker disconnects, instead of immediately removing it from data structur void handle_worker_disconnect(cluster_t* c, worker_t* w) { /* Instead of: destroy(w); kvmgr_del(...); */ w->state = WORKER_UNAVAILABLE; - w->disconnect_time = time(NULL); + w->host.dead_since = time(NULL); /* Worker remains in zarray (inuse=1) and kvtab. Scheduler already ignores UNAVAILABLE workers. @@ -1407,7 +2133,7 @@ struct RedirectInfo $C.mapname("xce_redirect_t") { } /* In RequestReply union: */ -redirect @7 :RedirectInfo $C.mapname("redirect"); +redirect @8 :RedirectInfo $C.mapname("redirect"); /* New error code in common/ */ #define E_XCE_REDIRECT (-100) /* Not an error — redirect instruction */ @@ -1437,12 +2163,13 @@ typedef struct { } xce_candidate_t; typedef struct { - xce_candidate_t candidates[N_CONTROLLER_MAX]; + xce_candidate_t candidates[MAX_HA_CANDIDATES]; int ncandidates; /* Number of candidates configured */ uint32_t connect_timeout_ms; /* Per-candidate connect timeout (default 2000) */ uint32_t retry_base_ms; /* Backoff base (default 100) */ uint32_t retry_max_ms; /* Backoff cap (default 5000) */ int max_retries; /* Max reconnection attempts (default 10) */ + char ca_cert_path[XCE_MAXPATH_LEN]; /* CA cert for TLS server verification (PEM) */ } xce_cluster_config_t; /* Connect to the cluster — returns fd connected to primary */ @@ -1538,17 +2265,30 @@ static int select_candidate(xce_cluster_config_t* conf, int* attempt, ### 10.5 Backoff with Jitter ```c -/* Exponential backoff with random jitter to spread reconnect storm */ +/* Thread-safe RNG initialization using pthread_once. + * Uses getrandom() for an unpredictable seed (avoids the correlated seeds + * from getpid() ^ time(NULL) on HPC clusters where many nodes boot simultaneously). + * The static seeded flag + double-checked locking in the original implementation + * was a data race — use pthread_once for correct one-time initialization. */ +static unsigned int rng_seed; /* per-process seed for rand_r() */ +static pthread_once_t rng_once = PTHREAD_ONCE_INIT; + +static void rng_do_init(void) { + if (getrandom(&rng_seed, sizeof(rng_seed), 0) != sizeof(rng_seed)) + rng_seed = (unsigned int)time(NULL) ^ (unsigned int)getpid(); +} + +/* Exponential backoff with random jitter to spread reconnect storm. + * Uses rand_r() (thread-safe) instead of rand() (per-process, not thread-safe). */ static void backoff_jitter(int attempt, xce_cluster_config_t* conf) { int delay_ms = conf->retry_base_ms * (1 << attempt); - /* Cap at max */ if (delay_ms > conf->retry_max_ms) delay_ms = conf->retry_max_ms; - /* Add jitter: random value in [0, delay/2] */ - int jitter = rand() % (delay_ms / 2 + 1); + pthread_once(&rng_once, rng_do_init); + int jitter = rand_r(&rng_seed) % (delay_ms / 2 + 1); delay_ms += jitter; usleep(delay_ms * 1000); @@ -1564,8 +2304,13 @@ int xce_connect_cluster(xce_cluster_config_t* conf) { int attempt = 0; int last_candidate = -1; + int redirect_count = 0; - for (;;) { + for (; attempt < conf->max_retries; attempt++) { +next_candidate: + /* redirect_count is NOT reset per candidate — it is a global hop counter + * across the entire connection attempt to prevent redirect loops that cycle + * through multiple candidates (e.g., A→B→A). */ int idx = select_candidate(conf, &attempt, last_candidate); xce_candidate_t* cand = &conf->candidates[idx]; @@ -1590,21 +2335,57 @@ int xce_connect_cluster(xce_cluster_config_t* conf) } if (reply.status == E_XCE_REDIRECT) { - /* Standby redirected us — follow immediately, no backoff */ + /* Standby redirected us — validate redirect target before following. + * Only follow to a known candidate from cluster.yaml to prevent + * a rogue endpoint from redirecting us to an arbitrary host. */ + int target_known = 0; + for (int k = 0; k < conf->ncandidates; k++) { + if (strcmp(conf->candidates[k].host, reply.data.redirect.host) == 0 + && conf->candidates[k].port == reply.data.redirect.port) { + target_known = 1; + break; + } + } + if (!target_known) { + /* Redirect to unknown host — treat as error, try next candidate */ + WARN("redirect to unknown host %s:%u — ignoring", + reply.data.redirect.host, reply.data.redirect.port); + close(fd); + backoff_jitter(attempt, conf); + continue; + } + + /* Follow redirect — no backoff since this is expected during discovery. + * MUST verify the server's certificate CN after TLS handshake: + * the peer cert must have CN=ctrl- matching the expected candidate + * from cluster.yaml, with OU=ha. A host:port match is necessary but not + * sufficient — a MITM could intercept the redirect response. CN validation + * ensures the TLS session is with the genuine controller peer. Reject the + * connection if CN validation fails (close fd, try next candidate). + * See the election probe peer verification (Part 5.4) for the same pattern. */ close(fd); fd = tcp_connect(reply.data.redirect.host, reply.data.redirect.port, conf->connect_timeout_ms); if (fd >= 0) { - /* Verify this is the primary */ + /* Re-send join request to verify this is actually the primary. + * TOCTOU protection: the primary may have changed between the + * redirect and our connection — the join reply confirms we + * reached the actual primary, not another standby. */ init_join_request(&req, &conf->myinfo); xce_send_request(fd, &req); xce_recv_reply(fd, &reply); if (reply.status == 0) { - return fd; /* Connected to primary */ + return fd; /* Confirmed primary */ + } + /* Redirect pointed to another standby? Follow again (up to limit) */ + if (++redirect_count >= 5) { + /* Too many redirects — return EXCE_ERR_REDIRECT_LOOP to caller */ + close(fd); + errno = ELOOP; + return EXCE_ERR_REDIRECT_LOOP; } - /* Redirect pointed to another standby? Follow again */ } close(fd); continue; @@ -1614,6 +2395,9 @@ int xce_connect_cluster(xce_cluster_config_t* conf) close(fd); backoff_jitter(attempt, conf); } + + /* All retries exhausted */ + return -1; } ``` @@ -1680,6 +2464,10 @@ controller: This file is already read by the controller for its own candidate configuration (Part 5.2). Workers and libxce clients read the same file for discovery. Distribution is an ops concern — shared NFS mount, config management tool (Ansible, Salt), or deployment pipeline. +**Candidate list consistency**: A stale `cluster.yaml` on one node could cause split-brain even without a network partition — for example, if a worker's config lists only ctrl-1 while ctrl-1 is down, the worker cannot discover ctrl-2 or ctrl-3 and will fail to connect. Operational practices should ensure atomic config updates (update all nodes before restarting any controller). + +**Config freshness control**: To detect stale `cluster.yaml` at runtime, the controller embeds a config version hash in its `OP_heartbeat` and join-reply messages (CRC32 of the serialized candidate list, offset by +1 so 0 is the sentinel for "not sent"). When a worker or API server receives a heartbeat with a mismatched config hash, it logs a warning and reloads `cluster.yaml` from disk. This is a lightweight freshness signal — it does not prevent a stale worker from connecting, but it surfaces divergence quickly so operators can investigate. The config version hash is added as an optional field in the `Request` Cap'n Proto schema (`configVersion @23 :UInt32`). **Sentinel**: 0 means "not sent" (pre-upgrade peer); valid hashes range from 1 to UINT32_MAX. On the receiving end, a `configVersion = 0` is treated as "peer does not support config versioning" and skipped — it is never compared against the local hash. This avoids the ambiguity of CRC32 potentially returning 0 for non-empty inputs. + ### 10.9 libxce Changes Summary | File | Change | @@ -1699,24 +2487,93 @@ This file is already read by the controller for its own candidate configuration **Goal**: Controller survives restart with job/queue/worker state intact. +P0 is broken into seven sub-phases with clear deliverables and dependency ordering. Each sub-phase produces a buildable, testable increment. + +--- + +#### P0-A: Cleanup & Prerequisites + +**Deliverable**: Clean codebase + rtti COW interface ready. No persistence behavior yet. + | Step | File | Description | |------|------|-------------| -| P0-1 | `src/controller/persist.c` + `include/persist.h` | WAL writer, segment management, fsync batching | -| P0-2 | `src/lib/zarray.c` + `include/zarray.h` | COW extensions: `shadow`, `shadow_caps`, `xzarray_freeze()`, `xzarray_thaw()` | -| P0-2a | `src/lib/zarray.c` | COW lock protocol: verify xzarray `mylock_t` mutex is taken correctly inside vget/vput during COW mode; kvmgr `my_rwlock_t` is the outer coordination lock (see Part 3.8) | -| P0-3 | `src/controller/persist.c` | Checkpoint via dual zarray freeze → serialize shadow → thaw + GC; use `delaymgr` for periodic checkpoint timer (see Part 3.9) | -| P0-4 | `src/controller/persist.c` | Recovery: load snapshot + replay WAL | -| P0-5 | `src/include/xce/cluster_def.h` | Add entity terminal states: `WORKER_UNAVAILABLE`, `WORKER_HIDDEN`, `QUEUE_HIDDEN`; add `persist_t persist;` to `cluster_t` | -| P0-6 | `src/controller/persist.c` | Hook functions: `persist_job_hook_fun`, `persist_queue_hook_fun`, `persist_worker_hook_fun` | +| P0-0 | `src/controller/job.c` + `worker.c` + `cluster.c` | Remove dead code (Part 1): unused `NJOBS`/`jobid` constants, `load_jobs()` `#if 0` body. Do this first so P0 builds on a clean foundation | +| P0-0a | `src/lib/zarray.c` + `include/zarray.h` | Merge `zarray_t` into `xzarray_t` (partially completed per git history) — enables uniform COW for all data structures including endpoints. **Scope**: move the `zarray_t` definition into `xzarray_t`, update all `zarray_*` call sites to use `xzarray_*` equivalents, verify that existing tests still pass. This step is a prerequisite for P0-2 (header move of `xzarray_t`) and P0-8a (deferred endpoint cleanup). **Owner**: this must be completed or explicitly handed off before P0-2 begins; if the merge is not yet complete, P0-8a falls back to the immediate-cleanup path (see Part 4.9 note). **IMPORTANT: P0-0a and P0-0b must be developed on the same branch or merged together before P0-2** — both steps modify `zarray.c` compound-literal initializers and the `xzarray_type` registration. P0-0a renames `zarray_*` functions to `xzarray_*`; P0-0b adds two new slots to `intf_varray_t`. If developed in parallel on separate branches, the initializer updates will conflict. Merge P0-0a first, then P0-0b (which builds on the renamed functions), or develop both on a single P0-A feature branch. | +| P0-0b | `rtti/typeinfo.h` + `zarray.c` + `kvmgr.c` | **rtti library enhancement** (see Appendix A.9): add `retain_fn_t` (`bool (*)(void*, void*)`), `vfreeze_fn_t`, and `vthaw_fn_t` function pointer types to `typeinfo.h`; grow `intf_varray_t` from 6 to 8 slots (`vfreeze`, `vthaw`); add `vfreeze()`/`vthaw()` global API wrappers to `typeinfo.h`; update all compound-literal `intf_varray_t` initializers (xzarray in `zarray.c`, kvmgr in `kvmgr.c`, kvtab in `kv.c`) to include the two new slots — use designated-initializer style `{ .vstore = ..., .vfreeze = (vfreeze_fn_t)NULL, .vthaw = (vthaw_fn_t)NULL }` for non-xzarray types; update nix derivation version from `rtti-0.0.1` to `rtti-0.1.0`. This is a prerequisite for P0-2 which wires `vfreeze`/`vthaw` to the xzarray implementations. Requires P0-0a to be merged first (same-branch constraint above). | +| P0-5 | `src/include/xce/cluster_def.h` + `include/config.h` | Add `persist_t persist;` to `cluster_t` — must precede P0-1 through P0-4 which reference `c->persist`. **Note**: `ha_state_t ha;` is added in P1-5 (HA wiring) — it is not needed in P0 since no P0 codepath references `c->ha`. P0-5 adds only `persist_t persist;`. Also create `include/config.h` with all system-level tunable constants (see Appendix A.10). | + +--- + +#### P0-B: WAL & COW Infrastructure + +**Deliverable**: WAL writes work; COW freeze/thaw round-trip on zarray. No checkpoint or recovery yet. **Requires: P0-A**. + +| Step | File | Description | +|------|------|-------------| +| P0-1 | `src/controller/persist.c` + `include/persist.h` | WAL writer, segment management, fsync batching. `persist_init()` must validate `wal_path` and `snap_path` against path traversal using `realpath()` — reject any path that resolves outside the intended base directory or contains `..` components — then create directories with `mode 0700` (owner-read-only) if they do not exist, and **verify permissions on existing directories at startup — refuse to start if `wal_path` or `snap_path` is not mode 0700 and owned by the controller UID** (rather than log-and-continue). Job payloads written to WAL/snapshot files may include environment variables and command lines containing credentials; this permission gate prevents a misconfigured mount from exposing them. **Dedicated fsync thread required**: `wal_sync_ms` defaults to 10ms; `delaymgr` has 2-second granularity (`sleep(precision)` where `precision=2`) so the WAL fsync timer must be a dedicated `pthread` using `nanosleep()` or `timerfd_create(CLOCK_MONOTONIC)` — do not use `delaymgr` for this. **Encryption note**: WAL and snapshot files are stored **unencrypted** by the controller. `persist_init()` must log an `INFO` message at startup: `"WAL/snapshot stored unencrypted; ensure wal_path and snap_path reside on an encrypted filesystem (e.g., LUKS/dm-crypt) if job payloads contain sensitive data"`. File-level encryption (e.g., AES-GCM per-record) is out of scope for this design. | +| P0-2 | `src/lib/zarray.c` + `include/zarray.h` | COW extensions: `shadow`, `shadow_caps`, `xzarray_freeze()`, `xzarray_thaw()`; modify `xzarray_iter_next()` for shadow-aware merge walk (see Part 3.5); modify `xzarray_freeze()` to set `pos = shadow_caps` (see Part 3.5 vstore invariant). **Header move required**: `xzarray_t` is currently defined as a file-private struct in `zarray.c`; it must be moved to `include/zarray.h` so `persist.c`, `repl.c`, and other callers can embed or reference it. `xzarray_private_t` (the internal struct containing `mylock_t`) is also currently file-private in `zarray.c` and must be moved to `include/zarray.h` as well — it is embedded inside `xzarray_t` and must be visible wherever `xzarray_t` is used. Without both moves, COW callers cannot access `shadow`/`shadow_caps` fields or embed the struct. **Requires: P0-0a** — P0-0a merges `zarray_t` into `xzarray_t`; P0-2 must not begin until P0-0a is merged and tested. Both touch the same file pair (`zarray.c` + `include/zarray.h`), so they must be serialized. | +| P0-2a | `src/lib/zarray.c` | COW lock protocol: verify xzarray `mylock_t` mutex is taken correctly inside vget/vput during COW mode; kvmgr `my_rwlock_t` is the outer coordination lock (see Part 3.9) | + +--- + +#### P0-C: Checkpoint & Recovery + +**Deliverable**: Full persist/recover round-trip — controller survives restart. **Requires: P0-B**. + +| Step | File | Description | +|------|------|-------------| +| P0-3 | `src/controller/persist.c` | Checkpoint via dual zarray freeze → serialize shadow → thaw + GC; use `delaymgr` for periodic checkpoint timer (see Part 3.10) | +| P0-4 | `src/controller/persist.c` | Recovery: load snapshot + replay WAL. Includes: (1) snapshot version check — reject snapshots where `SnapshotHeader.version > WAL_FORMAT_VERSION` (see Part 2.5 step 2a); (2) Cap'n Proto traversal limit — set `ReaderOptions.traversalLimitInWords` proportional to `file_size / 8` and reject snapshots larger than `MAX_SNAPSHOT_BYTES` (see Part 2.5 step 2b); (3) snapshot CRC verification over both header (`headerChecksum` covering lsn, term, version, counts) and body (`checksum`); (4) WAL entry type max guard — abort if any entry type exceeds `WAL_ENTRY_TYPE_MAX` (see Part 2.5). | +| P0-12a | `job.c` | Implement `restore_job(cluster_t* c, uint64_t jobid, const void* capnp_payload)` — inserts a job at its original jobid without allocating a new one. Used by `apply_wal_entry()` for `WAL_JOB_SUBMIT` during recovery and standby replay. Unlike `job_submit()`, `restore_job()` must accept an explicit jobid, bypass the jobid allocator, and skip BEFORE/AFTER_EVENT hooks. **Duplicate jobid handling**: if the jobid already exists in the kvmgr (e.g., from a duplicate WAL entry or a corrupt WAL that replays the same submit twice), `restore_job()` must log a `WARN` and return `-EEXIST` without overwriting the existing entry. `apply_wal_entry()` treats `-EEXIST` as a non-fatal skip. Required before P0-4 (recovery replay) and P2-2 (standby apply). **Note on `-EEXIST` occurrence**: During clean recovery (snapshot + WAL replay from a trimmed WAL), `-EEXIST` should never occur — the snapshot represents a consistent point-in-time state, and WAL entries after the snapshot's LSN contain only new state changes. `-EEXIST` is a safety guard for crash-recovery edge cases where: (1) a checkpoint failed mid-write but the WAL was not yet trimmed, causing the recovery algorithm to replay entries already present in the snapshot; or (2) a standby receives a duplicate replication entry due to reconnection. If `-EEXIST` occurs during what should be a clean recovery, it indicates a WAL trim bug or snapshot/WAL inconsistency and should be investigated. | +| P0-12 | `worker.c` | Wire `restore_worker()` into recovery path. **Adapter requirement**: the existing `restore_worker(void* cls, xce_worker_t host)` takes `xce_worker_t` by value (struct copy), but Cap'n Proto deserialization produces a pointer-based `Worker` struct. Implement a wrapper `worker_from_capnp()` that: (1) receives the deserialized `Worker` Cap'n Proto object, (2) allocates and populates an `xce_worker_t` by deep-copying pointer fields (`char* name` → strdup, `xce_config_t** resources` → allocate and copy each config item, `xce_res_threshold_t** thresholds` → same), (3) calls `restore_worker(cls, host)` with the populated struct. The existing `restore_worker()` function signature is unchanged — the adapter sits between Cap'n Proto and `restore_worker()`. | + +--- + +#### P0-D: Hooks & Reply Ordering + +**Deliverable**: Persistence hooks fire on AFTER_EVENT; `xce_reply()` moved after hooks for sync durability. **Requires: P0-C** (hooks need WAL to write to). + +| Step | File | Description | +|------|------|-------------| +| P0-4a | `src/include/xce/cluster_def.h` + `src/controller/job.c` | Add `HOOK_DETAIL_JOB_STOP` and `HOOK_DETAIL_JOB_RESUME` to the hook detail enum; update `on_job_stop()` to fire `HOOK_DETAIL_JOB_STOP` (currently fires `HOOK_DETAIL_JOB_EXIT`) and `on_job_resume()` to fire `HOOK_DETAIL_JOB_RESUME` — prerequisite for P0-6 WAL_JOB_STOP and WAL_JOB_RESUME hook wiring. **Audit required**: grep all call sites in `job.c` and `worker.c` that invoke `xce_run_hooks` with `HOOK_DETAIL_JOB_EXIT` to confirm that only genuinely terminal events (job done/fail/error) remain on that hook detail; any call site that fires `HOOK_DETAIL_JOB_EXIT` for a non-terminal event (stop, resume) must be updated. | +| P0-5a | `src/include/xce/cluster_def.h` + `types.h` | Add entity terminal states: `XCE_WORKER_UNAVAIL` (already exists in `types.h:165`), `XCE_WORKER_HIDDEN`, `QUEUE_STAT_HIDDEN`; add `JOB_STAT_TRANSIENT = 0x400` to job status bitmask (**must not use `0x200`** — `JOB_STAT_WAIT` already occupies `0x200`, see `types.h:351`). This fits in the existing `uint32_t state` field (current highest bitmask is `0x40000` for `JOB_STAT_ERR`). The scheduler must skip workers that own transient jobs (no new work dispatched to them). API queries must show transient jobs as status `unknown` or `transient`. Worker reconnection resolves the transient state by setting the job's actual terminal/running status (the worker reports job completion/exit). If the worker never reconnects, the job remains transient until operator intervention (same as existing `WORKER_UNAVAILABLE` behavior for workers). | +| P0-6 | `src/controller/persist.c` | Hook functions: `persist_job_hook_fun`, `persist_queue_hook_fun`, `persist_worker_hook_fun` (requires P0-4a for `HOOK_DETAIL_JOB_STOP` and `HOOK_DETAIL_JOB_RESUME` wiring) | | P0-7 | `src/controller/job.c` | Move `xce_reply()` after `AFTER_EVENT` hooks in `handle_jobsubmit()`; verify no kvmgr lock held during hooks (see Part 8.3 lock safety analysis) | -| P0-8 | `src/controller/worker.c` | Extend existing `mark_worker_unavail()` with `disconnect_time`; add `WORKER_HIDDEN` state path; already deferred — no `zarray_del` removal needed (see Part 4.10) | -| P0-8a | `src/controller/cluster.c` | Replace `zarray_del(&(c->endpoints), ep->idx)` in `handle_endpoint_error()` with deferred endpoint cleanup: close fd/SSL immediately, set `ENDPOINT_DISCONNECTED` state, cleanup slot at checkpoint (see Part 4.9) | -| P0-9 | `controller.capnp` | Add `PersistConfig`, `WALEntry`, `SnapshotHeader`, `Snapshot` schemas; add `HostState.unavail`, `HostState.hidden` | -| P0-10 | `cluster_def.h` | Add `persist_t persist;` to `cluster_t` | -| P0-11 | `cluster.c` | Wire `persist_init/recover/start/stop/fini` into `cluster_init/run/fini` | -| P0-12 | `myconf.c` + YAML config | Parse persistence configuration | -| P0-13 | `worker.c` | Wire `restore_worker()` into recovery path | -| P0-14 | Tests | WAL write/read round-trip, COW freeze/serialize/thaw, checkpoint+recover, hook invocation, retention/GC, endpoint deferred cleanup | + +--- + +#### P0-E: Deferred Cleanup & GC + +**Deliverable**: Entity lifecycle management — terminal entities cleaned at checkpoint, endpoints deferred. **Requires: P0-C** (checkpoint GC needs thaw infrastructure). + +| Step | File | Description | +|------|------|-------------| +| P0-8 | `src/controller/worker.c` | Extend existing `mark_worker_unavail()` to set `w->host.dead_since = time(NULL)` (field already exists in `xce_worker_t`); add `XCE_WORKER_HIDDEN` bitmask path; already deferred — no `zarray_del` removal needed (see Part 4.10) | +| P0-8a | `src/controller/cluster.c` | Replace `zarray_del()` at line ~85 and `zarray_clear()` at line ~322 in `handle_endpoint_error()` and endpoint finalization with deferred endpoint cleanup: close fd/SSL immediately, set `ENDPOINT_DISCONNECTED` state, cleanup slot at checkpoint (requires P0-0a zarray merge to enable COW for endpoints; until then, keep existing `zarray_del()`/`zarray_clear()`) | +| P0-13 | `job.c` | Implement `job_is_terminal()` (check `JOB_STAT_EXIT | JOB_STAT_DONE | JOB_STAT_PERR` bitmask) and `job_has_dependents()` (check pending_ctrl / array dependency tracking) — required by Part 4.5 retention callbacks | +| P0-14 | `kvmgr.c` + `htab.c` | Implement `kvtab_gc_stale_indices()` — walk hash chains, remove entries whose mapped zarray index has `inuse=0`. Called during thaw while kvmgr write lock is held. Internally uses `ht_del()` (not `del_key` — remains NULL per Part 4.1 invariant) | + +--- + +#### P0-F: Schema, Config, & Cluster Wiring + +**Deliverable**: Full end-to-end — config parsed from YAML, schemas defined, persistence wired into cluster lifecycle. **Requires: P0-D + P0-E**. + +| Step | File | Description | +|------|------|-------------| +| P0-9 | `controller.capnp` | Add `PersistConfig`, `WALEntry`, `SnapshotHeader`, `Snapshot` schemas; add `version` field (`UInt32`, default `1`) to `SnapshotHeader` for format version detection; add `deadSince` to `Worker` schema; add bitmask flags for `XCE_WORKER_HIDDEN`, `QUEUE_STAT_HIDDEN`; add `jobStatTransient = 0x400` constant comment in Job schema (`status` remains `UInt32` bitmask; **not** `0x200` which is `JOB_STAT_WAIT`) | +| P0-10 | `cluster.c` | Wire `persist_init/recover/start/stop/fini` into `cluster_init/run/fini` | +| P0-11 | `myconf.c` + YAML config | Parse persistence configuration | + +--- + +#### P0-G: Integration Tests + +**Deliverable**: Full persistence test suite. **Requires: P0-F**. + +| Step | File | Description | +|------|------|-------------| +| P0-15 | Tests | WAL write/read round-trip, COW freeze/serialize/thaw, checkpoint+recover, hook invocation, retention/GC, endpoint deferred cleanup | ### P1: HA Foundation (LSF-Style Election) @@ -1724,14 +2581,14 @@ This file is already read by the controller for its own candidate configuration | Step | File | Description | |------|------|-------------| -| P1-1 | `src/controller/ha.c` + `include/ha.h` | Role management, `am_i_primary()`, promotion/demotion | -| P1-2 | `src/controller/ha.c` | Monitor thread: probe higher-priority candidates | -| P1-3 | `src/controller/ha.c` | Promotion sequence: stop replay, increment term, recover, start primary | -| P1-4 | `controller.capnp` | Add `term` to Request, `standby` to HostRole, `replEntry/replAck/redirect` opcodes, `RedirectInfo` struct | -| P1-5 | `cluster_def.h` | Add `ha_state_t ha;` to `cluster_t` | -| P1-6 | `cluster.c` | Wire HA into startup sequence | -| P1-7 | `join.c` | Add redirect logic in `handle_join()` — standby sends REDIRECT for worker/scheduler/apiserver join requests | -| P1-8 | Tests | Election logic, promotion sequence, term fencing, redirect response | +| P1-1 | `src/controller/netio.c` + `src/include/xce/netio.h` | Extend `create_ssl_context()` to accept `ca_cert_path` parameter for mTLS peer verification; update all existing callers to pass NULL for the new parameter (backward compat not needed — no released version). See Part 5.9 | +| P1-2 | `src/controller/ha.c` + `include/ha.h` | Role management, `am_i_primary()`, promotion/demotion | +| P1-3 | `src/controller/ha.c` | Heartbeat watchdog: `OP_heartbeat` exchange via iomgr/vtask pipeline; `ha_check_heartbeat()` on a **dedicated `pthread` with `nanosleep()` or `timerfd_create(CLOCK_MONOTONIC)`** (NOT `delaymgr` — see Part 5.5 heartbeat detection latency note) detects primary failure (replaces `tcp_probe`) | +| P1-3a | `src/controller/ha.c` | Promotion sequence: stop replay, increment term, recover, start primary; SIGUSR1 handler for manual failover (see Part 5.8) | +| P1-4 | `controller.capnp` | Add `term` to Request, `standby` to HostRole, `replEntry/replAck/redirect/heartbeat/steppingDown` opcodes, `RedirectInfo` struct | +| P1-5 | `cluster.c` | Wire HA into startup sequence; add `ha_state_t ha;` to `cluster_t` in this step (note: `persist_t persist;` field was already added to `cluster_t` in P0-5) | +| P1-6 | `join.c` | Add redirect logic in `handle_join()` — standby sends REDIRECT for worker/scheduler/apiserver join requests | +| P1-7 | Tests | Election logic, promotion sequence, term fencing, redirect response | ### P2: Replication (Fan-Out TCP) @@ -1740,26 +2597,30 @@ This file is already read by the controller for its own candidate configuration | Step | File | Description | |------|------|-------------| | P2-1 | `src/controller/repl.c` + `include/repl.h` | Replicator: fan-out send via endpoint/iomgr SSL pipeline, ACK tracking; `standby_conn_t.ep` replaces `standby_conn_t.fd` (see Part 6.5) | -| P2-2 | `src/controller/repl.c` | Standby receiver: `OP_replEntry` dispatched via iomgr/vtask pipeline; `OP_replAck` sent via endpoint outq (not raw `send()`) | -| P2-3 | `src/controller/repl.c` | Sync replication: `repl_wait_ack()` with timeout; only triggered for `WAL_JOB_SUBMIT` from API server connections (1-3 connections, no pool exhaustion risk) | +| P2-2 | `src/controller/repl.c` | Standby receiver: `OP_replEntry` dispatched via iomgr/vtask pipeline; `OP_replAck` sent via endpoint outq (not raw `send()`). `apply_wal_entry()` does not fire hooks — persistence hooks are registered only on the primary path (in `cluster_init()` after recovery completes, or in `ha_promote_to_primary()` step 5 on promotion). No bypass flag is needed; the hook system is empty on standbys. Standbys apply WAL entries the same way recovery does (see Part 4.2 recovery) | +| P2-3 | `src/controller/repl.c` | Sync replication: `repl_wait_ack()` with timeout; only triggered for `WAL_JOB_SUBMIT` from API server connections. **ACK notification mechanism**: when the standby's `OP_replAck` arrives via iomgr/vtask, it must wake the blocked `repl_wait_ack()` call. Implement with a per-standby condition variable (`pthread_cond_t`) under `repl.lock`: the vtask handler signals it on receiving `OP_replAck`; `repl_wait_ack()` parks on it with `pthread_cond_timedwait()`. This keeps the blocking confined to the vtask thread running the `handle_jobsubmit()` path. **Pool floor note**: the default vcpu pool is 8 (`myconf.c` default). Under burst job submission, 8 simultaneous sync-replicating submits exhaust the pool. Clusters using sync replication should configure `vcpus >= 16`: the reasoning is that each blocked `repl_wait_ack()` occupies one vcpu for up to `sync_timeout_ms` (50ms default); at the typical API server connection count of 1–3, the realistic burst concurrency is low, but with multiple API servers sending simultaneous submits the pool can saturate quickly. 16 vcpus provides 2× headroom over the default and headroom for non-submit vtasks (worker joins, heartbeats) that must not be starved. Clusters with only 1 API server and low submit rates can stay at 8. **Startup enforcement**: `persist_start()` must log a `WARN` at startup if sync replication is enabled (`sync_timeout_ms > 0`) and the configured `vcpus < 16`: `WARN("sync replication enabled with vcpus=%d (< 16); recommend vcpus >= 16 to prevent repl_wait_ack pool saturation", vcpus)`. This is a warning, not a hard failure, so low-concurrency deployments can still operate. Operators should monitor `ctrl_vcpu_queue_depth` and increase `vcpus` if it stays above 0 during normal operation. **ACK deadlock prevention**: `repl_on_ack()` must be callable directly from the io_worker read callback (i.e., from `handle_msg()` for `OP_replAck` before vtask dispatch) — it only does `pthread_cond_signal()` and an `acked_lsn` update under `repl.ack_lock`, which is safe to call from io_worker without going through vtaskmgr. If `OP_replAck` were dispatched via vtaskmgr like all other messages, a pool of 8 vcpus all blocked in `repl_wait_ack()` would deadlock: the incoming ACK sits in the vtask queue, but no vcpu is free to process it. By handling ACKs in io_worker directly, the signal wakes a sleeping vcpu regardless of pool saturation. **Worker join lock constraint**: `handle_worker_join()` holds workermgr WRITE lock during `add_worker()` + AFTER_EVENT hooks + `xce_reply()`. The persistence hook for `HOOK_DETAIL_WORKER_JOIN` must use `persist_wal_async()` only — worker join is always async-replicated and must never block on `repl_wait_ack` while the write lock is held | | P2-4 | `src/controller/persist.c` | Wire sync/async replication into `persist_wal_sync_append()` / `persist_wal_async()` | -| P2-5 | `src/controller/repl.c` | Standby catch-up: when standby reconnects via endpoint, stream missing WAL entries | -| P2-5a | `src/controller/join.c` | Resolve `handle_worker_join()` WRITE lock during hooks — currently holds workermgr write lock during `add_worker()` + AFTER_EVENT hooks; if sync replication is needed for worker join, must release write lock before blocking (or accept that worker join is always async-replicated) | +| P2-5 | `src/controller/repl.c` | Standby catch-up: when standby reconnects via endpoint, stream missing WAL entries. The primary trims WAL only up to `min(acked_lsn)` across connected standbys (`standby_conn_t.acked_lsn`; falls back to `checkpoint_lsn` when no standbys — see Part 2.4 WAL trim guard). If a reconnecting standby needs WAL entries older than the primary's oldest surviving segment, transfer the latest snapshot first, then stream WAL entries from the snapshot's LSN forward | | P2-6 | Tests | Replication round-trip (via SSL pipeline), sync ACK timeout, standby catch-up | ### P2.5: libxce Smart Discovery -**Goal**: Workers, schedulers, and API servers can discover the primary via any candidate. +> **Note**: P2.5 carries the same implementation scope as P1 or P2 (8 steps). It is numbered 2.5 — not 3 — to reflect that it depends only on P1 and can proceed in parallel with P2 (the controller-side replication). It is independently schedulable, not half the work. + +> **Candidate count ceiling**: `MAX_STANDBYS = MAX_HA_CANDIDATES - 1` (defined in `repl.h`, currently 15) sizes the `standbys[]` array and represents the **structural maximum**. The **deployment ceiling** is ≤3 controllers (1 primary + 2 standbys): the LSF-style priority election is safe only when all candidates are in reliable contact with each other; larger clusters increase split-brain risk and election latency with no operational benefit for HPC scheduling. `xce_cluster_config_t` must enforce the structural ceiling when loading `cluster.yaml` (reject configs with more than `MAX_HA_CANDIDATES` candidates), but operators are strongly advised to deploy no more than 3. Add a `WARN` log at startup if `candidates_count > 3`: `"HA cluster has %d candidates; LSF-style election is validated for ≤3. Larger clusters increase split-brain risk."` + +**Goal**: Workers, schedulers, and API servers can discover the primary via any candidate. **Design decision**: libxce auto-discovery and auto-reconnect are foundational, not optional. All client-side controller connection code in the codebase will be migrated to this path — there is no "raw TCP to controller" fallback after this phase. This ensures the scheduler policy enhancement work that follows operates on a stable HA foundation. | Step | File | Description | |------|------|-------------| | P2.5-1 | `src/lib/include/xce/cluster.h` | Define `xce_cluster_config_t`, `xce_candidate_t`, API functions | -| P2.5-2 | `src/lib/cluster.c` (new) | Implement `xce_connect_cluster()`: candidate selection, redirect following, backoff+jitter | +| P2.5-2 | `src/lib/cluster.c` (new) | Implement `xce_connect_cluster()`: candidate selection, redirect following, backoff+jitter. **SSL**: all controller connections use SSL (enforced by `netio.c`); `xce_connect_cluster()` must perform the TLS handshake and return an SSL-wrapped connection, not a raw fd. Use the same CA cert path from `xce_cluster_config_t` for server certificate verification — `SSL_CTX_set_verify(SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT)` must be set on the client SSL context so the TLS library itself rejects an invalid server certificate before any application-layer CN check runs. The CN validation after handshake (Part 10.6) is defense-in-depth only. Workers and schedulers that previously connected via raw TCP must also go through this SSL path. | +| P2.5-2a | `src/lib/meson.build` | Add `cluster.c` to the `libxce_src` source list so the new file is compiled into `libxce` | | P2.5-3 | `src/lib/cluster.c` | Implement `xce_reconnect()`: auto-reconnect with state machine | | P2.5-4 | `src/lib/cluster.c` | Implement `xce_load_cluster_config()`: parse `cluster.yaml` candidate list | | P2.5-5 | `src/worker/main.c` | Replace hardcoded controller address with `xce_cluster_config_t`; add reconnection loop | | P2.5-6 | `src/scheduler/` | Same pattern as worker — use `xce_cluster_config_t` | -| P2.5-7 | `src/frontend/apiserver/backend/clib.go` | Use `xce_cluster_config_t` for controller discovery in `RegisterToBackend()` | +| P2.5-7 | `src/frontend/apiserver/backend/clib.go` | Use cluster discovery for controller connection in `RegisterToBackend()`. **Decision: use CGo binding (option a)**. Expose `xce_cluster_config_t` and `xce_connect_cluster()` through the existing `clib.go` CGo shim — `clib.go` already imports `xce` headers and wraps C calls for other controller interactions, so adding cluster discovery there is consistent and avoids duplicating config parsing logic in Go. Add a `#include "xce/cluster.h"` import and a `XceConnectCluster(conf *C.xce_cluster_config_t) int` Go wrapper, then call it from `RegisterToBackend()` instead of the current hardcoded dial. | | P2.5-8 | Tests | Candidate selection, redirect following, backoff+jitter, reconnection | ### P3: Integration & Hardening @@ -1773,17 +2634,29 @@ This file is already read by the controller for its own candidate configuration | P3-3 | WAL corruption handling: test torn writes, partial segments, CRC mismatch | | P3-4 | Performance: benchmark 1000 job submits/sec with sync replication | | P3-5 | Checkpoint GC: verify expired jobs/workers/queues are cleaned; verify dependents block cleanup | -| P3-6 | Admin commands: `xce hostcontrol --failover` for manual primary switch | -| P3-7 | Metrics: WAL size, replication lag, checkpoint duration, standby health, entity GC stats | +| P3-6 | Test the manual failover state machine path by calling `ha_demote_to_standby()` directly from a test harness (e.g., a test-only `ha_trigger_manual_failover(c)` shim or a dedicated integration test binary). This validates that the HA state machine correctly handles a manual promotion trigger — role transitions, WAL flush, hook re-registration — without requiring the unimplemented `xce hostcontrol --failover` CLI (see Out of Scope section). | +| P3-7 | Metrics: WAL size, replication lag, checkpoint duration, standby health, entity GC stats. **Metric naming convention**: All HA/persistence metrics use a `ctrl_` prefix (e.g., `ctrl_repl_degraded_count`, `ctrl_split_brain_detected`, `ctrl_async_events_lost`) to namespace them within the controller's existing metrics perimeter. Metric names must match the pattern `ctrl_[a-z_]+` and should be registered in a central metrics table with a description and type (counter/gauge/histogram). The implementation should define this table as part of `persist.c` or `ha.c` with a `metrics_init()` call in `persist_start()` / `ha_init()`. **Split-brain alert**: log a `WARN`-level message and expose a `ctrl_split_brain_detected` counter whenever a term conflict is detected (primary receives a message with a higher term than its own, or standby detects two primaries). This is the minimum operator signal that the cluster has entered a split-brain state. **Async event loss counter**: expose `ctrl_async_events_lost` — incremented for each async WAL entry that was in-flight (written locally but not yet replicated) when the primary failed. Computed by the newly promoted standby during promotion: `events_lost = primary_last_lsn - standby_applied_lsn` (where `primary_last_lsn` is recovered from the old primary's WAL at promotion time, if available; otherwise log `WARN("async event loss count unavailable — primary WAL not accessible")`). This gives operators visibility into the amount of async data loss after a failover. | ### P4: Cleanup & Documentation | Step | Description | |------|-------------| -| P4-1 | Remove dead code (Part 1) | -| P4-2 | Update `src/frontend/docs/` to remove stale PostgreSQL references | -| P4-3 | Update CLAUDE.md files with persistence/HA architecture | -| P4-4 | Operational runbook: how to add/remove standby, manual failover, disaster recovery | +| P4-1 | Update `src/frontend/docs/` to remove stale PostgreSQL references | +| P4-2 | Update CLAUDE.md files with persistence/HA architecture | +| P4-3 | Operational runbook: how to add/remove standby, manual failover, disaster recovery (include CA/cert provisioning requirements per Part 5.9) | +| P4-4 | Operational runbook section on certificate rotation: document how to rotate controller mTLS certs (update cert file, restart standbys one at a time, then restart primary). Include the `ha_cert_warn_days` (default 30) monitoring check. Add a guided `openssl` / `cfssl` procedure for issuing new controller certs signed by the cluster CA. | + +### Out of Scope + +The following are explicitly **not** covered by this design document and are deferred to future work: + +- **Admin CLI for manual failover**: `xce hostcontrol --failover` is not designed here. Manual failover is exercised in P3-6 via `ha_demote_to_standby()` called from a test harness. A production CLI surface for this operation is deferred to future work +- **WAL compaction beyond checkpoint trimming**: Checkpoint-based WAL segment deletion is specified, but online compaction (merging small segments, garbage-collecting within segments) is out of scope +- **Multi-data-center replication**: All replication targets are same-DC controllers connected via a dedicated management network. Cross-DC replication would require bandwidth-aware batching and is not addressed +- **Monitoring/alerting integration**: P3-7 lists metrics to expose, but the integration with Prometheus, Nagios, or other monitoring systems is out of scope +- **Runtime configuration changes**: Adding/removing standby candidates at runtime, changing `persist_config_t` values without restart, and online checkpoint interval adjustment are not designed here +- **Partial WAL re-sync**: After partition healing, the standby performs full re-sync from the primary. Incremental re-sync (sending only divergent entries) is out of scope +- **Snapshot compression**: Snapshots are stored as uncompressed Cap'n Proto. Compression (zstd, lz4) is a potential optimization deferred to future work --- @@ -1792,7 +2665,10 @@ This file is already read by the controller for its own candidate configuration ### A.1 Persistence Module ```c -/* persist.h */ +/* persist.h — see Appendix A.10 for system-level tunable constants in include/config.h */ + +#include "config.h" /* WAL_FORMAT_VERSION, MAX_SNAPSHOT_BYTES, MAX_WAL_PAYLOAD_BYTES, + WAL_ENTRY_TYPE_MAX, WAL_HDR_CRC_SIZE, DEFAULT_WAL_SYNC_MS, etc. */ typedef struct { int wal_fd; /* Current WAL segment fd */ @@ -1801,11 +2677,14 @@ typedef struct { uint64_t term; /* Current election term */ char seg_path[XCE_MAXPATH_LEN]; /* Current segment path */ uint32_t seg_offset; /* Write offset in current segment */ + uint64_t checkpoint_lsn; /* LSN of last completed checkpoint (for WAL trim guard) */ persist_config_t config; cluster_t* cluster; /* Back-reference */ - pthread_t ckpt_tid; /* Checkpoint thread */ - int running; /* Flag */ mylock_t lock; /* Protects WAL write + LSN */ + atomic_int checkpoint_in_progress; /* Non-zero while checkpoint is active */ + atomic_int checkpoint_requested; /* Set by inline WAL size trigger (Part 3.11); + cleared by checkpoint timer after completing */ + pthread_cond_t checkpoint_done; /* Signaled when checkpoint completes or is canceled */ } persist_t; int persist_init(persist_t* p, persist_config_t* config, cluster_t* c); @@ -1813,10 +2692,32 @@ int persist_start(persist_t* p); void persist_stop(persist_t* p); void persist_fini(persist_t* p); +/* Flush the current WAL segment to disk (fsync). + * Called during promotion (Part 5.6 step 3) to ensure all replicated + * entries are durable before the standby takes over as primary. */ +int persist_flush(persist_t* p); + /* Async WAL write — returns immediately after writing + sending to standbys */ int persist_wal_async(persist_t* p, uint32_t type, void* payload, uint32_t len); -/* Sync WAL write — waits for at least one standby ACK before returning */ +/* Sync WAL write — writes+fsyncs locally, then waits for at least one standby ACK. + * Return values distinguish the two failure modes: + * 0 — success (written + fsynced + ACKed) + * -EIO — local write or fsync failed (permanent; do not send to client) + * -ETIMEDOUT — wrote locally but no standby ACKed within sync_timeout_ms + * (transient; client may retry; entry is locally durable) + * Callers must treat -EIO and -ETIMEDOUT differently: -EIO aborts the operation, + * -ETIMEDOUT can be surfaced to the client as a "degraded durability" warning. + * + * IMPORTANT: -ETIMEDOUT is a durability warning, not a consistency error. When + * -ETIMEDOUT is returned, the in-memory mutation (e.g., job_submit()) has already + * completed — the job ID is assigned, the kvmgr entry exists, and the local WAL + * entry is fsynced to disk. There is NO rollback of the in-memory state. The + * persistence hook call from AFTER_EVENT cannot undo the business logic that + * preceded it. Callers that receive -ETIMEDOUT should still reply to the client + * with the result (e.g., the jobid), and log the degraded durability event. + * The entry will be replicated to standbys when they reconnect (see Part 7.4 + * degraded mode self-healing). */ int persist_wal_sync_append(persist_t* p, uint32_t type, void* payload, uint32_t len); /* Checkpoint — dual zarray freeze + serialize + thaw + GC */ @@ -1829,6 +2730,39 @@ int persist_recover(persist_t* p, cluster_t* c); int persist_job_hook_fun(int type, int situation, int detail, void* data, va_list ap); int persist_queue_hook_fun(int type, int situation, int detail, void* data, va_list ap); int persist_worker_hook_fun(int type, int situation, int detail, void* data, va_list ap); + +/* Apply a single WAL entry to in-memory state — used by both persist_recover() + * and the standby replication receiver. Persistence hooks are registered only + * on the primary path (after persist_recover() completes in cluster_init(), or + * in ha_promote_to_primary() step 5 on promotion), so apply_wal_entry() does + * not fire hooks — the hook system is empty during recovery and on standbys. + * + * Semantics: + * WAL_JOB_SUBMIT — calls restore_job(c, jobid, payload) to insert the job at its + * original jobid without allocating a new one. Do NOT call + * job_submit() here — job_submit() assigns a new jobid, which + * breaks replay by creating duplicate or wrong-ID jobs. + * If the jobid already exists, restore_job() returns -EEXIST; + * apply_wal_entry() logs a WARN and skips the entry (non-fatal). + * WAL_JOB_START — updates job stat, start_time, exec_host in-place + * WAL_JOB_FINISH — updates job stat, end_time, exit_code in-place + * WAL_JOB_FAIL — updates job stat, end_time, fail_reason in-place + * WAL_JOB_STOP — updates job stat to JOB_STAT_USUSP in-place + * WAL_JOB_RESUME — updates job stat from USUSP back to PEND in-place + * WAL_JOB_UPDATE — merges partial Job payload into existing job object + * WAL_JOB_CKPT — updates job ckpt_time in-place + * WAL_JOB_RUSAGE — updates job rusage fields in-place + * WAL_JOB_PREASON — updates job preason field in-place + * WAL_QUEUE_CTRL — creates or updates queue from QueueConf payload + * WAL_WORKER_CTRL — creates or updates worker from Worker payload + * WAL_CHECKPOINT — no-op (marker only, no payload) + * + * Returns 0 on success, -errno on deserialization or mutation failure. + * Unknown entry types are WARN-logged and skipped (return 0) to allow forward + * compatibility with future WAL types added by newer controller versions. + * WARNING: Skipping unknown entries during recovery silently loses the + * state changes those entries represent — see recovery forward-compat note. */ +int apply_wal_entry(cluster_t* c, const wal_entry_hdr_t* hdr, const void* payload); ``` ### A.2 zarray COW Extensions @@ -1836,35 +2770,56 @@ int persist_worker_hook_fun(int type, int situation, int detail, void* data, va ```c /* zarray.h — additions for COW snapshot support */ +/* NOTE on struct layout: Part 3.4 defines xzarray_private_t (the RTTI private + * section) as the home for shadow and shadow_caps fields, accessed via + * RTTI_OBJ_TO_PRIV(). The struct definition below shows these fields as members + * of xzarray_t for readability, but in the actual implementation they live in + * the private section and are accessed as priv->shadow and priv->shadow_caps. + * The RTTI type system allocates the private section alongside each instance. + * This is the same pattern used by xzarray_private_t.lock (mylock_t). */ + typedef struct { int count; size_t caps; size_t delta; int pos; xzarray_item_t* data; /* Active array */ - xzarray_item_t* shadow; /* Frozen snapshot (NULL = normal mode) */ - size_t shadow_caps; /* Shadow capacity when in COW mode */ destroy_fn_t killer; } xzarray_t; +/* Private section — allocated by makeInstance() alongside each xzarray_t instance. + * Accessed via RTTI_OBJ_TO_PRIV(obj). See Part 3.4 for the design rationale. */ +typedef struct { + mylock_t lock; + xzarray_item_t* shadow; /* Frozen snapshot (NULL when not in COW) */ + size_t shadow_caps; /* Shadow array capacity */ +} xzarray_private_t; + /* Freeze: swap data → shadow, create new empty active. - Caller must hold write lock. O(1) — pointer swap only. */ -void xzarray_freeze(xzarray_t* a); + Caller must hold write lock. O(1) — pointer swap only. + Returns 0 on success, -ENOMEM on allocation failure. + Public API: vfreeze(var) — RTTI dispatches to xzarray_freeze(meta_obj_t*). + Do not call xzarray_freeze(xzarray_t*) directly; it is a file-private + static function that takes meta_obj_t*, not xzarray_t*. */ +extern int vfreeze(var); /* Thaw: merge shadow back into active, apply GC. should_retain(arg, item) returns true if entity should survive. Caller must hold write lock. - Frees shadow when done. */ -typedef bool (*retain_fn_t)(void* arg, void* item); -void xzarray_thaw(xzarray_t* a, retain_fn_t should_retain, void* arg); + Frees shadow when done. Returns 0 on success. + Public API: vthaw(var, should_retain, arg) — RTTI dispatches to xzarray_thaw(meta_obj_t*, retain_fn_t, void*). + Do not call xzarray_thaw(xzarray_t*, ...) directly; it is a file-private + static function that takes meta_obj_t*, not xzarray_t*. */ +/* retain_fn_t is defined in rtti/typeinfo.h (see Appendix A.9) */ +extern int vthaw(var, retain_fn_t should_retain, void* arg); ``` ### A.3 Replication Module ```c -/* repl.h */ +/* repl.h — see Appendix A.10 for MAX_STANDBYS and other tunables in include/config.h */ -#define MAX_STANDBYS (N_CONTROLLER_MAX - 1) +#include "config.h" /* MAX_STANDBYS, DEFAULT_SYNC_TIMEOUT_MS */ typedef struct { endpoint_t* ep; /* Full endpoint (SSL, FSM, outq) — uses iomgr/vtask pipeline */ @@ -1881,12 +2836,24 @@ typedef struct { uint64_t pending_lsn; pthread_cond_t ack_cond; mylock_t ack_lock; + uint64_t last_disconnected_lsn[MAX_STANDBYS]; /* Per-slot: acked_lsn at disconnect + time, for one-interval trim delay */ + struct timespec last_disconnect_time[MAX_STANDBYS]; /* Per-slot: when the standby + disconnected, for trim delay */ } replicator_t; /* Fan-out send to all standbys */ int repl_broadcast(replicator_t* r, wal_entry_hdr_t* hdr, void* payload); -/* Wait for at least one standby to ACK the given LSN (sync mode) */ +/* Wait for at least one standby to ACK the given LSN (sync mode). + * Implementation MUST check the predicate before waiting to avoid a + * signal-before-wait race: if repl_on_ack() signals ack_cond before + * repl_wait_ack() parks on it, the signal is lost. Correct pattern: + * lock(ack_lock); + * while (r->acked_lsn < target_lsn && !timed_out) + * pthread_cond_timedwait(&r->ack_cond, &r->ack_lock, &deadline); + * unlock(ack_lock); + * The while-loop predicate check must be inside the lock, before the first wait. */ int repl_wait_ack(replicator_t* r, uint64_t target_lsn, uint32_t timeout_ms); /* Called when ACK received from a standby */ @@ -1912,17 +2879,20 @@ typedef struct { int my_priority; /* My index in candidates[] */ replicator_t repl; /* Replicate to standbys (primary) */ uint64_t applied_lsn; /* Last applied WAL entry (standby) */ - endpoint_t* primary_ep; /* Connection to current primary via endpoint/iomgr SSL pipeline (standby) */ + endpoint_t* primary_ep; /* Connection to current primary via iomgr SSL pipeline (standby) */ char primary_host[XCE_MAXNAME_LEN]; uint32_t primary_port; - pthread_t monitor_tid; /* Monitors higher-priority candidates */ + struct timespec last_heartbeat; /* Last heartbeat from primary (CLOCK_MONOTONIC, standby) */ + int promotion_failed; /* Non-zero if last promotion attempt failed — retry on next watchdog tick */ + int promotion_retry_count; /* Number of consecutive failed promotion attempts */ + ha_config_t config; /* HA configuration (timeouts, intervals) */ } ha_state_t; /* Check if this controller should be primary */ static int am_i_primary(cluster_t* c); -/* Monitor thread — runs on standby, watches higher-priority candidates */ -static void* ha_monitor_thread(void* arg); +/* Heartbeat watchdog — called from delaymgr timer on standby */ +static void ha_check_heartbeat(cluster_t* c); /* Promote self from standby to primary */ int ha_promote_to_primary(cluster_t* c); @@ -1959,12 +2929,15 @@ struct WALEntry $C.mapname("wal_entry_t") { # Snapshot header — first record in a checkpoint file struct SnapshotHeader $C.mapname("snapshot_hdr_t") { - lsn @0 :UInt64; - timestamp @1 :UInt64; - term @2 :UInt64; - nJobs @3 :UInt32; - nQueues @4 :UInt32; - nWorkers @5 :UInt32; + lsn @0 :UInt64; + timestamp @1 :UInt64; + term @2 :UInt64; + version @7 :UInt32 = 1; # WAL format version — bumped on incompatible schema changes + nJobs @3 :UInt32; + nQueues @4 :UInt32; + nWorkers @5 :UInt32; + checksum @6 :UInt32; # CRC32 over entire serialized Snapshot body (excluding this header) + headerChecksum @8 :UInt32; # CRC32 over header fields (lsn, timestamp, term, version, nJobs, nQueues, nWorkers) — protects security-critical term and lsn from tampering } # Full snapshot — checkpoint file contents @@ -1974,7 +2947,7 @@ struct Snapshot $C.mapname("snapshot_t") { queues @2 :List(QueueConf); workers @3 :List(Worker); resources @4 :List(Resource); - configs @5 :List(Config); + configs @5 :List(ConfigVar); } # Redirect response — standby tells client where the primary is @@ -1984,17 +2957,20 @@ struct RedirectInfo $C.mapname("xce_redirect_t") { } # In RequestReply union — add redirect variant: -# redirect @7 :RedirectInfo $C.mapname("redirect"); +# redirect @8 :RedirectInfo $C.mapname("redirect"); # New error code in common/ # #define E_XCE_REDIRECT (-100) /* Not an error — redirect instruction */ +# #define EXCE_ERR_REDIRECT_LOOP (-101) /* Too many redirect hops (>= 3) */ # Entity terminal states for deferred cleanup -# Added to HostState enum: -# unavail — Worker daemon died / disconnected -# hidden — Admin explicitly removed entity -# Added to QueueStatus enum: -# hidden — Admin deleted queue +# Worker status uses bitmask flags on UInt32, not enum (see Part 4.4): +# #define XCE_WORKER_UNAVAIL (0x00010000) — Worker daemon died / disconnected +# #define XCE_WORKER_HIDDEN (0x00200000) — Admin explicitly removed entity +# Job status — add transient bitmask (NOTE: 0x200 is JOB_STAT_WAIT, use 0x400): +# #define JOB_STAT_TRANSIENT (0x400) — Job state uncertain after failover +# Queue status — add hidden bitmask: +# #define QUEUE_STAT_HIDDEN (0x0100) — Admin deleted queue ``` ### A.6 libxce Discovery Module @@ -2008,12 +2984,13 @@ typedef struct { } xce_candidate_t; typedef struct { - xce_candidate_t candidates[N_CONTROLLER_MAX]; + xce_candidate_t candidates[MAX_HA_CANDIDATES]; int ncandidates; /* Number of candidates configured */ uint32_t connect_timeout_ms; /* Per-candidate connect timeout (default 2000) */ uint32_t retry_base_ms; /* Backoff base (default 100) */ uint32_t retry_max_ms; /* Backoff cap (default 5000) */ int max_retries; /* Max reconnection attempts (default 10) */ + char ca_cert_path[XCE_MAXPATH_LEN]; /* CA cert for TLS server verification (PEM) */ } xce_cluster_config_t; /* Connect to the cluster — returns fd connected to primary */ @@ -2033,7 +3010,8 @@ int xce_load_cluster_config(const char* yaml_path, xce_cluster_config_t* conf); | File | Purpose | Approximate Size | |------|---------|-----------------| -| `src/controller/persist.c` | WAL writer, checkpoint, recovery, hook functions | ~800 LOC | +| `src/controller/persist_wal.c` | WAL writer, segment management, fsync batching | ~400 LOC | +| `src/controller/persist_snap.c` | Checkpoint, recovery, hook functions | ~400 LOC | | `src/controller/include/persist.h` | Persistence API and types | ~80 LOC | | `src/controller/repl.c` | Fan-out replication via endpoint/iomgr SSL pipeline | ~500 LOC | | `src/controller/include/repl.h` | Replication API and types | ~60 LOC | @@ -2047,14 +3025,150 @@ int xce_load_cluster_config(const char* yaml_path, xce_cluster_config_t* conf); | File | Change | |------|--------| -| `src/lib/zarray.c` + `include/zarray.h` | Add `shadow`, `shadow_caps` fields; add `xzarray_freeze()`, `xzarray_thaw()`; modify `vget`, `vput`, `iter_next` for COW mode | -| `src/controller/job.c` | Move `xce_reply()` after `AFTER_EVENT` in `handle_jobsubmit()`; remove dead `NJOBS`/`jobid`/`load_jobs` body | -| `src/controller/cluster.c` | Wire `persist_init/recover/start/stop/fini` and HA into lifecycle; wire `cluster_load_jobs()` to persistence loader; replace `zarray_del(&(c->endpoints), ep->idx)` in `handle_endpoint_error()` with deferred endpoint cleanup (close fd/SSL, set `ENDPOINT_DISCONNECTED`, GC at checkpoint); use `delaymgr` for checkpoint timer | -| `src/controller/join.c` | Add redirect logic in `handle_join()` — standby sends `E_XCE_REDIRECT` + `RedirectInfo` for worker/scheduler/apiserver join requests; resolve WRITE lock held during hooks — worker join should use async replication (never blocks on `repl_wait_ack` while holding workermgr write lock) | -| `src/controller/worker.c` | Extend existing `mark_worker_unavail()` with `disconnect_time`; add `WORKER_HIDDEN` state path; `handle_worker_leave()` already calls `mark_worker_unavail()` and sets `w->ep = NULL` — no structural change needed; wire `restore_worker()` into recovery path | -| `src/include/xce/cluster_def.h` | Add `persist_t persist;` and `ha_state_t ha;` to `cluster_t`; add `WORKER_UNAVAILABLE`, `WORKER_HIDDEN` entity states | -| `src/schema/controller.capnp` | Add `PersistConfig`, `WALEntry`, `SnapshotHeader`, `Snapshot`, `RedirectInfo` schemas; add `term` to Request; add `standby` to HostRole; add `replEntry/replAck/redirect` opcodes; add `redirect` variant to `RequestReply`; add `HostState.unavail`/`HostState.hidden` | +| `src/lib/zarray.c` + `include/zarray.h` | Merge `zarray_t` into `xzarray_t` (P0-0a); add `shadow`, `shadow_caps` fields; add `xzarray_freeze()`, `xzarray_thaw()`; modify `vget`, `vput`, `iter_next` for COW mode | +| `src/controller/job.c` | Remove dead `NJOBS`/`jobid`/`load_jobs` body (P0-0); move `xce_reply()` after `AFTER_EVENT` in `handle_jobsubmit()` | +| `src/lib/kvmgr.c` | Register `persist_nop_freeze`/`persist_nop_thaw` for the **kvmgr hash table's own** varray interface (`kvmgr_type` in `kvmgr.c`). The kvmgr has TWO varray interfaces: (1) the hash table's varray (kvmgr itself) — which does NOT support COW and uses no-op freeze/thaw; and (2) the data zarray (`xzarray_type` in `zarray.c`) — which IS the COW target and uses the real `xzarray_freeze`/`xzarray_thaw` implementations. An implementer must NOT accidentally register no-ops for the data zarray, or checkpoint COW will silently skip freezing the data array. The `xzarray_type` registration in `zarray.c` already includes `xzarray_freeze`/`xzarray_thaw` slots — it is NOT modified by P0-0b. P0-0b only adds no-op slots to the kvmgr hash table's own varray compound literal in `kvmgr.c`. | +| `src/controller/cluster.c` | Wire `persist_init/recover/start/stop/fini` and HA into lifecycle; wire `cluster_load_jobs()` to persistence loader; deferred endpoint cleanup (after zarray merge); use `delaymgr` for checkpoint timer and heartbeat watchdog | +| `src/controller/include/persist.h` | Add `persist_nop_freeze`/`persist_nop_thaw` static inline no-ops for varray types that don't support COW | +| `src/controller/netio.c` + `src/include/xce/netio.h` | Extend `create_ssl_context()` to accept `ca_cert_path` parameter for mTLS (P1-1); update all existing callers to pass NULL for CA cert | +| `src/controller/join.c` | Add redirect logic in `handle_join()` — standby sends `E_XCE_REDIRECT` + `RedirectInfo` for worker/scheduler/apiserver join requests; worker join WRITE lock is acceptable — async replication only (never blocks on `repl_wait_ack` inside write lock) | +| `src/controller/worker.c` | Extend existing `mark_worker_unavail()` with `w->host.dead_since = time(NULL)`; add `WORKER_HIDDEN` state path; `handle_worker_leave()` already calls `mark_worker_unavail()` and sets `w->ep = NULL` — no structural change needed; wire `restore_worker()` into recovery path | +| `src/include/xce/cluster_def.h` | Add `persist_t persist;` (P0-5) and `ha_state_t ha;` (P1-5) to `cluster_t`; add `WORKER_UNAVAILABLE`, `WORKER_HIDDEN` entity states; worker status uses bitmask flags (e.g., `XCE_WORKER_HIDDEN = 0x00200000`), not state enum | +| `src/schema/controller.capnp` | Add `PersistConfig`, `WALEntry`, `SnapshotHeader`, `Snapshot`, `RedirectInfo` schemas; add `term` to Request; add `standby` to HostRole; add `replEntry/replAck/redirect/heartbeat` opcodes; add `redirect` variant to `RequestReply`; add `deadSince` to Worker; worker `status` remains `UInt32` (bitmask, not enum) | | `src/controller/myconf.c` | Parse persistence and HA configuration from YAML | | `src/worker/main.c` | Replace hardcoded controller address with `xce_cluster_config_t`; add reconnection loop | | `src/scheduler/` | Same pattern as worker — use `xce_cluster_config_t` | -| `src/frontend/apiserver/backend/clib.go` | Use `xce_cluster_config_t` for controller discovery in `RegisterToBackend()` | \ No newline at end of file +| `src/frontend/apiserver/backend/clib.go` | Use `xce_cluster_config_t` for controller discovery in `RegisterToBackend()` | +| `rtti/typeinfo.h` | Add `vfreeze_fn_t`/`vthaw_fn_t` types, grow `intf_varray_t` to 8 slots, add `vfreeze()`/`vthaw()` global API wrappers (see Appendix A.9) | +| `rtti-0.1.0` (nix) | Bump version from 0.0.1, update nix derivation hash and XCE import path | + +### A.9 rtti Library Enhancements + +The rtti library (`rtti-0.0.1`) is maintained alongside XCE and requires the following additions to support the COW snapshot mechanism. Since there are no released versions of rtti, backward compatibility is not a constraint — these changes are made in-place and version bumped to `rtti-0.1.0`. + +#### A.9.1 New Function Pointer Types + +Add to `rtti/typeinfo.h` before the `intf_varray_t` struct definition, after the existing `vclear_fn_t` typedef (around line 172): + +```c +/* COW snapshot support — freeze/thaw for application-level copy-on-write */ +typedef bool (*retain_fn_t)(void* arg, void* item); +typedef int (*vfreeze_fn_t)(meta_obj_t* obj); +typedef int (*vthaw_fn_t)(meta_obj_t* obj, retain_fn_t should_retain, void* arg); +``` + +`retain_fn_t` is the callback used by `vthaw` to decide whether an entity survives checkpoint GC. It returns `true` to retain, `false` to garbage-collect. Passing `NULL` for `should_retain` means "keep all shadow entities, no GC" — used when aborting a checkpoint (merge shadow back without discarding anything). This type must live in rtti (not in `persist.h` or `zarray.h`) because `vthaw_fn_t` references it in the `intf_varray_t` interface — callers that invoke `vthaw(var, my_retain, arg)` need the type visible through the rtti header. + +#### A.9.2 Updated `intf_varray_t` + +Add two new slots after the existing six: + +```c +typedef struct { + vstore_fn_t vstore; + vget_fn_t vget; + vget_vec_fn_t vget_vec; + vput_fn_t vput; + vput_vec_fn_t vput_vec; + vclear_fn_t vclear; + vfreeze_fn_t vfreeze; /* NEW — freeze (O(1) ptr swap) */ + vthaw_fn_t vthaw; /* NEW — thaw + merge + GC */ +} intf_varray_t; +``` + +**Layout**: The struct grows from 24 bytes (6 × 8 byte pointers on LP64) to 32 bytes (8 × 8 byte pointers). The `interface_table_t.varray` field is already a pointer to `intf_varray_t`, so the struct size change is isolated to the varray interface — no other interface structs are affected. All existing compound-literal initializers (like the one in `kvmgr.c:288` and `zarray.c:666`) must be updated to supply the two new NULL initializers. The C99 designated-initializer pattern `{ .vstore = ..., .vget = ..., }` is recommended to make this robust against future reordering. + +#### A.9.3 New Global API Functions + +Add extern declarations after the existing varray API block (around line 259 of `typeinfo.h`): + +```c +extern int vfreeze(var); /* O(1) freeze — pointer swap */ +extern int vthaw(var, retain_fn_t should_retain, void* arg); /* merge shadow + optional GC; should_retain=NULL keeps all */ +``` + +These dispatch through `RTTI_OBJ_TO_TYPE(obj)->interfaces.varray->vfreeze` / `->vthaw`, following the same pattern as the existing `vstore`, `vget`, etc. wrappers. + +#### A.9.4 No-op Implementations in XCE + +librtti provides only the abstract interface — the no-op implementations belong in XCE, not in librtti. Registration sites for varray types that don't support COW supply their own no-ops: + +```c +/* In persist.h — shared no-ops for XCE types that don't support COW */ +static inline int persist_nop_freeze(meta_obj_t* obj) { return 0; } +static inline int persist_nop_thaw(meta_obj_t* obj, retain_fn_t f, void* a) { return 0; } +``` + +Types like kvmgr that don't support COW register these no-ops in their compound literal: + +```c +/* kvmgr.c */ +.vfreeze = (vfreeze_fn_t)persist_nop_freeze, +.vthaw = (vthaw_fn_t)persist_nop_thaw, +``` + +The library dispatch wrappers (`vfreeze()`, `vthaw()`) return `-EINVAL` when the method pointer is NULL — this is a bug catcher, not a feature gate. Every registration site must supply all 8 function pointers; a NULL slot is a programming error. + +#### A.9.5 xzarray Registration + +In `zarray.c`, the `typeinfo_t xzarray_type` registration must update the varray interface compound literal to include the new slots: + +```c +.varray = &(intf_varray_t) { + .vstore = (vstore_fn_t)xzarray_store, + .vget = (vget_fn_t)xzarray_get_vec, /* xzarray_get_vec handles single-get too */ + .vget_vec = (vget_vec_fn_t)xzarray_get_vec, + .vput = (vput_fn_t)xzarray_put, + .vput_vec = (vput_vec_fn_t)xzarray_put_vec, + .vclear = (vclear_fn_t)xzarray_clear, + .vfreeze = (vfreeze_fn_t)xzarray_freeze, /* NEW */ + .vthaw = (vthaw_fn_t)xzarray_thaw, /* NEW */ +}, +``` + +#### A.9.6 Build Integration + +- **Version**: Bump `rtti-0.0.1` → `rtti-0.1.0` in the nix derivation (or equivalent build system metadata) +- **Nix store path**: The updated library gets a new hash; update the import path in XCE's `meson.build` or nix flake accordingly +- **Compiler flag**: No new dependencies; `` (for the `atomic_int election_done` flag in Part 5.4) is C11 and should already be in the toolchain +- **Test coverage**: Add tests for `xzarray_freeze`/`xzarray_thaw` round-trip, COW-mode vstore linear scan, and shadow merge correctness with overlapping indices + +### A.10 System-Level Tunable Constants — `include/config.h` + +All system-level tunable constants are consolidated into `include/config.h` so that operators and developers can find and adjust limits in one place. Module-specific headers (`persist.h`, `repl.h`, `cluster_def.h`) `#include "config.h"` and use these definitions — they do not define their own local copies. + +```c +/* include/config.h — System-level tunable constants for XCE controller + * All #define values below are the system-wide defaults. + * Runtime-overridable values (persist_config_t, ha_config_t) are NOT here — + * they live in their respective config structs and are set from cluster.yaml. */ + +/* --- HA limits --- */ +#define MAX_HA_CANDIDATES 16 /* Structural max candidates in cluster.yaml */ +#define DEFAULT_HB_INTERVAL_MS 3000 /* Primary heartbeat interval (ms) */ +#define DEFAULT_HB_TIMEOUT_MS 10000 /* Standby heartbeat timeout (ms) */ +#define DEFAULT_ELECTION_TIMEOUT_MS 2000 /* Election probe deadline (ms) */ +#define DEFAULT_SYNC_TIMEOUT_MS 50 /* Sync replication ACK timeout (ms) */ +#define DEFAULT_MAX_PROMOTION_RETRIES 5 /* Max failed promotion attempts */ +#define DEFAULT_HA_CERT_WARN_DAYS 30 /* Warn if cert expires within N days */ + +/* --- Persistence limits --- */ +#define WAL_FORMAT_VERSION 1 /* Bumped on incompatible schema changes */ +#define WAL_SEG_SIZE (64 * 1024 * 1024) /* 64 MB pre-allocated WAL segment */ +#define WAL_HDR_CRC_SIZE 32 /* sizeof(wal_entry_hdr_t) - 2*sizeof(uint32_t) */ +#define MAX_WAL_PAYLOAD_BYTES (1u << 20) /* 1 MB per WAL entry */ +#define WAL_ENTRY_TYPE_MAX 100 /* Highest valid entry type — bump when new types added */ +#define MAX_SNAPSHOT_BYTES (256 * 1024 * 1024) /* 256 MB — reject larger snapshots */ + +/* --- Persistence defaults (runtime-overridable via persist_config_t) --- */ +#define DEFAULT_WAL_SYNC_MS 10 /* fsync interval (ms) */ +#define DEFAULT_SNAP_INTERVAL_S 60 /* checkpoint interval (s) */ +#define DEFAULT_SNAP_WAL_SIZE_MB 64 /* checkpoint when WAL exceeds this (MB) */ +#define DEFAULT_FINISHED_RETAIN_S 3600 /* retain finished jobs (s) */ +#define DEFAULT_WORKER_RETAIN_S 86400 /* retain unavailable workers (s) */ + +/* --- Derived --- */ +#define MAX_STANDBYS (MAX_HA_CANDIDATES - 1) /* 15 — structural; ≤3 deployment ceiling */ +``` + +**Design rule**: If a constant affects more than one module or is an operator-visible limit, it belongs in `config.h`. If it is a purely internal implementation detail of a single module (e.g., an internal struct size or a local retry count), it may stay in the module header. The test: if an operator would ask "what's the maximum?" or a developer would search for the constant when debugging, it goes in `config.h`. \ No newline at end of file