Reclaim hotblocks disk: point-delete cleanup + startup DeleteFilesInRange#79
Conversation
…9, NET-798) Replace the per-key tombstone purge with a two-phase table cleanup that can actually reclaim disk space, including at a full disk. Phase 1 (logical, snapshot-safe): cleanup() drops each deleted table with a single range tombstone (OptimisticTransactionDB::delete_range_cf) instead of millions of point deletes, then marks it reclaim-pending. Range tombstones respect snapshots, so in-flight queries are unaffected. Phase 2 (physical): reclaim_disk_space(grace) unlinks whole SST files below the live watermark (min live TableId across all chunks + dirty tables) via DeleteFilesInRange. It performs no writes and needs no scratch space, so it makes progress even at 100% disk. A per-table deletion timestamp in CF_DELETED_TABLES gates the unlink behind a grace period -- the file unlink ignores snapshots, so grace must exceed the max query/snapshot lifetime. Also: - TABLES CF: compact-on-deletion collector + 24h periodic compaction so compaction finds tombstone-heavy / boundary files (NET-819). - hotblocks: the cleanup loop runs both phases each tick and backs off on error instead of busy-looping failing writes; startup reclaims unconfigured datasets' files before serving with a zero grace, since no readers exist yet (NET-798). New --reclaim-grace-secs flag (default 15m). Tests: logical-delete snapshot safety, physical reclaim after grace, watermark pinning by a live table, idempotency, end-to-end delete_dataset + reclaim, and a value-codec/migration unit test. Supersedes the sync_dataset_cleanup flag (draft PR #79). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0e7ab30 to
be14d62
Compare
…9, NET-798) Replace the per-key tombstone purge with a two-phase table cleanup that can actually reclaim disk space, including at a full disk. Phase 1 (logical, snapshot-safe): cleanup() drops each deleted table with a single range tombstone (OptimisticTransactionDB::delete_range_cf) instead of millions of point deletes, then marks it reclaim-pending. Range tombstones respect snapshots, so in-flight queries are unaffected. Phase 2 (physical): reclaim_disk_space(grace) unlinks whole SST files below the live watermark (min live TableId across all chunks + dirty tables) via DeleteFilesInRange. It performs no writes and needs no scratch space, so it makes progress even at 100% disk. A per-table deletion timestamp in CF_DELETED_TABLES gates the unlink behind a grace period -- the file unlink ignores snapshots, so grace must exceed the max query/snapshot lifetime. Also: - TABLES CF: compact-on-deletion collector + 24h periodic compaction so compaction finds tombstone-heavy / boundary files (NET-819). - hotblocks: the cleanup loop runs both phases each tick and backs off on error instead of busy-looping failing writes; startup reclaims unconfigured datasets' files before serving with a zero grace, since no readers exist yet (NET-798). New --reclaim-grace-secs flag (default 15m). Tests: logical-delete snapshot safety, physical reclaim after grace, watermark pinning by a live table, idempotency, end-to-end delete_dataset + reclaim, and a value-codec/migration unit test. Supersedes the sync_dataset_cleanup flag (draft PR #79). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b19920a to
7659b7c
Compare
98eac3f to
63dddfc
Compare
63dddfc to
718995e
Compare
| for id in &pending { | ||
| let mut start = TableKeyFactory::new(id); | ||
| let mut end = TableKeyFactory::new(id); | ||
| db.delete_range_cf(cf_tables, start.start(), end.end())?; |
There was a problem hiding this comment.
Range deletions are not officially supported for transactional databases - facebook/rocksdb#4812.
According to rust-rocksdb/rust-rocksdb#839 , it seems support for this feature was added to the Rust wrapper simply because someone was able to chase all the required pointers, despite lacking a full understanding of all potential edge cases.
There was a problem hiding this comment.
Thanks. Reverted the old behaviour for now
Range deletions (delete_range_cf) are not officially supported on OptimisticTransactionDB, the transactional DB this storage uses (facebook/rocksdb#4812, rust-rocksdb#839). Per review on #79, revert Phase-1 cleanup to point deletes while keeping the rest of the disk reclaim machinery. logical_cleanup and purge_orphan_dirty_tables now enumerate each deleted table's CF_TABLES key range and point-delete every key, via a shared purge_table helper that writes one batch per table (bounding a large multi-table purge's memory, as the pre-tombstone code did). Point tombstones are MVCC-versioned, so in-flight snapshots are unaffected, identical to the range tombstones they replace. The startup DeleteFilesInRange unlink, watermark logic, CLI flag, and tests are unchanged. Updated the compact-on-deletion-collector rationale: deleting a table now writes one tombstone per key, so dead SSTs are dense with tombstones and the collector becomes the primary compaction trigger. All cleanup_reclaim tests and the full storage suite pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ff4c648 to
b8444a6
Compare
|
Fresh recurrence of the full-disk condition this PR targets, in case it helps prioritize review. On 2026-07-04 ~00:37 UTC the The recovery matched this PR's premise exactly: the pod was restarted ~00:30 UTC and free space jumped from 0 → ~674 GiB in a single step (768Gi volume went from 100% to ~13% used). That is precisely the "unlink whole SST files below the live- Evidence during the incident: retention was advancing logically (per-dataset Until this ships and the deployed image is bumped, the operational mitigation is a manual |
bf6ebbb to
028f8d6
Compare
Fixes the NET-819 full-disk incident and NET-798 (blocking startup cleanup). Two independent mechanisms: Phase 1 -- routine logical cleanup (every 10s): logical_cleanup and purge_orphan_dirty_tables enumerate each deleted table's CF_TABLES key range and point-delete every key, via a shared purge_table helper that writes one batch per table (bounding a large multi-table purge's memory). Point tombstones are MVCC-versioned, so in-flight snapshots are unaffected. (delete_range_cf is not officially supported on OptimisticTransactionDB -- facebook/rocksdb#4812, rust-rocksdb#839 -- so range tombstones are not used.) Compaction tuning (now load-bearing): compact-on-deletion collector plus periodic compaction to find tombstone-heavy / stale SSTs, and level_compaction_dynamic_level_bytes. RocksDB leaves periodic_compaction_seconds disabled for leveled compaction without a compaction filter, so the effective baseline today is the separate 30-day ttl default; we set 7 days, exposed as --rocksdb-periodic-compaction-secs since it rewrites live data along with dead. max_background_jobs derives from the core count clamped to 2..=8 (the default 2 could not keep up during the incident, but a fixed 8 would thrash a small node) and is overridable via --rocksdb-max-background-jobs. Physical reclaim -- startup only, off by default: a single startup_disk_recovery routine folds orphan purge, unconfigured-dataset deletes, and the DeleteFilesInRange file unlink. The unlink pass runs FIRST because it needs no scratch space, so it makes progress on a near-full disk where the bookkeeping writes fail; a second pass then frees what those writes unpinned. It unlinks whole SST files below the min-live-TableId watermark. Malformed bookkeeping keys are deleted rather than rescanned forever (restoring pre-rewrite self-healing); one scan_table_ids helper is shared across the three CF scans. Note what the unlink is not. It needs no scratch space, where compaction must write its merged output before dropping the inputs and so deadlocks on a full disk -- but it still appends a VersionEdit to the MANIFEST, and getting to it at all requires the database to have opened, which replays the WAL and flushes it to L0. A volume at literally zero free bytes still needs manual headroom before the process starts. Only the deferred runtime disk-pressure trigger, firing before the volume fills, actually closes that gap. DeleteFilesInRange also skips level 0 outright, so freshly flushed dead data is left to compaction. The file unlink ignores snapshots, so it can break an in-flight pre-deletion query; it therefore runs only at startup, where there are no live readers, and only under --startup-disk-reclaim (default off) so the image can ship to prod inertly and be switched on deliberately. The orphan purge is gated together with the unlink, not just the unlink: the purge is what lifts the watermark, and reclaim-measure reports "below wm now" vs "below wm after purge". Purging by default would collapse that comparison to zero on the first restart and destroy the measurement. Deleting unconfigured datasets is unchanged and still runs unconditionally -- it predates this branch. reclaim-measure joins the workspace and is built into the hotblocks image. It opens the live db as a RocksDB secondary instance (no lock, no writes; CF_TABLES sizes come from live_files() metadata rather than SST opens) and reports how much --startup-disk-reclaim would actually free, excluding the L0 bytes the unlink cannot touch and reporting them separately. Its db path defaults to /run/db, so it can be exec'd straight into a running pod. The rules that decide what is unlinkable -- range bounds, watermark, the level-0 and entirely-below-the-bound predicate -- live in sqd_storage::db::reclaim and are shared with the database, so the number the probe prints cannot drift from what the flag frees. This supersedes the earlier grace-period design: per-table deletion timestamps, the wall-clock grace gate, and the runtime orphan reaper are all removed, along with their CLI flags. That eliminates a class of wall-clock data-loss bugs (clock jumps, slow clients outliving the grace) in exchange for running the unlink only at startup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
028f8d6 to
020c58f
Compare
delete_range_cf is not officially supported on OptimisticTransactionDB (facebook/rocksdb#4812, rust-rocksdb#839 - same finding as PR #79), so purge the dataset's CF_BLOCK_HASHES prefix with bounded write batches of point deletes instead. Stays outside the metadata transaction, so crash semantics are unchanged: index gone before metadata -> hashes 404, retried next startup. Also trim the doc comments across the index code down to their load-bearing parts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The implementation-free behavioral specification (15 documents: state model, write/read paths, invariant catalog INV-*, liveness LIV-*, failure model FM-*, retention/space RS-*, performance SLI-*/HZ-*, observability OB-*, conformance plan CT-* with the reference-model oracle, HTTP binding IB-*, parameter registry P-*), plus the full spec-vs-implementation audit (audit-2026-07-12.md) that re-verified every gap-register entry against the code. Audit outcomes already folded into the normative text: - RP-5b: an anchored query at from == head+1 (fin+1 for finalized-only) with a mismatched parent conflicts immediately, before the head-wait -- the implementation was right, the reference model was wrong - RP-9: the coverage carrier is emission itself -- the coverage-end block is always emitted (header-only when unmatched); extra boundary markers are layout-dependent and exempt from INV-22 determinism - WP 2.5: a downward retention bound is an explicit, alarmed RESET (matching the implementation's intent); refused at boot for Pinned - WP-6: rollback may be batch-granular (chunk-boundary matching) - Gap register rewritten: GAP-6 demoted P0->P2 (runtime reclaim landed in PR #79), GAP-22 upgraded P2->P1 (a deep fork can silently replace the finalized prefix), GAP-2/4/8/9/11/20/21 corrected, GAP-23..37 added - Harness README: first four CT-4 scripts planned; the header-only-record open question resolved (it is the RP-9 carrier) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The implementation-free behavioral specification (15 documents: state model, write/read paths, invariant catalog INV-*, liveness LIV-*, failure model FM-*, retention/space RS-*, performance SLI-*/HZ-*, observability OB-*, conformance plan CT-* with the reference-model oracle, HTTP binding IB-*, parameter registry P-*), plus the full spec-vs-implementation audit (audit-2026-07-12.md) that re-verified every gap-register entry against the code. Audit outcomes already folded into the normative text: - RP-5b: an anchored query at from == head+1 (fin+1 for finalized-only) with a mismatched parent conflicts immediately, before the head-wait -- the implementation was right, the reference model was wrong - RP-9: the coverage carrier is emission itself -- the coverage-end block is always emitted (header-only when unmatched); extra boundary markers are layout-dependent and exempt from INV-22 determinism - WP 2.5: a downward retention bound is an explicit, alarmed RESET (matching the implementation's intent); refused at boot for Pinned - WP-6: rollback may be batch-granular (chunk-boundary matching) - Gap register rewritten: GAP-6 demoted P0->P2 (runtime reclaim landed in PR #79), GAP-22 upgraded P2->P1 (a deep fork can silently replace the finalized prefix), GAP-2/4/8/9/11/20/21 corrected, GAP-23..37 added - Harness README: first four CT-4 scripts planned; the header-only-record open question resolved (it is the RP-9 carrier) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Codex <codex@openai.com>
The implementation-free behavioral specification (15 documents: state model, write/read paths, invariant catalog INV-*, liveness LIV-*, failure model FM-*, retention/space RS-*, performance SLI-*/HZ-*, observability OB-*, conformance plan CT-* with the reference-model oracle, HTTP binding IB-*, parameter registry P-*), plus the full spec-vs-implementation audit (audit-2026-07-12.md) that re-verified every gap-register entry against the code. Audit outcomes already folded into the normative text: - RP-5b: an anchored query at from == head+1 (fin+1 for finalized-only) with a mismatched parent conflicts immediately, before the head-wait -- the implementation was right, the reference model was wrong - RP-9: the coverage carrier is emission itself -- the coverage-end block is always emitted (header-only when unmatched); extra boundary markers are layout-dependent and exempt from INV-22 determinism - WP 2.5: a downward retention bound is an explicit, alarmed RESET (matching the implementation's intent); refused at boot for Pinned - WP-6: rollback may be batch-granular (chunk-boundary matching) - Gap register rewritten: GAP-6 demoted P0->P2 (runtime reclaim landed in PR #79), GAP-22 upgraded P2->P1 (a deep fork can silently replace the finalized prefix), GAP-2/4/8/9/11/20/21 corrected, GAP-23..37 added - Harness README: first four CT-4 scripts planned; the header-only-record open question resolved (it is the RP-9 carrier) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Codex <codex@openai.com>
What & why
Fixes the NET-819 mainnet full-disk incident and NET-798 (blocking disk cleanup on startup).
During the incident the hotblocks volume hit 100% and compaction could not reclaim space: at a full disk compaction deadlocks (it needs scratch space to write merged output before deleting its inputs), and write-based deletion only grows the DB until compaction runs. The one operation that frees space without writing is
DeleteFilesInRange, which unlinks whole SST files at the filesystem level.Approach
Two independent mechanisms.
Phase 1 — routine logical cleanup (
logical_cleanup, every 10s). Point-deletes a deleted table's keys and drops its bookkeeping entry, one write batch per table. Snapshot-safe: the deletes are MVCC-versioned, so an in-flight query holding an older snapshot is unaffected and no grace period is needed. Frees no space directly; it lets compaction drop the data. This is the normal-operation reclaim path, and it is always on.Point deletes rather than a single
delete_rangetombstone per table, because range deletions are not officially supported on the transactionalOptimisticTransactionDBwe run.Compaction tuning (now load-bearing, since routine reclaim relies entirely on it):
max_background_jobs=8,max_subcompactions=4,level_compaction_dynamic_level_bytes→ the default 2 background jobs could not keep up during the incident.Physical reclaim — startup only, off by default (
reclaim_disk_space). Unlinks wholeCF_TABLESSST files below the min-live-TableIdwatermark viaDeleteFilesInRange. No writes, no scratch space → frees disk even at 100%, where compaction deadlocks. This is the "reclaim before ingest" step (NET-798). Crash-orphanedDIRTY_TABLESmarkers are purged at startup first, so they don't pin the watermark.Startup order matters at a full disk: unlink (write-free) → bookkeeping writes that lift the watermark → unlink again. Every step is best-effort and never blocks startup.
Rollout: measure first, then enable
Physical reclaim is gated behind
--startup-disk-reclaim, default off, so this can ship to prod inertly and be switched on deliberately.The orphan purge is gated together with the unlink rather than left on: the purge is what lifts the watermark, and the measurement tool below reports "below wm now" vs "below wm after purge". Purging by default would collapse that comparison to zero on the first restart and destroy the measurement.
crates/reclaim-measureis a read-only probe, now built into the hotblocks image. It opens the live DB as a RocksDB secondary instance — no lock, no writes, no SST opens (sizes come fromlive_files()metadata) — and reports how much--startup-disk-reclaimwould actually free. Its DB path defaults to/run/db, so it can be exec'd straight into a running pod:It prints
CF_TABLESsplit into live / dirty / dead (reclaimable) / boundary bytes, plus how much a startup unlink would free now and after the orphan purge. Deploy → run it → if the number justifies it, add--startup-disk-reclaimand restart.Deleting unconfigured datasets at startup is not gated; it predates this branch and still runs unconditionally.
Trade-offs & deferred work
The file unlink ignores snapshots, so it can break an in-flight query whose snapshot predates a deletion. We therefore run it only at startup, where there are no live readers — it never corrupts the DB, only risks an in-flight read, and only at a moment when there are none.
The watermark is a global minimum over all datasets, so one old live table (or one undecodable chunk) blocks the unlink everywhere until it is gone. Above-watermark and boundary garbage is left to compaction.
A runtime disk-pressure trigger (free-space threshold via
statvfs, or on a writeENOSPC) — the emergency reclaim compaction cannot deliver at a full disk — is documented in code as a future improvement, not built here.Changed from the earlier iteration
This supersedes the grace-period design. Removed: per-table deletion timestamps, the wall-clock grace gate (
--reclaim-grace-secs), the runtime orphan reaper (--max-build-secs), and the two-phase bookkeeping. That eliminates a class of wall-clock data-loss bugs (clock jumps, slow clients outliving the grace, malformed-value coercion) in exchange for running the unlink only at startup.reclaim_disk_spaceand Phase 1 are now fully decoupled (watermark vs. work-queue).Testing
crates/storage/tests/cleanup_reclaim.rs(8 tests): snapshot-safe logical delete, file unlink below the watermark, live-table watermark pinning, idempotency, startup orphan purge, reclaim/Phase-1 decoupling, and the documented "unlink breaks a live pre-deletion reader" trade-off. Fullsqd-storagesuite green (cleanup 8, compaction 5, database_ops 10, write_read 15, lib 1).The flag was exercised end-to-end against a real DB: with it off, startup logs
startup disk reclaim is off; enable with --startup-disk-reclaimand neither reclaim step runs; with it on, both run clean.reclaim-measurewas run against the same DB. The docker image build itself was not verified locally (no daemon available) — the Dockerfile change is a second-pon the existingcargo buildplus one extraCOPY.