diff --git a/.kiro/specs/config-upgrade-snapshot/tasks.md b/.kiro/specs/config-upgrade-snapshot/tasks.md index 25607ead..82624de2 100644 --- a/.kiro/specs/config-upgrade-snapshot/tasks.md +++ b/.kiro/specs/config-upgrade-snapshot/tasks.md @@ -136,7 +136,7 @@ - _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 +- [x] 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_ @@ -150,6 +150,8 @@ ## Implementation Notes +- **Task 8.8 (unit-level, no live soak needed — deliberately excludes 8.6):** proves Req 4.4/7.2 (a second marker committing before the first's install completes must cleanly supersede it, never conflate) via two independently-built `t42`/`t43`-family fixtures in `test_config_repl.c`: `c_control` commits+applies+checkpoints marker A then marker B normally; `c_raced` holds the shared `checkpoint_in_progress` CAS across *both* markers' commit+apply (inlining `t43_commit_marker`'s 4-step body rather than changing its signature), so both markers' forced-checkpoint attempts hit `-EBUSY` and skip — genuinely "still installing." Mid-race assertions (captured before releasing the CAS) prove `mc->cluster_config` already reflects marker B while `cfgref_present` is still false and the fence is set; post-resolve assertions (after releasing the CAS, calling `persist_checkpoint()`, then an explicit `upgrade_materialization_verify()`) prove the settled state reflects only B. Discovered mid-implementation that whole-file `snapdiff_paths` byte-identity between the two fixtures' final checkpoints does NOT hold — `c_control`'s marker-A checkpoint physically consumes one extra WAL LSN (`persist_write_checkpoint_marker`, persist.c:1684) before marker B is appended, so B lands at a different absolute LSN in each fixture, and `write_snapshot`'s sentinel header embeds `checkpoint_lsn` at byte offset 8. Adapted to a field-aware comparison (full `memcmp` from byte 16 onward) plus `snapdiff_paths` diagnostic corroboration that offset 8 is the *only* divergence — a more precise proof than blind byte-identity, reviewer independently re-derived and confirmed this against `write_snapshot`'s actual layout rather than trusting the claim. One RED→GREEN cycle (`bug-1473`, test-construction gap not a production bug): the bare `persist_checkpoint()` resolve call doesn't itself trigger `upgrade_materialization_verify()` in a delaymgr-less unit fixture — production's self-healing (`schedule_upgrade_materialization_retry`, fired from `ha.c:2541-2542` on every commit-advance) only runs on a live cluster; the fixture needs the manual call, mirroring the existing 6.1-c fence tests' pattern. Reviewer's only findings were non-blocking style notes (could have used the public `upgrade_materialization_retry_fn` wrapper instead of calling `upgrade_materialization_verify` directly — functionally identical here since the fence was already independently confirmed set). Suite: 1027 → 1028 tests, 0 failures. + - **Task 8.7 (both scenarios, unit-level, no live soak needed):** design.md's Testing Strategy was explicit this task should use deterministic fault injection rather than chaotic-soak timing probability, and both hooks needed already existed from earlier tasks (`persist_marker_checkpoint_fault_hook`, task 8.1; `xce_upgrade_marker_committed_hook`, task 7) — task 8.7 was purely additive test work, zero production code touched. **Scenario 1** (`test_marker_checkpoint_fault_hook_crash_then_restart_resumes_materialization`, `test_upgrade_snapshot_apply.c`) builds on the existing seam test (which only proved the fault hook fires and gates the checkpoint call) by clearing the fault, tearing down without deleting the on-disk WAL, and booting a genuinely fresh `persist_t`/`cluster_t` via `persist_recover()` — a real second-process boot-replay simulation, deliberately not a same-process re-call (which the test's own comment argues would be a weaker, trivially-passing proof). **Scenario 2** (`test_leadership_change_mid_materialization_completes_via_same_generic_apply_drain`, `test_config_repl.c`) proves Req 5.4 by building a promoted member and a non-promoted control member from byte-identical committed-but-unapplied markers, draining each via the respective real production path (`persist_apply_pump_run_sync`, which production promotion — `raft_cb_become_leader`, `ha.c` — calls unconditionally right after the role flip; vs. plain `persist_apply_pump_run` for the control), and asserting the resulting checkpoints are byte-identical via `snapdiff_paths` (task 8.4's tool). Reviewer independently verified (not trusted) both design claims by reading `ha.c` directly (`upgrade_materialization_fenced` only refuses an *already*-fenced member; the sync-drain call is unconditional and unguarded by role-specific logic) and specifically interrogated whether this was a tautology (calling the same underlying function twice) — concluded it genuinely is not, since the real variable under test is role state at drain time plus the production entry point, and the test separately asserts promotion alone has zero effect on the fence. Also confirmed via direct comparison against the existing 6.2/8.3 fence tests that this is new *continuation* coverage (fence unset → promotion succeeds → drain completes → config activates), not a duplicate of the existing *gating* coverage (fence set → promotion refused, never reaching the drain). - **Task 8.6 progress (infrastructure + two real production bugs found and fixed; task NOT YET complete — one more bug blocks a clean live-soak PASS):** building this task's live 3-node chaotic-upgrade soak required substantial new infrastructure that never existed anywhere in this codebase: a client-side send path for `OP_upgradeConfig` (`xce_upgradeconfig_req_raw`/`xce_upgradeconfig`/`xcecli_upgradeconfig` — the auto-generated capnp encoder for this request type is a literal no-op for its `Data` fields, worked around via a hand-rolled bypass mirroring the existing `xce_replconfigpush_req_raw` idiom), a `cmd_hostctrl`/`cmd_upgradeconfig` CLI pair in `test.c`, and a new `--config-upgrade` mode in `run-ha-soak.sh` (+ `SKILL.md` docs). Building and running this surfaced two real, independently-confirmed production defects: diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index 0d40ffb9..967e9030 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -2491,6 +2491,376 @@ static void test_wal_config_marker_apply_activates_originating_member_config(voi printf("test_wal_config_marker_apply_activates_originating_member_config: PASS\n"); } +/* ── config-upgrade-snapshot task 8.8: a second configuration-change marker + * committing before the first's snapshot install completes everywhere must + * cleanly SUPERSEDE the first's in-flight materialization -- never leave it + * partially applied or conflated with the newer one (Req 4.4, 7.2) ──────── + * + * design.md ("Overlapping-marker -EBUSY handling", ~line 258, and + * "Overlapping upgrades", ~line 413): when marker B's own apply runs while + * marker A's forced checkpoint is still physically in flight (holding the + * shared checkpoint_in_progress CAS), marker B's own checkpoint call ALSO + * hits -EBUSY -- not an error, self-resolving. Content is a deterministic + * function of (term, lsn, config-hash), and config_committed_marker always + * resolves to the LATEST committed marker within the committed prefix + * (scan_latest_committed_marker, config_repl.c:124-215, "later markers + * overwrite -- latest wins", line 198). So once checkpointing is unblocked + * and a fresh checkpoint attempt runs, it always reflects the latest marker + * -- never a stale/partial mix of the two. + * + * Two independently-built single-member fixtures process the IDENTICAL + * entry sequence (same term, same two deterministic config blobs via + * t1469_make_config_blob -- byte-identical across calls per its own doc + * comment, the same determinism-across-fixtures property task 8.7 + * Scenario 2 above already relies on): + * + * c_control -- marker A commits+applies+checkpoints normally, THEN marker + * B commits+applies+checkpoints normally. No racing. + * + * c_raced -- the shared checkpoint_in_progress CAS is held BEFORE marker A + * commits and never released until AFTER both markers have committed and + * applied: marker A's own forced checkpoint attempt -EBUSYs and skips + * (still "installing"), then marker B commits+applies while A's install is + * still outstanding, and marker B's own forced checkpoint attempt ALSO + * -EBUSYs and skips. Only after releasing the CAS and running + * persist_checkpoint() once directly (the same resolve step + * test_marker_apply_busy_checkpoint_handled_gracefully in + * test_upgrade_snapshot_apply.c uses) does a checkpoint actually land. + * + * NOTE on why this test does NOT assert whole-file byte-identity between the + * two fixtures' final checkpoints: c_control's marker A checkpoint + * physically WRITES a WAL_CHECKPOINT entry (persist_write_checkpoint_marker) + * before marker B is ever appended, consuming one physical WAL lsn slot that + * c_raced's fixture never consumes at that point (its checkpoint attempt for + * A never gets past the checkpoint_in_progress CAS check, so nothing is + * physically written for it). Marker B therefore lands at a DIFFERENT + * absolute physical lsn in each fixture (one higher in c_control) purely as + * a side effect of how many checkpoints happened to physically land before + * it -- exactly the "content is a deterministic function of (term, lsn, + * config-hash)" property this task's own description calls out (the + * checkpoint's own lsn is legitimately part of "content"). write_snapshot's + * sentinel header embeds this lsn verbatim at a fixed byte offset + * (persist.c: "magic (8) + checkpoint_lsn (8) + ..."), so the two files are + * expected to diverge at EXACTLY that one field. The decisive proof is + * instead: byte-identical content everywhere else -- the committed-config + * reference (cfgref_tag/cfgref_sha) and the entire entity+config-bytes + * payload -- which is precisely what would carry any trace of marker A's + * incomplete install, if one existed. snapdiff_paths is still used (for the + * diagnostic corroboration that the divergence, if any, is exactly and only + * that one field), alongside a direct field-aware comparison of everything + * from byte 16 onward. */ +static void test_second_config_marker_supersedes_first_still_installing_on_member(void) +{ + char tmp_control[256], tmp_raced[256]; + strcpy(tmp_control, "/tmp/xce_cr88_control_XXXXXX"); + strcpy(tmp_raced, "/tmp/xce_cr88_raced_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp_control)); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp_raced)); + + const uint64_t term = 88; + + cluster_t *c_control = t42_make_cluster(tmp_control); + cluster_t *c_raced = t42_make_cluster(tmp_raced); + TEST_ASSERT_NOT_NULL(c_control); + TEST_ASSERT_NOT_NULL(c_raced); + t42_make_leader(c_control, term, /*n_peers*/1); + t42_make_leader(c_raced, term, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c_control->commit, &c_control->persist, 1, 8)); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c_raced->commit, &c_raced->persist, 1, 8)); + repl_set_commit_engine(&c_control->persist.repl, &c_control->commit); + repl_set_commit_engine(&c_raced->persist.repl, &c_raced->commit); + myconf_t *mc_control = t43_attach_myconf(c_control, tmp_control); + myconf_t *mc_raced = t43_attach_myconf(c_raced, tmp_raced); + + /* Identical deterministic blobs across BOTH fixtures -- independent + * encode calls per fixture (t1469_make_config_blob's own doc comment: + * byte-identical for the same (gen, name) parameters). */ + size_t oldlen = 0, newlen = 0; + uint8_t *old_control = t1469_make_config_blob(1, "t1469-old", &oldlen); + uint8_t *new_control = t1469_make_config_blob(2, "t1469-new", &newlen); + uint8_t *old_raced = t1469_make_config_blob(1, "t1469-old", &oldlen); + uint8_t *new_raced = t1469_make_config_blob(2, "t1469-new", &newlen); + uint8_t oldsha[32], newsha[32]; + SHA256(old_control, oldlen, oldsha); + SHA256(new_control, newlen, newsha); + uint64_t oldtag; { uint64_t t; memcpy(&t, oldsha, 8); oldtag = be64toh(t); } + uint64_t newtag; { uint64_t t; memcpy(&t, newsha, 8); newtag = be64toh(t); } + + /* ── Fixture 1: control -- no racing; both markers checkpoint normally. */ + TEST_ASSERT_EQUAL_INT(0, + conf_save_config(mc_control, old_control, oldlen, oldsha)); + free(old_control); + t43_commit_marker(c_control, oldtag, oldsha); + TEST_ASSERT_EQUAL_INT(0, + conf_save_config(mc_control, new_control, newlen, newsha)); + free(new_control); + t43_commit_marker(c_control, newtag, newsha); + uint64_t control_ckpt_lsn = c_control->persist.checkpoint_lsn; + bool control_config_present = (mc_control->cluster_config != NULL); + uint64_t gen_control = + control_config_present ? mc_control->cluster_config->config_generation : 0; + int fenced_control = upgrade_materialization_fenced(c_control); + + /* ── Fixture 2: raced -- marker A's install is still genuinely + * outstanding (its checkpoint attempt never lands) when marker B + * commits. Hold the shared checkpoint_in_progress CAS BEFORE marker A + * commits and never release it until AFTER marker B has also committed + * and applied -- this is what makes marker A genuinely "still + * installing" at the moment marker B supersedes it, not a race that + * might or might not manifest. */ + TEST_ASSERT_EQUAL_INT(0, + conf_save_config(mc_raced, old_raced, oldlen, oldsha)); + free(old_raced); + + TEST_ASSERT_EQUAL_INT( + 0, atomic_exchange_explicit(&c_raced->persist.checkpoint_in_progress, + 1, memory_order_acq_rel)); + + /* Marker A: inline t43_commit_marker's 4-step body -- its signature must + * stay unmodified for other callers, and the CAS needs to stay held + * across BOTH markers, which a single call to t43_commit_marker cannot + * do. */ + uint64_t lsn_a = 0; + TEST_ASSERT_EQUAL_INT(0, + config_append_marker(c_raced, oldtag, oldsha, &lsn_a)); + TEST_ASSERT_EQUAL_INT(0, persist_flush(&c_raced->persist)); + TEST_ASSERT_EQUAL_INT(0, + commit_submit(&c_raced->commit, lsn_a, COMMIT_MODE_WAIT, 1000)); + t42_wait_applied(&c_raced->persist, lsn_a, 2000); + TEST_ASSERT_TRUE(persist_last_applied_lsn(&c_raced->persist) >= lsn_a); + + /* Marker B: committed and applied while the CAS is STILL held -- marker + * A's install has not happened yet. */ + TEST_ASSERT_EQUAL_INT(0, + conf_save_config(mc_raced, new_raced, newlen, newsha)); + free(new_raced); + uint64_t lsn_b = 0; + TEST_ASSERT_EQUAL_INT(0, + config_append_marker(c_raced, newtag, newsha, &lsn_b)); + TEST_ASSERT_EQUAL_INT(0, persist_flush(&c_raced->persist)); + TEST_ASSERT_EQUAL_INT(0, + commit_submit(&c_raced->commit, lsn_b, COMMIT_MODE_WAIT, 1000)); + t42_wait_applied(&c_raced->persist, lsn_b, 2000); + TEST_ASSERT_TRUE(persist_last_applied_lsn(&c_raced->persist) >= lsn_b); + + /* ── Mid-scenario: genuinely "still installing" -- captured BEFORE + * releasing the CAS. */ + bool config_present_mid = (mc_raced->cluster_config != NULL); + uint64_t gen_mid = + config_present_mid ? mc_raced->cluster_config->config_generation : 0; + char *clustername_mid = + (config_present_mid && mc_raced->cluster_config->clustername) + ? strdup(mc_raced->cluster_config->clustername) : NULL; + bool cfgref_present_mid = c_raced->persist.cfgref_present; + int fenced_mid = upgrade_materialization_fenced(c_raced); + + /* Release the CAS -- whatever else was (simulated to be) holding it has + * finished; marker A's and marker B's own checkpoint attempts are both + * long past (both -EBUSY'd and skipped). */ + atomic_store_explicit(&c_raced->persist.checkpoint_in_progress, 0, + memory_order_release); + + /* Resolve: a fresh checkpoint attempt, self-resolving per design.md -- + * mirrors test_marker_apply_busy_checkpoint_handled_gracefully's own + * resolve step (test_upgrade_snapshot_apply.c). A bare persist_checkpoint() + * call is NOT itself a marker-apply dispatch, so it does not call + * upgrade_materialization_verify() on its own -- that call only happens + * from apply_committed_entry's WAL_CONFIG_MARKER case (a NEW marker + * applying) or from the delaymgr-driven retry trampoline + * (schedule_upgrade_materialization_retry), and this bare fixture never + * initializes a delaymgr. schedule_upgrade_materialization_retry's own + * doc comment is explicit about this exact situation: "No-op when... + * the delaymgr is not running (the unit-test fixture path...) -- caller + * drives directly". Explicitly re-verify here, mirroring the 6.1-c fence + * tests above (e.g. immediately after t43_commit_marker) which do the + * same for the same reason. */ + int resolve_rc = persist_checkpoint(&c_raced->persist); + uint64_t raced_ckpt_lsn = c_raced->persist.checkpoint_lsn; + int verify_rc = upgrade_materialization_verify(c_raced); + + /* ── Post-resolve: the "settled" state. */ + uint64_t committed_tag_after = 0; + uint8_t committed_sha_after[32]; + int rc_committed_after = config_committed_marker( + c_raced, &committed_tag_after, committed_sha_after); + bool cfgref_matches_after = c_raced->persist.cfgref_present && + c_raced->persist.cfgref_tag == newtag && + memcmp(c_raced->persist.cfgref_sha, newsha, 32) == 0; + int fenced_after = upgrade_materialization_fenced(c_raced); + bool config_present_after = (mc_raced->cluster_config != NULL); + uint64_t gen_after = + config_present_after ? mc_raced->cluster_config->config_generation : 0; + char *clustername_after = + (config_present_after && mc_raced->cluster_config->clustername) + ? strdup(mc_raced->cluster_config->clustername) : NULL; + + /* ── Cross-fixture proof: settle both checkpoint file paths. */ + char control_snap_path[4096], raced_snap_path[4096]; + snprintf(control_snap_path, sizeof(control_snap_path), + "%s/snap-%016" PRIx64 ".capnp", c_control->persist.snap_path, + control_ckpt_lsn); + snprintf(raced_snap_path, sizeof(raced_snap_path), + "%s/snap-%016" PRIx64 ".capnp", c_raced->persist.snap_path, + raced_ckpt_lsn); + bool control_snap_exists = (access(control_snap_path, F_OK) == 0); + bool raced_snap_exists = (access(raced_snap_path, F_OK) == 0); + + snapdiff_report_t diff_rep; + memset(&diff_rep, 0, sizeof(diff_rep)); + int diff_rc = snapdiff_paths(control_snap_path, raced_snap_path, NULL, + &diff_rep); + + /* Independent, field-aware comparison: read both files fully and compare + * everything AFTER the 16-byte [magic(8) + checkpoint_lsn(8)] prefix -- + * i.e. cfgref_present/tag/sha, format_version, and the entire + * entity+config-bytes payload (write_snapshot's sentinel layout, + * persist.c). This excludes the ONE field expected to legitimately + * differ (the checkpoint's own physical WAL lsn) and asserts everything + * else -- which is what would carry any trace of marker A's incomplete + * install -- is byte-for-byte identical. */ + long control_size = -1, raced_size = -1; + uint8_t *control_buf = NULL, *raced_buf = NULL; + FILE *fp_control = fopen(control_snap_path, "rb"); + if (fp_control) { + fseek(fp_control, 0, SEEK_END); + control_size = ftell(fp_control); + fseek(fp_control, 0, SEEK_SET); + if (control_size > 0) { + control_buf = (uint8_t *)malloc((size_t)control_size); + if (control_buf) + TEST_ASSERT_EQUAL_INT((int)control_size, + (int)fread(control_buf, 1, (size_t)control_size, fp_control)); + } + fclose(fp_control); + } + FILE *fp_raced = fopen(raced_snap_path, "rb"); + if (fp_raced) { + fseek(fp_raced, 0, SEEK_END); + raced_size = ftell(fp_raced); + fseek(fp_raced, 0, SEEK_SET); + if (raced_size > 0) { + raced_buf = (uint8_t *)malloc((size_t)raced_size); + if (raced_buf) + TEST_ASSERT_EQUAL_INT((int)raced_size, + (int)fread(raced_buf, 1, (size_t)raced_size, fp_raced)); + } + fclose(fp_raced); + } + + bool sizes_match = (control_size >= 0 && control_size == raced_size); + bool payload_beyond_lsn_matches = false; + if (sizes_match && control_buf && raced_buf && control_size > 16) { + payload_beyond_lsn_matches = + (memcmp(control_buf + 16, raced_buf + 16, + (size_t)(control_size - 16)) == 0); + } + uint64_t control_lsn_in_file = 0, raced_lsn_in_file = 0; + if (control_buf && control_size >= 16) + memcpy(&control_lsn_in_file, control_buf + 8, 8); + if (raced_buf && raced_size >= 16) + memcpy(&raced_lsn_in_file, raced_buf + 8, 8); + + free(control_buf); + free(raced_buf); + + t43_free_myconf(c_control); + t43_free_myconf(c_raced); + commit_engine_fini(&c_control->commit); + commit_engine_fini(&c_raced->commit); + t42_teardown(c_control); + t42_teardown(c_raced); + + /* ── Assertions ──────────────────────────────────────────────────── */ + + /* Control baseline sanity. */ + TEST_ASSERT_TRUE(control_config_present); + TEST_ASSERT_EQUAL_UINT64(2, gen_control); + TEST_ASSERT_EQUAL_INT(0, fenced_control); + + /* Mid-scenario: genuinely "still installing". */ + TEST_ASSERT_TRUE_MESSAGE(config_present_mid, + "member B's own apply must have activated config independently of " + "marker A's install state"); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(2, gen_mid, + "activation must reflect the LATEST marker (B) purely from B's own " + "apply, never re-activating to A afterward nor staying short of B"); + TEST_ASSERT_NOT_NULL(clustername_mid); + if (clustername_mid) + TEST_ASSERT_EQUAL_STRING("t1469-new", clustername_mid); + TEST_ASSERT_FALSE_MESSAGE(cfgref_present_mid, + "no checkpoint has physically landed for EITHER marker yet -- both " + "A's and B's checkpoint attempts were skipped via -EBUSY, proving " + "this is genuinely 'still installing', not a coincidental pass"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, fenced_mid, + "materialization for the latest committed marker (B) has not " + "completed yet -- the fence must still be set"); + + /* Post-resolve: settled state reflects ONLY marker B. */ + TEST_ASSERT_EQUAL_INT(0, rc_committed_after); + TEST_ASSERT_EQUAL_HEX64(newtag, committed_tag_after); + TEST_ASSERT_EQUAL_INT(0, memcmp(newsha, committed_sha_after, 32)); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, resolve_rc, + "the resolving checkpoint attempt must succeed once the CAS guard " + "is free -- self-resolving, per design.md"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, verify_rc, + "the explicit re-verify (standing in for this bare fixture's absent " + "delaymgr-driven retry trampoline) must succeed once a checkpoint " + "has actually captured marker B"); + TEST_ASSERT_TRUE_MESSAGE(cfgref_matches_after, + "the checkpoint sentinel must now reflect marker B, never marker A"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, fenced_after, + "the fence must clear once materialization for the latest marker " + "(B) completes"); + TEST_ASSERT_TRUE(config_present_after); + TEST_ASSERT_EQUAL_UINT64(2, gen_after); + TEST_ASSERT_NOT_NULL(clustername_after); + if (clustername_after) + TEST_ASSERT_EQUAL_STRING("t1469-new", clustername_after); + + /* Cross-fixture proof -- the decisive assertion for Req 4.4/7.2. */ + TEST_ASSERT_TRUE_MESSAGE(control_snap_exists, + "control fixture must have produced a checkpoint at marker B's lsn"); + TEST_ASSERT_TRUE_MESSAGE(raced_snap_exists, + "raced fixture must have produced a checkpoint once resolved"); + TEST_ASSERT_TRUE_MESSAGE(sizes_match, + "both fixtures' settled checkpoints must be the same total size -- " + "identical entity/config content, differing at most in the " + "embedded physical WAL lsn"); + TEST_ASSERT_TRUE_MESSAGE(payload_beyond_lsn_matches, + "Req 4.4/7.2: beyond the one field that legitimately differs " + "(the checkpoint's own physical WAL lsn -- itself content per this " + "task's own 'deterministic function of (term, lsn, config-hash)' " + "framing), the settled checkpoint produced after racing marker A's " + "incomplete install must be byte-for-byte identical to the " + "non-raced control -- no trace of marker A's partially-applied " + "intermediate state survives into marker B's final materialized " + "output"); + + /* Diagnostic corroboration: snapdiff_paths independently confirms + * whether the files are literally byte-identical end-to-end, and if + * not, that the sole divergence is exactly that one embedded-lsn field + * (offset 8, per write_snapshot's sentinel layout). */ + TEST_ASSERT_TRUE_MESSAGE(diff_rc >= 0, + "snapdiff_paths must not error opening either checkpoint file"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, diff_rep.size_mismatch, + "file sizes must match even if the embedded physical lsn differs"); + if (!diff_rep.identical) { + TEST_ASSERT_EQUAL_INT_MESSAGE(8, (int)diff_rep.diff_offset, + "the only expected divergence is the embedded checkpoint_lsn " + "field at byte offset 8 (write_snapshot's sentinel layout)"); + } + TEST_ASSERT_EQUAL_UINT64(control_ckpt_lsn, control_lsn_in_file); + TEST_ASSERT_EQUAL_UINT64(raced_ckpt_lsn, raced_lsn_in_file); + + free(clustername_mid); + free(clustername_after); + + printf("test_second_config_marker_supersedes_first_still_installing_on_" + "member: PASS\n"); +} + /* ── config-upgrade-snapshot task 8.7 Scenario 2: leadership change mid- * materialization does not special-case, and completes via the SAME generic * apply-drain path (Req 5.4) ──────────────────────────────────────────── @@ -5560,6 +5930,11 @@ void test_config_repl(void) /* bug-1469: WAL_CONFIG_MARKER apply must activate config on the * ORIGINATING member too, not just receiving standbys. */ RUN_TEST(test_wal_config_marker_apply_activates_originating_member_config); + /* config-upgrade-snapshot task 8.8: a second configuration-change + * marker committing before the first's snapshot install completes + * everywhere must cleanly supersede it, never leave it partially + * applied or conflated with the newer one. */ + RUN_TEST(test_second_config_marker_supersedes_first_still_installing_on_member); /* config-upgrade-snapshot task 8.7 Scenario 2: leadership change * mid-materialization completes via the same generic apply-drain path. */ RUN_TEST(test_leadership_change_mid_materialization_completes_via_same_generic_apply_drain);