Verify a backup can be read before restore_backup replaces the database

[cb1kenobi]

⊙ Problem

restore_backup purged the database before anything checked that this build could read the backup (#2965: restore_backup purges the destination before anything verifies the backup can be read). A corrupt file, a table in a newer format_version, or an unreadable transaction log destroyed the database it was meant to restore.

💡 Solution

Verify the backup in place, in its repository, before the purge, with no staging copy and no extra disk. Online, the check runs after the database is closed, so no write is acknowledged and then lost. The restoring marker is now published only immediately before the purge, so a crash during the check leaves the database loadable.

⚖️ Alternatives

• Stage, prove, then swap (this PR's first design). Rejected in review (Kris): it needs a third copy on disk, so a restore on a tight disk is always refused. It also proved less than it seemed, since opening a database leaves tables outside the cache unread.
• Verify while the database still serves. Rejected: writes acknowledged during a minutes-long checksum read would be lost to the purge. Before this PR they failed instead.
• Publish the marker at the claim, as before. Rejected: a crash during the check would leave an intact database marked incomplete, and with an unreadable backup the only way out is restoring older data over it.
• Re-check the backup's identity after an unlocked verify. Rejected for holding the repository lock from selection to pin. Timestamp, size and file count do not identify a backup.

❓ Your call: verify_backup keeps its existing checks and does not open tables, so it can report a backup as valid that a restore then refuses for its format. I kept it narrow to avoid changing its cost and results for every caller.

🔧 Changes

• Check before the purge (dataLayer/rocksdbBackup.ts).
  • assertBackupReadable runs verifyBackupFiles: backups.verify with checksums and verifyTables (rocksdb-js 2.12.0), plus a check that a declared blob snapshot exists. A refusal keeps a client error's status and says this restore did not modify the database, or that an interrupted restore stays blocked.
  • Online, selection and its manifest checks run under the repository lock before the claim. Then close, closure check, verify, closure check again, mark, and purge.
  • Offline, with no live writers, verification runs under the repository lock before the claim, and the marker follows the lock probe.
  • The repository README states that restore verifies first. verify_backup is unchanged.
• Claim, then mark (dataLayer/restoreMarker.ts).
  • claimRestore takes the lock and runs the pin callback but writes no marker for a fresh claim. markRestoreDestructive publishes it before the purge, and beginRestore is both together.
  • The held lock already blocks every open, so checkRestoreState's marker-first contract still holds. restoreLockHeldExclusively is factored out of it, and clearing a never-marked claim is a no-op.
  • Callers: restore uses claimRestore, pins count while the lock is held, and an offline create_backup treats a held claim as an active restore.
• Design notes. The verify-before-purge note and where verification runs on each path in dataLayer/DESIGN.md, pin lifetime, and its index line.

✅ Verification

End-to-end route: the new integration test, through the Operations API and the CLI.

• restore-backup-preflight.test.ts: an online corrupt-backup refusal leaves the database writable, an offline refusal survives a restart, and a valid backup restores offline and online. It declares its table up front so no insert redeclares it (#3120 leaks handles on a redeclare).
• rocksdbBackup.test.js, 20 cases on both paths:
  • refusals of a corrupt file, a table patched to format_version 99, an unreadable log, and a missing blob snapshot;
  • an interrupted restore's marker kept;
  • where verification runs (online: claimed, closed, unmarked);
  • selection under the lock;
  • the marker before the purge;
  • a reopen during verification and a delete during verification, both refused.
• restoreMarker.test.js: a claim holds opens off without a marker, an abandoned claim leaves the database loadable, and a recovery claim keeps its marker.
• Each guard was deleted from dist/ and fails its test. On rocksdb-js 2.12.0: test:unit:backup 182 passing, test:unit:resources 4283 passing; typecheck, lint, format and check:design-docs clean.
• Not run locally: test:unit:main (it opens the local install) and the integration test (port in use); both run in CI. qa822-purge-log-invariants fails on main too.

Docs: HarperFast/documentation#721: Document that restore_backup checks a backup before replacing the database.

_{🤖 Generated by Anthropic Claude (Opus); posted via @cb1kenobi.}

Related PRs: #2648 independent, #2649 independent, #3107 independent, #2155 independent
Complexity: complicated
Release target: v5.4 — a behavior change to restore, milestoned for the next minor

Framing-Verdict: chosen-approach-sound (round roll-up)

_{Review-Coverage: authored=claude; ran=gemini,codex,cursor-composer,cursor-muse,cursor-grok; adjudicated=domain; declined=cursor-kimi; rounds=17; full=8 @ dd51b03}

_{Review-Attention: deep ~40m (critical: backupRepository.ts, restoreMarker.ts +1; decisions: markerless-claim, online-verify-after-close, verify-backup-skips-tables, checksum-on-every-restore, do-less-alternative, release-target-vs-evidence) @ dd51b03}

[github-actions]

Release cherry-pick v5.3: cancelled

Cherry-pick branch cherry-pick/v5.3/pr-3121 was deleted — this PR no longer targets v5.3 (milestone is now v5.4).

[kriszyp]

I'm skeptical of this approach, as it significantly increases the disk space required to do a restore. If the remaining free space is less than the database size, it will now fail. Before this PR, there is some slight chance that this would detect a failure ahead of time, but if there is too little disk space, there is now a 100% chance of failure. Reporting the failure loudly ahead of time seems like a poor consolation in the midst of an emergency restore, if the reporting was the actual cause of the failure. I would think most of the likely failures could be checked in place, and others could still be remedied after the main database was deleted in a failed restore (if you are doing a restore, you are probably pretty committed to replacing the main database).

_{🤖 Reviewed with Codex}

[cb1kenobi]

Thanks Kris. That was the open "your call" on disk cost, and your point about the emergency restore on a tight disk carries it. Staging is gone; the restore now checks the backup in place and needs no extra disk.

Before anything is deleted, restore_backup now runs backups.verify against the repository with checksums and the new verifyTables (rocksdb-js 2.12.0, which opens every SST in the backup one at a time), validates the transaction logs, and checks that a declared blob snapshot is present. A backup that fails is refused with the database untouched.

Ordering, since it is the subtle part:

• Selection and every manifest-derived check run under the backup repository lock, and the backup is pinned before that lock is released, so the backup that gets verified and restored is the one that was selected.
• Online, the check runs after the close and the closure check, so no write is acknowledged and then lost to the purge; the closure check runs again right before the purge.
• The restoring marker is now published only immediately before the purge. Until then the held restore lock alone keeps every open off the database, so a crash while the backup is still being checked leaves the database loadable instead of marked incomplete.

The cost is time rather than disk: the check reads the whole backup once, and online that read is inside the downtime.

_{🤖 Generated by Anthropic Claude (Opus); posted via @cb1kenobi.}