Skip to content

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

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

Document copy-db's blob companion directory and restore steps#620
kriszyp wants to merge 11 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:

Engine scope is carried by <EngineBadge engines="LMDB" /> on Copy Compaction, Compact on Start, and the copy-db CLI entry instead of prose, and Compact on Start now carries a <VersionBadge type="changed" version="v5.3.0" /> for its two new skips.

For the human reviewer

  • Do not merge before Fix copy-db producing a silently corrupt, non-restorable database copy harper#2098. These pages publish live — docusaurus.config.ts sets lastVersion: 'current', so reference/ is the published v5 documentation and the version badge labels the change without gating it. Published against 5.2.x, an operator follows these restore steps, gets no blob directory, and may still treat the copy as a working backup.
  • The v5.3.0 badge is an inference. #2098 is still open and carries no milestone; the newest core tag is v5.2.6, 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.
  • No release-notes/v5-lincoln/5.3.md exists, so the 5.3 copy-db change has no release-note entry — reviewers flagged this against AGENTS.md in three rounds. Left as-is deliberately: that file covers a whole release, and authoring it from this one change is the release-notes pass's job, not this PR's.
  • Two reviewer findings are core behaviors, 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 the target/companion cleanup after a failed copy is best effort, so a copy whose own cleanup fails still blocks the next run. Both are now documented as behavior; neither is fixed here.
  • reference/backups/overview.md contradicts this page on LMDB get_backup and blobs. :8 tells every reader a backup is "all tables, the audit/transaction log, and any file-backed blobs", carving an LMDB exception only for table subsets and audit, and :13 describes get_backup as streaming "a full snapshot" — while compaction.md:88 now says an LMDB get_backup streams the .mdb file only. The page an operator lands on when choosing a backup strategy is the one that is wrong, so the correction belongs in its own issue against backups/overview.md rather than here.

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).
  • npm run format:check clean.
  • 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 rootStores.size > 1 throw, the RocksDB rejection, the best-effort cleanup in the catch, the compact-on-start skips) and dataLayer/blobBackup.ts (copyBlobRootsByIndex, copyTree's missing-root no-op, the README.md mapping listing every configured root), plus resources/blob.ts getBlobPathsForDatabaseName. main does not carry this code.
  • The mkdir -p added to the restore example is load-bearing: cp -r src/. dest/ creates one missing directory level but fails with two, which is the case for a database whose blob root does not exist yet.

Refs HarperFast/harper#2048

Generated by Claude Opus 5.

Review-Coverage: authored=claude; ran=gemini,codex; declined=cursor-grok,cursor-composer,domain; rounds=8 @ 04536bd

Human-Review-Need: 4 @ 04536bd

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
github-actions Bot temporarily deployed to pr-620 August 26, 2026 06:29 Inactive
@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.

@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 the content — but I'd hold the merge until HarperFast/harper#2098 lands.

🧊 In plain terms: this page documents a copy-db that copies your blob files alongside the database, and gives restore steps that depend on that. That version of copy-db isn't released yet. Published as-is, someone on 5.2.x follows the steps, gets no blob directory, and may still treat the copy as a working backup — losing every blob at restore time, which is the exact bug the unmerged fix exists to stop.

Why I think the sequencing matters more than usual here rather than being the normal docs-ahead-of-release case:

  • harper#2098 is still open, the newest core tag is v5.2.6, and no -blobs logic exists on main in harper or harper-pro.
  • These pages publish live: docusaurus.config.ts sets lastVersion: 'current' with current: { label: 'v5' }, so reference/ is the published v5 documentation. The <VersionBadge version="v5.3.0" /> labels the change but doesn't gate it.
  • I checked the merge base: this is the first mention of blobs on either page, so it introduces the claim rather than refining one.

The content itself is accurate, and I verified it against #2098's implementation rather than against main, since main doesn't have it: the target-and-companion-directory refusal, compact-on-start skipping RocksDB and skipping multi-environment databases, and the existsSync filtering of blob roots all check out. The cp -rcp correction on the .mdb file is right too.

One substantive addition. On the unmounted-blob-root case, the page tells readers to confirm <target-database-path>-blobs exists afterwards. #2098 actually warns at copy time, naming the missing roots and stating plainly that the copy is missing those blobs. Pointing readers at that warning is more actionable than post-hoc inspection, and it turns a silent-ish trap into something the command tells you about.

Two threads inline: a gap on multi-environment databases, and a cross-page contradiction that wants its own issue.

Three nits, take or leave: Compact on Start gains two v5.3.0 behaviour changes with no <VersionBadge type="changed"> while the other two sections this diff touches carry one; engine scope is prose where the repo has <EngineBadge engines="LMDB" /> and the backups pages use it; and the unmounted-root case is the highest-stakes point on the page but sits in plain prose while lower-stakes material gets > **Note:** callouts.

— DAIvid (Claude Opus 5) · cross-model: Codex (graded) + Gemini + Harper domain adjudication · core-behaviour claims traced against harper#2098, not main

Comment thread reference/database/compaction.md Outdated
Comment thread reference/database/compaction.md
kriszyp and others added 3 commits August 27, 2026 09:21
A database whose tables use per-table `path` settings spans several storage
environments. The page already said copy-db is LMDB-only and that compact on
start skips such a database, but never said which way copy-db goes -- refuse, or
copy one environment and exit 0. Say that it errors out.

Also from review:

- Use `<EngineBadge engines="LMDB" />` for engine scope instead of prose, and
  badge Compact on Start's v5.3.0 behaviour changes, matching the two other
  sections this branch touches.
- Promote the unmounted-blob-root case to an `> **Important:**` callout and point
  at the warning copy-db emits at copy time (naming each missing root) rather
  than at post-hoc inspection of the companion directory.

Co-Authored-By: Claude Opus <noreply@anthropic.com>
From the pre-push review:

- A failed `copy-db` removes the target, its lock file, and the companion
  directory it created before reporting the error (harper#2098 `copyDb`'s catch),
  so "retrying an interrupted copy means removing both" was wrong about the
  common case. Say what actually needs clearing by hand: a previous *successful*
  copy to the same path, or a run killed before cleanup could run.
- Say that the exit status does not carry the missing-blob-root warning, so a
  scheduled copy checking only the exit code keeps rotating incomplete copies.
- `cp -r src/. dest/` creates one missing directory level but not two, so the
  restore example fails for a database whose blob root does not exist yet. Add
  `mkdir -p`.

Co-Authored-By: Claude Opus <noreply@anthropic.com>
`copyDb`'s catch removes each artifact with `.catch(() => {})`, so a removal that
itself fails is swallowed and the next run is still refused for an existing
target. The previous wording promised the cleanup unconditionally.

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 Ethan-Arrowood added the waiting-for-companion PR is generally ready to go as soon as companion (usually implementation) PR merges. label Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-companion PR is generally ready to go as soon as companion (usually implementation) PR merges.

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