HarperFast / HarperFast/harper
create_backup: serialize backup management ops to close a finalize/purge race
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
Follow-up from #1831 review (flagged by @kriszyp on `dataLayer/rocksdbBackup.ts`).
## Problem
`create_backup` is two-phase: `rootStore.backup()` writes the RocksDB engine backup, then `finalizeBackup()` copies the blob snapshot and publishes the completion manifest.
The rocksdb-js backup-directory **writer lock is released when `rootStore.backup()` resolves** — *before* `finalizeBackup()` copies blobs and writes the manifest. So a concurrent `purge_backups` / `delete_backup` on the same database can acquire that lock, see and purge the just-created engine backup, and return — while `create_backup` keeps copying blobs and then writes a manifest (and reports a `backup_id`) for an engine backup that no longer exists.
Net: `create_backup` can report success with a phantom `backup_id`, and a manifest can outlive its engine backup.
## Scope / severity
Only bites when two backup **admin** operations run concurrently on the same database — uncommon, but a real correctness gap.
## Suggested fix
Hold a per-database backup-management lock (an OS flock like the restore lock in `dataLayer/restoreMarker.ts`) across `create_backup` / `delete_backup` / `purge_backups` so they serialize through completion, i.e. the whole engine-backup + blob-snapshot + manifest sequence is exclusive. This is the 'serialize management operations through completion' aspect deferred during #1831.
Add a regression that races a create (during its blob-copy phase) against a purge.
Contributor guide
Research direction
Read dataLayer/rocksdbBackup.ts and dataLayer/restoreMarker.ts first, then locate the create_backup, delete_backup, and purge_backups entry points. Run the existing backup tests before adding a regression that races creation during blob copying against purge; done means the operations serialize and no manifest or reported backup_id remains for a removed engine backup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 62/100