diff --git a/.kiro/specs/ha-entity-snapshots/tasks.md b/.kiro/specs/ha-entity-snapshots/tasks.md index 1bbcad8..25f7a23 100644 --- a/.kiro/specs/ha-entity-snapshots/tasks.md +++ b/.kiro/specs/ha-entity-snapshots/tasks.md @@ -137,8 +137,8 @@ lands, then re-run with a forced empty-rejoin round to get a true signal on this spec's own entity-restore correctness._ -- [ ] 8. Validation: rolling-upgrade compatibility -- [ ] 8.1 Verify a pre-feature checkpoint file is handled safely by the new binary in a live +- [x] 8. Validation: rolling-upgrade compatibility +- [x] 8.1 Verify a pre-feature checkpoint file is handled safely by the new binary in a live cluster - Plant a checkpoint file in the legacy (position-only) format, start a member on the upgraded binary against it, and observe its behavior @@ -147,6 +147,18 @@ completeness - _Requirements: 5.1, 5.2_ - _Depends: 6.1_ + - (8.1) `persist_recover`'s local boot-time snapshot load (persist.c ~2790) passed `NULL` for + `out_entities_restored` -- it had no observability parity with the wire receive path's + (`ha_handle_snapshot_chunk`, task 6.1) legacy-vs-entity-bearing INFO log. Added the same + conditional INFO logging at this call site (small, additive, no behavior change -- WAL replay + in Step 3 still applies unconditionally either way). Live-verified: planted a hand-crafted + 16-byte legacy sentinel (`XCESNAP\0` + LE checkpoint_lsn=1, no entity frames) as ctrl-1's own + `snap-0000000000000001.capnp` in a 3-node cluster (reusing the ha-entity-snap soak deploy, + WAL segments left intact), restarted all 3. ctrl-1: no crash, logged + "persist_recover: ... is a legacy position-only checkpoint ... continuing via normal WAL + replay", was elected primary, served the full 37-job list, and `walcompare` reported all 3 + members' committed prefixes (LSN 1..49) byte-identical -- confirms full committed consistency + reached via ordinary WAL replay, no false-positive completeness. ## Implementation Notes - (1.1) Test fixture `snapent_make`/`snapent_teardown` lives in `test_persist_snapshot_entities.c`; seeded jobs currently carry only jobid+state+an all-zeroed submit. Task 2.1's "field-identical round-trip" assertion needs richer submit-payload seeding (e.g. non-zero cmdline/user/queue in `xce_job_submit_t`) to be a meaningful field-by-field comparison — extend `snapent_seed_jobs`, do not re-seed separately. diff --git a/src/controller/persist.c b/src/controller/persist.c index d578a76..455d9cb 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -2789,16 +2789,30 @@ int persist_recover(persist_t* p) /* --- Step 2: load snapshot (if any) --- */ if (snap_lsn > 0 && snap_file[0] != '\0') { - /* Local recovery doesn't need the entities-restored indicator: the - * WAL replay in Step 3 applies everything above checkpoint_lsn - * regardless, unchanged by ha-entity-snapshots. */ - int rc = load_snapshot(p, snap_file, snap_lsn, NULL); + /* Local recovery doesn't need the entities-restored indicator to + * decide behavior: the WAL replay in Step 3 applies everything + * above checkpoint_lsn regardless, unchanged by ha-entity-snapshots. + * It IS consumed for observability parity with the wire receive + * path (ha_handle_snapshot_chunk, task 6.1) — a rolling-upgrade + * restart against a pre-feature (legacy, position-only) local + * checkpoint is an expected rollout state, not an error. */ + bool entities_restored = false; + int rc = load_snapshot(p, snap_file, snap_lsn, &entities_restored); if (rc != 0) { ERROR("persist_recover: failed to load snapshot %s: %s", snap_file, strerror(-rc)); return rc; } /* checkpoint_lsn and current_lsn set by load_snapshot */ + if (entities_restored) { + INFO("persist_recover: snapshot %s loaded lsn=%" PRIu64 + " with full job/queue entity restoration", snap_file, + p->checkpoint_lsn); + } else { + INFO("persist_recover: snapshot %s lsn=%" PRIu64 " is a legacy " + "position-only checkpoint (no entity content) — continuing " + "via normal WAL replay", snap_file, p->checkpoint_lsn); + } } /* --- Step 3: replay WAL entries after checkpoint_lsn --- */