Skip to content

Reclaim hotblocks disk: point-delete cleanup + startup DeleteFilesInRange#79

Merged
mo4islona merged 1 commit into
masterfrom
sync-dataset-cleanup-flag
Jul 9, 2026
Merged

Reclaim hotblocks disk: point-delete cleanup + startup DeleteFilesInRange#79
mo4islona merged 1 commit into
masterfrom
sync-dataset-cleanup-flag

Conversation

@mo4islona

@mo4islona mo4islona commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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_range tombstone per table, because range deletions are not officially supported on the transactional OptimisticTransactionDB we run.

Compaction tuning (now load-bearing, since routine reclaim relies entirely on it):

  • compact-on-deletion collector → picks up the SSTs a table purge turned tombstone-dense;
  • 3-day periodic compaction → backstops files that never cross the density threshold. Not 24h: each pass rewrites every SST older than the period, live data included, so a large stable dataset would be rewritten daily for nothing;
  • 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 whole CF_TABLES SST files below the min-live-TableId watermark via DeleteFilesInRange. No writes, no scratch space → frees disk even at 100%, where compaction deadlocks. This is the "reclaim before ingest" step (NET-798). Crash-orphaned DIRTY_TABLES markers 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-measure is 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 from live_files() metadata) — and reports how much --startup-disk-reclaim would actually free. Its DB path defaults to /run/db, so it can be exec'd straight into a running pod:

kubectl exec -it <hotblocks-pod> -- /app/reclaim-measure

It prints CF_TABLES split 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-reclaim and 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 write ENOSPC) — 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_space and 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. Full sqd-storage suite 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-reclaim and neither reclaim step runs; with it on, both run clean. reclaim-measure was run against the same DB. The docker image build itself was not verified locally (no daemon available) — the Dockerfile change is a second -p on the existing cargo build plus one extra COPY.

mo4islona added a commit that referenced this pull request Jun 25, 2026
…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>
@mo4islona mo4islona force-pushed the sync-dataset-cleanup-flag branch from 0e7ab30 to be14d62 Compare June 25, 2026 15:08
@mo4islona mo4islona changed the title Add sync_dataset_cleanup flag to defer table purge Reclaim hotblocks disk via DeleteFilesInRange + range deletes (NET-819, NET-798) Jun 25, 2026
mo4islona added a commit that referenced this pull request Jun 25, 2026
…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>
@mo4islona mo4islona force-pushed the sync-dataset-cleanup-flag branch 2 times, most recently from b19920a to 7659b7c Compare June 25, 2026 15:22
@mo4islona mo4islona changed the title Reclaim hotblocks disk via DeleteFilesInRange + range deletes (NET-819, NET-798) Reclaim hotblocks disk via DeleteFilesInRange + range deletes Jun 25, 2026
@mo4islona mo4islona force-pushed the sync-dataset-cleanup-flag branch 5 times, most recently from 98eac3f to 63dddfc Compare June 26, 2026 08:41
@mo4islona mo4islona changed the title Reclaim hotblocks disk via DeleteFilesInRange + range deletes Reclaim hotblocks disk: range-tombstone cleanup + startup DeleteFilesInRange Jun 26, 2026
@mo4islona mo4islona marked this pull request as ready for review June 26, 2026 09:25
@mo4islona mo4islona force-pushed the sync-dataset-cleanup-flag branch from 63dddfc to 718995e Compare June 26, 2026 09:27
Comment thread crates/storage/src/db/write/ops.rs Outdated
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())?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Reverted the old behaviour for now

mo4islona added a commit that referenced this pull request Jun 29, 2026
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>
@mo4islona mo4islona force-pushed the sync-dataset-cleanup-flag branch from ff4c648 to b8444a6 Compare July 2, 2026 10:40
@elina-chertova

Copy link
Copy Markdown

Fresh recurrence of the full-disk condition this PR targets, in case it helps prioritize review.

On 2026-07-04 ~00:37 UTC the network-hotblocks-mainnet hotblocks-db volume (local local-storage PV, 768Gi, on-prem node) hit 100% and fired a critical node-disk alert. The deployed image is subsquid/data-hotblocks:b5b8886 (2026-05-20), which predates this PR, so none of the reclaim mechanisms here are present.

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-TableId watermark, reclaim even at 100% where compaction deadlocks" behavior described here — today it only happened because of a manual restart, not the routine logical_cleanup / startup reclaim_disk_space this PR adds.

Evidence during the incident: retention was advancing logically (per-dataset hotblocks_first_block was moving forward for all datasets), yet on-disk usage had grown monotonically and sat >96% full for the preceding ~30 days with no sawtooth — consistent with tombstone/compaction not reclaiming space until a restart.

Until this ships and the deployed image is bumped, the operational mitigation is a manual hotblocks-db restart to trigger reclaim (and/or capping the heaviest datasets via a retention: <N> Head strategy).

@mo4islona mo4islona requested a review from define-null July 5, 2026 08:53
@mo4islona mo4islona changed the title Reclaim hotblocks disk: range-tombstone cleanup + startup DeleteFilesInRange Reclaim hotblocks disk: point-delete cleanup + startup DeleteFilesInRange Jul 9, 2026
@mo4islona mo4islona force-pushed the sync-dataset-cleanup-flag branch 2 times, most recently from bf6ebbb to 028f8d6 Compare July 9, 2026 15:11
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>
@mo4islona mo4islona force-pushed the sync-dataset-cleanup-flag branch from 028f8d6 to 020c58f Compare July 9, 2026 16:37
@mo4islona mo4islona merged commit 22e990a into master Jul 9, 2026
3 checks passed
@mo4islona mo4islona deleted the sync-dataset-cleanup-flag branch July 9, 2026 16:58
mo4islona added a commit that referenced this pull request Jul 10, 2026
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>
mo4islona added a commit that referenced this pull request Jul 13, 2026
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>
mo4islona added a commit that referenced this pull request Jul 13, 2026
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>
mo4islona added a commit that referenced this pull request Jul 14, 2026
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>
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.

3 participants