Skip to content

Commit 122f928

Browse files
committed
mason: add age-based orphaned-temporary sweep to callgraph store
Cold builds write <key>.g...sqlite.tmp.<pid>.<ts> and rename into place on success; a killed/crashed build leaves the temporary behind. The existing cleanup only fires per-root when that root builds, so orphans for a root that stopped building (e.g. after the root-keyed migration) accumulate forever. Add a store-wide sweep on the same cadence as generation GC (after a generation is published) covering both the root-keyed and legacy per-harness layouts. The deletion predicate is age-based (mtime >= 24h), not pid-liveness: pid reuse makes liveness read false-positive on exactly the oldest orphans. remove_file ENOENT is treated as success (a concurrent rename won) and the parent dir is fsynced after removals. Completed .sqlite generations and read markers are untouched.
1 parent cd6ec3b commit 122f928

1 file changed

Lines changed: 220 additions & 0 deletions

File tree

  • crates/aft/src/callgraph_store

crates/aft/src/callgraph_store/mod.rs

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1885,6 +1885,10 @@ impl CallGraphStore {
18851885
verify_writer_lease(&writer_lease)?;
18861886
publish_pointer(callgraph_dir, project_key, &generation)?;
18871887
gc_old_generations(callgraph_dir, project_key, &generation);
1888+
// Store-wide orphan sweep on the same cadence: reclaims aged build
1889+
// temps for roots that no longer build here, which the per-root GC
1890+
// above never reaches.
1891+
sweep_orphaned_build_temps_store_wide(callgraph_dir);
18881892
Ok(())
18891893
});
18901894
if matches!(publication, Err(CallGraphStoreError::Superseded)) {
@@ -5733,6 +5737,106 @@ fn remove_sqlite_sidecars(path: &Path) {
57335737
let _ = std::fs::remove_file(PathBuf::from(format!("{path_text}-journal")));
57345738
}
57355739

5740+
/// Minimum age before a cold-build temporary is treated as orphaned and deleted.
5741+
///
5742+
/// A cold build writes `<key>.g...sqlite.tmp.<pid>.<ts>` and renames it into
5743+
/// place on success; a build that dies (process kill, crash, host restart) leaves
5744+
/// the temporary behind. The largest observed cold build finishes well under a
5745+
/// day, so a temporary that has sat for 24 hours belongs to a dead build that will
5746+
/// never rename. A live build's temporary is minutes old at most.
5747+
///
5748+
/// The predicate is deliberately AGE-based, not pid-liveness. Pid reuse makes a
5749+
/// liveness check read false-positive on exactly the oldest files — the ones most
5750+
/// worth deleting: in production an orphan's embedded pid had been recycled to an
5751+
/// unrelated live process, so "is the pid alive?" answered yes for garbage. Age
5752+
/// cannot lie that way, so it is the honest orphan predicate.
5753+
const ORPHANED_BUILD_TEMP_MIN_AGE: Duration = Duration::from_secs(24 * 60 * 60);
5754+
5755+
/// Best-effort store-wide sweep of orphaned cold-build temporaries. Runs at the
5756+
/// same cadence as [`gc_old_generations`] (after a generation is published) but,
5757+
/// unlike it, is not scoped to the building root: it covers every directory in the
5758+
/// callgraph store so orphans left by a root that STOPPED building are reclaimed.
5759+
///
5760+
/// That last case is the production hole this fixes. The per-root cleanup in
5761+
/// [`gc_old_generations`] only fires when a root actually builds, so when activity
5762+
/// moves away (e.g. the root-keyed migration moved builds to a new store) the old
5763+
/// store's orphans become permanent — gigabytes accumulated in a legacy store
5764+
/// whose roots no longer built there, while the active store stayed clean. A
5765+
/// sibling root that still builds triggers this pass and cleans both layouts.
5766+
fn sweep_orphaned_build_temps_store_wide(callgraph_dir: &Path) {
5767+
sweep_orphaned_build_temps(callgraph_dir);
5768+
let Some(storage_root) = root_storage_dir(callgraph_dir) else {
5769+
return;
5770+
};
5771+
let domain = crate::root_cache::RootCacheDomain::Callgraph.as_str();
5772+
5773+
// Root-keyed layout: every `<storage>/callgraph/<key>` directory.
5774+
if let Ok(entries) = std::fs::read_dir(storage_root.join(domain)) {
5775+
for entry in entries.flatten() {
5776+
if entry.path().is_dir() {
5777+
sweep_orphaned_build_temps(&entry.path());
5778+
}
5779+
}
5780+
}
5781+
5782+
// Legacy per-harness layout: every `<storage>/<harness>/callgraph` directory.
5783+
if let Ok(entries) = std::fs::read_dir(&storage_root) {
5784+
for entry in entries.flatten() {
5785+
let legacy_dir = entry.path().join(domain);
5786+
if legacy_dir.is_dir() {
5787+
sweep_orphaned_build_temps(&legacy_dir);
5788+
}
5789+
}
5790+
}
5791+
}
5792+
5793+
/// Sweep one callgraph directory, removing build temporaries older than
5794+
/// [`ORPHANED_BUILD_TEMP_MIN_AGE`].
5795+
fn sweep_orphaned_build_temps(callgraph_dir: &Path) {
5796+
sweep_orphaned_build_temps_older_than(callgraph_dir, ORPHANED_BUILD_TEMP_MIN_AGE);
5797+
}
5798+
5799+
/// Inner sweep with an explicit age threshold so tests can exercise the predicate.
5800+
/// See [`ORPHANED_BUILD_TEMP_MIN_AGE`] for why the predicate is age, not pid.
5801+
fn sweep_orphaned_build_temps_older_than(callgraph_dir: &Path, min_age: Duration) {
5802+
let now = SystemTime::now();
5803+
let Ok(entries) = std::fs::read_dir(callgraph_dir) else {
5804+
return;
5805+
};
5806+
let mut removed_any = false;
5807+
for entry in entries.flatten() {
5808+
let name = entry.file_name().to_string_lossy().to_string();
5809+
// Build-temporary shape: `<key>.g...sqlite.tmp.<pid>.<ts>`. The
5810+
// `-journal`/`-wal`/`-shm` sidecars append their suffix AFTER the temp
5811+
// name, so they still contain `.sqlite.tmp.` and match here too. Anything
5812+
// without that substring — a completed `.sqlite` generation, a pointer, a
5813+
// read-marker dir — is left alone: those belong to generation GC.
5814+
if !name.contains(".sqlite.tmp.") {
5815+
continue;
5816+
}
5817+
let mtime = entry
5818+
.metadata()
5819+
.and_then(|meta| meta.modified())
5820+
.unwrap_or(now);
5821+
if now.duration_since(mtime).unwrap_or(Duration::ZERO) < min_age {
5822+
continue;
5823+
}
5824+
// Deletion races a concurrent build finishing: that build renames the temp
5825+
// into place, so the file is gone by the time we unlink. The 24h age makes
5826+
// this overlap practically impossible, but treat a missing file as success
5827+
// (the rename won) rather than an error, and never touch a path that does
5828+
// not match the temporary shape above.
5829+
match std::fs::remove_file(entry.path()) {
5830+
Ok(()) => removed_any = true,
5831+
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
5832+
Err(_) => {}
5833+
}
5834+
}
5835+
if removed_any {
5836+
crate::fs_lock::sync_parent(callgraph_dir);
5837+
}
5838+
}
5839+
57365840
/// Bound the cold-build's tree-sitter pass to half the cores (cap 8) instead of
57375841
/// the global all-cores rayon pool. The store cold-build is the heaviest
57385842
/// background pass (parse-dominated) and runs on a separate thread off the
@@ -11109,6 +11213,122 @@ mod cold_build_insert_tests {
1110911213
assert!(!dir.path().join(&old).exists());
1111011214
}
1111111215

11216+
fn write_build_temp_with_age(dir: &Path, name: &str, age: Duration) -> PathBuf {
11217+
let path = dir.join(name);
11218+
fs::write(&path, b"temp placeholder").unwrap();
11219+
let mtime = SystemTime::now().checked_sub(age).unwrap_or(UNIX_EPOCH);
11220+
filetime::set_file_mtime(&path, filetime::FileTime::from_system_time(mtime)).unwrap();
11221+
path
11222+
}
11223+
11224+
#[test]
11225+
fn orphan_temp_sweep_removes_aged_orphan_and_journal_but_spares_fresh() {
11226+
let dir = tempdir().unwrap();
11227+
// One directory holds both an aged orphan (with its journal sidecar) and a
11228+
// fresh temporary, so this proves the sweep SELECTS by age rather than
11229+
// deleting everything in the directory.
11230+
let aged = "project.g100.1.sqlite.tmp.1.200";
11231+
let aged_journal = "project.g100.1.sqlite.tmp.1.200-journal";
11232+
let fresh = "project.g300.1.sqlite.tmp.1.400";
11233+
let aged_age = ORPHANED_BUILD_TEMP_MIN_AGE + Duration::from_secs(60);
11234+
write_build_temp_with_age(dir.path(), aged, aged_age);
11235+
write_build_temp_with_age(dir.path(), aged_journal, aged_age);
11236+
write_build_temp_with_age(dir.path(), fresh, Duration::ZERO);
11237+
11238+
sweep_orphaned_build_temps(dir.path());
11239+
11240+
assert!(
11241+
!dir.path().join(aged).exists(),
11242+
"aged orphan must be removed"
11243+
);
11244+
assert!(
11245+
!dir.path().join(aged_journal).exists(),
11246+
"aged journal sidecar must be removed"
11247+
);
11248+
assert!(
11249+
dir.path().join(fresh).is_file(),
11250+
"fresh temporary must survive"
11251+
);
11252+
}
11253+
11254+
#[test]
11255+
fn orphan_temp_sweep_reaches_legacy_store_for_root_with_no_pointer_or_build() {
11256+
let storage = tempdir().unwrap();
11257+
let storage_root = storage.path();
11258+
// The production shape: a legacy per-harness store whose root no longer
11259+
// builds there — no `.current` pointer, no running build — so the per-root
11260+
// cleanup never fires for it. A sibling root still building in the
11261+
// root-keyed store triggers the store-wide sweep, which must reach into the
11262+
// legacy directory and reclaim the orphan.
11263+
let legacy_dir = storage_root.join("opencode").join("callgraph");
11264+
fs::create_dir_all(&legacy_dir).unwrap();
11265+
let orphan = "deadbeef.g100.1.sqlite.tmp.1.200";
11266+
write_build_temp_with_age(
11267+
&legacy_dir,
11268+
orphan,
11269+
ORPHANED_BUILD_TEMP_MIN_AGE + Duration::from_secs(60),
11270+
);
11271+
assert!(
11272+
!legacy_dir.join("deadbeef.current").exists(),
11273+
"the dead root has no current pointer"
11274+
);
11275+
11276+
let root_keyed_dir = storage_root.join("callgraph").join("livekey");
11277+
fs::create_dir_all(&root_keyed_dir).unwrap();
11278+
11279+
sweep_orphaned_build_temps_store_wide(&root_keyed_dir);
11280+
11281+
assert!(
11282+
!legacy_dir.join(orphan).exists(),
11283+
"legacy orphan must be reclaimed by the store-wide sweep"
11284+
);
11285+
}
11286+
11287+
#[test]
11288+
fn orphan_temp_sweep_negative_control_age_predicate_is_what_spares_fresh() {
11289+
// NEGATIVE CONTROL, mutation-proved: forcing the age predicate to accept
11290+
// everything (min_age = 0) removes the fresh temporary that the real 24h
11291+
// threshold spares in the test above. If a mutation to the age check leaves
11292+
// the fresh file in place here, the predicate is no longer doing the
11293+
// selection work the fresh-survives assertion relies on.
11294+
let dir = tempdir().unwrap();
11295+
let fresh = "project.g300.1.sqlite.tmp.1.400";
11296+
write_build_temp_with_age(dir.path(), fresh, Duration::ZERO);
11297+
11298+
sweep_orphaned_build_temps_older_than(dir.path(), Duration::ZERO);
11299+
11300+
assert!(
11301+
!dir.path().join(fresh).exists(),
11302+
"with the age predicate forced open, the fresh temporary is removed"
11303+
);
11304+
}
11305+
11306+
#[test]
11307+
fn orphan_temp_sweep_leaves_completed_generation_and_read_marker_alone() {
11308+
let dir = tempdir().unwrap();
11309+
// A completed generation (its name has no `.sqlite.tmp.`) that is old enough
11310+
// to be swept, plus a live read marker, is generation GC's jurisdiction.
11311+
// The orphan sweep must not intersect it.
11312+
let generation = write_generation_with_age(
11313+
dir.path(),
11314+
"project",
11315+
400,
11316+
ORPHANED_BUILD_TEMP_MIN_AGE + Duration::from_secs(60),
11317+
);
11318+
let _marker = crate::root_cache::ReadMarker::create(dir.path(), &generation).unwrap();
11319+
11320+
sweep_orphaned_build_temps(dir.path());
11321+
11322+
assert!(
11323+
dir.path().join(&generation).is_file(),
11324+
"completed generation must survive the orphan sweep"
11325+
);
11326+
assert!(
11327+
crate::root_cache::read_marker_dir(dir.path(), &generation).exists(),
11328+
"read marker must survive the orphan sweep"
11329+
);
11330+
}
11331+
1111211332
#[test]
1111311333
fn atomic_swap_checkpoint_uses_passive_when_live_marker_exists() {
1111411334
let dir = tempfile::tempdir().unwrap();

0 commit comments

Comments
 (0)