Document copy-db's blob companion directory and restore steps - #620
Document copy-db's blob companion directory and restore steps#620kriszyp wants to merge 8 commits into
Conversation
copy-db now writes the database's blob roots to `<target>-blobs/<rootIndex>/` and refuses a target that already exists (HarperFast/harper#2048). Blob files live outside the database file and are addressed by database name, so a copy is not restorable without them — document the layout, the restore steps for restoring under a different name, and that copy compaction is LMDB-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for the harper copy-db command and database compaction, adding details about file-backed blobs, target path constraints, and database engine applicability (LMDB vs. RocksDB). The review feedback suggests adding <VersionBadge> elements to properly document these behavior changes on existing surfaces, following repository guidelines.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-620 This preview will update automatically when you push new commits. |
DavidCockerill
left a comment
There was a problem hiding this comment.
Approving. No findings.
"The copy is not restorable without this directory" in bold is the sentence that matters, and it is not hedged. Anyone who took a copy-db backup before this was written could reasonably believe they hold a restorable copy and not.
The <rootIndex> explanation earns its length: preserving the source root's position is what lets a multi-root database restore each root to its original slot, and writing a README.md into the directory recording the mapping means the copy carries its own restore instructions rather than depending on someone finding this page later. Showing the restore under a different database name is also the case people actually hit.
The LMDB-only scope appears in both the CLI reference and the compaction page, so a reader arriving from either gets it.
— DAIvid (Claude Opus 5)
- Add <VersionBadge type="changed" version="v5.3.0" /> for the new blob copying and target-existence behavior on both surfaces - Scope the "not restorable" warning to databases that hold file-backed values, and note that others restore from the database file alone - Rename the section to "File-backed blobs copied separately" - Use ASCII "..." in the blob path notation Co-Authored-By: Claude Opus <noreply@anthropic.com>
- Bytes values live inside the record; only Blob values are file-backed - Document that the -blobs companion directory must also be absent - Limit the "database file alone is enough" case to an immediate in-place replacement; a retained backup needs the companion directory because the source can reclaim blob files afterwards - Do not assert that a blob-free database emits no companion directory - Place version badges on their own line under the nearest heading Co-Authored-By: Claude Opus <noreply@anthropic.com>
copy-db throws on a RocksDB source; only compact-on-start skips them. Co-Authored-By: Claude Opus <noreply@anthropic.com>
From the pre-push review: configured blob roots are <storage.blobPaths[n]>/<database>, not the bare configured path; copy-db writes no companion directory at all when every blob root is missing on disk; a multi-root database needs every <rootIndex> tree restored. Co-Authored-By: Claude Opus <noreply@anthropic.com>
From the round-3 pre-push review: the companion directory only matters for databases with Blob values; the backup compactOnStartKeepBackup retains carries no blobs; copy.mdb is a file, so cp needs no -r. Co-Authored-By: Claude Opus <noreply@anthropic.com>
create_backup is RocksDB-only, so the compact-on-start note now names volume snapshots, get_backup, or a copy-db copy kept with its companion directory. Also warns that refusing an existing target breaks scripts that re-copy to a fixed path. Co-Authored-By: Claude Opus <noreply@anthropic.com>
It carries no blobs, so it is not a complete point-in-time backup for a database with Blob values. Co-Authored-By: Claude Opus <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-620 This preview will update automatically when you push new commits. |
|
@kriszyp fyi you have some ghost commits in here! Not a big deal cause I'll squash merge anyways but just calling out incase. |
|
I'm going to review this myself shortly, but rerequesting reviews since additional commits were pushed since the previous approvals. I thought GH was supposed to do this automatically; maybe thats buried in a setting somewhere. |
Companion to HarperFast/harper#2098 (fixes HarperFast/harper#2048).
copy-dbpreviously left the database's file-backed blobs behind, so a copy moved to another host, or restored under a different database name, silently lost every blob. It now copies each blob root to<target>-blobs/<rootIndex>/alongside the database file, and refuses a target — or a companion directory — that already exists rather than merging the copy into it.Documents:
<rootIndex>layout, and the restore steps (including restoring under a different database name, which is where the blob roots have to be placed by hand), plus that a multi-root database needs every<rootIndex>tree restored into the slot of the same index-blobscompanion directory may already exist — stricter than earlier v5 releases, so a script re-copying to a fixed path has to remove both first (CLI parameter list)Blobvalues are file-backed (Bytesvalues live inside the record), and that the roots are<storage.blobPaths[n]>/<database>blobPathsvolume produces a database-file-only copy without failing the commandcopy-dbfails on a RocksDB source, while compact-on-start skips RocksDB and multi-environment databasescompactOnStartKeepBackupretains is the pre-compaction database file only, so it is a compaction rollback rather than a point-in-time backup (LMDBget_backuphas the same limitation)For the human reviewer
v5.3.0badge is an inference. Fix copy-db producing a silently corrupt, non-restorable database copy harper#2098 is still open and carries no milestone; corepackage.jsonis at5.2.2, so the next minor isv5.3.0. If #2098 lands elsewhere, the four<VersionBadge type="changed" version="v5.3.0" />tags and the three "As of v5.3.0" sentences need updating. This PR should not merge before #2098.release-notes/v5-lincoln/5.3.mdexists, so the 5.3copy-dbchange has no release-note entry — the reviewers flagged this againstAGENTS.md. That file covers a whole release, so it is left for the release-notes pass rather than created here.copy-dbexits 0 when every blob root is missing on disk (an unmountedblobPathsvolume yields a database-file-only copy that a backup script records as success), and a failed or interrupted copy leaves its partial target and-blobsdirectory behind for the operator to remove by hand. Both are documented here as behavior; neither is fixed here.reference/backups/overview.md:35tells LMDB users to reach forget_backupor volume snapshots without mentioning thatget_backupomits blobs. Not touched in this PR — same gap, different page.Verification
npm run buildpasses. The two broken anchors it reports are pre-existing and in files this PR does not touch (backups/overview,release-notes/v5-lincoln/5.1).npx prettier --checkclean on both changed files.bin/copyDb.ts(target and companion existence guards, thepopulatedRootsshort-circuit, the RocksDB rejection, the compact-on-start skips) andresources/blob.tsgetBlobPathsForDatabaseName(roots are<storage.blobPaths[n]>/<database>), and theget_backupengine behavior againstreference/backups/operations.md.Refs HarperFast/harper#2048
Generated by Claude Opus 5.
Review-Coverage: authored=claude; ran=codex; blocked=gemini(no-output); declined=cursor-grok,cursor-composer,domain; rounds=5 @ 5caf515
Human-Review-Need: 4 @ 5caf515