HarperFast / HarperFast/harper
beginRestore() truncates the restoring marker before rewriting it, so a crash on a recovery attempt can unblock a half-purged database
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
Pre-existing defect in `restore_backup`, independent of the archive work but a prerequisite for it (#2632, item 2; design note §7.4 in `docs/proposals/archive-restore.md` on branch `design/archive-restore`). Belongs to #2100.
## Problem
`beginRestore()` opens the restoring marker with `openSync(markerPath, 'w')`, which truncates it, and only then writes the two lines (`dataLayer/restoreMarker.ts:193-199`). `scanBlockedRestores()` skips any marker whose first line is empty (`:272`).
On a first attempt that is benign — nothing destructive has run. On a **recovery** attempt, where `lock.preexisting` is true and the database directory may already be half-purged by a failed restore, `beginRestore()` re-truncates the marker that was correctly blocking the database. A crash in that window (between the truncate and the write, or before the file fsync) leaves an empty marker, and on the next boot `databasesBlockedByRestore` reads it as no block: a half-purged database loads as healthy. Silent, and it defeats the one guarantee the marker exists to give.
Both planned restore routes rerun `restoreBackupOffline` over exactly this marker, so it cannot be built on as-is.
## Fix
Write the marker temp → `fsync` → `rename` → parent `fsync`, so a torn write can never replace a valid marker. `beginRestore()` on a pre-existing marker should leave it untouched (it already carries the right database name) rather than rewrite it.
## Acceptance
- A fails-on-base test: a torn marker write (kill or injected error between truncate and write on the current code) must leave the database blocked. On the fix, the same injection leaves the original marker intact.
- Crash-window tests on the replacement write (before rename, after rename before directory fsync).
- Windows: `fsyncDir` treats directory fsync as a no-op (`:143-156`); document what durability the marker has there.
Contributor guide
Assessment
This issue has not been assessed yet.