diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f68698..b97efdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,24 @@ and the versions follow [semver](https://semver.org). ## [Unreleased] +### Fixed + +- A run that failed no longer raises a job's memory need above what its + passing runs took, also with fewer than three runs and when it is no + outlier. One `tsc` run that failed at 14.5 GB made every later run reserve + 18 GB. When every recent run failed, one high peak alone does not set the + need: the job reserves `new_job_mem` until a peak comes back. +- A failed run starved of memory adds the 25% boost only when its peak stayed + within that 25% of its need. +- `taskguard outliers`, `doctor --explain` and the job's end line show failed + runs whose peak is ignored, and why. + +### Changed + +- `outlier_ratio` defaults to 1.5 (was 2): a one-off 17 GB peak where a job + usually takes 9 GB no longer sets the need for its next 10 runs. A job with + two sizes, such as a cache hit and a full build, still reserves the big one. + ## [0.9.0] - 2026-10-12 ### Added diff --git a/README.md b/README.md index 84406b6..c72b0c6 100644 --- a/README.md +++ b/README.md @@ -59,12 +59,17 @@ below). the need. A memory need never passes `mem_max`: a job that once took more is capped there, and says so. - **One wild run does not set the need.** A peak more than `outlier_ratio` - (2) times the next peak below it, and at least `outlier_min` (1 GB) above + (1.5) times the next peak below it, and at least `outlier_min` (1 GB) above it, is an outlier: a CI run that took 12 GB where it usually takes 4.5 GB. The first time it does not count, the second time it counts half, the third - time in full (`outlier_weights`). A run that failed never counts: a run that - goes wrong can take far more than the job needs, and on a real machine most - outliers were such runs. Jobs whose usual peak is under `outlier_min` have + time in full (`outlier_weights`). A run that failed (exit not 0) never + raises the need above the runs that passed, outlier or not, and with any + number of runs: a run that goes wrong can take far more than the job needs, + and on a real machine most outliers were such runs. When every recent run + failed, the need is the second-highest failed peak, at least `new_job_mem` + but never more than the highest: one failed peak alone does not set it. + A failed run that was starved of memory adds the 25% only when its peak + stayed within that 25% of its need. Jobs whose usual peak is under `outlier_min` have no outliers: there a jump is mostly a full build after cache hits, and holding it back would start the next full build into a full machine. The job's line says when its run was an outlier, `taskguard outliers` lists @@ -531,7 +536,7 @@ most used ones: cpu_max = 100 # percent of all cores mem_max = 85 # percent of RAM hints = true # agent hints on status lines -outlier_ratio = 2 # a peak this many times the next one is an outlier; 0 = off +outlier_ratio = 1.5 # a peak this many times the next one is an outlier; 0 = off outside_admit = true # start jobs that only other programs keep out partial_fit = 0.5 # a job that cannot fit starts on half its CPU need; 0 = off max_backfill = 600 # small jobs pass a stuck job this long, then the machine drains @@ -582,7 +587,9 @@ repo. Put a repo's own scripts in its `.taskguard.toml`: similar jobs can still take more than its estimate before it is learned. - An outlier that is real growth is learned late: the next run still reserves the usual peak, and only the third high run counts in full. A run - that failed is never learned, even when it really needed that memory. Set + that failed is not learned above the runs that passed, even when it really + needed that memory: a job that only reaches its full size when it fails, + such as a type check that passes on small changes, can be under-reserved. Set `outlier_weights = [0.5]` to learn sooner, or `outlier_ratio = 0` to learn every peak at once. - A job that starts early on `noise_mem` or `outside_admit` can push memory diff --git a/src/commands.rs b/src/commands.rs index b8d1e93..58186b5 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -316,7 +316,13 @@ pub fn outliers(args: &[String]) -> Result { let p = parse_prune(args)?; let rules = learn_rules(&p)?; let db = Db::open_dir(&config::state_dir())?; - let found = find_all_outliers(&db, p.pattern.as_deref(), &rules)?; + let mut found = Vec::new(); + for k in db.keys(p.pattern.as_deref())? { + let l = db.learned(&k, &rules)?; + if l.outliers.is_some() || !l.failed.is_empty() { + found.push((k, l)); + } + } println!( "a peak counts as an outlier when it is more than {}x and {} above the next peak below it (outlier_ratio, outlier_min)", rules.outlier_ratio, @@ -328,19 +334,29 @@ pub fn outliers(args: &[String]) -> Result { } println!(); println!("{:<52} {:>6} {:>9} {:>9} {:>9} {:>6} {:>9} why", "key", "run", "ended", "peak", "usual", "x", "next run"); - for (k, o) in &found { - for r in &o.runs { + for (k, l) in &found { + let next = l.mem_kb.unwrap_or(0); + let row = |r: &db::Outlier, usual: u64, why: String| { println!( "{:<52} {:>6} {:>9} {:>9} {:>9} {:>6.1} {:>9} {}", k, r.run_id, ago(r.ended_at), gb(r.peak_kb), - gb(o.normal_kb), - r.peak_kb as f64 / o.normal_kb.max(1) as f64, - gb(o.mem_kb), - o.why(r) + gb(usual), + r.peak_kb as f64 / usual.max(1) as f64, + gb(next), + why ); + }; + let outliers = l.outliers.iter().flat_map(|o| o.runs.iter().map(move |r| (o, r))); + for (o, r) in outliers.clone() { + row(r, o.normal_kb, o.why(r)); + } + // Failed runs above the need that are no ratio outlier: usual is the + // need the runs that passed leave. + for r in l.failed.iter().filter(|f| !outliers.clone().any(|(_, r)| r.run_id == f.run_id)) { + row(r, next, r.why_ignored()); } } println!(); @@ -560,6 +576,15 @@ fn explain(cmdline: &str, cwd: &Path, cfg: &Config, dir: &Path) -> Result { if l.mem_boosted { " (+25% after a memory-starved run)" } else { "" }, l.dur_s.map(dur).unwrap_or("?".into()) ); + if let Some(f) = l.failed.first() { + let top = l.failed.iter().map(|r| r.peak_kb).max().unwrap_or(0); + println!( + "failed: {} run(s) up to {} above the need; {}, a run that goes wrong can take far more than the job needs (taskguard outliers)", + l.failed.len(), + gb(top), + f.why_ignored() + ); + } if let Some(o) = &l.outliers { let top = o.runs.iter().map(|r| r.peak_kb).max().unwrap_or(0); println!( diff --git a/src/config.rs b/src/config.rs index 849a69c..bbbb4d8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -200,7 +200,7 @@ impl Default for Config { max_bypass: 120, max_backfill: 600, boost_runs: 5, - outlier_ratio: 2.0, + outlier_ratio: 1.5, outlier_min_kb: 1024 * 1024, outlier_weights: vec![0.0, 0.5], noise_mem: 2.0, @@ -370,6 +370,7 @@ impl Config { outlier_ratio: self.outlier_ratio, outlier_min_kb: self.outlier_min_kb, outlier_weights: self.outlier_weights.clone(), + new_job_mem_kb: self.new_job_mem_kb, } } diff --git a/src/db.rs b/src/db.rs index d2a4a5a..1861573 100644 --- a/src/db.rs +++ b/src/db.rs @@ -221,10 +221,16 @@ pub struct Estimate { pub struct Learned { pub runs: usize, /// Max of the recent peaks, with outliers held back (see `Outliers`). - /// None when no run was ever measured. + /// Runs that failed never raise it above the runs that passed; when every + /// recent run failed, see `failed_need`. None when no run was ever + /// measured. pub mem_kb: Option, /// The recent memory peaks that stand far above the others. pub outliers: Option, + /// The recent runs that failed with a peak at least `outlier_min_kb` + /// above `mem_kb`, newest first: held back, as a run that goes wrong can + /// take far more memory than the job needs. + pub failed: Vec, /// The cores recent runs used, weighted toward the newest runs, with /// outliers held back (see `cpu_need`). None when unknown. pub cpu: Option, @@ -245,6 +251,10 @@ pub struct Learned { /// A run starved of memory raises the need when its peak reached this share /// of the need it started with. pub const BOOST_REACHED: f64 = 0.9; +/// ... and, for a run that failed, when its peak stayed within what the boost +/// gives. One that went far past its need and failed would not have passed +/// with 25% more room. +pub const BOOST_FAILED_MAX: f64 = 1.25; /// How history turns into needs. #[derive(Debug, Clone, PartialEq)] @@ -261,11 +271,21 @@ pub struct Learn { /// How much the outliers count, by how many there are: the first entry /// for one, the second for two. Past the list they count in full. pub outlier_weights: Vec, + /// What a job with no history reserves: also a job whose runs all failed + /// (see `failed_need`). + pub new_job_mem_kb: u64, } impl Default for Learn { fn default() -> Self { - Learn { keep: 10, boost_runs: 5, outlier_ratio: 2.0, outlier_min_kb: 1024 * 1024, outlier_weights: vec![0.0, 0.5] } + Learn { + keep: 10, + boost_runs: 5, + outlier_ratio: 1.5, + outlier_min_kb: 1024 * 1024, + outlier_weights: vec![0.0, 0.5], + new_job_mem_kb: 1536 * 1024, + } } } @@ -284,6 +304,23 @@ impl Outlier { pub fn failed(&self) -> bool { self.exit.is_some_and(|e| e != 0) } + + /// Why a failed run's peak does not count, in words. + pub fn why_ignored(&self) -> String { + format!("ignored: the run failed (exit {})", self.exit.unwrap_or(-1)) + } +} + +/// The memory need when every recent run failed: the second-highest peak, +/// at least `new_job_mem_kb` but never more than the highest. One failed run +/// far above the others alone does not set the need, as the job is then as +/// little known as a new one; a job that fails at the same peak again and +/// again does need it. +pub fn failed_need(peaks: &[u64], new_job_mem_kb: u64) -> Option { + let mut by_peak = peaks.to_vec(); + by_peak.sort_unstable_by(|a, b| b.cmp(a)); + let top = *by_peak.first()?; + Some(by_peak.get(1).copied().unwrap_or(0).max(new_job_mem_kb).min(top)) } /// The highest recent peaks when they stand far above the rest: one run that @@ -311,7 +348,7 @@ impl Outliers { /// Why an outlier counts as it does, in words. pub fn why(&self, o: &Outlier) -> String { if o.failed() { - return format!("ignored: the run failed (exit {})", o.exit.unwrap_or(-1)); + return o.why_ignored(); } let n = self.repeats(); let times = if n == 1 { "once".to_string() } else { format!("{n} times") }; @@ -576,6 +613,18 @@ impl Db { Some(o) => Some(o.mem_kb), None => recent.iter().map(|r| r.0 as u64).max(), }; + // A run that failed never raises the need above the runs that passed. + let passed = runs[..recent.len()].iter().filter(|o| !o.failed()).map(|o| o.peak_kb).max(); + let failed_peaks: Vec = runs[..recent.len()].iter().filter(|o| o.failed()).map(|o| o.peak_kb).collect(); + let mem = match passed { + Some(p) => mem.map(|m| m.min(p)), + None => failed_need(&failed_peaks, rules.new_job_mem_kb), + }; + let failed: Vec = runs[..recent.len()] + .iter() + .filter(|o| o.failed() && mem.is_some_and(|m| o.peak_kb >= m + rules.outlier_min_kb)) + .cloned() + .collect(); let cpu_runs: Vec = recent .iter() .zip(&runs) @@ -590,10 +639,19 @@ impl Db { // was given: then its need was too small. One that starved far below // its need was squeezed by others, and more room for it would have // gone unused. A run from before the need was recorded still counts. - let boosted = rows - .iter() - .take(boost_runs) - .any(|r| r.3.as_deref() == Some("memory") && r.6.is_none_or(|need| r.0 as f64 >= need as f64 * BOOST_REACHED)); + // One that failed counts only when its peak also stayed within what + // the boost gives: a run that went to 50x its need and crashed tells + // nothing about its need. + let boosted = rows.iter().zip(&runs).take(boost_runs).any(|(r, o)| { + r.3.as_deref() == Some("memory") + && match (r.6, o.failed()) { + (None, failed) => !failed, + (Some(need), failed) => { + let peak = r.0 as f64; + peak >= need as f64 * BOOST_REACHED && (!failed || peak <= need as f64 * BOOST_FAILED_MAX) + } + } + }); let steady_mem_kb = recent.iter().filter_map(|r| r.4).max().map(|m| m as u64); let steady_cpu = median_of(recent.iter().filter_map(|r| r.5).collect()); let nested_jobs = recent.iter().filter_map(|r| r.7).filter(|n| *n > 0).max().map(|n| n as u64); @@ -601,6 +659,7 @@ impl Db { runs: recent.len(), mem_kb: mem, outliers, + failed, cpu, dur_s: dur, mem_boosted: boosted, @@ -1415,6 +1474,140 @@ mod tests { assert_eq!(o.why(&o.runs[0]), "ignored: the run failed (exit 137)"); } + /// A run that peaked at `peak` GB, ended with `exit`, and started with a + /// memory need of `need` GB. + fn run_mem(db: &Db, key: &str, peak: f64, exit: i32, starved: Option<&str>, need: f64) -> i64 { + let gb = |x: f64| (x * GB as f64) as u64; + let id = db.insert_run(&NewRun { ns: "n", key, need_mem_kb: gb(need), ..Default::default() }).unwrap(); + db.mark_started(id, now() - 10.0, 0.0, None).unwrap(); + let r = RunResult { + ended_at: now(), + exit, + peak_mem_kb: gb(peak), + cores_wanted: 1.0, + measured: true, + starved: starved.map(str::to_string), + ..Default::default() + }; + db.finish_run(id, &r).unwrap(); + id + } + + fn mem_gb(l: &Learned) -> f64 { + (l.mem_kb.unwrap() as f64 / GB as f64 * 10.0).round() / 10.0 + } + + #[test] + fn a_failed_run_never_raises_the_need_above_the_runs_that_passed() { + // root:tsc_-p_tsconfig.json: two small runs, then one that failed at + // 14.5 GB starved of memory. With fewer than three runs there is no + // outlier check, so it set the need to 14.5 GB, 18 GB with the boost. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + run_mem(&db, "k", 0.2, 0, None, 0.0); + run_mem(&db, "k", 0.3, 2, None, 0.2); + let big = run_mem(&db, "k", 14.5, 1, Some("memory"), 0.3); + let l = db.learned("k", &Learn::default()).unwrap(); + assert_eq!(mem_gb(&l), 0.2, "only the run that passed counts"); + assert!(!l.mem_boosted, "it went to 48x its need and failed: more room would not have saved it"); + let ids: Vec = l.failed.iter().map(|o| o.run_id).collect(); + assert_eq!(ids, vec![big], "the failed 0.3 GB run is too close to the need to list"); + assert_eq!(l.failed[0].why_ignored(), "ignored: the run failed (exit 1)"); + + // host/dapi tsc: peaks of 1-4.5 GB, then one that failed at 5.9 GB + // starved of memory, only 1.3x its need of 4.5 GB. It is no ratio + // outlier, so it counted in full and the need went to 7.4 GB. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + for (peak, exit) in [(4.5, 0), (1.0, 0), (4.2, 0), (2.8, 0), (4.3, 0), (3.5, 0), (3.7, 0), (3.8, 2), (2.4, 0)] { + run_mem(&db, "k", peak, exit, None, 4.5); + } + run_mem(&db, "k", 5.9, 2, Some("memory"), 4.5); + let l = db.learned("k", &Learn::default()).unwrap(); + assert_eq!(mem_gb(&l), 4.5); + assert!(!l.mem_boosted, "it went past what the boost would have given"); + assert_eq!(l.failed.len(), 1, "only the 5.9 GB run: the failed 3.8 GB run is under the need"); + assert_eq!(l.failed[0].peak_kb, (5.9 * GB as f64) as u64); + } + + #[test] + fn when_every_run_failed_one_high_peak_alone_does_not_set_the_need() { + // prd14394-gate: 0.2 GB, then 12.3 GB, both failed. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + run_mem(&db, "k", 0.2, 1, None, 16.0); + run_mem(&db, "k", 12.3, 1, None, 16.0); + let rules = Learn::default(); + let l = db.learned("k", &rules).unwrap(); + assert_eq!(l.mem_kb, Some(rules.new_job_mem_kb), "as little known as a new job"); + assert_eq!(l.failed.len(), 1); + + // One failed run alone is no better. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + run_mem(&db, "k", 14.5, 1, None, 0.0); + assert_eq!(db.learned("k", &rules).unwrap().mem_kb, Some(rules.new_job_mem_kb)); + + // Under new_job_mem, the failed peak is all it reserves. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + run_mem(&db, "k", 0.5, 1, None, 0.0); + assert_eq!(mem_gb(&db.learned("k", &rules).unwrap()), 0.5); + + // A job that fails at the same peak again and again does need it. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + run_mem(&db, "k", 12.0, 1, None, 0.0); + run_mem(&db, "k", 12.3, 1, None, 0.0); + let l = db.learned("k", &rules).unwrap(); + assert_eq!(mem_gb(&l), 12.0); + assert!(l.failed.is_empty(), "12.3 GB is too close to the need to list"); + } + + #[test] + fn a_failed_run_starved_at_its_need_still_raises_it() { + // Killed for memory at its need: the need was too small. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + for _ in 0..3 { + run_mem(&db, "k", 4.0, 0, None, 4.5); + } + run_mem(&db, "k", 4.4, 137, Some("memory"), 4.5); + assert!(db.learned("k", &Learn::default()).unwrap().mem_boosted); + // Up to what the boost gives still counts. + run_mem(&db, "k", 5.6, 137, Some("memory"), 4.5); + assert!(db.learned("k", &Learn::default()).unwrap().mem_boosted); + } + + #[test] + fn a_one_off_peak_under_twice_the_usual_is_held_back() { + // 17 GB where the job usually takes 9 GB: 1.9x. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + for _ in 0..4 { + run_mem(&db, "k", 9.0, 0, None, 9.0); + } + run_mem(&db, "k", 17.0, 0, None, 9.0); + let l = db.learned("k", &Learn::default()).unwrap(); + assert_eq!(mem_gb(&l), 9.0); + assert_eq!(l.outliers.as_ref().map(|o| o.why(&o.runs[0])).as_deref(), Some("ignored: seen once, maybe a one-off")); + } + + #[test] + fn a_job_with_two_sizes_reserves_the_big_one() { + // tsc-queue:packages_dapi_server: 2 GB on a cache hit, 25 GB on a + // full build. Holding the 25 GB back would start a full build into a + // machine with no room for it. + let tmp = tempfile::tempdir().unwrap(); + let db = Db::open_dir(tmp.path()).unwrap(); + for peak in [24.0, 2.0, 1.7, 1.8, 25.0, 25.0, 25.0, 25.0, 1.9, 2.1] { + run_mem(&db, "k", peak, 0, None, 25.0); + } + let l = db.learned("k", &Learn::default()).unwrap(); + assert_eq!(mem_gb(&l), 25.0); + assert_eq!(l.outliers, None); + } + #[test] fn small_jobs_and_steady_steps_have_no_outliers() { let rules = Learn::default(); diff --git a/src/help.rs b/src/help.rs index f469e89..f4236ed 100644 --- a/src/help.rs +++ b/src/help.rs @@ -124,7 +124,8 @@ A peak is an outlier when it is more than R times the next peak below it, and at least SIZE above it. Jobs whose usual peak is under SIZE have no outliers: there a jump is mostly a full build after cache hits. Outliers count by outlier_weights: the first time not at all, the second time half, the third -time in full. A run that failed (exit not 0) never counts. +time in full. A run that failed (exit not 0) never counts above the runs that +passed, outlier or not: it shows here as ignored. arguments: PATTERN the keys to look at, as `taskguard history` shows them; @@ -132,8 +133,8 @@ arguments: shell leaves the * alone. Default: every key. options: - --ratio R how many times the next peak (default outlier_ratio, 2). - 1.5 finds more outliers, 3 fewer. + --ratio R how many times the next peak (default outlier_ratio, 1.5). + 1.2 finds more outliers, 2 fewer. --min SIZE how far above the next peak (default outlier_min, 1G). columns: @@ -145,7 +146,7 @@ columns: examples: taskguard outliers - taskguard outliers 'root:*' --ratio 1.5 + taskguard outliers 'root:*' --ratio 1.2 "; const PRUNE: &str = "\ diff --git a/src/runner.rs b/src/runner.rs index 30a196b..58df647 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -900,21 +900,23 @@ pub fn run(mut o: Opts) -> Result { Ok(code) } -/// When the run that just ended stands far above the job's other runs, say -/// how much it counts, and record it with the reason. +/// When the run that just ended stands far above the job's other runs, or +/// failed above its need, say how much it counts, and record it with the +/// reason. fn note_outlier(d: &Db, cfg: &Config, me: &Entry) { let Ok(l) = d.learned(&me.key, &cfg.learn()) else { return }; - let Some(o) = &l.outliers else { return }; - let Some(run) = o.runs.iter().find(|r| r.run_id == me.run_id) else { return }; - let why = o.why(run); - let _ = d.adjustment(&me.key, me.run_id, "mem_outlier", Some(o.normal_kb as f64), Some(o.mem_kb as f64), &why); + let next = l.mem_kb.unwrap_or(0); + let found = l.outliers.as_ref().and_then(|o| o.runs.iter().find(|r| r.run_id == me.run_id).map(|r| (r, o.normal_kb, o.why(r)))); + let found = found.or_else(|| l.failed.iter().find(|r| r.run_id == me.run_id).map(|r| (r, next, r.why_ignored()))); + let Some((run, usual, why)) = found else { return }; + let _ = d.adjustment(&me.key, me.run_id, "mem_outlier", Some(usual as f64), Some(next as f64), &why); say(&format!( "warning {} - took {} of memory, {:.1}x its usual {}; {why}. The next run reserves {} (taskguard outliers; taskguard prune --outliers)", me.key, report::gb(run.peak_kb), - run.peak_kb as f64 / o.normal_kb.max(1) as f64, - report::gb(o.normal_kb), - report::gb(o.mem_kb) + run.peak_kb as f64 / usual.max(1) as f64, + report::gb(usual), + report::gb(next) )); } diff --git a/taskguard.example.toml b/taskguard.example.toml index 708c597..669aa7f 100644 --- a/taskguard.example.toml +++ b/taskguard.example.toml @@ -75,9 +75,10 @@ boost_runs = 5 # runs that get +25% memory after a memory-starved run # A peak more than outlier_ratio times the next peak below it, and outlier_min # above it, is an outlier. Outliers count by outlier_weights: the first entry # when there is one, the second when there are two, in full past the list. A -# run that failed never counts. Jobs that usually take under outlier_min have -# no outliers. See them with taskguard outliers. 0 turns it off. -outlier_ratio = 2 +# run that failed never counts above the runs that passed. Jobs that usually +# take under outlier_min have no outliers. See them with taskguard outliers. +# 0 turns it off. +outlier_ratio = 1.5 outlier_min = "1G" outlier_weights = [0, 0.5] # A job that falls short by this little starts anyway, with a warning.