Add RocksDB blob_dir patch so blob files can live on a separate volume

[kriszyp]

Adds a downstream RocksDB patch introducing AdvancedColumnFamilyOptions::blob_dir, so blob files can be placed on a different volume than the SST files.

Stock RocksDB derives every blob file path from cf_paths.front().path. That means db_paths/cf_paths cannot tier large values at all — they distribute SST files by level while every blob file stays put. Without this, there is no way to keep the LSM tree (and all of its compaction write traffic) on fast local storage while large values live on cheaper attached storage.

All blob path derivation now routes through a single ImmutableCFOptions::GetBlobDir(), so the option and its cf_paths.front() default cannot drift apart. ROCKSDB_HAS_CF_BLOB_DIR lets downstream code feature-detect and still compile against an unpatched RocksDB. Consumed by @harperfast/rocksdb-js via its new blobs.dir option.

The patch is authored against v11.8.1, the version rocksdb-js pins, and applied with patch -p1 -F0 so a version bump fails loudly instead of fuzzy-matching a hunk into a plausible-but-wrong site.

Where to look

GetBlobDir() is the whole contract; most of the rest is call sites. The parts worth reading closely are the ones review found rather than the ones the first pass wrote.

• Checkpoint and BackupEngine refuse a blob_dir DB. CreateCustomCheckpoint — the shared implementation behind both — asks for a portable capture through LiveFilesStorageInfoOptions::require_portable_files, and GetLiveFilesStorageInfo evaluates it under mutex_ before the flush and again after, because FlushForGetLiveFiles() releases the mutex. The state is a monotone latch (has_ever_had_blob_dir_) set at column-family creation and never cleared, so dropping the only blob_dir CF does not re-open the hole. Reasoning below.
• FlushJob::HasBlobFileAdditions() is total, not asserted. It answers false for a null edit_. edit_ is null exactly when PickMemTable() picked nothing — reachable when an overlapping flush already claimed the memtables, or when they all sit above the request's max_memtable_id — and Run() accepts that state and returns OK. A job that picked nothing wrote no blob files, so the predicate has a correct answer; guarding the call site instead would have been wrong, because pick_status[i] records that PickMemTable() ran, not that it picked.
• Directory fsync. A blob directory outside cf_paths has no FSDirectory handle in Directories, so its entries were never fsynced after flush, compaction, or WAL recovery — a crash could leave a durable SST referencing a blob file with no directory entry. ColumnFamilyData now owns a handle, created through the shared created_dirs map so no directory is opened twice.
• The audit's second regex was half dead. cf_paths(\.front\(\)|\[0\])\.path — the dot used to sit before the alternation, so cf_paths[0].path (which has no dot after cf_paths) could never match and the guard silently covered half of what it claimed. Fixing it surfaced seven call sites; all seven derive SST paths, and they are now pinned in cf-blob-path-inventory.txt so one that later derives a blob path shows up as a diff.

Checkpoint and backup

Two review threads asked for backup/checkpoint to be rejected while blob_dir is non-empty. Building the patched tree and measuring says the stated failure mode does not happen, but the conclusion is right anyway, for a worse reason.

Flattening is already refused. CreateCustomCheckpoint rejects any DB whose non-WAL files span more than one directory, and reporting the real blob directory from GetLiveFilesStorageInfo is what puts a tiered DB in that category. Measured: with blob_dir set and one live blob file, CreateCheckpoint and CreateNewBackup both return NotSupported and leave no partial artifact.

But only once a blob file exists to report. A DB with blob_dir set that has not written one yet backs up cleanly — and that backup is a landmine. The copied OPTIONS file carries blob_dir, an absolute path, so restoring elsewhere and opening produces a second DB pointed at the source's blob directory, whose obsolete-file scan deletes the source's live blob files. Measured on the patched build: source blob files 1 before, 0 after opening the restore, and the source DB then fails its own read with No such file or directory.

So the refusal keys on the option being set at all, not on the files reported and not on it differing from dbname_ — the path is absolute either way.

This is upstream's hazard, not one blob_dir introduces. The same sequence with cf_paths pointing at an external directory corrupts the source DB on stock v11.8.1, verified by building an unpatched tree. It is closed here because blob_dir is the option this patch owns, and because a tiered DB spends its early life blob-free — exactly the window that backs up cleanly and restores into a landmine.

Rebased onto the experimental-patches pipeline

This PR had been conflicting with main since #15 merged, which is why no PR workflow had ever run on it — GitHub cannot compute a merge commit for a conflicting PR, so the gate this branch adds had never once executed. Rebased; three conflicts, all in build.yml, all resolved in favour of keeping both sides:

conflict	resolution
patch -F0 invocation	took main — it had already adopted -F0 and a rationale comment, so this branch's intent was already there
audit call vs. the new EXPERIMENTAL_DIRS loop	kept both, audit first: the inventories describe the official patch set, and an opt-in experiment is not drift
a later commit deletes that audit call	honoured — it moves the audit into the dedicated nightly audit job

No RocksDB patch content changed in the rebase; vcpkg-overlays/ is byte-identical to the pre-rebase tree.

CI

A PR gate applies all patches at zero fuzz, runs the call-site audit, then builds and runs a focused set of upstream tests. It resolves the release from patched-version.txt rather than releases/latest: the inventories pin upstream line numbers, so resolving "latest" at run time would have turned every upstream release day into a red gate on every open PR. The nightly audit still runs against the latest release, which is where drift belongs and where it must block the publishing build.

The gate no longer shares a concurrency group with the release build. main grew a workflow-level concurrency key for build.yml in #19 on 2026-09-15, six days after this branch last ran its gate, and the two changes had never met. A pull_request run has no inputs, so its key resolved to the same Automated RocksDB Builds--- string the nightly and an input-less workflow_dispatch resolve to — and a group holds one in-progress run plus one pending run, cancelling the pending one when a third queues. A nightly that arrived while a PR was validating could therefore be cancelled outright by the next PR push, and PR feedback could sit behind a multi-platform release build. Pull requests now key on the PR number; non-PR runs compose the identical string as before (format renders the empty inputs to --), so release serialisation is untouched, and cancel-in-progress is true only for pull requests, where superseding an earlier run of the same PR is what should happen.

Verification

All on a patched v11.8.1 tree built from this patch (make, debug, GCC 15).

suite	result
db_basic_test *PortableLiveFileCapture*, *RecoveryBlobDirSynced*	4/4
db_flush_test.BlobDirIsFsyncedForOrdinaryAndAtomicFlush	1/1
checkpoint_test (full)	42/42
backup_engine_test (full)	108/108
options_test (full)	79/79
options_settable_test	ColumnFamilyOptionsAllFieldsSettable fails — identically on a pristine v11.8.1 tree (unset_bytes_base 102 vs 126, both trees), so it is a GCC 15 artifact the patch neither causes nor masks

• The PR gate this branch adds is green on its first-ever run (run 34369386485): the pinned download, the zero-fuzz apply, the call-site audit, and all five test binaries — options_settable_test included, so the GCC 15 failure above does not reproduce on ubuntu-latest.

• The regenerated patch applies to a fresh v11.8.1 tarball at patch -p1 -F0 and git apply --check, and the tree it produces is byte-identical across all 28 patched files to the tree that was built and tested.

• The inventory audit passes against that freshly reproduced tree.

• RocksDB's own make check was run against the patched tree in an earlier round and found two real defects, both fixed here (a186bc7).

• Behavior of the checkpoint/backup guard, measured against the patched library rather than traced:

  configuration	CreateCheckpoint / CreateNewBackup
  blob_dir unset	OK — and restore OK. Unchanged from stock.
  blob_dir set, one live blob file	NotSupported, no partial artifact
  blob_dir set, no blob file yet	NotSupported (the case the guard adds)
  blob_dir set to the DB directory itself	NotSupported — still an absolute path that would not follow a copy

• The resulting library passes @harperfast/rocksdb-js's tiered-storage suite 19/19. That predates this round; it does not exercise checkpoint or backup.

For the human reviewer

Reviewer sign-off at 729c24a. cb1kenobi's review closes the earlier blockers — "portable capture is latched under the DB mutex, backup and checkpoint refuse a configured blob_dir, and directory fsync plus call-site audit coverage match the stated contract" — and rules the items below "documented C++ layering tradeoffs that rocksdb-js already enforces, not unfixed defects on this diff." So none of these blocks merge. Items 1, 4, 6 and 9 remain live only as offers to do more than this diff does.

Open question on the 2026-09-21 review. cb1kenobi requested changes at 729c24a with an empty body and no line comments, so there is nothing in it to act on. Read against the record it most likely restates the #24 ordering block in item 7 — the same reviewer cleared this diff's content a week earlier — but that is an inference, not something the review says. Asked on the thread; nothing in the code was changed on the strength of a guess.

1. Refusing every backup of a blob_dir database is a real operational constraint, and it is the decision most worth disagreeing with. It is strictly more conservative than the status quo — it only refuses backups that were already dangerous — but a tiered database can never be checkpointed or backed up through RocksDB's own tooling, at any point in its life. Say if you would rather have documentation only and accept the footgun.
2. The portable-capture latch reflects the descriptors this DB was opened with, not its persisted OPTIONS. A read-only open whose descriptors omit blob_dir, against a DB whose OPTIONS file still names one, admits a capture that copies that OPTIONS file — and restoring it can again aim obsolete-file cleanup at the source volume. rocksdb-js refuses such an open; the C++ API does not. Closing it in the engine means validating the captured OPTIONS artifact, which is the same layering question as item 3. Raised by review, left open deliberately.
3. Enforcement of the immutability rule lives in the consumer. A blob file's directory is re-derived from the option on every open rather than recorded per file, so changing it on a populated database strands the existing blob files. RocksDB cannot enforce that without persisting more state, so the constraint is documented on the option and enforced by rocksdb-js, which compares against the persisted blob_dir. The layering is worth agreeing on.
4. No regression test for the empty-pick window. The HasBlobFileAdditions() fix is total, so the crash is gone, but nothing pins it. Reaching that window from a test needs a second flush to claim the memtables inside the SyncClosedWals mutex-release window, and any second flush re-enters the same sync point, so the usual LoadDependency idiom deadlocks — forcing it means adding a new TEST_SYNC_POINT to upstream db_impl_compaction_flush.cc and widening the vendored patch. Say if you want that; it is a small change, just not a free one.
5. The identical cf_paths hole upstream is left open. Same sequence, same corruption, stock v11.8.1. Fixing it would change behavior for configurations this patch does not own.
6. Direct-write blob partition manager. New in 11.8.1 and off by default. It wrote to dbname_ regardless of blob placement; it now honors blob_dir only when explicitly set, so the default path is byte-for-byte unchanged. Two review legs argued it should unconditionally use GetBlobDir() — that is a real upstream inconsistency (with blob_dir unset and cf_paths.front() != dbname_, direct-write blobs are written to one and read from the other), but fixing it changes stock behavior on a path this patch does not own. Say if you would rather it were fixed here.
7. Placement: answered — this moves to experimental-patches/. Add opt-in experimental patches pipeline #15 landed the opt-in patch layer after this branch was written, and I had left the question open here because making the patch opt-in means the default prebuild loses the feature. #24 "Move the cf-blob-dir patch into experimental-patches/" settles it: it targets this branch, not main, so it lands here first and this PR then merges to main with the patch already in the opt-in slot. It is a strict fast-forward of this branch, the patch file is byte-identical across the move (sha256 931b8b0a…), and its gate reproduces this PR's full test result under the new pipeline, so the move costs no coverage. The one thing that move does not settle is the consumer: official prebuilds will no longer carry ROCKSDB_HAS_CF_BLOB_DIR, so rocksdb-js #767 must either pin an experimental prebuild or wait for blob_dir to graduate into the official overlay. That decision is tracked on Move the cf-blob-dir patch into experimental-patches/ #24, not here.
8. Rebase burden. A vendored patch across 19 files. 11.8.1 already introduced one new path-derivation site relative to 11.1.2, which is why the header carries a re-audit note and patched-version.txt now pins what the inventories were generated from. If that burden looks wrong, the alternative is proposing blob_dir upstream and waiting.
9. A dynamically created column family does not get its blob_dir created. DBImpl::Open adds blob_dir to its CreateDirIfMissing loop, but CreateColumnFamilyImpl does not — and upstream v11.8.1 does create every missing cf_paths entry there (db/db_impl/db_impl.cc:3866). So CreateColumnFamily with a blob_dir that does not exist yet fails where the same call with a fresh cf_paths entry succeeds. It fails with an IO error rather than silently, and rocksdb-js sets blobs.dir at open, so nothing in the shipping path reaches it. Left alone deliberately rather than overlooked: the fix is three lines, but it would edit a vendored patch that is signed off and byte-identical to the copy Move the cf-blob-dir patch into experimental-patches/ #24 moves, no test in the gate would exercise it, and it would invalidate the byte-identity claim in Verification. Say if you want it; it is a small follow-up.

Review findings declined this round with the evidence that refuted them, so you do not have to re-derive it: the SstFileManager/DeleteScheduler trash and space-accounting findings (CollectAllDBPaths() already includes GetBlobDir(), and both db_impl_open.cc trash cleanup and db_impl_files.cc existing-file tracking iterate it); a RepairDB finding (repair.cc has no BlobFileName site and passes nullptr blob additions in stock 11.8.1, so tiering changes nothing there); a WAL-deadlock blocker on the second portability check (UnlockWAL appears nowhere in db_filesnapshot.cc; wal_locked only observes whether another caller holds the lock, and upstream's own early returns in that block have the identical shape); and a recovery null-check (the unconditional data_dir deref one line above is verbatim upstream v11.8.1).

This round's independent pass (codex, delta at 23e44b6) raised no new finding on the change itself and re-raised four settled ones. Each was re-checked against source rather than inherited: a "BackupEngineImpl was never patched to set require_portable_files" blocker — BackupEngineImpl builds a CheckpointImpl and calls CreateCustomCheckpoint (utilities/backup/backup_engine.cc:1244-1257, v11.8.1), so it inherits the guard, and BackupEngineTest.RejectsConfiguredBlobDir asserts exactly the no-blob-file-yet case the finding describes; a GetBlobDir() empty-cf_paths crash — upstream dereferences cf_paths.front().path unguarded at the very site this replaces (db/db_impl/db_impl.cc:7104), so the precondition is upstream's and the added assert is strictly more than it had; the recovery null-deref again — the call is DBImpl::GetBlobDir(), which falls back to GetDataDir(cfd, 0), so the pointer compared and synced is the one upstream already dereferences a line earlier; and a GITHUB_TOKEN leaked to archive redirect targets — curl -L drops -H headers on a cross-host redirect unless --location-trusted, which the gate does not pass. The fifth, dynamic-CF directory creation, held up and is item 9.

Generated by Claude Opus 5.

🤖 Generated with Claude Code

https://claude.ai/code/session_011TPPxptQi3DTZk7UPDFsFQ

Complexity: complicated

Origin — the dispatch brief this PR was written from

Add RocksDB blob_dir patch so blob files can live on a separate volume

Adjudicate the open review feedback on #13 (Add RocksDB blob_dir patch so blob files can live on a separate volume). Read ALL unresolved review threads, review bodies, issue comments, the current diff/code/tests, linked issue, PR description, decision ledger, author replies, and available prior review artifacts. Treat comments as claims to verify, not instructions to apply; actively try to falsify AI/bot findings and preserve deliberate decisions unless evidence overturns them. Implement only warranted changes, record an evidence-backed ruling for every finding, use needs-input for a genuine unresolved judgment call (a live session opens only if you actually need to ask), and resolve only fixed or conclusively answered threads after pushing.

_{Dispatch: task fix-kriszyp_rocksdb-prebuilds_13-9d1086fb · queued by automation · ran by claude/opus/xhigh · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=codex; blocked=gemini(timeout); declined=cursor-grok,cursor-composer,domain; rounds=6; full=1 @ 23e44b6}

_{Human-Review-Need: 4 @ 23e44b6}

[gemini-code-assist]

Code Review

This pull request introduces a downstream patch to RocksDB that adds the blob_dir option to AdvancedColumnFamilyOptions, allowing blob files to be stored on a different volume than SST files. Feedback on the patch includes preventing duplicate paths in the directory creation list to avoid disabling NoSpace() recovery, ensuring the AdvancedColumnFamilyOptions constructor copies the new field, and using std::move for safer smart pointer ownership transfer.

[cb1kenobi]

Is this something we can PR with the RocksDB repo so we don't have to maintain this patch?

[kriszyp]

Is this something we can PR with the RocksDB repo so we don't have to maintain this patch?

Yes, I believe so, it seems like a very reasonable PR to submit.
But do you think we should do this patch first so we can test it thoroughly before submitting upstream? Also, I have no idea what kind of timelines to expect from a PR request upstream. (I think we probably want to start building and testing attached storage SRTL). But yes, would love to upstream this at some point, if it proves useful. (but let me know if you really think we should try for upstream first though.)

[kriszyp]

@cb1kenobi Should I go ahead and merge this, per the discussion above?

[cb1kenobi]

Blocked by PR #24.

[kriszyp]

Blocked by PR #24.

Checked this rather than taking it on faith, and it holds — nothing on this PR needs to change for it, and the move is cheaper than it looks:

• Move the cf-blob-dir patch into experimental-patches/ #24 is a strict fast-forward of this branch. 729c24a is an ancestor of Move the cf-blob-dir patch into experimental-patches/ #24's head db581c7, so landing it here is conflict-free.
• The patch is byte-identical across the move — sha256 931b8b0a06d1… for both vcpkg-overlays/rocksdb/patches/0002-cf-blob-dir.patch and experimental-patches/0001-cf-blob-dir/0001-cf-blob-dir.patch. Git tracks it as a 100% rename.
• No coverage is lost. Move the cf-blob-dir patch into experimental-patches/ #24's gate runs this patch's own audit.sh and test.sh and reproduces this PR's full result under the new pipeline: 4 / 1 / 42 / 108 / 79 / 4 across db_basic_test, db_flush_test, checkpoint_test, backup_engine_test, options_test, options_settable_test (job log).

So the ordering is right, and I've updated item 7 of the description — it had the placement question as still open, and your comment answers it.

What is left is on #24 rather than here: two review threads are still unresolved against db581c7, and I confirmed both still hold at that commit —

1. The nightly/dispatch audit checks one patch per fresh tree (apply-patches.sh "$SRC" "$name", build.yml:367) while the release build applies the composed stack (build.yml:472). Latent today, since 0001-cf-blob-dir is the only non-noop patch — it becomes reachable with the second one.
2. .github/scripts/apply-patches.sh is in neither workflow's path filter — build.yml lists .github/workflows/build.yml only, and validate-experimental-patches.yml lists experimental-ids.sh but not apply-patches.sh, so a helper-only change exercises neither gate.

The one thing #24 does not settle is the consumer: once the patch is opt-in, official prebuilds stop carrying ROCKSDB_HAS_CF_BLOB_DIR, so rocksdb-js #767 needs either a pinned experimental prebuild or an eventual graduation into the official overlay. You flagged that on #24 already; noting it here so the sequencing is in one place.

Nice call on the move, by the way — the per-patch patched-version.txt is the part that makes it hold up beyond this one patch.

— Claude Opus 5

[cb1kenobi]

The blob_dir patch's earlier blockers are closed: portable capture is latched under the DB mutex, backup and checkpoint refuse a configured blob_dir, and directory fsync plus call-site audit coverage match the stated contract. Remaining items in the PR body are documented C++ layering tradeoffs that rocksdb-js already enforces, not unfixed defects on this diff. No new blocking issue on the changed lines at 729c24a.

—
Reviewed 729c24a

[kriszyp]

@cb1kenobi your review at 729c24a came through with an empty body and no line comments, so I can't tell what it's asking for. My best guess is that it formalizes the #24 ordering block from your earlier comment — you cleared this diff's content a week ago, and a blocking review is the thing that actually stops the merge button. If that's it, nothing here needs to change and it can stay as-is until #24 lands. If you meant something specific, say the word and I'll pick it up.

Rather than guess at it, I re-checked this branch against current main, which has moved seven commits since this PR last ran its gate — and that turned up one real thing:

main #19's concurrency guard and this PR's pull_request trigger had never met. They landed six days apart on the same workflow. A PR run has no inputs, so its group key resolved to the same Automated RocksDB Builds--- string the nightly and an input-less workflow_dispatch resolve to. A concurrency group holds one in-progress run plus one pending run and cancels the pending one when a third queues — so a nightly that arrived while a PR was validating could be cancelled outright by the next PR push, and PR feedback could sit behind a multi-platform release build. Pushed 23e44b6: pull requests key on the PR number, non-PR runs compose the byte-identical string they did before, and cancel-in-progress is true only for pull requests.

Two things worth knowing about the shape of that fix:

• I merged main in rather than rebasing. Move the cf-blob-dir patch into experimental-patches/ #24 targets this branch and has 729c24a as an ancestor, so a rebase would have stranded it. I re-ran the merge both ways afterwards: Move the cf-blob-dir patch into experimental-patches/ #24 still merges into this branch cleanly, and the combined result keeps both your experimental-patches/** path entry and the new group key.
• The independent pre-push round (codex, delta) found nothing new on the change and re-raised four already-settled findings. I re-checked each against upstream v11.8.1 source rather than inheriting the earlier ruling; all four are refuted, with the evidence written into the PR description so it doesn't have to be re-derived. The fifth held up and is now item 9 in For the human reviewer: a dynamically created column family doesn't get its blob_dir created, where upstream does create a missing cf_paths entry. It errors rather than corrupting, and rocksdb-js sets the directory at open, so I left the vendored patch alone rather than modify a signed-off file for it — happy to do it as a follow-up if you want the parity.

Thanks for the earlier clearance — it made this round a lot cheaper to scope.

— Claude Opus 5

[cb1kenobi]

No new blocking defects were found on the changed lines. The per-PR concurrency group correctly isolates validation from release workflows.

—
Reviewed 23e44b6

[cb1kenobi]

No confirmed blocking defects were found on changed lines. The per-PR concurrency group correctly isolates validation from release workflows.

—
Reviewed 23e44b6