From 9fdb6778d4b96468aea7a1788f25fae3de0da7e5 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 1 Jul 2026 08:30:10 +0000 Subject: [PATCH] feat(ha-entity-snapshots): restore-before-advance load_snapshot implementation (task 5.2) Replaces the naive task-5.1 stopgap with the real restore contract: detects legacy vs entity-bearing checkpoints by remaining file size; restores every job then every queue status via restore_job/ restore_queue_state, in file order, with bounds-checked length-prefixed reads; only after every entity restores successfully does current_lsn/checkpoint_lsn commit to the checkpoint's position. Every failure path (bounds check, decode, restore error, unrecognized format_version) returns before that single commit point -- position is provably untouched on failure, never partially advanced. Also fixes the pre-existing restore_job decode leak (job.c): job_init deep-copies submit (never nulled) and MOVE_PTRs allocs/pidinfo/ pend_reasons (nulled on the one path that runs) -- free_Job(&j) is safe unconditionally on every reachable return path (EEXIST early-return, create_job success, create_job failure), verified against the exact generated free_*_ptr null-check guards. Two pre-existing test_config_repl.c wire-protocol tests needed their synthetic garbage snapshot bodies updated to a well-formed empty-entity tail, since load_snapshot now genuinely parses past the legacy header (previously ignored); their actual assertions (chunk-reassembly correctness) are unchanged. This is the exact function/contract that caused task 2.2's rejection (position advancing before restore validated). Independent review traced the memory-safety fix through the generated capnp code field-by- field, confirmed the test_config_repl.c change doesn't weaken any assertion, and confirmed restore_queue_state's ENOENT-aborts-the-whole- checkpoint behavior is faithful to design.md's unconditional contract (flagged as a watch-item for task 7.1's soak, not a defect). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JDYFbrgyDHbvNYcxApKhrX --- .kiro/specs/ha-entity-snapshots/tasks.md | 6 +- src/controller/job.c | 31 ++- src/controller/persist.c | 268 ++++++++++++++++---- src/controller/testsuite/test_config_repl.c | 48 +++- 4 files changed, 297 insertions(+), 56 deletions(-) diff --git a/.kiro/specs/ha-entity-snapshots/tasks.md b/.kiro/specs/ha-entity-snapshots/tasks.md index b189da2..4fd1032 100644 --- a/.kiro/specs/ha-entity-snapshots/tasks.md +++ b/.kiro/specs/ha-entity-snapshots/tasks.md @@ -69,7 +69,7 @@ - Observable: the RED test from 4.2 passes - _Requirements: 3.1, 3.2, 3.3_ -- [ ] 5. Core: entity restoration on load +- [x] 5. Core: entity restoration on load - [x] 5.1 RED: restore-before-advance, failure-safety, and legacy-format tests - Assert loading a checkpoint with N jobs + M queues restores all of them before the member's position advances @@ -81,7 +81,7 @@ - _Requirements: 2.1, 2.2, 2.3, 2.4, 5.1, 5.2_ - _Boundary: Persistence Layer (load_snapshot)_ - _Depends: 2.2, 3.2_ -- [ ] 5.2 Restore entities before advancing the member's position, with legacy-format detection +- [x] 5.2 Restore entities before advancing the member's position, with legacy-format detection - Detect whether a checkpoint carries entity content or is a legacy position-only file - Restore every job then every queue status via the existing restore functions, in file order; on any failure, stop and leave position unchanged @@ -134,3 +134,5 @@ - (4.1) `snapshot_inflight_lsn[peer_idx]` clears to 0 BEFORE `snapshot_inflight[peer_idx]` clears to 0 in every path (completion + all 3 failure sites in `ha_snapshot_send_dispatch`). Task 4.3's `rotate_snapshots` consultation must therefore treat `snapshot_inflight_lsn[i] != 0` as the "protected" signal directly (checkpoint LSNs are never 0), not "check flag==1 first, then trust LSN" -- there's a narrow window where the flag is still 1 but the LSN has already cleared. - (execution note, after 4.1) Remaining tasks split by risk given a mid-session spend-limit hit: low-risk/small/isolated tasks (4.2, 4.3, 7.1, 8.1) executed directly in the main context with self-review (build + test + boundary/TBD/secrets grep), no subagent dispatch. High-risk tasks (5.1/5.2 `load_snapshot` restore -- the exact function that caused task 2.2's rejection; 6.1 wiring into the live `ha_handle_snapshot_chunk` production path) keep the full implementer + independent-reviewer subagent cycle, since that adversarial review already caught a real live-hazard bug once this spec (task 2.2) and the remaining risk is concentrated there, not spread evenly. - (5.1) `persist_load_snapshot`'s wrapper currently maps `*out_entities_restored = (rc==0)` -- a deliberately naive stopgap so the 3 RED tests fail for the right reasons. `load_snapshot`'s real body is completely untouched (still watermark-only, byte-identical to before this task). Task 5.2 must replace the wrapper's naive mapping wholesale with real format-detection + restore-then-gate-position logic; it owns turning `load_snapshot` from a no-op-on-entities function into the real restore path. +- (5.2) `restore_queue_state`'s `-ENOENT` ("queue not yet built from config") aborts the ENTIRE checkpoint restore, including already-successfully-restored jobs earlier in the same file -- this is design-compliant (design.md/Req 2.3 are unconditional: any restore failure leaves position unchanged) and not a defect, but it is a real residual availability risk: a benign config-lag on the destination member (a queue the source references not yet locally provisioned) would make catch-up fail outright rather than skip-and-continue. WATCH-ITEM for task 7.1's chaotic soak: if a catch-up round gets stuck/never completes, this exact code path (queue.c's restore_queue_state ENOENT -> persist.c's load_snapshot_entities abort) is the first place to look. +- (5.2) Test-only, non-blocking: the destination fixture's `snapent_teardown` does not release job_t objects added via `restore_job` (only the ones seeded at fixture creation) -- a pre-existing category of test-fixture-teardown gap (same class as tasks.md's (3.2) note), now slightly enlarged since load-path tests exercise restore_job against a destination fixture. Not production code; extend snapent_teardown if this file keeps growing. diff --git a/src/controller/job.c b/src/controller/job.c index a03c3be..3227dc0 100644 --- a/src/controller/job.c +++ b/src/controller/job.c @@ -118,12 +118,37 @@ int restore_job(cluster_t* c, uint64_t jobid_unused, lock(c->jobmgr, RTTI_LOCK_READ); int found = key_find(c->jobmgr, (void*)j.jobid, (void**)&existing); unlock(c->jobmgr); - if (found > 0 && existing != NULL) + if (found > 0 && existing != NULL) { + /* j was fully decoded above (submit/allocs/pidinfo/pend_reasons all + * populated by decode_Job) and create_job() is never reached on this + * path, so ownership never moved -- every decoded pointer is still + * ours to release here (ha-entity-snapshots task 5.2: fixes the + * pre-existing decode leak on this early-return path, called out in + * tasks.md's Implementation Notes). */ + free_Job(&j); return -EEXIST; + } + + job_t* job = create_job(c, &j); + int saved_rc = job ? 0 : -errno; + + /* create_job() (jobj.c job_init) takes ownership of j.allocs/j.pidinfo/ + * j.pend_reasons via MOVE_PTR (nulling them here on success) and makes + * its OWN deduplicated copies of j.submit/j.ckptinfo (makeJobsub/ + * makeString) -- so j.submit, the one field decode_Job always populates + * that job_init never moves, is never released by create_job and must + * always be freed here. free_Job() null-checks each of its four fields + * before freeing (submit/allocs/pidinfo/pend_reasons), so calling it + * unconditionally is safe whether create_job succeeded (three of the + * four already nulled by the MOVE_PTR above) or failed before job_init + * ran any MOVE_PTR (none nulled, all four correctly released) -- never a + * double free either way (ha-entity-snapshots task 5.2, pre-existing + * leak noted in tasks.md's Implementation Notes). */ + free_Job(&j); - job_t* job = create_job(c, &j); if (!job) - return -errno; + return saved_rc; + /* ha-convergence-recovery (3.3): tag the restored job with its submit * entry's LSN so the egress watermark predicate can decide its commit * status (job_submit_committed). On a promoted primary, replayed entries diff --git a/src/controller/persist.c b/src/controller/persist.c index fc8d44b..d578a76 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -2162,10 +2162,137 @@ done: /* Recovery internals */ /* ------------------------------------------------------------------ */ -/* P0 sentinel format: "XCESNAP\0" (8 bytes) + checkpoint_lsn (8 bytes LE). - * Returns 0 on success and sets p->checkpoint_lsn / p->current_lsn. - * Returns -EINVAL on bad magic, -EIO on short read, -errno on open failure. */ -static int load_snapshot(persist_t* p, const char* path, uint64_t lsn) +/* Parse and restore every job frame then every queue frame from the entity + * payload beginning at the current `fd` offset (design.md "Checkpoint File + * Layout"): [job_count][job_count * {payload_len; capnp Job bytes}] + * [queue_count][queue_count * {name_len; name; status}]. + * + * `file_size` is the checkpoint file's total size (from fstat) and + * `consumed` is the number of bytes already read from it (legacy header + + * format_version) — every length-prefixed frame's length is bounds-checked + * against the ACTUAL remaining bytes in the file before being trusted for a + * read or allocation, never blindly followed (a corrupt/attacker-controlled + * length must fail fast, not drive an oversized read). + * + * Restores in file order: every job via restore_job, then every queue via + * restore_queue_state (design.md sequence diagram). Returns 0 only if every + * frame restored; on the FIRST failure of any kind (short read, oversized + * length, decode error, or restore error) returns immediately with a + * negative errno — the caller MUST NOT commit any position change on a + * non-zero return (Requirement 2.3). + * + * restore_job's -EEXIST is treated as success, not a failure: it is job.c's + * own documented idempotent-replay guard (job already present), needed so a + * retried load — after an earlier partial attempt already created some jobs + * before failing on a later one — can continue past already-restored entries + * rather than aborting on them again. restore_queue_state has no such + * idempotent-duplicate case (re-applying the same status is already a + * harmless overwrite), so any non-zero return from it is a genuine failure. */ +static int load_snapshot_entities(int fd, off_t file_size, off_t consumed, + persist_t* p, uint64_t stored_lsn) +{ + uint32_t job_count = 0; + if (read(fd, &job_count, sizeof(job_count)) != (ssize_t)sizeof(job_count)) + return -EIO; + consumed += (off_t)sizeof(job_count); + + if (job_count > 0 && (p->cluster == NULL || p->cluster->jobmgr == NULL)) + return -EINVAL; /* entities present but nowhere to restore them into */ + + for (uint32_t i = 0; i < job_count; i++) { + uint32_t payload_len = 0; + if (read(fd, &payload_len, sizeof(payload_len)) != + (ssize_t)sizeof(payload_len)) + return -EIO; + consumed += (off_t)sizeof(payload_len); + + if (payload_len == 0 || (off_t)payload_len > file_size - consumed) + return -EIO; + + uint8_t* buf = (uint8_t*)malloc(payload_len); + if (buf == NULL) + return -ENOMEM; + if (read(fd, buf, payload_len) != (ssize_t)payload_len) { + free(buf); + return -EIO; + } + consumed += (off_t)payload_len; + + int rrc = restore_job(p->cluster, 0, buf, payload_len, stored_lsn); + free(buf); + if (rrc != 0 && rrc != -EEXIST) + return rrc; + } + + uint32_t queue_count = 0; + if (read(fd, &queue_count, sizeof(queue_count)) != + (ssize_t)sizeof(queue_count)) + return -EIO; + consumed += (off_t)sizeof(queue_count); + + if (queue_count > 0 && (p->cluster == NULL || p->cluster->qmgr == NULL)) + return -EINVAL; /* entities present but nowhere to restore them into */ + + for (uint32_t i = 0; i < queue_count; i++) { + uint32_t name_len = 0; + if (read(fd, &name_len, sizeof(name_len)) != (ssize_t)sizeof(name_len)) + return -EIO; + consumed += (off_t)sizeof(name_len); + + if (name_len == 0 || (off_t)name_len > file_size - consumed) + return -EIO; + + char* name = (char*)malloc((size_t)name_len + 1); + if (name == NULL) + return -ENOMEM; + if (read(fd, name, name_len) != (ssize_t)name_len) { + free(name); + return -EIO; + } + name[name_len] = '\0'; + consumed += (off_t)name_len; + + uint32_t status = 0; + if (read(fd, &status, sizeof(status)) != (ssize_t)sizeof(status)) { + free(name); + return -EIO; + } + consumed += (off_t)sizeof(status); + + int qrc = restore_queue_state(p->cluster, name, status); + free(name); + if (qrc != 0) + return qrc; + } + + return 0; +} + +/* Load a snapshot: legacy header (magic + checkpoint_lsn + optional + * config-ref tail), then — for an entity-bearing checkpoint — every job and + * queue frame that follows it (ha-entity-snapshots task 5.2, Requirements + * 2.1-2.4, 5.1, 5.2). See design.md "load_snapshot (extended)" and its + * sequence diagram for the exact contract this implements. + * + * Ordering (Requirement 2.1, 2.2, 2.3 — restore FIRST, validate FULLY, THEN + * advance position, never the reverse, never partial): the header and any + * config-ref tail are parsed into LOCAL variables only. Format is then + * detected by whether any bytes remain past the header (design.md's stated + * discriminator — not a flag bit). A short file (nothing remains) is a + * legacy, pre-feature checkpoint: not an error, so its recorded position is + * committed immediately and *out_entities_restored is false. Otherwise every + * job then every queue frame is restored via load_snapshot_entities() before + * ANYTHING is written to p->checkpoint_lsn/p->current_lsn/p->cfgref_*; on + * any restore failure this function returns non-zero with p's fields + * PROVABLY untouched (the single commit point below is only ever reached + * after load_snapshot_entities() returns 0). *out_entities_restored is only + * set on a rc==0 return, matching persist.h's documented contract. + * + * Returns 0 on success, -EINVAL on bad magic / unrecognized entity format, + * -EIO on a short/corrupt read, -ENOMEM on allocation failure, -errno on + * open/fstat failure, or a restore_job/restore_queue_state error code. */ +static int load_snapshot(persist_t* p, const char* path, uint64_t lsn, + bool* out_entities_restored) { (void)lsn; /* lsn is encoded in the file itself — we re-read it */ @@ -2173,66 +2300,116 @@ static int load_snapshot(persist_t* p, const char* path, uint64_t lsn) if (fd < 0) return -errno; + struct stat st; + if (fstat(fd, &st) != 0) { + int saved = errno; + close(fd); + return -saved; + } + /* magic(8) + checkpoint_lsn(8) + optional config-ref tail(1+8+32). */ char header[16 + 1 + 8 + 32]; ssize_t n = read(fd, header, sizeof(header)); - close(fd); - if (n < 16) + if (n < 16) { + close(fd); return -EIO; + } - if (memcmp(header, "XCESNAP\0", 8) != 0) + if (memcmp(header, "XCESNAP\0", 8) != 0) { + close(fd); return -EINVAL; + } - /* Extract checkpoint_lsn from bytes 8-15 (little-endian on this arch) */ + /* Extract checkpoint_lsn from bytes 8-15 (little-endian on this arch). + * Held LOCALLY -- not yet written to p-> -- until the single commit + * point below (legacy short-circuit, or after every entity restores). */ uint64_t stored_lsn; memcpy(&stored_lsn, header + 8, 8); + /* Optional committed-config reference (ha-term-safety task 4.2 r1): + * present only when the sentinel carries the 41-byte tail. Also held + * locally until the same commit point. */ + int cfgref_present = 0; + uint64_t cfgref_tag = 0; + uint8_t cfgref_sha[32]; + memset(cfgref_sha, 0, sizeof(cfgref_sha)); + if (n >= 16 + 1 + 8 + 32 && header[16] != 0) { + cfgref_present = 1; + memcpy(&cfgref_tag, header + 17, 8); + memcpy(cfgref_sha, header + 25, 32); + } + + off_t consumed = (off_t)n; + + /* Legacy-format detection (Requirement 2.4, 5.1, 5.2): "does more data + * exist" is the discriminator (design.md), not a flag bit -- a file + * ending exactly at the legacy header is unambiguously a pre-feature + * checkpoint. This is an expected rollout shape, not corruption: commit + * the position it records (existing, preserved legacy behavior) and + * report entities_restored=false so the caller falls back to normal + * catch-up rather than treating this checkpoint as fully complete. */ + if (st.st_size <= consumed) { + close(fd); + p->checkpoint_lsn = stored_lsn; + p->current_lsn = stored_lsn; + if (cfgref_present) { + p->cfgref_present = 1; + p->cfgref_tag = cfgref_tag; + memcpy(p->cfgref_sha, cfgref_sha, sizeof(p->cfgref_sha)); + } + if (out_entities_restored) + *out_entities_restored = false; + return 0; + } + + /* Entity-bearing checkpoint: format_version, then job frames, then queue + * frames (design.md "Checkpoint File Layout"). */ + uint32_t fmt = 0; + if (read(fd, &fmt, sizeof(fmt)) != (ssize_t)sizeof(fmt)) { + close(fd); + return -EIO; + } + consumed += (off_t)sizeof(fmt); + + /* An unrecognized format_version is corruption or a future layout this + * binary cannot read -- fail loud rather than silently treating it as + * legacy (task 5.1 review note: a version mismatch must never be masked + * as an expected legacy shape, or real corruption goes undetected). */ + if (fmt != SNAPSHOT_FORMAT_VERSION) { + close(fd); + return -EINVAL; + } + + int rc = load_snapshot_entities(fd, st.st_size, consumed, p, stored_lsn); + close(fd); + if (rc != 0) + return rc; /* Requirement 2.3: p->*_lsn / p->cfgref_* untouched */ + + /* Single commit point (Requirement 2.1, 2.2): reached ONLY after every + * job and queue frame restored successfully above. */ p->checkpoint_lsn = stored_lsn; p->current_lsn = stored_lsn; - - /* Optional committed-config reference (ha-term-safety task 4.2 r1): present - * only when the sentinel carries the 41-byte tail. An older 16-byte sentinel - * (n == 16) leaves the cached ref untouched (cfgref_present stays 0 from - * persist_init), so a member that never checkpointed a config marker simply - * has no fallback reference — the live WAL scan is authoritative. */ - if (n >= 16 + 1 + 8 + 32 && header[16] != 0) { + if (cfgref_present) { p->cfgref_present = 1; - memcpy(&p->cfgref_tag, header + 17, 8); - memcpy(p->cfgref_sha, header + 25, 32); + p->cfgref_tag = cfgref_tag; + memcpy(p->cfgref_sha, cfgref_sha, sizeof(p->cfgref_sha)); } + if (out_entities_restored) + *out_entities_restored = true; return 0; } -/* Public wrapper for load_snapshot — exposed via persist.h for ha.c (Task 3.5c). - * - * ha-entity-snapshots task 5.1 (RED, signature-compat stopgap ONLY — task 5.2 - * owns the real restore-before-advance implementation): `load_snapshot` above - * is UNCHANGED, still today's watermark-only behavior (never inspects/restores - * any entity payload). This wrapper's *out_entities_restored mapping is - * deliberately the most naive one possible — "the underlying call reported - * success" — precisely so it does NOT mask the two real gaps task 5.2 must - * close: (1) a legacy (position-only) file will report entities_restored=true - * here even though it never carried an entity, since load_snapshot() returns - * 0 for ANY well-formed header regardless of what (if anything) follows it — - * a live instance of the exact false-completeness hazard Requirement 2.4 - * exists to prevent; (2) a corrupted entity frame is never even inspected — - * load_snapshot() returns 0 and already advances p->checkpoint_lsn/ - * current_lsn before this wrapper is reached, so the "position unchanged on - * failure" contract (Requirement 2.3) does not hold yet either. Both are - * exactly the behaviors the task 5.1 RED tests assert against. */ +/* Public wrapper for load_snapshot — exposed via persist.h for ha.c (Task + * 3.5c). load_snapshot() above now owns the full restore-before-advance + * implementation (ha-entity-snapshots task 5.2): this wrapper is a thin + * pass-through, since load_snapshot() already gates *out_entities_restored + * and the p->checkpoint_lsn/current_lsn commit on entity-restore success + * (Requirements 2.1-2.4, 5.1, 5.2). */ int persist_load_snapshot(persist_t *p, const char *path, uint64_t lsn, bool *out_entities_restored) { - int rc = load_snapshot(p, path, lsn); - - /* Contract: only set on rc==0 (design.md "load_snapshot (extended)" — - * "*out_entities_restored is only set when rc==0"); left untouched - * otherwise so a failed call never overwrites a caller's own state. */ - if (rc == 0 && out_entities_restored != NULL) - *out_entities_restored = true; - - return rc; + return load_snapshot(p, path, lsn, out_entities_restored); } /* P2-2: Standby WAL replay — apply a single replicated entry to in-memory state. @@ -2612,7 +2789,10 @@ int persist_recover(persist_t* p) /* --- Step 2: load snapshot (if any) --- */ if (snap_lsn > 0 && snap_file[0] != '\0') { - int rc = load_snapshot(p, snap_file, snap_lsn); + /* 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); if (rc != 0) { ERROR("persist_recover: failed to load snapshot %s: %s", snap_file, strerror(-rc)); diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index 34335b7..b279a0f 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -2954,16 +2954,38 @@ static void test_snapshot_recv_two_chunks_assembles_and_renames(void) /* Build a fake 128-byte snapshot body (two 64-byte chunks). * payload_bytes contains the raw file bytes for each chunk. * We skip writing an actual snapshot source file — the receiver - * writes snap.part and renames it; we use the resulting file to verify. */ + * writes snap.part and renames it; we use the resulting file to verify. + * + * The body must be a WELL-FORMED entity-bearing checkpoint, not opaque + * filler (ha-entity-snapshots task 5.2): load_snapshot() now actually + * parses everything past the legacy header — a 0xAA/0xBB filler tail + * used to be silently ignored (load_snapshot only ever read the 57-byte + * header) but is now correctly rejected as an unrecognized + * format_version. Layout (design.md "Checkpoint File Layout"): the + * 57-byte legacy header (cfgref_present=0 at byte 16, so bytes 17-56 are + * unread and left zero) + format_version=1 + job_count=0 + queue_count=0 + * (69 bytes total), padded with zero filler out to 128 bytes to keep the + * two-64-byte-chunk shape this test exercises — the padding is never + * read (load_snapshot_entities stops right after the queue_count=0 + * loop), so its content is immaterial. */ uint64_t snap_lsn = 100; size_t total_size = 128; + uint8_t body[128]; + memset(body, 0, sizeof(body)); + memcpy(body, "XCESNAP\0", 8); + memcpy(body + 8, &snap_lsn, 8); + /* body[16] (cfgref_present) left 0 by the memset above. */ + uint32_t fake_fmt = 1u; /* must match persist.c's SNAPSHOT_FORMAT_VERSION */ + uint32_t fake_job_count = 0u; + uint32_t fake_queue_count = 0u; + memcpy(body + 57, &fake_fmt, sizeof(fake_fmt)); + memcpy(body + 61, &fake_job_count, sizeof(fake_job_count)); + memcpy(body + 65, &fake_queue_count, sizeof(fake_queue_count)); + uint8_t chunk0[64]; uint8_t chunk1[64]; - memset(chunk0, 0xAA, 64); - memset(chunk1, 0xBB, 64); - /* Embed XCESNAP header in chunk0 so load_snapshot can parse it */ - memcpy(chunk0, "XCESNAP\0", 8); - memcpy(chunk0 + 8, &snap_lsn, 8); + memcpy(chunk0, body, 64); + memcpy(chunk1, body + 64, 64); persist_t *p = calloc(1, sizeof(persist_t)); TEST_ASSERT_NOT_NULL(p); @@ -3140,9 +3162,21 @@ static void test_snapshot_recv_128kb_two_chunks(void) TEST_ASSERT_NOT_NULL(chunk0); TEST_ASSERT_NOT_NULL(chunk1); - /* Embed valid XCESNAP header in chunk0 so persist_load_snapshot succeeds */ + /* Embed valid XCESNAP header in chunk0 so persist_load_snapshot succeeds. + * chunk0/chunk1 are otherwise zero-filled, but a well-formed entity- + * bearing checkpoint (ha-entity-snapshots task 5.2) needs format_version + * = SNAPSHOT_FORMAT_VERSION right after the 57-byte legacy header, or + * load_snapshot() correctly rejects an all-zero format_version as + * unrecognized rather than silently ignoring the body as before. The + * job_count=0/queue_count=0 that follow are already satisfied by the + * zero-fill; the rest of the 128KB body is never read (load_snapshot + * stops right after the queue_count=0 loop) so it stays harmless + * filler. */ memcpy(chunk0, "XCESNAP\0", 8); memcpy(chunk0 + 8, &snap_lsn, 8); + /* chunk0[16] (cfgref_present) already 0 from calloc. */ + const uint32_t fake_fmt = 1u; /* must match persist.c's SNAPSHOT_FORMAT_VERSION */ + memcpy(chunk0 + 57, &fake_fmt, sizeof(fake_fmt)); /* ── Chunk 0: not last ── */ snapshot_transfer_t c0;