myxce/.kiro/specs/config-upgrade-snapshot/tasks.md
Rongsong Shen 89b85e1680 feat(config-upgrade-snapshot): gate primary serving and promotion on the activation fence
Wires task 6.1's upgrade_materialization_verify/upgrade_materialization_fenced
into ha_dispatch_gate's CTRL_PRIMARY branch (sequential block alongside,
not merged into, the existing config_serve_fenced check -- same
replication/read-only opcode allowlist, refuses state-changing ops
with -XCE_ERR_NOT_READY while fenced) and the promotion gate
(raft_cb_become_leader refuses to become CTRL_PRIMARY while fenced,
introduces no new WAL/log-completeness eligibility mechanism, never
self-terminates). Adds a 4-function self-triggering retry trampoline
(upgrade_materialization_retry_fn/_vtask_entry/_delaymgr_trampoline/
schedule_upgrade_materialization_retry) mirroring
config_activate_retry_fn's exact shape, hooked into
ha_cb_commit_advanced_stub alongside the existing config-byte retry.

Fixes a real bug caught on review: the fence had no production path
that could ever engage it from its zero-initialized default -- every
setter call site was gated on the fence already being set (closed
loop), making the new gate checks permanently dead code. Fixed with
two unconditional upgrade_materialization_verify() call sites:
persist.c's apply_committed_entry WAL_CONFIG_MARKER case (right after
persist_checkpoint_marker_triggered returns, regardless of its rc) and
myapp.c's boot sequence (right after config_activate_committed, for
Req 5.2 crash/restart resume). New test drives the fence through the
real production trigger (a forced -EBUSY checkpoint miss on a real
marker commit+apply), not a direct verify() call, proving the
end-to-end path actually works. Task 6.2 of 19.

Requirements: 3.1, 3.2, 3.3, 3.4, 4.3, 5.5
2026-07-25 04:58:37 +00:00

23 KiB

Implementation Plan

  • 1. Define HostAdminState and QueueAdminState Cap'n Proto structs and bump the snapshot format version

    • Add HostAdminState { name; status } and QueueAdminState { name; status } messages to the controller schema, independently extensible for future admin-state fields
    • Bump the on-disk checkpoint format version so any binary reading a checkpoint under the old queue-frame layout hard-rejects it rather than misinterpreting the bytes
    • Observable: schema compiles, generates the new message types, and a checkpoint file written under the new version is rejected by code still expecting the old version
    • Requirements: 1.9, 5.7
  • 2. Core: snapshot entity and configuration payload — collection and write side

  • 2.1 Collect host admin-state frames into the checkpoint

    • Walk the live host table the same way existing job/queue frame collection does, encoding only administratively-set status bits (open/closed/locked/tagged) per host as a HostAdminState frame
    • Exclude every worker-reported, dynamically-changing field (resource values, liveness timestamps, non-admin status bits) from the encoded frame
    • Observable: a checkpoint taken while a host is administratively closed contains exactly one HostAdminState frame for that host with only the admin bits set, and none of its dynamic fields
    • Requirements: 1.6, 1.7, 1.9
  • 2.2 Migrate queue admin-state frames to the new wire format

    • Replace the queue frame's fixed raw byte layout with QueueAdminState encoding, preserving the same name+status content
    • Observable: a checkpoint's queue section decodes via the new message type, and a checkpoint written under the prior raw layout is rejected fail-loud rather than silently reinterpreted
    • Requirements: 1.9, 5.7
  • 2.3 Embed the active configuration bytes into every checkpoint

    • Read the locally-held active configuration bytes directly from where they are already durably saved on disk, and append them as a new checkpoint section after the host frames
    • Ensure this section is populated on every checkpoint (not only a marker-triggered one), so a member joining or catching up after the marker-triggered snapshot has rotated away still receives configuration and state together
    • Keep configuration bytes out of WAL entry payloads entirely — only the checkpoint file carries them
    • Observable: any checkpoint file, marker-triggered or periodic, contains a configuration-bytes section whose content hash matches the currently active configuration; two independent materializations at the same marker boundary and configuration produce byte-identical checkpoint files
    • Requirements: 2.1, 2.3, 2.4, 5.1
  • 3. (P) Restore a host admin-state frame safely regardless of whether the host has ever joined this member

    • Look up the host by name in the live host table; if present, OR the restored admin-status bits into its current status
    • If absent, create a minimal placeholder entry (unjoined, dead-until-connected) carrying only the restored admin-status bits, using the same host-creation mechanism the existing synthetic sentinel host already uses without requiring a live join
    • Ensure the placeholder is valid enough that the host's real, later join is treated as an ordinary reconnect rather than a first-time join, so its existing admin-bit-preserving behavior applies unchanged
    • Guard against a retried restore attempt re-creating or conflicting with a placeholder an earlier, partially-failed attempt already created — find first, create only if truly missing
    • Observable: restoring a host admin-state frame for a host with no local record creates exactly one placeholder entry carrying the correct admin bits; restoring it again (simulating a retry) does not create a duplicate or error; when that host later joins for real, its admin status (e.g. closed) is still set afterward
    • Requirements: 1.10, 1.11, 5.8
    • Boundary: Host Restore (worker.c)
  • 4. Decode host and queue admin-state frames on checkpoint load

    • Add host-frame decoding to the checkpoint load path, dispatching each decoded frame to the host restore capability
    • Update queue-frame decoding to read the new QueueAdminState encoding instead of the old raw layout, without changing the existing queue-state-restore behavior itself
    • Preserve the existing restore discipline: every frame in the checkpoint restores successfully before any position is advanced; any restore failure leaves prior state untouched
    • Observable: loading a checkpoint containing host and queue admin-state frames restores every host and queue's admin status correctly regardless of restore order between entity types
    • Requirements: 1.10
    • Depends: 2.1, 2.2, 3
  • 5. Materialize the upgrade snapshot when a configuration-change marker is applied

    • Add explicit handling for the configuration-change marker in the single, unified entity-apply dispatch point (the same seam every member's apply already routes through — primary, standby, and restart replay alike), replacing its current no-op treatment
    • Trigger a forced checkpoint at that exact point, so materialization happens immediately and deterministically at the marker's boundary rather than waiting for a periodic trigger
    • Ensure the forced checkpoint shares the existing single-in-flight checkpoint guard, so a concurrent periodic checkpoint and this one can never both proceed at once
    • Treat a "checkpoint already in progress" outcome (e.g. a second configuration marker committing while an earlier one's checkpoint is still running) as an expected, self-resolving condition, not a failure to retry specially
    • Ensure this trigger only materializes state — it must not itself change how entities are interpreted or re-homed under the new configuration
    • Observable: applying a configuration-change marker produces a checkpoint at that marker's exact position, on both the primary and a standby, without requiring a controller restart; a second marker committing while the first's checkpoint is still running does not corrupt or lose either checkpoint
    • Requirements: 1.1, 1.5, 1.8, 4.4, 5.2, 5.3
    • Depends: 2.1, 2.2, 2.3
  • 6. Core: activation fence gating serving and promotion on materialization

  • 6.1 (P) Add a materialization-and-hash-verification fence independent of the existing configuration-byte fence

    • Introduce a new fence condition distinct from the existing configuration-byte-mismatch fence, since "bytes don't match" and "materialization for a matching configuration isn't done yet" are different conditions that must be distinguishable
    • Verify by reading the current committed configuration-change marker and this member's own materialized-and-verified state for it, always re-reading the live, currently-active configuration bytes on disk on every verification attempt rather than trusting a previously-embedded or cached copy
    • Set the fence and log the actionable mismatch loudly on failure; clear it on success; never terminate the process on this condition
    • Observable: a member whose materialization for the current marker is incomplete or mismatched shows the fence set and a loud log entry naming the marker; once materialization completes and bytes match, the fence clears without any process restart
    • Requirements: 1.8, 3.2, 3.4
    • Boundary: Activation Fence (ha.c/ha.h)
  • 6.2 Gate primary request-serving and promotion on the new fence

    • Extend the primary request-dispatch gate with this new fence check, composed alongside the existing configuration-byte fence rather than merged into it, admitting replication and read-only operations while fenced and refusing state-changing client requests
    • Extend the promotion gate the same way, so a member whose materialization is incomplete cannot become primary and immediately violate the serving gate the moment it wins an election
    • Introduce no new promotion-eligibility mechanism for WAL/log completeness — this fence is about materialization completeness only, layered on top of the existing Raft log-freshness election gate, not replacing or duplicating it
    • Wire a non-blocking, self-triggering retry so the fence re-verifies and clears automatically once conditions allow, without any operator action
    • Observable: a fenced primary continues answering read-only and status requests but rejects a state-changing client request with a not-ready response; a fenced member that wins an election does not begin serving under the new configuration until the fence clears on its own
    • Requirements: 3.1, 3.2, 3.3, 3.4, 4.3, 5.5
    • Depends: 6.1
  • 7. Remove the per-node restart/replay fallback and replace it with the marker-triggered materialization

    • Delete the configuration-upgrade handler's restart-request step now that materialization is driven entirely by the marker-apply trigger, decoupled from that request-handling code path
    • Add a deterministic test seam that can force a leadership change to land exactly between a configuration-change marker's commit and every member completing its snapshot install, for later regression use
    • Observable: pushing a configuration upgrade no longer restarts any controller process, and every member reaches the new configuration through the marker-apply trigger alone
    • Requirements: 1.2, 1.3, 1.4, 2.2, 5.4, 6.3
    • Depends: 5
  • 8. Validation: regression coverage for every new mechanism

  • 8.1 (P) Unit-test the marker-apply trigger and its interaction with WAL retention

    • Assert the configuration-marker case fires a checkpoint exactly once per apply and is safe under a forced retry
    • Assert the trigger does not itself re-home entities
    • Add a deterministic fault-injection seam at the marker-apply-to-checkpoint-durable boundary, mirroring the existing fault-hook pattern already used elsewhere in this codebase, for later chaotic-scenario use
    • Assert a snapshot encoded in an unrecognized format version (simulating a rolling binary upgrade or the prior raw queue-frame layout) is rejected fail-loud, never partially installed
    • Assert that when a standby is genuinely mid-catch-up (has not yet acknowledged the marker's position) at the moment the marker-triggered checkpoint's WAL trim runs, that standby's still-needed WAL segments are not deleted
    • Observable: the new test suite passes, demonstrating exactly-once checkpoint firing, safe retry, format-mismatch rejection, and trim safety under the new trigger path
    • Requirements: 1.1, 1.5, 1.8, 5.2, 5.3, 5.6, 5.7, 6.3, 7.3, 7.4
    • Depends: 5
    • Boundary: Marker Apply Trigger (test_upgrade_snapshot_apply.c)
  • 8.2 (P) Unit-test host placeholder restore and rejoin preservation

    • Assert restoring a host admin-state frame for a never-joined host creates a placeholder carrying the correct admin bits
    • Assert a retried restore after a simulated partial failure finds and reuses the existing placeholder rather than conflicting with it
    • Assert that a host's real first join after a placeholder was created preserves the admin status through the ordinary reconnect path
    • Observable: the new test suite passes, demonstrating create-if-missing correctness, idempotent retry, and admin-status survival through a real join
    • Requirements: 1.10, 1.11, 5.8, 7.1
    • Depends: 3
    • Boundary: Host Restore (test_upgrade_snapshot_restore.c)
  • 8.3 (P) Unit-test the activation fence

    • Assert the fence sets on incomplete materialization or a configuration-hash mismatch and clears on successful verification
    • Assert the request-dispatch gate's allowlist behavior while fenced, and that promotion is refused while fenced
    • Observable: the new test suite passes, demonstrating fence set/clear transitions and correct gating of both serving and promotion
    • Requirements: 3.2, 3.3, 3.4, 4.3, 5.5
    • Depends: 6.2
    • Boundary: Activation Fence (test_upgrade_activation_fence.c)
  • 8.4 (P) Build a cross-member active-configuration comparator and extend checkpoint verification tooling

    • Build a new offline, file-based tool that compares each member's active configuration content hash, recomputed fresh from each member's own on-disk configuration file, mirroring the existing checkpoint-comparison tool's shape
    • Extend the existing checkpoint-structure verification tool to decode and assert the embedded configuration reference's tag and hash (not merely report their presence), and to report host-frame count and configuration-bytes-section presence
    • Observable: the comparator correctly reports two members with matching configuration as identical and two with differing configuration as divergent; the extended verification tool reports a concrete tag/hash value and section counts for a real checkpoint file
    • Requirements: 4.2
    • Depends: 2.3
    • Boundary: Regression Tooling (configcompare, snapverify)
  • 8.5 Reproduce bug-857 on the pre-change behavior and confirm it no longer occurs

    • Run the original materialized-state-divergence repro scenario against the pre-change code path and confirm the divergence occurs
    • Run the same scenario against the post-change behavior and confirm every member's materialized state converges identically
    • Observable: the same repro scenario demonstrably fails on the old behavior and passes on the new one
    • Requirements: 6.2
    • Depends: 7, 8.1, 8.2, 8.3, 8.4
  • 8.6 Exercise a configuration upgrade interleaved with chaotic multi-kill failover

    • Drive a configuration upgrade while repeatedly killing and restarting the primary, and assert every member's materialized state and active configuration are identical afterward
    • Assert a host's administratively-set status specifically survives the same scenario, not merely inferred from the aggregate state check
    • Observable: the chaotic-upgrade soak run reports zero materialized-state divergence, zero configuration divergence, and correct host admin-status across all members
    • Requirements: 4.1, 4.3, 6.1, 6.4
    • Depends: 7, 8.1, 8.2, 8.3, 8.4
  • 8.7 Inject a crash and a leadership change at the two critical upgrade windows

    • Force a controller crash/restart between a configuration-change marker's commit and that member's own snapshot write becoming durable, and assert materialization resumes correctly from the marker on restart
    • Force a leadership change between a configuration-change marker's commit and every member completing its snapshot install, and assert the new leader continues driving materialization to completion without any special resume logic
    • Observable: both fault-injection scenarios complete with the post-upgrade consistency invariant and the crash-safety invariant intact
    • Requirements: 5.2, 5.4, 5.5, 6.3
    • Depends: 7, 8.1, 8.2, 8.3, 8.4
  • 8.8 Exercise a second configuration upgrade committing before the first's installation completes everywhere

    • Commit a second configuration-change marker while the first upgrade's snapshot is still installing on at least one member, and assert the earlier upgrade's in-flight state is cleanly superseded rather than left partially applied or conflated with the newer one
    • Observable: after both markers settle, every member reflects only the newer configuration and state, with no trace of a partially-applied intermediate state
    • Requirements: 4.4, 7.2
    • Depends: 7, 8.1, 8.2, 8.3, 8.4
  • 8.9 Exercise the WAL-trim-under-marker-trigger scenario under chaotic conditions

    • Repeat the mid-catch-up WAL-trim assertion from the unit-level fixture, but under the full chaotic multi-kill soak rather than an isolated fixture
    • Observable: across a full chaotic-soak run with configuration upgrades in flight, no standby ever loses a WAL segment it still needed for catch-up
    • Requirements: 7.4
    • Depends: 7, 8.1, 8.2, 8.3, 8.4

Implementation Notes

  • Task 6.2 round-1 rejection (real bug, caught by review): the first implementation wired the fence into both gates + a retry trampoline correctly, but upgrade_materialization_fence had no production path that could ever SET it from its zero-initialized default — every setter call site was gated on the fence already being set (closed loop). Fixed with two unconditional call sites: persist.c's apply_committed_entry WAL_CONFIG_MARKER case now unconditionally calls upgrade_materialization_verify(c) right after persist_checkpoint_marker_triggered returns (regardless of its rc), and myapp.c's boot sequence unconditionally calls it right after config_activate_committed (Req 5.2, crash/restart resume). New cross-module direction: persist.c now externs an ha.c function (mirrors the existing persist.c → worker.c extern pattern, no circular include). Lock-reentrancy re-verified safe (neither call chain touches apply_pump_lock).

  • Task 6.1 process note: the Claude account's monthly API spend limit was hit mid-task (implementer subagent cut off after landing ha.c/ha.h but before tests/build). The orchestrator finished this task directly (no subagent implementer/reviewer pair) rather than stopping: verified the partial diff, wrote 4 tests into test_config_repl.c (reusing existing t42_/t43_ fixtures rather than creating test_upgrade_activation_fence.c — that file is task 8.3's own boundary per design.md, not 6.1's), built, ran the full suite (1001 Tests 0 Failures 0 Ignored), and self-reviewed the diff in lieu of an independent reviewer subagent. Flagging this so a future session/reviewer knows this task's diff did not go through the normal two-subagent review gate.

  • Task 5 correction to design.md: design.md's "Marker Apply Trigger" section says "Call persist_checkpoint() directly" — this is unsafe as written. persist_apply_pump_run holds apply_pump_lock across the whole dispatch loop (including the apply_committed_entry call), and persist_checkpoint() itself re-acquires that same non-recursive mutex — a real self-deadlock on the live pump path (primary AND standby online-apply). Separately, last_applied_lsn/replay position only advance AFTER apply_committed_entry returns at both call sites, so pinning checkpoint_lsn from last_applied_lsn inside the marker case would mislabel the checkpoint with the previous entry's LSN, not the marker's own. Fixed via persist_checkpoint_write_locked(p, checkpoint_lsn) (no self-lock, explicit LSN param) + persist_checkpoint_marker_triggered(p, lsn), called from apply_committed_entry's new WAL_CONFIG_MARKER case with the marker's own lsn parameter. design.md should be updated to reflect this; not done as part of task 5 (docs-only, deferred).

  • Task 5 → ha-term-safety/raft-log-replication finding (bug-1466, NOT this spec's to fix): persist_write_checkpoint_marker advances p->current_lsn directly, bypassing ha_primary_log_append/raft_log_append, breaking ha_primary_log_append's own documented p->current_lsn == s->log_len + 1 invariant (ha.c:4054-4060) once any checkpoint interleaves with primary appends. Pre-existing (periodic checkpoint already did this); task 5 just made the interleaving common instead of rare, surfacing it as real test flakiness for the first time. ha->raft.log_len feeds real RequestVote freshness comparisons (ha.c:258-261, 401-406) — NOT advisory-only as bug-1466's original text claimed; severity corrected in .wolf/buglog.json. Out of this spec's boundary (Raft log/term safety is ha-term-safety/raft-log-replication's territory) — flagging for whoever next owns that area.

  • Task 3 → task 4 heads-up: restore_host_state(cluster_t*, const char*, uint32_t) (worker.c) is standalone and NOT yet wired into load_snapshot. Task 4 dispatches decoded HostAdminState frames to it. restore_host_state runs on the live vtask pool (called from ha_handle_snapshot_chunk → persist_load_snapshot), not a quiesced phase — a concurrent real first-time join for the same host is architecturally possible; verified safe by convergent key_find/makeWorker semantics (see worker.c comment above restore_host_state's key_find call), not by absence of concurrency. Task 4 should confirm this contract still holds once wired in.

  • Task 2.3 → task 6.1 heads-up: read_active_config_bytes (persist.c) is the fully-inlined (no config_repl.c call) pattern for reading live config bytes off disk fresh each time — task 6.1's activation-fence verify function needs the identical "always re-read live bytes, never trust a cached/embedded copy" discipline (design.md is explicit about this). Reuse/mirror read_active_config_bytes rather than re-deriving the config_repl.c-avoidance reasoning from scratch.

  • Task 2.2 → task 4 MUST re-enable — DONE (task 4): test_snapent_load_snapshot_restores_entities_before_advancing_position is un-ignored and passes for real now that load_snapshot's queue decode reads QueueAdminState capnp.

  • test_snapent_checkpoint_queue_status_roundtrip's comments (~line 1019) are stale (still reference "format_version=1" / "task 3.2 not yet implemented" from an earlier spec pass) — harmless, not a task 2.2 defect, but worth cleaning up whenever that test is next touched.

  • Task 2.1 → task 3 heads-up: host-frame collection is unconditional (every host in c->workermgr gets a frame, status masked to admin bits, may be 0) — confirmed correct against requirements.md/design.md by both implementer and reviewer. Consequence for task 3 (restore_host_state): a checkpoint may contain many zero-admin-status host frames on large clusters. Not required to special-case this, but consider whether restore_host_state should skip placeholder creation when admin_status == 0 on a miss, purely as a cost optimization — current design.md contract doesn't require it either way.

  • Build environment (bug-918 recurrence 4, fixed in this worktree): nix develop was hard-broken — xbuild's capnpc package pins its src to the MUTABLE main ref of shen390s/c-capnproto with a hardcoded sha256, so it hash-mismatches every time that branch moves upstream (recurred 3x before in this same worktree per .wolf/buglog.json). Fixed durably by overriding capnpc's src in src/nixSupport/xce.nix to a pkgs.fetchFromGitHub pinned at an immutable commit (rev+sha256) instead of tracking main. Any future task in this spec that needs a fresh nix develop should not hit this again; if it does, the fix pattern is the same (re-pin to the then-current upstream commit).

  • Test-runner gotcha: running controller_unit_test with default ASan settings hangs indefinitely in this sandbox — LeakSanitizer's stop-the-world thread scan never completes (confirmed: 50+ min wall clock, ~5s CPU, futex_do_wait). Always run with ASAN_OPTIONS=detect_leaks=0 (matches the project's own meson test registration). Do not wrap the binary in stdbuf or anything else that adds its own LD_PRELOAD — conflicts with ASan's preload requirement and the binary refuses to start.

  • After any SNAPSHOT_FORMAT_VERSION bump: grep the whole testsuite/ tree for other hardcoded copies of the old format-version literal before considering the bump done — test_persist_snapshot_entities.c (a different, already-shipped spec's test file) had its own independent SNAPENT_TEST_FORMAT_VERSION constant that silently went stale after task 1's bump and broke 3 unrelated tests until caught.