Skip to content

Document copy-db's blob companion directory and restore steps - #620

Open
kriszyp wants to merge 8 commits into
mainfrom
kris/copy-db-blob-companion
Open

Document copy-db's blob companion directory and restore steps#620
kriszyp wants to merge 8 commits into
mainfrom
kris/copy-db-blob-companion

Conversation

@kriszyp

@kriszyp kriszyp commented Aug 6, 2026

Copy link
Copy Markdown
Member

Companion to HarperFast/harper#2098 (fixes HarperFast/harper#2048).

copy-db previously 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:

For the human reviewer

  • The v5.3.0 badge is an inference. Fix copy-db producing a silently corrupt, non-restorable database copy harper#2098 is still open and carries no milestone; core package.json is at 5.2.2, so the next minor is v5.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.
  • No release-notes/v5-lincoln/5.3.md exists, so the 5.3 copy-db change has no release-note entry — the reviewers flagged this against AGENTS.md. That file covers a whole release, so it is left for the release-notes pass rather than created here.
  • Two reviewer findings are core defects, not docs changes, and are left for Fix copy-db producing a silently corrupt, non-restorable database copy harper#2098: copy-db exits 0 when every blob root is missing on disk (an unmounted blobPaths volume yields a database-file-only copy that a backup script records as success), and a failed or interrupted copy leaves its partial target and -blobs directory behind for the operator to remove by hand. Both are documented here as behavior; neither is fixed here.
  • reference/backups/overview.md:35 tells LMDB users to reach for get_backup or volume snapshots without mentioning that get_backup omits blobs. Not touched in this PR — same gap, different page.

Verification

  • npm run build passes. 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 --check clean on both changed files.
  • Every claim checked against Fix copy-db producing a silently corrupt, non-restorable database copy harper#2098's bin/copyDb.ts (target and companion existence guards, the populatedRoots short-circuit, the RocksDB rejection, the compact-on-start skips) and resources/blob.ts getBlobPathsForDatabaseName (roots are <storage.blobPaths[n]>/<database>), and the get_backup engine behavior against reference/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

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>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread reference/cli/commands.md Outdated
Comment thread reference/database/compaction.md Outdated
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-620

This preview will update automatically when you push new commits.

@github-actions
github-actions Bot temporarily deployed to pr-620 August 6, 2026 02:57 Inactive
@kriszyp
kriszyp marked this pull request as ready for review August 24, 2026 12:16
@kriszyp
kriszyp requested a review from a team as a code owner August 24, 2026 12:16

@DavidCockerill DavidCockerill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment thread reference/database/compaction.md Outdated
kriszyp and others added 7 commits August 25, 2026 23:49
- 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>
@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-620

This preview will update automatically when you push new commits.

@Ethan-Arrowood

Copy link
Copy Markdown
Member

@kriszyp fyi you have some ghost commits in here! Not a big deal cause I'll squash merge anyways but just calling out incase.

@Ethan-Arrowood

Copy link
Copy Markdown
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

copy-db produces a silently corrupt, non-restorable copy and exits 0 — four independent channels in bin/copyDb.ts

4 participants