From 7b507156a0dc634e115f2ada170c350d5df339df Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 25 Jul 2026 02:51:49 +0000 Subject: [PATCH] feat(config-upgrade-snapshot): add materialization-and-hash-verification activation fence Adds a new, independent fence (ha_state_t.upgrade_materialization_fence) distinct from the existing config-byte serve-fence: that fence tracks whether the locally-held config bytes hash-match the committed marker; this one tracks whether THIS MEMBER'S OWN upgrade-snapshot materialization (task 5's marker-triggered checkpoint) has actually completed for the current committed marker, with the live on-disk config bytes still hash-verifying against it right now. upgrade_materialization_verify(c) mirrors config_activate_committed's shape: reads the current committed marker, checks whether persist.cfgref_present/cfgref_tag/cfgref_sha already reflect it (stamped by every checkpoint via scan_latest_committed_config_ref -- proof this member's own materialization ran for this exact marker), and separately re-reads the LIVE on-disk config bytes fresh on every call (never a cached/embedded copy) to hash-verify them too. Both must hold for the fence to clear. Logs loud only on the 0->1 transition, never exits/aborts, always fail-loud + fence + stay alive + retriable. upgrade_materialization_fenced(c) is the trivial atomic-read accessor. Not yet wired into the dispatch gate, promotion gate, or an automatic retry trigger -- that's task 6.2. Task 6.1 of 19. Requirements: 1.8, 3.2, 3.4 --- .kiro/specs/config-upgrade-snapshot/tasks.md | 4 +- src/controller/ha.c | 183 ++++++++++++++ src/controller/ha.h | 73 ++++++ src/controller/testsuite/test_config_repl.c | 250 +++++++++++++++++++ 4 files changed, 509 insertions(+), 1 deletion(-) diff --git a/.kiro/specs/config-upgrade-snapshot/tasks.md b/.kiro/specs/config-upgrade-snapshot/tasks.md index dc78d2f..dd5fef8 100644 --- a/.kiro/specs/config-upgrade-snapshot/tasks.md +++ b/.kiro/specs/config-upgrade-snapshot/tasks.md @@ -53,7 +53,7 @@ - _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 +- [x] 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 @@ -149,6 +149,8 @@ ## Implementation Notes +- **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. diff --git a/src/controller/ha.c b/src/controller/ha.c index eb7f9fe..73444fe 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -22,6 +22,7 @@ #include #include +#include /* Circular-include safety: cluster_def.h includes ha.h (include-guarded), * and ha.h only forward-declares cluster_t — no cycle. */ @@ -105,6 +106,188 @@ extern int config_serve_fenced(const cluster_t *c); * stale serve-fence (non-blocking ordq_put only). */ extern int config_schedule_activate_retry(cluster_t *c); +/* ── config-upgrade-snapshot task 6.1: Activation Fence ─────────────────── + * upgrade_materialization_verify (below) is a SEPARATE fence from the + * config-byte serve-fence above — see ha_state_t's upgrade_materialization_ + * fence field doc comment (ha.h) for the full "why a second fence" rationale. + * It reads the SAME committed-marker authority config_activate_committed + * itself reads. */ +extern int config_committed_marker(const cluster_t *c, uint64_t *version_tag, + uint8_t sha256_out[32]); + +/* upgrade_materialization_read_live_config_sha256 (config-upgrade-snapshot + * task 6.1, Req 3.2/3.4): resolve and hash THIS MEMBER'S CURRENTLY-ACTIVE + * config bytes fresh, right now — never a cached/frozen copy (in particular, + * never c->persist.cfgref_sha, which is only a snapshot-time REFERENCE + * captured at whatever moment the last checkpoint ran, not proof the bytes + * are still current this instant). + * + * DELIBERATE DESIGN CHOICE: this calls the same two already-PUBLIC + * config_repl.c entry points (conf_read_current_filename, conf_read_raw) + * that config_activate_committed's own local_config_sha256 helper uses + * internally, rather than re-deriving the //conf/current -> + * cluster.bin. resolution a third time (persist.c's + * read_active_config_bytes already duplicates that resolution inline once, + * with its own documented rationale for why persist.c specifically avoids a + * persist.c -> config_repl.c call edge). ha.c carries no such constraint — + * it already depends on config_repl.c directly for config_activate_ + * committed/config_serve_fenced/config_schedule_activate_retry/conf_read_raw + * above, so calling these two existing, already-public accessors here is + * in-boundary, ordinary integration along an established edge, not a new + * layering violation or a third duplicate implementation of path validation. + * + * Returns 0 with out_sha populated on success; -ENOENT when no local config + * bytes exist yet (benign — e.g. c->myconf unset, or no conf/current pointer + * written yet); another negative errno surfaces a genuine read/validation + * failure from the underlying calls. */ +static int upgrade_materialization_read_live_config_sha256(cluster_t *c, + uint8_t out_sha[32]) +{ + extern int conf_read_current_filename(const char *workdir, char *out, + size_t outsz); + extern int conf_read_raw(const char *conffile, uint8_t **bytes, + size_t *len); + + myconf_t *mc = c->myconf; + if (!mc || !mc->workdir || !mc->name) + return -ENOENT; + + /* effective_dir = workdir/name — must match conf_save_config's own path + * construction (config_repl.c) exactly, mirroring local_config_sha256. */ + char conf_dir[4096]; + snprintf(conf_dir, sizeof(conf_dir), "%s/%s", mc->workdir, mc->name); + + char fname[256] = {0}; + int rc = conf_read_current_filename(conf_dir, fname, sizeof(fname)); + if (rc != 0) + return -ENOENT; /* no local bytes yet — caller fences + retries */ + + char path[4352]; + snprintf(path, sizeof(path), "%s/conf/%s", conf_dir, fname); + + uint8_t *bytes = NULL; + size_t blen = 0; + rc = conf_read_raw(path, &bytes, &blen); + if (rc != 0) + return -ENOENT; + + SHA256(bytes, blen, out_sha); + free(bytes); + return 0; +} + +/* upgrade_materialization_verify (config-upgrade-snapshot task 6.1, Req 1.8, + * 3.2, 3.4): the Activation Fence's own verify function — mirrors + * config_activate_committed's exact shape (config_repl.c:304-377) for a + * DIFFERENT condition. config_serve_fence (config_repl.c) asks "do the + * locally-held config BYTES hash-match the committed marker?"; THIS fence + * asks "has THIS MEMBER'S OWN materialization (the upgrade-snapshot + * checkpoint, config-upgrade-snapshot task 5) actually completed for the + * CURRENT committed marker, with matching config bytes?" — a distinct, + * independently-trackable condition: bytes can already hash-match + * (config_serve_fence clear) while this member's own checkpoint for that + * exact marker has not run yet, e.g. immediately after a marker commits but + * before apply_committed_entry's WAL_CONFIG_MARKER case has actually fired + * and written a checkpoint. + * + * "Materialization complete" is determined via c->persist.cfgref_present / + * cfgref_tag / cfgref_sha — persist.c's scan_latest_committed_config_ref, + * invoked from EVERY checkpoint (persist_checkpoint_write_locked), stamps + * these fields with the latest committed WAL_CONFIG_MARKER as of that + * checkpoint. cfgref_present && cfgref_tag == marker_tag && cfgref_sha == + * marker_sha therefore means "some checkpoint on THIS member has already + * captured a reference to this exact committed marker" — materialization for + * it has happened. This is the SAME field config_committed_marker's own + * compaction fallback already reads (config_repl.c), so this is a read of an + * existing, already-correct signal, not a new one. + * + * That alone is not sufficient: the checkpoint's cfgref_sha is a REFERENCE + * captured AT CHECKPOINT TIME, not proof the bytes are STILL current right + * now (the WAL marker's raft-commit and the R7 config-push byte delivery are + * two independent channels with no strict ordering guarantee). So this + * function ALSO re-reads the live on-disk config bytes fresh on every call + * (never c->persist.cfgref_sha, never any other cached/embedded copy) via + * upgrade_materialization_read_live_config_sha256 (above), and hash-verifies + * that fresh read against the committed marker too. Both must hold for the + * fence to clear. + * + * Never exits/aborts on this condition (controller/CLAUDE.md's never-self- + * terminate invariant) — always fail loud + fence + stay alive + retriable, + * mirroring config_activate_committed exactly: logs loud only on the 0->1 + * fence transition (never spams on repeated failed retries), returns + * -EAGAIN on failure so the caller retries, clears + returns 0 on success. */ +int upgrade_materialization_verify(cluster_t *c) +{ + if (!c) + return -EINVAL; + + /* 1. The committed marker is the authority for the target upgrade. An + * absent marker (no configuration change has ever committed on this + * cluster, or the marker has not committed yet) is NOT a divergence — + * clear any stale fence and succeed, mirroring config_activate_ + * committed's own -ENOENT handling exactly. */ + uint64_t marker_tag = 0; + uint8_t marker_sha[32]; + int rc = config_committed_marker(c, &marker_tag, marker_sha); + if (rc == -ENOENT) { + atomic_store_explicit(&c->ha.upgrade_materialization_fence, 0, + memory_order_release); + return 0; + } + if (rc != 0) + return rc; /* -EINVAL / -EIO surfaced to the caller */ + + /* 2. Materialization-complete signal: has ANY checkpoint on THIS member + * already captured a reference to this EXACT committed marker? */ + bool materialized = c->persist.cfgref_present && + c->persist.cfgref_tag == marker_tag && + memcmp(c->persist.cfgref_sha, marker_sha, 32) == 0; + + /* 3. Hash-verify: re-read the LIVE, currently-active config bytes fresh + * on every attempt and compare against the committed marker. */ + uint8_t live_sha[32]; + int live_rc = upgrade_materialization_read_live_config_sha256(c, live_sha); + bool bytes_verified = (live_rc == 0) && + (memcmp(live_sha, marker_sha, 32) == 0); + + if (!materialized || !bytes_verified) { + int was = atomic_exchange_explicit(&c->ha.upgrade_materialization_fence, + 1, memory_order_acq_rel); + if (!was) + ERROR("upgrade materialization fence: committed config marker " + "tag=%016" PRIx64 " -- materialization %s " + "(cfgref_present=%d cfgref_tag=%016" PRIx64 "), live config " + "bytes %s (rc=%d) -- member's OWN upgrade-snapshot " + "materialization for this marker is INCOMPLETE or " + "MISMATCHED; member NON-SERVING and NON-PROMOTABLE under the " + "new configuration, staying alive and retrying " + "(config-upgrade-snapshot R1.8/3.2/3.4)", + marker_tag, materialized ? "captured" : "NOT YET captured", + c->persist.cfgref_present, c->persist.cfgref_tag, + bytes_verified ? "hash-match" : "do NOT hash-match", live_rc); + return -EAGAIN; + } + + atomic_store_explicit(&c->ha.upgrade_materialization_fence, 0, + memory_order_release); + INFO("upgrade materialization VERIFIED for committed config marker " + "tag=%016" PRIx64 " -- materialization complete, live config bytes " + "hash-verified", marker_tag); + return 0; +} + +/* upgrade_materialization_fenced: trivial atomic-read accessor — mirrors + * config_serve_fenced (config_repl.c:262-268) exactly, for the new + * independent fence. Pure atomic read (no I/O), safe on the election/ + * dispatch path. */ +int upgrade_materialization_fenced(const cluster_t *c) +{ + if (!c) + return 0; + return atomic_load_explicit(&c->ha.upgrade_materialization_fence, + memory_order_acquire) ? 1 : 0; +} + static void on_vote_connected(endpoint_t *ep); static void on_pre_vote_connected(endpoint_t *ep); static int vote_ep_err_handler(void *ep_raw); diff --git a/src/controller/ha.h b/src/controller/ha.h index c6dc080..88b218a 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -320,6 +320,44 @@ typedef struct { * cleared and the config is activated. * NEVER self-terminates (loud non-serving * fence only). 0 at init (ha_init memset). */ + /* ── New field added by config-upgrade-snapshot spec (Task 6.1) ────── */ + _Atomic int upgrade_materialization_fence; /* Upgrade-snapshot + * materialization-and-hash-verify + * fence (Req 1.8, 3.2, 3.4) — a NEW, + * INDEPENDENT fence, deliberately NOT + * merged into config_serve_fence + * above: that fence tracks whether the + * out-of-band config-PUSH bytes match + * the committed marker; THIS fence + * tracks whether THIS MEMBER'S OWN + * upgrade-snapshot materialization + * (the marker-triggered checkpoint, + * config-upgrade-snapshot task 5) has + * actually completed for the CURRENT + * committed marker, with the live + * on-disk config bytes still + * hash-verifying against it right + * now. These are logically distinct + * and can disagree in either + * direction (e.g. bytes can already + * hash-match — config_serve_fence + * clear — while this member's own + * checkpoint for that exact marker + * has not run yet, immediately after + * a marker commits but before its + * forced checkpoint fires). Set by + * upgrade_materialization_verify() + * on materialization-incomplete or a + * hash mismatch against a FRESH + * re-read of the live config bytes + * (never a cached/embedded copy); + * cleared on success. While set, the + * member MUST NOT serve under the new + * configuration or become primary; it + * STAYS ALIVE and keeps retrying. + * NEVER self-terminates (loud + * non-serving fence only). 0 at init + * (ha_init memset). */ /* ── New field added by raft-drive-async spec (Task 2.1) ──────── */ _Atomic int reconnect_inflight[MAX_HA_CANDIDATES]; /* Per-peer * single-flight guard for the offloaded @@ -755,6 +793,41 @@ int ha_dispatch_gate(ha_state_t *ha, int opcode); */ int ha_config_promotion_fenced(const ha_state_t *ha); +/* + * upgrade_materialization_verify (config-upgrade-snapshot task 6.1, Req 1.8, + * 3.2, 3.4): the Activation Fence's own verify function. Mirrors + * config_activate_committed's exact shape (config_repl.c:304-377) for a + * DIFFERENT, independently-tracked condition — see ha_state_t's + * upgrade_materialization_fence field doc comment above for the full + * rationale on why this must be a separate fence from config_serve_fence. + * + * On every call: reads the current committed WAL_CONFIG_MARKER (the + * authority for the target upgrade), checks whether THIS MEMBER'S OWN + * checkpoint machinery has already captured a materialization reference for + * that EXACT marker (c->persist.cfgref_present/cfgref_tag/cfgref_sha, + * stamped by every checkpoint via persist.c's scan_latest_committed_config_ + * ref), and separately re-reads the LIVE, currently-active config bytes off + * disk FRESH (never a cached/embedded copy) to hash-verify them against the + * marker too. Both must hold for the fence to clear. + * + * Returns 0 (fence cleared or absent-marker benign case) on success; + * -EAGAIN when materialization is incomplete or the hash mismatches (fence + * SET, logged loud only on the 0->1 transition); -EINVAL/-EIO on a genuine + * error reading the committed marker. NEVER exits/aborts on this condition + * (controller/CLAUDE.md never-self-terminate invariant) — always fail loud + + * fence + stay alive + retriable. + */ +int upgrade_materialization_verify(cluster_t *c); + +/* + * upgrade_materialization_fenced: 1 when this member's own upgrade-snapshot + * materialization for the current committed config marker is incomplete or + * hash-mismatched (upgrade_materialization_fence set); 0 otherwise. Trivial + * atomic-read accessor — mirrors config_serve_fenced (config_repl.c:262-268) + * exactly, for the new independent fence. Pure atomic read (no I/O). + */ +int upgrade_materialization_fenced(const cluster_t *c); + /* * ha_apply_wal_catchup_entry: apply a single WAL catch-up entry to local * persistent storage and advance ha->applied_lsn on success. diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index 98cf24f..542fa90 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -1382,6 +1382,251 @@ static void test_config_activate_no_marker_falls_back_and_loads(void) printf("test_config_activate_no_marker_falls_back_and_loads: PASS\n"); } +/* ── config-upgrade-snapshot task 6.1: Activation Fence + * (upgrade_materialization_verify / upgrade_materialization_fenced, ha.c) + * ────────────────────────────────────────────────────────────────────── + * These tests reuse this file's own t42_/t43_ fixture helpers rather than + * duplicating them into a new file — the Activation Fence's verify function + * is a close sibling of config_activate_committed (tested just above), and + * every fixture piece it needs (a real cluster_t, a real commit engine, a + * real myconf/config file, real committed WAL_CONFIG_MARKERs) already lives + * here. task 8.3 owns broader dispatch-gate/promotion-gate fence coverage in + * its own new file (test_upgrade_activation_fence.c per design.md); this + * task's own boundary is ha.c/ha.h only, so its tests stay narrowly scoped + * to upgrade_materialization_verify/upgrade_materialization_fenced + * themselves. */ + +extern int upgrade_materialization_verify(cluster_t *c); +extern int upgrade_materialization_fenced(const cluster_t *c); + +/* (6.1-a) No committed marker yet: benign, not a divergence — fence stays + * clear, verify succeeds (mirrors config_activate_committed's own -ENOENT + * handling). */ +static void test_upgrade_materialization_verify_no_marker_clears_fence(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ucaf61_nomarker_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/20, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_verify(c)); + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_fenced(c)); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_upgrade_materialization_verify_no_marker_clears_fence: PASS\n"); +} + +/* (6.1-b) A committed marker exists and its bytes are locally correct and + * hash-verify, AND this member's own checkpoint has already captured a + * reference to it (task 5's marker-triggered checkpoint runs synchronously + * on apply, so t43_commit_marker's wait-for-applied already guarantees this) + * — materialization is genuinely complete, fence clears. */ +static void test_upgrade_materialization_verify_matching_materialization_clears_fence(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ucaf61_match_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/21, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + size_t blen = 0; + uint8_t *bytes = t43_make_config_blob(&blen); + uint8_t sha[32]; + SHA256(bytes, blen, sha); + uint64_t tag; { uint64_t t; memcpy(&t, sha, 8); tag = be64toh(t); } + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytes, blen, sha)); + free(bytes); + + t43_commit_marker(c, tag, sha); + + /* Task 5's synchronous marker-triggered checkpoint already ran by the + * time t43_commit_marker (which waits for apply) returns — cfgref_* + * already reflects this exact marker. */ + TEST_ASSERT_TRUE(c->persist.cfgref_present); + TEST_ASSERT_EQUAL_HEX64(tag, c->persist.cfgref_tag); + + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_verify(c)); + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_fenced(c)); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_upgrade_materialization_verify_matching_materialization_clears_fence: PASS\n"); +} + +/* (6.1-c) A NEW marker commits while an earlier checkpoint is (simulated) + * still in-flight, so THIS marker's own materialization never actually runs + * (task 5's -EBUSY path, Req 4.4) -- config_committed_marker now returns the + * NEW marker but c->persist.cfgref_* still reflects the OLDER one. + * Materialization is genuinely incomplete for the CURRENT marker -> fence + * SETS (distinct from config_serve_fence, which would stay clear here since + * the bytes for the new marker are already present and correct). */ +static void test_upgrade_materialization_verify_incomplete_materialization_fences(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ucaf61_incomplete_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/22, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* Marker A: commits, applies, materializes normally (baseline). */ + size_t blenA = 0; + uint8_t *bytesA = t43_make_config_blob(&blenA); + uint8_t shaA[32]; + SHA256(bytesA, blenA, shaA); + uint64_t tagA; { uint64_t t; memcpy(&t, shaA, 8); tagA = be64toh(t); } + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytesA, blenA, shaA)); + free(bytesA); + t43_commit_marker(c, tagA, shaA); + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_verify(c)); + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_fenced(c)); + TEST_ASSERT_EQUAL_HEX64(tagA, c->persist.cfgref_tag); + + /* Marker B: its bytes are saved correctly (config_serve_fence would stay + * clear), but its own checkpoint is forced to skip (-EBUSY) by holding + * the shared checkpoint_in_progress CAS open across the commit -- the + * exact real mechanism task 5's marker-apply case hits under Req 4.4. */ + size_t blenB = 0; + uint8_t *bytesB = t43_make_config_blob(&blenB); + /* t43_make_config_blob always produces byte-identical content -- mutate + * a trailing byte so blob B genuinely differs from blob A (this test + * only ever hashes/saves the raw bytes via upgrade_materialization_verify, + * which never parses them as capnp, so a one-byte mutation is safe). */ + bytesB[blenB - 1] ^= 0xFF; + uint8_t shaB[32]; + SHA256(bytesB, blenB, shaB); + uint64_t tagB; { uint64_t t; memcpy(&t, shaB, 8); tagB = be64toh(t); } + TEST_ASSERT_FALSE(memcmp(shaA, shaB, 32) == 0); + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytesB, blenB, shaB)); + free(bytesB); + + TEST_ASSERT_EQUAL_INT( + 0, atomic_exchange_explicit(&c->persist.checkpoint_in_progress, 1, + memory_order_acq_rel)); + uint64_t mlsnB = 0; + TEST_ASSERT_EQUAL_INT(0, config_append_marker(c, tagB, shaB, &mlsnB)); + TEST_ASSERT_EQUAL_INT(0, persist_flush(&c->persist)); + TEST_ASSERT_EQUAL_INT(0, + commit_submit(&c->commit, mlsnB, COMMIT_MODE_WAIT, 1000)); + /* apply runs (marker B's WAL_CONFIG_MARKER case fires and hits -EBUSY on + * the still-held CAS -- treated as expected/self-resolving, not a + * failure, so apply itself succeeds); wait for it directly since + * t43_commit_marker's own wait helper is not used here. */ + uint64_t deadline_iters = 200; + while (deadline_iters-- > 0 && + atomic_load_explicit(&c->persist.last_applied_lsn, + memory_order_acquire) < mlsnB) { + struct timespec ts = { .tv_sec = 0, .tv_nsec = 10 * 1000 * 1000 }; + nanosleep(&ts, NULL); + } + TEST_ASSERT_TRUE(atomic_load_explicit(&c->persist.last_applied_lsn, + memory_order_acquire) >= mlsnB); + + /* Release the simulated in-flight checkpoint now that marker B's apply + * (and its -EBUSY'd checkpoint attempt) has already happened. */ + atomic_store_explicit(&c->persist.checkpoint_in_progress, 0, + memory_order_release); + + /* config_committed_marker now authoritatively returns marker B, but this + * member's own materialization never ran for it (cfgref_tag is still A) + * -- genuinely incomplete, not a bytes mismatch. */ + TEST_ASSERT_EQUAL_HEX64(tagA, c->persist.cfgref_tag); + TEST_ASSERT_EQUAL_INT(-EAGAIN, upgrade_materialization_verify(c)); + TEST_ASSERT_EQUAL_INT(1, upgrade_materialization_fenced(c)); + + /* Idempotent retry: calling again while still genuinely unresolved stays + * fenced, does not error/crash/toggle unexpectedly. */ + TEST_ASSERT_EQUAL_INT(-EAGAIN, upgrade_materialization_verify(c)); + TEST_ASSERT_EQUAL_INT(1, upgrade_materialization_fenced(c)); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_upgrade_materialization_verify_incomplete_materialization_fences: PASS\n"); +} + +/* (6.1-d) Materialization for the current marker DID complete (cfgref_* + * correctly captured it, matching), but the LOCAL config file has since been + * overwritten with different bytes (without a new marker committing) -- + * simulates the exact "stale embedded copy" trap design.md warns about. + * A correct implementation re-reads the LIVE bytes fresh every call rather + * than trusting cfgref_sha (a checkpoint-time reference) -- so this MUST + * fence, proving the fresh re-read actually happens. */ +static void test_upgrade_materialization_verify_stale_local_bytes_after_materialization_fences(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ucaf61_stale_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/23, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + size_t blen = 0; + uint8_t *bytes = t43_make_config_blob(&blen); + uint8_t sha[32]; + SHA256(bytes, blen, sha); + uint64_t tag; { uint64_t t; memcpy(&t, sha, 8); tag = be64toh(t); } + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytes, blen, sha)); + free(bytes); + t43_commit_marker(c, tag, sha); + + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_verify(c)); + TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_fenced(c)); + + /* The local config file is overwritten with DIFFERENT bytes, without any + * new marker committing (simulates local corruption/drift, or a config + * push that lands out of order) -- config_committed_marker still + * returns the SAME marker/sha as before. */ + size_t blen2 = 0; + uint8_t *bytes2 = t43_make_config_blob(&blen2); + /* t43_make_config_blob always produces byte-identical content -- mutate + * a trailing byte so the "overwritten" bytes genuinely differ. */ + bytes2[blen2 - 1] ^= 0xFF; + uint8_t sha2[32]; + SHA256(bytes2, blen2, sha2); + TEST_ASSERT_FALSE(memcmp(sha, sha2, 32) == 0); + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytes2, blen2, sha2)); + free(bytes2); + + /* cfgref_sha (captured at checkpoint time) still shows the OLD matching + * value -- a naive implementation trusting it would wrongly stay clear. + * The correct implementation re-reads the file fresh and fences. */ + TEST_ASSERT_EQUAL_HEX64(tag, c->persist.cfgref_tag); + TEST_ASSERT_EQUAL_INT(-EAGAIN, upgrade_materialization_verify(c)); + TEST_ASSERT_EQUAL_INT(1, upgrade_materialization_fenced(c)); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_upgrade_materialization_verify_stale_local_bytes_after_materialization_fences: PASS\n"); +} + /* (4.3-C) FENCE IS ENFORCED by a production reader: while the config serve-fence * is set, ha_dispatch_gate refuses config-dependent state-changing operations on * the PRIMARY (loud non-serving, member stays alive) and a fenced member is @@ -4407,6 +4652,11 @@ void test_config_repl(void) * no-marker fallback, serve-fence enforced by dispatch gate + promotion). */ RUN_TEST(test_push_path_mismatch_does_not_preactivate); RUN_TEST(test_config_activate_no_marker_falls_back_and_loads); + /* config-upgrade-snapshot task 6.1: Activation Fence. */ + RUN_TEST(test_upgrade_materialization_verify_no_marker_clears_fence); + RUN_TEST(test_upgrade_materialization_verify_matching_materialization_clears_fence); + RUN_TEST(test_upgrade_materialization_verify_incomplete_materialization_fences); + RUN_TEST(test_upgrade_materialization_verify_stale_local_bytes_after_materialization_fences); RUN_TEST(test_serve_fence_enforced_by_dispatch_gate_and_promotion); /* Task 4.3 r2 remediation: commit-advance retry clears a push-before-commit * serve-fence (Finding D / R7.9). */