disk_watch: silence the todo when the agent's total footprint is tiny
gate the big-dir todo on agent_footprint_bytes() >= 3*BIG_DIR_BYTES (15gib) in addition to the existing pct/empty-dirs checks, so a small agent whose disk-usage% crosses the warn line because the *filesystem* is full (not because this agent owns much of it) stays silent even when big_dirs() happens to name one oversized subdir inside an otherwise small tree. closes #4635
This commit is contained in:
parent
849bf46550
commit
a8f47b3f69
1 changed files with 86 additions and 16 deletions
|
|
@ -57,6 +57,13 @@ const TODO_KEY: &str = "usage";
|
||||||
/// Only directories at least this large are worth naming. Coarse on
|
/// Only directories at least this large are worth naming. Coarse on
|
||||||
/// purpose: the listed set shouldn't churn while a build runs.
|
/// purpose: the listed set shouldn't churn while a build runs.
|
||||||
const BIG_DIR_BYTES: u64 = 5 << 30;
|
const BIG_DIR_BYTES: u64 = 5 << 30;
|
||||||
|
/// Floor on the agent's total footprint (sum across every `roots()`
|
||||||
|
/// tree) below which the todo never fires, regardless of any single
|
||||||
|
/// subdir crossing `BIG_DIR_BYTES` on its own — an agent whose whole
|
||||||
|
/// tree is this small cannot meaningfully move a large filesystem's
|
||||||
|
/// usage even by deleting everything it owns, so naming one oversized
|
||||||
|
/// subdir inside a small total isn't worth a turn to act on.
|
||||||
|
const MIN_AGENT_FOOTPRINT_BYTES: u64 = 3 * BIG_DIR_BYTES;
|
||||||
/// At most this many directories are named in the todo.
|
/// At most this many directories are named in the todo.
|
||||||
const MAX_LISTED: usize = 5;
|
const MAX_LISTED: usize = 5;
|
||||||
/// Directories deeper than this (relative to a scan root) are summed into
|
/// Directories deeper than this (relative to a scan root) are summed into
|
||||||
|
|
@ -119,21 +126,23 @@ fn probe() -> Option<String> {
|
||||||
if pct < WARN_PCT {
|
if pct < WARN_PCT {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
summary_for(pct, &big_dirs())
|
summary_for(pct, agent_footprint_bytes(), &big_dirs())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Pure rendering half of [`probe`]: the todo text for a given usage
|
/// Pure rendering half of [`probe`]: the todo text for a given usage
|
||||||
/// percentage + set of oversized directories. Separated so the threshold
|
/// percentage, total agent footprint, and set of oversized directories.
|
||||||
/// and — more importantly — the *stability* of the summary are unit-tested
|
/// Separated so the thresholds and — more importantly — the *stability*
|
||||||
/// without a real filesystem.
|
/// of the summary are unit-tested without a real filesystem.
|
||||||
///
|
///
|
||||||
/// Silent unless the agent owns something worth deleting. A full disk the
|
/// Silent unless the agent owns something worth deleting. A full disk the
|
||||||
/// agent did not fill is not its problem to solve: it cannot free host
|
/// agent did not fill is not its problem to solve: it cannot free host
|
||||||
/// bytes, so the todo would only ever cost a turn to conclude "not
|
/// bytes, so the todo would only ever cost a turn to conclude "not
|
||||||
/// actionable". The shared store filling up is the host's signal, not an
|
/// actionable". The shared store filling up is the host's signal, not an
|
||||||
/// agent's.
|
/// agent's. Same reasoning extends to [`MIN_AGENT_FOOTPRINT_BYTES`]: a
|
||||||
fn summary_for(pct: u64, dirs: &[PathBuf]) -> Option<String> {
|
/// small total footprint means there is nothing here worth the bother
|
||||||
if pct < WARN_PCT || dirs.is_empty() {
|
/// even when one subdir happens to cross [`BIG_DIR_BYTES`] on its own.
|
||||||
|
fn summary_for(pct: u64, footprint: u64, dirs: &[PathBuf]) -> Option<String> {
|
||||||
|
if pct < WARN_PCT || dirs.is_empty() || footprint < MIN_AGENT_FOOTPRINT_BYTES {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
let bucket = pct / BUCKET_PCT * BUCKET_PCT;
|
let bucket = pct / BUCKET_PCT * BUCKET_PCT;
|
||||||
|
|
@ -201,6 +210,26 @@ fn roots() -> Vec<PathBuf> {
|
||||||
kept
|
kept
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Sum of bytes under every `roots()` tree combined — the agent's total
|
||||||
|
/// footprint on the filesystem [`probe`] measured. A second full walk
|
||||||
|
/// alongside [`big_dirs`]'s own: both are already gated behind
|
||||||
|
/// `pct >= WARN_PCT`, so neither runs on a healthy disk, and the extra
|
||||||
|
/// walk trades a little redundant `read_dir` work for keeping this and
|
||||||
|
/// [`big_dirs`] as two small, independently obvious functions instead of
|
||||||
|
/// one that returns a tuple nobody asked for.
|
||||||
|
fn agent_footprint_bytes() -> u64 {
|
||||||
|
let mut budget = SCAN_BUDGET;
|
||||||
|
let mut discard = Vec::new();
|
||||||
|
let mut total: u64 = 0;
|
||||||
|
for root in roots() {
|
||||||
|
let Ok(md) = std::fs::metadata(&root) else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
total = total.saturating_add(dir_bytes(&root, md.dev(), 0, &mut budget, &mut discard));
|
||||||
|
}
|
||||||
|
total
|
||||||
|
}
|
||||||
|
|
||||||
/// Walk the agent's own tree and return the directories worth naming:
|
/// Walk the agent's own tree and return the directories worth naming:
|
||||||
/// at least [`BIG_DIR_BYTES`], deepest-first (a `target/` dir, not the
|
/// at least [`BIG_DIR_BYTES`], deepest-first (a `target/` dir, not the
|
||||||
/// `/agents/<label>` that contains it), capped at [`MAX_LISTED`].
|
/// `/agents/<label>` that contains it), capped at [`MAX_LISTED`].
|
||||||
|
|
@ -284,14 +313,26 @@ mod tests {
|
||||||
PathBuf::from(s)
|
PathBuf::from(s)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A footprint comfortably over the floor, for tests that aren't
|
||||||
|
/// exercising the floor itself.
|
||||||
|
const BIG_FOOTPRINT: u64 = MIN_AGENT_FOOTPRINT_BYTES * 2;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn under_threshold_is_silent() {
|
fn under_threshold_is_silent() {
|
||||||
assert!(summary_for(WARN_PCT - 1, &[p("/agents/a/state/hh/target")]).is_none());
|
assert!(
|
||||||
|
summary_for(
|
||||||
|
WARN_PCT - 1,
|
||||||
|
BIG_FOOTPRINT,
|
||||||
|
&[p("/agents/a/state/hh/target")]
|
||||||
|
)
|
||||||
|
.is_none()
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn at_threshold_speaks_up() {
|
fn at_threshold_speaks_up() {
|
||||||
let s = summary_for(WARN_PCT, &[p("/agents/a/state/hh/target")]).expect("todo");
|
let s =
|
||||||
|
summary_for(WARN_PCT, BIG_FOOTPRINT, &[p("/agents/a/state/hh/target")]).expect("todo");
|
||||||
assert!(s.contains("over 80% full"));
|
assert!(s.contains("over 80% full"));
|
||||||
assert!(s.contains("tens of GiB"));
|
assert!(s.contains("tens of GiB"));
|
||||||
assert!(s.contains("tell the operator"));
|
assert!(s.contains("tell the operator"));
|
||||||
|
|
@ -302,7 +343,35 @@ mod tests {
|
||||||
/// free host bytes, so naming no directories means saying nothing.
|
/// free host bytes, so naming no directories means saying nothing.
|
||||||
#[test]
|
#[test]
|
||||||
fn full_disk_with_nothing_big_of_ours_is_silent() {
|
fn full_disk_with_nothing_big_of_ours_is_silent() {
|
||||||
assert!(summary_for(99, &[]).is_none());
|
assert!(summary_for(99, BIG_FOOTPRINT, &[]).is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The bug this exists for (hive-agent#4635's report): one subdir can
|
||||||
|
/// cross `BIG_DIR_BYTES` on its own while the agent's whole footprint
|
||||||
|
/// stays small enough that deleting it wouldn't move a large
|
||||||
|
/// filesystem's usage at all — that must stay silent too.
|
||||||
|
#[test]
|
||||||
|
fn a_small_total_footprint_is_silent_even_with_a_named_dir() {
|
||||||
|
assert!(
|
||||||
|
summary_for(
|
||||||
|
90,
|
||||||
|
MIN_AGENT_FOOTPRINT_BYTES - 1,
|
||||||
|
&[p("/agents/a/state/hh/target")]
|
||||||
|
)
|
||||||
|
.is_none()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_footprint_at_the_floor_speaks_up() {
|
||||||
|
assert!(
|
||||||
|
summary_for(
|
||||||
|
90,
|
||||||
|
MIN_AGENT_FOOTPRINT_BYTES,
|
||||||
|
&[p("/agents/a/state/hh/target")]
|
||||||
|
)
|
||||||
|
.is_some()
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The anti-nag property: usage drifting inside one bucket must render
|
/// The anti-nag property: usage drifting inside one bucket must render
|
||||||
|
|
@ -311,23 +380,23 @@ mod tests {
|
||||||
#[test]
|
#[test]
|
||||||
fn drift_inside_a_bucket_is_identical() {
|
fn drift_inside_a_bucket_is_identical() {
|
||||||
let dirs = [p("/agents/a/state/hh/target")];
|
let dirs = [p("/agents/a/state/hh/target")];
|
||||||
let a = summary_for(85, &dirs).expect("todo");
|
let a = summary_for(85, BIG_FOOTPRINT, &dirs).expect("todo");
|
||||||
let b = summary_for(89, &dirs).expect("todo");
|
let b = summary_for(89, BIG_FOOTPRINT, &dirs).expect("todo");
|
||||||
assert_eq!(a, b);
|
assert_eq!(a, b);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn crossing_a_bucket_changes_the_summary() {
|
fn crossing_a_bucket_changes_the_summary() {
|
||||||
let dirs = [p("/agents/a/state/hh/target")];
|
let dirs = [p("/agents/a/state/hh/target")];
|
||||||
let a = summary_for(89, &dirs).expect("todo");
|
let a = summary_for(89, BIG_FOOTPRINT, &dirs).expect("todo");
|
||||||
let b = summary_for(90, &dirs).expect("todo");
|
let b = summary_for(90, BIG_FOOTPRINT, &dirs).expect("todo");
|
||||||
assert_ne!(a, b);
|
assert_ne!(a, b);
|
||||||
assert!(b.contains("over 90% full"));
|
assert!(b.contains("over 90% full"));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn big_dirs_are_named() {
|
fn big_dirs_are_named() {
|
||||||
let s = summary_for(90, &[p("/agents/x/state/wt/target")]).expect("todo");
|
let s = summary_for(90, BIG_FOOTPRINT, &[p("/agents/x/state/wt/target")]).expect("todo");
|
||||||
assert!(s.contains("/agents/x/state/wt/target"));
|
assert!(s.contains("/agents/x/state/wt/target"));
|
||||||
assert!(s.contains("du -sh"));
|
assert!(s.contains("du -sh"));
|
||||||
}
|
}
|
||||||
|
|
@ -376,7 +445,8 @@ mod tests {
|
||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
let todos = Todos::open(&dir.path().join("state.sqlite")).expect("open");
|
let todos = Todos::open(&dir.path().join("state.sqlite")).expect("open");
|
||||||
let wake = Notify::new();
|
let wake = Notify::new();
|
||||||
let summary = summary_for(90, &[p("/agents/a/state/hh/target")]).expect("todo");
|
let summary =
|
||||||
|
summary_for(90, BIG_FOOTPRINT, &[p("/agents/a/state/hh/target")]).expect("todo");
|
||||||
|
|
||||||
reconcile(&todos, &wake, Some(&summary));
|
reconcile(&todos, &wake, Some(&summary));
|
||||||
assert_eq!(todos.list(Some(SUBSYSTEM)).expect("list").len(), 1);
|
assert_eq!(todos.list(Some(SUBSYSTEM)).expect("list").len(), 1);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue