Skip to content

Commit b19920a

Browse files
mo4islonaclaude
andcommitted
Reclaim hotblocks disk via DeleteFilesInRange + range deletes (NET-819, 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>
1 parent eaa9f86 commit b19920a

6 files changed

Lines changed: 540 additions & 55 deletions

File tree

crates/hotblocks/src/cli.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ pub struct CLI {
6161
#[arg(long, value_name = "N", default_value = "10")]
6262
pub rocksdb_keep_log_file_num: usize,
6363

64+
/// Grace period, in seconds, before a deleted table's data files are
65+
/// physically unlinked by the background cleanup. MUST exceed the longest
66+
/// query/snapshot lifetime: file unlink ignores snapshots, so a shorter
67+
/// value can silently drop rows from an in-flight query. Default: 15 min.
68+
#[arg(long, value_name = "SECS", default_value = "900")]
69+
pub reclaim_grace_secs: u64,
70+
6471
/// Known client IDs for metrics labeling. Client IDs not in this list
6572
/// will be reported as "unknown" to prevent metrics cardinality abuse.
6673
#[arg(long = "known-client", value_name = "ID")]

crates/hotblocks/src/main.rs

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,23 @@ fn main() -> anyhow::Result<()> {
3636
.block_on(async {
3737
let app = args.build_app().await?;
3838

39-
tokio::spawn(db_cleanup_task(app.db.clone()));
39+
// Reclaim disk from unconfigured datasets deleted during startup,
40+
// before serving. No in-flight queries exist yet, so a zero grace is
41+
// safe and frees the most -- this replaces the old blocking purge and
42+
// works even at a full disk (file unlink, no writes).
43+
{
44+
let db = app.db.clone();
45+
match tokio::task::spawn_blocking(move || db.reclaim_disk_space(Duration::ZERO)).await {
46+
Ok(Ok(n)) => debug!("startup reclaim freed {n} table(s)"),
47+
Ok(Err(err)) => error!(error =? err, "startup reclaim failed"),
48+
Err(_) => error!("startup reclaim panicked")
49+
}
50+
}
51+
52+
tokio::spawn(db_cleanup_task(
53+
app.db.clone(),
54+
Duration::from_secs(args.reclaim_grace_secs)
55+
));
4056

4157
let api = build_api(app);
4258

@@ -89,24 +105,41 @@ async fn shutdown_signal() {
89105
}
90106
}
91107

108+
const CLEANUP_INTERVAL: Duration = Duration::from_secs(10);
109+
/// Backoff after a failed cleanup tick, so a persistent error (e.g. a full disk)
110+
/// doesn't busy-loop failing writes.
111+
const CLEANUP_ERROR_BACKOFF: Duration = Duration::from_secs(30);
112+
92113
#[instrument(name = "db_cleanup", skip_all)]
93-
async fn db_cleanup_task(db: DBRef) {
94-
tokio::time::sleep(Duration::from_secs(10)).await;
114+
async fn db_cleanup_task(db: DBRef, reclaim_grace: Duration) {
115+
tokio::time::sleep(CLEANUP_INTERVAL).await;
95116
loop {
96-
debug!("db cleanup started");
97117
let db = db.clone();
98-
let result = tokio::task::spawn_blocking(move || db.cleanup()).await;
118+
let result = tokio::task::spawn_blocking(move || {
119+
// Phase 1: logical, snapshot-safe purge (one range tombstone per table).
120+
let purged = db.cleanup()?;
121+
// Phase 2: physically unlink files whose grace has elapsed. No writes
122+
// / scratch space, so it makes progress even at a full disk.
123+
let reclaimed = db.reclaim_disk_space(reclaim_grace)?;
124+
anyhow::Ok((purged, reclaimed))
125+
})
126+
.await;
127+
99128
match result {
100-
Ok(Ok(deleted)) => {
101-
if deleted > 0 {
102-
debug!("purged {} tables", deleted)
103-
} else {
104-
debug!("nothing to purge, pausing cleanup for 10 seconds");
105-
tokio::time::sleep(Duration::from_secs(10)).await;
129+
Ok(Ok((purged, reclaimed))) => {
130+
if purged > 0 || reclaimed > 0 {
131+
debug!("cleanup: purged {purged} tables, reclaimed {reclaimed} files");
106132
}
133+
tokio::time::sleep(CLEANUP_INTERVAL).await;
134+
}
135+
Ok(Err(err)) => {
136+
error!(error =? err, "database cleanup failed; backing off");
137+
tokio::time::sleep(CLEANUP_ERROR_BACKOFF).await;
138+
}
139+
Err(_) => {
140+
error!("database cleanup task panicked; backing off");
141+
tokio::time::sleep(CLEANUP_ERROR_BACKOFF).await;
107142
}
108-
Ok(Err(err)) => error!(error =? err, "database cleanup task failed"),
109-
Err(_) => error!("database cleanup task panicked")
110143
}
111144
}
112145
}

crates/storage/src/db/db.rs

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use super::{
1212
use crate::db::{
1313
ops::{perform_dataset_compaction, CompactionStatus},
1414
read::datasets::list_all_datasets,
15-
write::{ops::deleted_deleted_tables, table_builder::TableBuilder, tx::Tx},
15+
write::{ops as cleanup_ops, table_builder::TableBuilder, tx::Tx},
1616
Chunk, DatasetUpdate
1717
};
1818

@@ -36,7 +36,8 @@ pub struct DatabaseSettings {
3636
direct_io: bool,
3737
cache_index_and_filter_blocks: bool,
3838
max_log_file_size: usize,
39-
keep_log_file_num: usize
39+
keep_log_file_num: usize,
40+
auto_compactions: bool
4041
}
4142

4243
impl Default for DatabaseSettings {
@@ -48,7 +49,8 @@ impl Default for DatabaseSettings {
4849
direct_io: false,
4950
cache_index_and_filter_blocks: false,
5051
max_log_file_size: 10,
51-
keep_log_file_num: 10
52+
keep_log_file_num: 10,
53+
auto_compactions: true
5254
}
5355
}
5456
}
@@ -91,6 +93,14 @@ impl DatabaseSettings {
9193
self
9294
}
9395

96+
/// Enable/disable RocksDB background auto-compaction of the table data.
97+
/// Defaults to `true`; mainly for tests that need deterministic control over
98+
/// when compaction runs (manual `compact_tables`/reclaim still work).
99+
pub fn with_auto_compactions(mut self, yes: bool) -> Self {
100+
self.auto_compactions = yes;
101+
self
102+
}
103+
94104
fn db_options(&self) -> RocksOptions {
95105
let mut options = RocksOptions::default();
96106
options.create_if_missing(true);
@@ -141,6 +151,16 @@ impl DatabaseSettings {
141151
let mut options = RocksOptions::default();
142152
options.set_block_based_table_factory(&block_based_table_factory);
143153
options.set_compression_type(rocksdb::DBCompressionType::Lz4);
154+
// Help compaction find tombstone-heavy SSTs (table deletes leave range
155+
// tombstones), and bound staleness so no dead file lingers uncompacted.
156+
// A lone range tombstone over a whole table has low deletion *density*,
157+
// so periodic compaction is the real backstop here; the collector mostly
158+
// catches denser boundary files. Thresholds are provisional.
159+
options.add_compact_on_deletion_collector_factory(128 * 1024, 64 * 1024, 0.5);
160+
options.set_periodic_compaction_seconds(24 * 60 * 60);
161+
if !self.auto_compactions {
162+
options.set_disable_auto_compactions(true);
163+
}
144164
options
145165
}
146166

@@ -287,8 +307,38 @@ impl Database {
287307
Ok(())
288308
}
289309

310+
/// Phase 1 -- logically purge deleted tables (snapshot-safe range tombstones).
311+
/// Returns the number of tables logically deleted by this call.
290312
pub fn cleanup(&self) -> anyhow::Result<usize> {
291-
deleted_deleted_tables(&self.db)
313+
cleanup_ops::logical_cleanup(&self.db)
314+
}
315+
316+
/// Phase 2 -- physically reclaim disk space from tables deleted longer than
317+
/// `grace` ago, by unlinking whole SST files below the live watermark.
318+
///
319+
/// Performs no writes and needs no scratch space, so it works at a full disk.
320+
/// `grace` MUST exceed the maximum in-flight query/snapshot lifetime: file
321+
/// unlinks ignore snapshots, so a shorter grace can drop rows from a running
322+
/// query. Use [`Duration::ZERO`] only where no readers exist (e.g. startup).
323+
/// Returns the number of deleted-table records reclaimed.
324+
pub fn reclaim_disk_space(&self, grace: std::time::Duration) -> anyhow::Result<usize> {
325+
cleanup_ops::reclaim_disk_space(&self.db, grace)
326+
}
327+
328+
/// Flush the table-data column family's memtable to SST files. Useful before
329+
/// a reclaim/shutdown so freshly written data is on disk as files.
330+
pub fn flush(&self) -> anyhow::Result<()> {
331+
self.db.flush_cf(self.db.cf_handle(CF_TABLES).unwrap())?;
332+
Ok(())
333+
}
334+
335+
/// Force a full compaction of the table-data column family. Unlike
336+
/// [`Database::reclaim_disk_space`] this *writes* (needs scratch space, so it
337+
/// is not safe at a full disk); it exists to push data into the bottom level
338+
/// and to rewrite tombstone-heavy boundary files that file-unlink can't reach.
339+
pub fn compact_tables(&self) {
340+
let cf = self.db.cf_handle(CF_TABLES).unwrap();
341+
self.db.compact_range_cf(cf, None::<&[u8]>, None::<&[u8]>);
292342
}
293343

294344
pub fn get_statistics(&self) -> Option<String> {

0 commit comments

Comments
 (0)