Skip to content

Commit 1126661

Browse files
sokraclaude
andauthored
Delete blob files during compaction when entries are superseded (#91314)
### What? During compaction in `turbo-persistence`, when entries are dropped (superseded by newer values or pruned by tombstones), blob files referenced by those entries are now marked for deletion. ### Why? Previously, compaction would merge SST files and correctly drop stale entries, but blob files referenced by those dropped entries were leaked on disk (marked with a TODO at the time). Over time this would cause unbounded disk usage growth for databases that overwrite or delete blob-sized values. ### How? When the compaction merge loop skips an entry (because `skip_remaining_for_this_key` is `true`), it now checks if the dropped entry is a `LookupValue::Blob` and, if so, pushes its sequence number to `blob_seq_numbers_to_delete`. The existing `commit()` infrastructure already handles the rest — writing `.del` files and removing the actual `.blob` files after the CURRENT pointer is updated. The change is minimal (4 lines of logic in `db.rs`): - Made `blob_seq_numbers_to_delete` mutable - Added an `else` branch to collect blob sequence numbers from dropped entries This covers both cases: - **SingleValue**: After the first (newest) entry for a key is written, all older entries are skipped. Blob references in those older entries are marked for deletion. - **MultiValue**: After a tombstone is encountered, all older entries for that key are skipped. Blob references in those older entries are marked for deletion. ### Tests Added 4 new tests: - `compaction_deletes_superseded_blob` — blob overwritten by smaller value → blob deleted after compaction - `compaction_deletes_blob_on_tombstone` — blob deleted via tombstone → blob deleted after compaction - `compaction_deletes_blob_multi_value_tombstone` — MultiValue: tombstone prunes blob → blob deleted - `compaction_preserves_active_blob` — blob still referenced → blob preserved after compaction All existing compaction tests (23) and full turbo-persistence test suite (60) continue to pass. Co-authored-by: Tobias Koppers <sokra@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent d5e8863 commit 1126661

File tree

2 files changed

+231
-4
lines changed

2 files changed

+231
-4
lines changed

turbopack/crates/turbo-persistence/src/db.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,9 +1085,7 @@ impl<S: ParallelScheduler, const FAMILIES: usize> TurboPersistence<S, FAMILIES>
10851085

10861086
let iter = MergeIter::new(iters.into_iter())?;
10871087

1088-
// TODO figure out how to delete blobs when they are no longer
1089-
// referenced
1090-
let blob_seq_numbers_to_delete: Vec<u32> = Vec::new();
1088+
let mut blob_seq_numbers_to_delete: Vec<u32> = Vec::new();
10911089

10921090
struct Collector {
10931091
/// The active writer and its sequence number. `None` if no
@@ -1241,6 +1239,16 @@ impl<S: ParallelScheduler, const FAMILIES: usize> TurboPersistence<S, FAMILIES>
12411239
sequence_number,
12421240
&mut keys_written,
12431241
)?;
1242+
} else {
1243+
// Entry is being dropped (superseded by newer entry or
1244+
// pruned by tombstone). If it references a blob file,
1245+
// mark that blob for deletion.
1246+
if let LazyLookupValue::Eager(LookupValue::Blob {
1247+
sequence_number,
1248+
}) = &entry.value
1249+
{
1250+
blob_seq_numbers_to_delete.push(*sequence_number);
1251+
}
12441252
}
12451253
}
12461254

turbopack/crates/turbo-persistence/src/tests.rs

Lines changed: 220 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{fs, time::Instant};
1+
use std::{fs, path::Path, time::Instant};
22

33
use anyhow::Result;
44
use rayon::iter::{IntoParallelIterator, ParallelIterator};
@@ -1968,3 +1968,222 @@ fn multi_value_tombstone_shadows_older_sst_only() -> Result<()> {
19681968

19691969
Ok(())
19701970
}
1971+
1972+
/// Returns the number of `.blob` files in the given directory.
1973+
fn count_blob_files(dir: &Path) -> usize {
1974+
fs::read_dir(dir)
1975+
.unwrap()
1976+
.filter_map(|e| e.ok())
1977+
.filter(|e| e.path().extension().is_some_and(|ext| ext == "blob"))
1978+
.count()
1979+
}
1980+
1981+
/// Test that compaction deletes blob files when their entries are superseded
1982+
/// by newer values (SingleValue family).
1983+
#[test]
1984+
fn compaction_deletes_superseded_blob() -> Result<()> {
1985+
let tempdir = tempfile::tempdir()?;
1986+
let path = tempdir.path();
1987+
1988+
let db = TurboPersistence::<_, 1>::open_with_parallel_scheduler(
1989+
path.to_path_buf(),
1990+
RayonParallelScheduler,
1991+
)?;
1992+
1993+
let blob_value = vec![42u8; MAX_MEDIUM_VALUE_SIZE + 1];
1994+
1995+
// Write a blob-sized value
1996+
let batch = db.write_batch()?;
1997+
batch.put(0, vec![1u8], blob_value.clone().into())?;
1998+
db.commit_write_batch(batch)?;
1999+
2000+
assert_eq!(
2001+
count_blob_files(path),
2002+
1,
2003+
"Should have 1 blob file after first write"
2004+
);
2005+
2006+
// Verify we can read it
2007+
let result = db.get(0, &vec![1u8])?;
2008+
assert_eq!(result.as_deref(), Some(&blob_value[..]));
2009+
2010+
// Overwrite the key with a small (non-blob) value in a new batch
2011+
let batch = db.write_batch()?;
2012+
batch.put(0, vec![1u8], vec![99u8].into())?;
2013+
db.commit_write_batch(batch)?;
2014+
2015+
// Blob file still exists before compaction (old SST still references it)
2016+
assert_eq!(
2017+
count_blob_files(path),
2018+
1,
2019+
"Blob file should still exist before compaction"
2020+
);
2021+
2022+
// Compact — the old blob entry is superseded by the newer small value
2023+
db.full_compact()?;
2024+
2025+
// After compaction, the old blob file should be deleted
2026+
assert_eq!(
2027+
count_blob_files(path),
2028+
0,
2029+
"Old blob file should be deleted after compaction"
2030+
);
2031+
2032+
// The new value should still be readable
2033+
let result = db.get(0, &vec![1u8])?;
2034+
assert_eq!(result.as_deref(), Some(&[99u8][..]));
2035+
2036+
db.shutdown()?;
2037+
Ok(())
2038+
}
2039+
2040+
/// Test that compaction deletes blob files when a key is deleted via tombstone
2041+
/// (SingleValue family).
2042+
#[test]
2043+
fn compaction_deletes_blob_on_tombstone() -> Result<()> {
2044+
let tempdir = tempfile::tempdir()?;
2045+
let path = tempdir.path();
2046+
2047+
let db = TurboPersistence::<_, 1>::open_with_parallel_scheduler(
2048+
path.to_path_buf(),
2049+
RayonParallelScheduler,
2050+
)?;
2051+
2052+
let blob_value = vec![42u8; MAX_MEDIUM_VALUE_SIZE + 1];
2053+
2054+
// Write a blob-sized value
2055+
let batch = db.write_batch()?;
2056+
batch.put(0, vec![1u8], blob_value.clone().into())?;
2057+
db.commit_write_batch(batch)?;
2058+
2059+
assert_eq!(count_blob_files(path), 1);
2060+
2061+
// Delete the key
2062+
let batch = db.write_batch()?;
2063+
batch.delete(0, vec![1u8])?;
2064+
db.commit_write_batch(batch)?;
2065+
2066+
// Blob file still exists before compaction
2067+
assert_eq!(
2068+
count_blob_files(path),
2069+
1,
2070+
"Blob file should still exist before compaction"
2071+
);
2072+
2073+
// Compact — tombstone supersedes the blob entry
2074+
db.full_compact()?;
2075+
2076+
// After compaction, the blob file should be deleted
2077+
assert_eq!(
2078+
count_blob_files(path),
2079+
0,
2080+
"Blob file should be deleted after compaction"
2081+
);
2082+
2083+
// Key should not be found
2084+
let result = db.get(0, &vec![1u8])?;
2085+
assert!(result.is_none());
2086+
2087+
db.shutdown()?;
2088+
Ok(())
2089+
}
2090+
2091+
/// Test that compaction deletes blob files for MultiValue families when a
2092+
/// tombstone prunes older blob entries.
2093+
#[test]
2094+
fn compaction_deletes_blob_multi_value_tombstone() -> Result<()> {
2095+
let tempdir = tempfile::tempdir()?;
2096+
let path = tempdir.path();
2097+
2098+
let config = DbConfig {
2099+
family_configs: [FamilyConfig {
2100+
kind: FamilyKind::MultiValue,
2101+
}],
2102+
};
2103+
2104+
let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler(
2105+
path.to_path_buf(),
2106+
config,
2107+
RayonParallelScheduler,
2108+
)?;
2109+
2110+
let blob_value = vec![42u8; MAX_MEDIUM_VALUE_SIZE + 1];
2111+
2112+
// Write a blob-sized value
2113+
let batch = db.write_batch()?;
2114+
batch.put(0, vec![1u8], blob_value.clone().into())?;
2115+
db.commit_write_batch(batch)?;
2116+
2117+
assert_eq!(count_blob_files(path), 1);
2118+
2119+
// Delete the key (tombstone) and write a new small value in a new batch
2120+
let batch = db.write_batch()?;
2121+
batch.delete(0, vec![1u8])?;
2122+
batch.put(0, vec![1u8], vec![99u8].into())?;
2123+
db.commit_write_batch(batch)?;
2124+
2125+
// Blob file still exists before compaction
2126+
assert_eq!(count_blob_files(path), 1);
2127+
2128+
// Compact — tombstone prunes the old blob entry
2129+
db.full_compact()?;
2130+
2131+
// After compaction, the old blob file should be deleted
2132+
assert_eq!(
2133+
count_blob_files(path),
2134+
0,
2135+
"Blob file should be deleted after compaction"
2136+
);
2137+
2138+
// The new value should still be readable
2139+
let results = db.get_multiple(0, &vec![1u8].as_slice())?;
2140+
assert_eq!(results.len(), 1);
2141+
assert_eq!(results[0].as_ref(), &[99u8]);
2142+
2143+
db.shutdown()?;
2144+
Ok(())
2145+
}
2146+
2147+
/// Test that compaction preserves blob files that are still referenced
2148+
/// (not superseded).
2149+
#[test]
2150+
fn compaction_preserves_active_blob() -> Result<()> {
2151+
let tempdir = tempfile::tempdir()?;
2152+
let path = tempdir.path();
2153+
2154+
let db = TurboPersistence::<_, 1>::open_with_parallel_scheduler(
2155+
path.to_path_buf(),
2156+
RayonParallelScheduler,
2157+
)?;
2158+
2159+
let blob_value = vec![42u8; MAX_MEDIUM_VALUE_SIZE + 1];
2160+
2161+
// Write a blob-sized value
2162+
let batch = db.write_batch()?;
2163+
batch.put(0, vec![1u8], blob_value.clone().into())?;
2164+
db.commit_write_batch(batch)?;
2165+
2166+
// Write a different key to create a second SST (so compaction has work to do)
2167+
let batch = db.write_batch()?;
2168+
batch.put(0, vec![2u8], vec![1u8].into())?;
2169+
db.commit_write_batch(batch)?;
2170+
2171+
assert_eq!(count_blob_files(path), 1);
2172+
2173+
// Compact — the blob entry is still the latest, should be preserved
2174+
db.full_compact()?;
2175+
2176+
// Blob file should still exist
2177+
assert_eq!(
2178+
count_blob_files(path),
2179+
1,
2180+
"Active blob file should be preserved after compaction"
2181+
);
2182+
2183+
// Value should still be readable
2184+
let result = db.get(0, &vec![1u8])?;
2185+
assert_eq!(result.as_deref(), Some(&blob_value[..]));
2186+
2187+
db.shutdown()?;
2188+
Ok(())
2189+
}

0 commit comments

Comments
 (0)