diff --git a/Cargo.lock b/Cargo.lock index a4257c74..41f38fa9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2916,7 +2916,7 @@ dependencies = [ [[package]] name = "kache" -version = "0.12.0" +version = "0.13.0" dependencies = [ "anyhow", "assert_cmd", @@ -2964,7 +2964,7 @@ dependencies = [ [[package]] name = "kache-core" -version = "0.12.0" +version = "0.13.0" dependencies = [ "anyhow", "async-trait", @@ -2976,7 +2976,7 @@ dependencies = [ [[package]] name = "kache-e2e" -version = "0.12.0" +version = "0.13.0" dependencies = [ "anyhow", "chrono", @@ -2990,7 +2990,7 @@ dependencies = [ [[package]] name = "kache-service" -version = "0.12.0" +version = "0.13.0" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index b005ecd7..a202f0db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kache" -version = "0.12.0" +version = "0.13.0" edition = "2024" description = "Zero-copy, content-addressed build cache for Rust, C/C++ and more, with S3 and shared-filesystem remotes." license = "Apache-2.0" @@ -36,7 +36,7 @@ reqsign-aws-v4 = { version = "=3.0.2", default-features = false } reqsign-core = { version = "=3.1.0", default-features = false } # Direct dep so OpenDAL/reqwest can use ring without reintroducing aws-lc. rustls = { version = "0.23", default-features = false, features = ["ring"] } -kache-core = { version = "0.12.0", path = "crates/kache-core", default-features = false, features = ["planning"] } +kache-core = { version = "0.13.0", path = "crates/kache-core", default-features = false, features = ["planning"] } async-trait = "0.1" tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros", "process", "fs", "io-util", "net", "time", "signal", "sync"] } serde = { version = "1", features = ["derive"] } diff --git a/crates/kache-core/Cargo.toml b/crates/kache-core/Cargo.toml index 1d3af67d..243dd49c 100644 --- a/crates/kache-core/Cargo.toml +++ b/crates/kache-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kache-core" -version = "0.12.0" +version = "0.13.0" edition = "2024" description = "Core planner data structures and algorithms for kache." license = "Apache-2.0" diff --git a/crates/kache-core/src/lib.rs b/crates/kache-core/src/lib.rs index 23d5a629..a1cfcdba 100644 --- a/crates/kache-core/src/lib.rs +++ b/crates/kache-core/src/lib.rs @@ -17,10 +17,79 @@ pub struct BuildIntent { pub cargo_lock_deps: Vec<(String, String)>, } +/// Which source produced a candidate, i.e. how much to trust it +/// (kunobi-ninja/kache#617). +/// +/// `Unknown` is the `#[serde(other)]` arm so a newer planner naming a source +/// this build has never heard of degrades to "untrusted" instead of failing +/// the whole plan. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(rename_all = "snake_case")] +pub enum CandidateSource { + /// Exact lockfile-shard match: this build's dependency set produced it. + Shard, + /// This machine built this crate before. + History, + /// A crate NAME matched something in the remote listing. A crate name is + /// not a build identity, so this is a guess. + KeyCache, + #[default] + #[serde(other)] + Unknown, +} + +impl CandidateSource { + /// Confidence rank, lower is better. Only the ORDER matters; these are not + /// probabilities and must not be presented as any. + pub fn confidence_rank(self) -> u8 { + match self { + CandidateSource::Shard => 0, + CandidateSource::History => 1, + CandidateSource::KeyCache => 2, + CandidateSource::Unknown => 3, + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct PrefetchCandidate { pub cache_key: String, pub crate_name: String, + /// What a miss would cost to rebuild. `None` = unknown, which is NOT the + /// same as zero: an un-backfilled store row reads 0, and treating that as + /// "free to fetch and worthless to have" would bury it (#617). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub compile_time_ms: Option, + /// Stored artifact size. An admission and ranking ESTIMATE, never a + /// promise about compressed transfer bytes: anything enforced has to be + /// counted on the wire. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub size_bytes: Option, + #[serde(default)] + pub source: CandidateSource, + /// Position in the build's dependency order, i.e. roughly when the build + /// will ask for it. `None` = not in the intent's crate list. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub demand_index: Option, +} + +impl PrefetchCandidate { + /// A candidate with no metadata, as the pre-#617 wire produced. + pub fn new(cache_key: String, crate_name: String) -> Self { + Self { + cache_key, + crate_name, + compile_time_ms: None, + size_bytes: None, + source: CandidateSource::Unknown, + demand_index: None, + } + } + + pub fn with_source(mut self, source: CandidateSource) -> Self { + self.source = source; + self + } } #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] @@ -42,6 +111,116 @@ pub struct PrefetchPlan { pub candidates: Vec, } +/// How many candidates each source may contribute to one plan +/// (kunobi-ninja/kache#616). +/// +/// These bound plan COMPOSITION, which is a different job from the daemon's +/// key/byte/time budgets: those bound resource use and are the trust boundary, +/// these stop one low-confidence source from crowding out better candidates +/// before the budget is even reached. `0` disables a cap. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PlanLimits { + /// Key-cache variants per crate. A crate name is not a build identity, so + /// of `n` variants at most one can be the right one; taking many is paying + /// `n` downloads for at most one hit. + pub key_cache_per_crate: usize, + /// Key-cache candidates across the whole plan, so a build with hundreds of + /// unresolved crates cannot fill the plan with guesses. + pub key_cache_total: usize, + /// History entries per crate. Unlike shards, where one crate legitimately + /// has several compile units, extra history rows for one crate are older + /// variants. + pub history_per_crate: usize, +} + +impl Default for PlanLimits { + fn default() -> Self { + Self { + key_cache_per_crate: 2, + key_cache_total: 64, + history_per_crate: 2, + } + } +} + +/// Urgency bucket width, in dependency-order positions (#617). +/// +/// Demand order is bucketed rather than used exactly because it comes from a +/// guppy graph traversal, which only approximates when cargo will actually ask +/// (cargo reorders for parallelism, build scripts, proc macros, features). +/// Treating position 40 and 45 as meaningfully different is false precision; +/// 40 versus 400 is real. Roughly the prefetch concurrency, so one window is +/// about one wave of downloads. +#[cfg(feature = "planning")] +const URGENCY_BUCKET: u32 = 16; + +/// Sort key for dispatch order, lowest first (#617). +/// +/// Lexicographic, deliberately, rather than a weighted score: a weighted sum +/// needs coefficients, and nothing can calibrate them until #618 makes +/// "arrived before it was demanded" measurable. Every element here is an +/// ordering, not a magnitude. +/// +/// 1. Urgency bucket. Prefetch races the build, so a high-value artifact +/// needed at minute eight loses to a medium-value one needed at second +/// five. Candidates with no demand index sort last. +/// 2. Confidence. Within one wave, prefer the source most likely to be right. +/// 3. Value, descending. Expensive rebuilds first, so the limited slots buy +/// the most avoided work. Unknown cost sorts after known cost rather than +/// being scored as zero. +/// +/// Callers must apply this as a STABLE sort: equal keys keep source order, +/// which is the planner's confidence-merge order. +#[cfg(feature = "planning")] +pub fn dispatch_sort_key(candidate: &PrefetchCandidate) -> (u32, u8, std::cmp::Reverse) { + let bucket = candidate + .demand_index + .map(|index| index / URGENCY_BUCKET) + .unwrap_or(u32::MAX); + ( + bucket, + candidate.source.confidence_rank(), + // `None` -> 0 -> sorts last under Reverse, without claiming the + // candidate is worthless. + std::cmp::Reverse(candidate.compile_time_ms.unwrap_or(0)), + ) +} + +/// Truncate `items` to `limit`, returning how many were dropped. `0` disables. +/// +/// Shared by every composition cap so "0 means unlimited" is defined once +/// rather than re-derived at each call site. +#[cfg(feature = "planning")] +fn cap_to(items: &mut Vec, limit: usize) -> usize { + if limit == 0 || items.len() <= limit { + return 0; + } + let dropped = items.len() - limit; + items.truncate(limit); + dropped +} + +/// What a plan left out, so a truncated plan is distinguishable from one that +/// had nothing more to offer (#616). +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct PlanComposition { + pub from_shards: usize, + pub from_history: usize, + pub from_key_cache: usize, + /// Dropped by [`PlanLimits`], by source. + pub dropped_history_per_crate: usize, + pub dropped_key_cache_per_crate: usize, + pub dropped_key_cache_total: usize, +} + +impl PlanComposition { + pub fn dropped_total(&self) -> usize { + self.dropped_history_per_crate + + self.dropped_key_cache_per_crate + + self.dropped_key_cache_total + } +} + #[cfg(feature = "planning")] #[async_trait] pub trait PlannerDataSource { @@ -62,13 +241,32 @@ pub async fn build_prefetch_plan( intent: &BuildIntent, planner_name: &str, ) -> Result +where + T: PlannerDataSource + Sync + ?Sized, +{ + build_prefetch_plan_with_limits(source, intent, planner_name, PlanLimits::default()) + .await + .map(|(plan, _composition)| plan) +} + +/// [`build_prefetch_plan`] with explicit composition limits, also returning +/// what the limits dropped so a caller can report it (#616). +#[cfg(feature = "planning")] +pub async fn build_prefetch_plan_with_limits( + source: &T, + intent: &BuildIntent, + planner_name: &str, + limits: PlanLimits, +) -> Result<(PrefetchPlan, PlanComposition)> where T: PlannerDataSource + Sync + ?Sized, { let crate_order = crate_query_order(intent); + let demand_index = demand_index_map(&crate_order); let mut seen = HashSet::new(); let mut resolved_crates = HashSet::new(); let mut candidates = Vec::new(); + let mut composition = PlanComposition::default(); // Sources are merged in descending order of confidence, each one filling // only the crates the ones before it left unresolved. Shard lookups used @@ -88,7 +286,7 @@ where for candidate in order_candidates_by_crate_order(shard_candidates, intent) { resolved_crates.insert(candidate.crate_name.clone()); if seen.insert(candidate.cache_key.clone()) { - candidates.push(candidate); + candidates.push(candidate.with_source(CandidateSource::Shard)); } } } @@ -102,31 +300,93 @@ where .collect() }; + composition.from_shards = candidates.len(); + let history_query = unresolved(&resolved_crates); if !history_query.is_empty() { + // Group by crate so the per-crate cap keeps the FIRST entries, which + // both data sources return most-recently-used first. + let mut by_crate: HashMap> = HashMap::new(); for candidate in order_candidates_by_crate_order( source.history_candidates(&history_query).await?, intent, ) { - resolved_crates.insert(candidate.crate_name.clone()); - if seen.insert(candidate.cache_key.clone()) { - candidates.push(candidate); + by_crate + .entry(candidate.crate_name.clone()) + .or_default() + .push(candidate.with_source(CandidateSource::History)); + } + + for crate_name in &history_query { + let Some(mut for_crate) = by_crate.remove(crate_name) else { + continue; + }; + composition.dropped_history_per_crate += + cap_to(&mut for_crate, limits.history_per_crate); + for candidate in for_crate { + resolved_crates.insert(candidate.crate_name.clone()); + if seen.insert(candidate.cache_key.clone()) { + composition.from_history += 1; + candidates.push(candidate); + } } } } + // The key cache is the weakest source: it maps a crate NAME to every cache + // key in the bucket, with no target, toolchain, profile, or feature + // dimension, so of `n` variants at most one can be right. Capping is not a + // fix for that (dimensioning the remote layout is, separately) but it stops + // the guesses crowding out better candidates (#616). for crate_name in unresolved(&resolved_crates) { - for cache_key in source.key_cache_keys_for_crate(&crate_name).await? { - if seen.insert(cache_key.clone()) { - candidates.push(PrefetchCandidate { - cache_key, - crate_name: crate_name.clone(), - }); + if limits.key_cache_total > 0 && composition.from_key_cache >= limits.key_cache_total { + // Whatever this crate would have offered is dropped wholesale; count + // it so the truncation is visible rather than inferred. + composition.dropped_key_cache_total += source + .key_cache_keys_for_crate(&crate_name) + .await? + .into_iter() + .filter(|key| !seen.contains(key)) + .count(); + continue; + } + + let mut for_crate: Vec = source + .key_cache_keys_for_crate(&crate_name) + .await? + .into_iter() + .map(|cache_key| { + PrefetchCandidate::new(cache_key, crate_name.clone()) + .with_source(CandidateSource::KeyCache) + }) + .collect(); + + composition.dropped_key_cache_per_crate += + cap_to(&mut for_crate, limits.key_cache_per_crate); + + for candidate in for_crate { + if limits.key_cache_total > 0 && composition.from_key_cache >= limits.key_cache_total { + composition.dropped_key_cache_total += 1; + continue; + } + if seen.insert(candidate.cache_key.clone()) { + composition.from_key_cache += 1; + candidates.push(candidate); } } } - Ok(execute_plan(planner_name, candidates)) + // Stamp demand position so the daemon can rank without the intent: it only + // receives the plan, and `PrefetchRequest::from_plan` drops everything else. + for candidate in &mut candidates { + candidate.demand_index = demand_index.get(&candidate.crate_name).copied(); + } + + // Dispatch order (#617). Stable, so equal keys keep the confidence-merge + // order the sources were appended in. + candidates.sort_by_key(dispatch_sort_key); + + Ok((execute_plan(planner_name, candidates), composition)) } #[cfg(feature = "planning")] @@ -144,6 +404,16 @@ fn execute_plan(planner_name: &str, candidates: Vec) -> Prefe } } +/// crate name -> position in dependency order, for [`dispatch_sort_key`]. +#[cfg(feature = "planning")] +fn demand_index_map(crate_order: &[String]) -> HashMap { + crate_order + .iter() + .enumerate() + .map(|(index, name)| (name.clone(), index as u32)) + .collect() +} + #[cfg(feature = "planning")] fn crate_query_order(intent: &BuildIntent) -> Vec { let mut seen = HashSet::new(); @@ -234,12 +504,9 @@ mod tests { Ok(crate_names .iter() .filter_map(|crate_name| { - self.history_by_crate - .get(crate_name) - .map(|cache_key| PrefetchCandidate { - cache_key: cache_key.clone(), - crate_name: crate_name.clone(), - }) + self.history_by_crate.get(crate_name).map(|cache_key| { + PrefetchCandidate::new(cache_key.clone(), crate_name.clone()) + }) }) .collect()) } @@ -276,10 +543,7 @@ mod tests { plan_id: Some("plan-1".into()), planner: Some("local".into()), disposition: PrefetchDisposition::Execute, - candidates: vec![PrefetchCandidate { - cache_key: "abc".into(), - crate_name: "serde".into(), - }], + candidates: vec![PrefetchCandidate::new("abc".into(), "serde".into())], }; let json = serde_json::to_string(&plan).unwrap(); @@ -314,10 +578,7 @@ mod tests { #[tokio::test] async fn test_build_prefetch_plan_prefers_shard_candidates() { let source = FakePlannerDataSource { - shard_candidates: vec![PrefetchCandidate { - cache_key: "from-shard".into(), - crate_name: "serde".into(), - }], + shard_candidates: vec![PrefetchCandidate::new("from-shard".into(), "serde".into())], ..Default::default() }; let intent = BuildIntent { @@ -341,10 +602,7 @@ mod tests { async fn test_build_prefetch_plan_falls_back_to_history_and_key_cache() { let mut source = FakePlannerDataSource { shard_error: true, - history_candidates: vec![PrefetchCandidate { - cache_key: "history-key".into(), - crate_name: "serde".into(), - }], + history_candidates: vec![PrefetchCandidate::new("history-key".into(), "serde".into())], ..Default::default() }; source.key_cache.insert( @@ -373,18 +631,9 @@ mod tests { async fn test_build_prefetch_plan_orders_shard_candidates_by_crate_order() { let source = FakePlannerDataSource { shard_candidates: vec![ - PrefetchCandidate { - cache_key: "app-key".into(), - crate_name: "app".into(), - }, - PrefetchCandidate { - cache_key: "dep-key".into(), - crate_name: "dep".into(), - }, - PrefetchCandidate { - cache_key: "middle-key".into(), - crate_name: "middle".into(), - }, + PrefetchCandidate::new("app-key".into(), "app".into()), + PrefetchCandidate::new("dep-key".into(), "dep".into()), + PrefetchCandidate::new("middle-key".into(), "middle".into()), ], ..Default::default() }; @@ -419,10 +668,7 @@ mod tests { async fn test_build_prefetch_plan_fills_crates_a_partial_shard_hit_missed() { let mut source = FakePlannerDataSource { // Only `dep` is in a bucket that still matches. - shard_candidates: vec![PrefetchCandidate { - cache_key: "dep-shard-key".into(), - crate_name: "dep".into(), - }], + shard_candidates: vec![PrefetchCandidate::new("dep-shard-key".into(), "dep".into())], history_by_crate: HashMap::from([("middle".into(), "middle-history-key".into())]), ..Default::default() }; @@ -459,10 +705,10 @@ mod tests { #[tokio::test] async fn test_build_prefetch_plan_does_not_requery_shard_resolved_crates() { let mut source = FakePlannerDataSource { - shard_candidates: vec![PrefetchCandidate { - cache_key: "serde-shard-key".into(), - crate_name: "serde".into(), - }], + shard_candidates: vec![PrefetchCandidate::new( + "serde-shard-key".into(), + "serde".into(), + )], history_by_crate: HashMap::from([("serde".into(), "serde-stale-history-key".into())]), ..Default::default() }; @@ -573,9 +819,8 @@ mod tests { let candidates = candidate_names .into_iter() .enumerate() - .map(|(index, crate_name)| PrefetchCandidate { - cache_key: format!("key-{index}"), - crate_name, + .map(|(index, crate_name)| { + PrefetchCandidate::new(format!("key-{index}"), crate_name) }) .collect::>(); @@ -593,4 +838,434 @@ mod tests { ); } } + // ── Composition caps and ranking (#616, #617) ──────────────────────── + + #[cfg(feature = "planning")] + fn candidate( + key: &str, + crate_name: &str, + source: CandidateSource, + compile_time_ms: Option, + demand_index: Option, + ) -> PrefetchCandidate { + PrefetchCandidate { + cache_key: key.into(), + crate_name: crate_name.into(), + compile_time_ms, + size_bytes: None, + source, + demand_index, + } + } + + /// Confidence ordering is the whole point of the rank; assert the ORDER, + /// not the literal numbers. + #[test] + fn test_confidence_rank_orders_sources() { + assert!( + CandidateSource::Shard.confidence_rank() < CandidateSource::History.confidence_rank() + ); + assert!( + CandidateSource::History.confidence_rank() + < CandidateSource::KeyCache.confidence_rank() + ); + assert!( + CandidateSource::KeyCache.confidence_rank() + < CandidateSource::Unknown.confidence_rank() + ); + } + + /// An unrecognised source name degrades to `Unknown` instead of failing the + /// whole plan (forward compatibility with a newer planner). + #[test] + fn test_unknown_candidate_source_deserializes() { + let candidate: PrefetchCandidate = + serde_json::from_str(r#"{"cache_key":"k","crate_name":"c","source":"telepathy"}"#) + .unwrap(); + assert_eq!(candidate.source, CandidateSource::Unknown); + } + + /// A pre-#617 candidate has no metadata and must not be rejected, and the + /// missing fields must read as unknown rather than zero. + #[test] + fn test_legacy_candidate_wire_still_parses() { + let candidate: PrefetchCandidate = + serde_json::from_str(r#"{"cache_key":"k","crate_name":"c"}"#).unwrap(); + assert_eq!(candidate.compile_time_ms, None); + assert_eq!(candidate.size_bytes, None); + assert_eq!(candidate.demand_index, None); + assert_eq!(candidate.source, CandidateSource::Unknown); + } + + /// Urgency dominates value: prefetch races the build, so a costly artifact + /// needed late loses to a cheaper one needed now (#617). + #[cfg(feature = "planning")] + #[test] + fn test_dispatch_order_prefers_urgency_over_value() { + let urgent_cheap = candidate("a", "a", CandidateSource::Shard, Some(10), Some(0)); + let late_expensive = candidate("b", "b", CandidateSource::Shard, Some(100_000), Some(500)); + assert!(dispatch_sort_key(&urgent_cheap) < dispatch_sort_key(&late_expensive)); + } + + /// Within one urgency window, value decides. + #[cfg(feature = "planning")] + #[test] + fn test_dispatch_order_prefers_value_within_a_bucket() { + let cheap = candidate("a", "a", CandidateSource::Shard, Some(10), Some(0)); + let costly = candidate("b", "b", CandidateSource::Shard, Some(5_000), Some(1)); + assert!( + dispatch_sort_key(&costly) < dispatch_sort_key(&cheap), + "same bucket, same source: the expensive rebuild goes first" + ); + } + + /// Positions inside one bucket are treated as equally urgent: guppy order + /// only approximates demand time, so finer distinctions are false precision. + #[cfg(feature = "planning")] + #[test] + fn test_dispatch_order_buckets_nearby_positions_together() { + let first = candidate("a", "a", CandidateSource::Shard, Some(10), Some(0)); + let nearby = candidate("b", "b", CandidateSource::Shard, Some(10), Some(15)); + let next_bucket = candidate("c", "c", CandidateSource::Shard, Some(10), Some(16)); + assert_eq!( + dispatch_sort_key(&first), + dispatch_sort_key(&nearby), + "positions 0 and 15 share a bucket" + ); + assert!( + dispatch_sort_key(&nearby) < dispatch_sort_key(&next_bucket), + "position 16 starts the next bucket" + ); + } + + /// Within a bucket, confidence beats value: a shard match is worth more + /// than a bigger number from a guess. + #[cfg(feature = "planning")] + #[test] + fn test_dispatch_order_prefers_confidence_within_a_bucket() { + let shard_cheap = candidate("a", "a", CandidateSource::Shard, Some(1), Some(0)); + let guess_costly = candidate("b", "b", CandidateSource::KeyCache, Some(99_999), Some(0)); + assert!(dispatch_sort_key(&shard_cheap) < dispatch_sort_key(&guess_costly)); + } + + /// Unknown cost sorts after known cost, but is not dropped and is not + /// treated as zero-value against a *different* bucket. + #[cfg(feature = "planning")] + #[test] + fn test_dispatch_order_places_unknown_cost_last_within_its_bucket() { + let known = candidate("a", "a", CandidateSource::Shard, Some(1), Some(0)); + let unknown = candidate("b", "b", CandidateSource::Shard, None, Some(0)); + assert!(dispatch_sort_key(&known) < dispatch_sort_key(&unknown)); + + // ...but an unknown-cost candidate needed NOW still beats a known-cost + // one needed much later. + let late_known = candidate("c", "c", CandidateSource::Shard, Some(50_000), Some(999)); + assert!(dispatch_sort_key(&unknown) < dispatch_sort_key(&late_known)); + } + + /// A candidate outside the intent's crate list sorts last rather than first. + #[cfg(feature = "planning")] + #[test] + fn test_dispatch_order_places_unknown_demand_last() { + let known = candidate("a", "a", CandidateSource::Shard, Some(1), Some(100_000)); + let no_demand = candidate("b", "b", CandidateSource::Shard, Some(50_000), None); + assert!(dispatch_sort_key(&known) < dispatch_sort_key(&no_demand)); + } + + /// The per-crate key-cache cap keeps the first N and reports the rest. + #[cfg(feature = "planning")] + #[tokio::test] + async fn test_key_cache_per_crate_cap() { + let mut source = FakePlannerDataSource::default(); + source.key_cache.insert( + "serde".into(), + vec!["k1".into(), "k2".into(), "k3".into(), "k4".into()], + ); + let intent = BuildIntent { + crate_names: vec!["serde".into()], + ..Default::default() + }; + + let (plan, composition) = build_prefetch_plan_with_limits( + &source, + &intent, + "fallback", + PlanLimits { + key_cache_per_crate: 2, + ..PlanLimits::default() + }, + ) + .await + .unwrap(); + + assert_eq!(plan.candidates.len(), 2, "capped to two variants"); + assert_eq!(composition.from_key_cache, 2); + assert_eq!(composition.dropped_key_cache_per_crate, 2); + assert!(composition.dropped_total() > 0, "truncation is visible"); + } + + /// The plan-wide key-cache cap stops one weak source filling the plan, and + /// counts what whole crates it skipped. + #[cfg(feature = "planning")] + #[tokio::test] + async fn test_key_cache_total_cap_counts_skipped_crates() { + let mut source = FakePlannerDataSource::default(); + for name in ["a", "b", "c"] { + source + .key_cache + .insert(name.into(), vec![format!("{name}-k1")]); + } + let intent = BuildIntent { + crate_names: vec!["a".into(), "b".into(), "c".into()], + ..Default::default() + }; + + let (plan, composition) = build_prefetch_plan_with_limits( + &source, + &intent, + "fallback", + PlanLimits { + key_cache_total: 1, + ..PlanLimits::default() + }, + ) + .await + .unwrap(); + + assert_eq!(plan.candidates.len(), 1); + assert_eq!(composition.from_key_cache, 1); + assert_eq!( + composition.dropped_key_cache_total, 2, + "the two skipped crates are counted, not silently missing" + ); + } + + /// `0` disables a cap rather than dropping everything. + #[cfg(feature = "planning")] + #[tokio::test] + async fn test_zero_limit_disables_the_cap() { + let mut source = FakePlannerDataSource::default(); + source + .key_cache + .insert("serde".into(), vec!["k1".into(), "k2".into(), "k3".into()]); + let intent = BuildIntent { + crate_names: vec!["serde".into()], + ..Default::default() + }; + + let (plan, composition) = build_prefetch_plan_with_limits( + &source, + &intent, + "fallback", + PlanLimits { + key_cache_per_crate: 0, + key_cache_total: 0, + history_per_crate: 0, + }, + ) + .await + .unwrap(); + + assert_eq!(plan.candidates.len(), 3); + assert_eq!(composition.dropped_total(), 0); + } + + /// Candidates are stamped with their source and demand position, so the + /// daemon can rank without the intent (it only receives the plan). + #[cfg(feature = "planning")] + #[tokio::test] + async fn test_plan_stamps_source_and_demand_index() { + let source = FakePlannerDataSource { + history_by_crate: HashMap::from([ + ("dep".into(), "dep-key".into()), + ("app".into(), "app-key".into()), + ]), + ..Default::default() + }; + let intent = BuildIntent { + crate_names: vec!["dep".into(), "app".into()], + ..Default::default() + }; + + let (plan, _) = + build_prefetch_plan_with_limits(&source, &intent, "fallback", PlanLimits::default()) + .await + .unwrap(); + + let dep = plan + .candidates + .iter() + .find(|c| c.crate_name == "dep") + .expect("dep planned"); + let app = plan + .candidates + .iter() + .find(|c| c.crate_name == "app") + .expect("app planned"); + assert_eq!(dep.source, CandidateSource::History); + assert_eq!(dep.demand_index, Some(0)); + assert_eq!(app.demand_index, Some(1), "position follows crate order"); + } + + /// `cap_to` computes the DROPPED count, not a ratio. 5 items capped to 2 + /// drops 3; a divide would say 2. (Mutation-driven: `-` vs `/` agree on + /// 4-and-2, so the earlier cap test could not tell them apart.) + #[cfg(feature = "planning")] + #[test] + fn test_cap_to_reports_the_dropped_count() { + let mut items: Vec = (0..5) + .map(|i| PrefetchCandidate::new(format!("k{i}"), "c".into())) + .collect(); + assert_eq!(cap_to(&mut items, 2), 3); + assert_eq!(items.len(), 2); + assert_eq!(items[0].cache_key, "k0", "keeps the FIRST N"); + + let mut exact: Vec = + vec![PrefetchCandidate::new("a".into(), "c".into())]; + assert_eq!(cap_to(&mut exact, 1), 0, "exactly at the limit drops none"); + assert_eq!(cap_to(&mut exact, 0), 0, "0 disables the cap"); + assert_eq!(exact.len(), 1); + } + + /// `dropped_total` sums every drop class. Each field is distinct so a + /// swapped operator or a missing term changes the result. + #[test] + fn test_dropped_total_sums_every_class() { + let composition = PlanComposition { + dropped_history_per_crate: 1, + dropped_key_cache_per_crate: 2, + dropped_key_cache_total: 4, + ..PlanComposition::default() + }; + assert_eq!(composition.dropped_total(), 7); + assert_eq!(PlanComposition::default().dropped_total(), 0); + } + + /// The per-crate history cap keeps the most-recent entries and reports the + /// rest. History is ordered most-recently-used first by both data sources, + /// so the cap keeps the freshest variants. + #[cfg(feature = "planning")] + #[tokio::test] + async fn test_history_per_crate_cap() { + let source = FakePlannerDataSource { + history_candidates: vec![ + PrefetchCandidate::new("serde-newest".into(), "serde".into()), + PrefetchCandidate::new("serde-older".into(), "serde".into()), + PrefetchCandidate::new("serde-oldest".into(), "serde".into()), + ], + ..Default::default() + }; + let intent = BuildIntent { + crate_names: vec!["serde".into()], + ..Default::default() + }; + + let (plan, composition) = build_prefetch_plan_with_limits( + &source, + &intent, + "fallback", + PlanLimits { + history_per_crate: 1, + ..PlanLimits::default() + }, + ) + .await + .unwrap(); + + assert_eq!(composition.from_history, 1); + assert_eq!(composition.dropped_history_per_crate, 2); + assert_eq!( + plan.candidates + .iter() + .map(|c| c.cache_key.as_str()) + .collect::>(), + vec!["serde-newest"] + ); + } + + /// Once the plan-wide key-cache budget is spent, a later crate is skipped + /// WHOLESALE, and everything it would have offered is counted, not just the + /// per-crate-capped subset. + #[cfg(feature = "planning")] + #[tokio::test] + async fn test_key_cache_total_skips_a_whole_crate_and_counts_all_of_it() { + let mut source = FakePlannerDataSource::default(); + source.key_cache.insert("a".into(), vec!["a1".into()]); + source.key_cache.insert( + "b".into(), + vec![ + "b1".into(), + "b2".into(), + "b3".into(), + "b4".into(), + "b5".into(), + ], + ); + let intent = BuildIntent { + crate_names: vec!["a".into(), "b".into()], + ..Default::default() + }; + + let (_plan, composition) = build_prefetch_plan_with_limits( + &source, + &intent, + "fallback", + PlanLimits { + key_cache_per_crate: 2, + key_cache_total: 1, + history_per_crate: 2, + }, + ) + .await + .unwrap(); + + assert_eq!(composition.from_key_cache, 1, "only crate `a` fits"); + assert_eq!( + composition.dropped_key_cache_total, 5, + "all five of `b`'s variants are counted, not the two a per-crate \ + cap would have admitted" + ); + assert_eq!( + composition.dropped_key_cache_per_crate, 0, + "`b` was skipped before the per-crate cap applied" + ); + } + + /// The budget can also run out PART WAY through one crate's variants: the + /// remainder is dropped and counted individually. + #[cfg(feature = "planning")] + #[tokio::test] + async fn test_key_cache_total_can_run_out_mid_crate() { + let mut source = FakePlannerDataSource::default(); + source + .key_cache + .insert("serde".into(), vec!["k1".into(), "k2".into(), "k3".into()]); + let intent = BuildIntent { + crate_names: vec!["serde".into()], + ..Default::default() + }; + + let (_plan, composition) = build_prefetch_plan_with_limits( + &source, + &intent, + "fallback", + PlanLimits { + key_cache_per_crate: 2, + key_cache_total: 1, + history_per_crate: 2, + }, + ) + .await + .unwrap(); + + assert_eq!(composition.from_key_cache, 1); + assert_eq!( + composition.dropped_key_cache_per_crate, 1, + "k3 dropped by the per-crate cap" + ); + assert_eq!( + composition.dropped_key_cache_total, 1, + "k2 dropped by the plan-wide cap, mid-crate" + ); + } } diff --git a/crates/kache-e2e/Cargo.toml b/crates/kache-e2e/Cargo.toml index 840200b7..4e88c779 100644 --- a/crates/kache-e2e/Cargo.toml +++ b/crates/kache-e2e/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kache-e2e" -version = "0.12.0" +version = "0.13.0" edition = "2024" license = "Apache-2.0" repository = "https://github.com/kunobi-ninja/kache" diff --git a/crates/kache-service/Cargo.toml b/crates/kache-service/Cargo.toml index ccc2d052..d6287a3e 100644 --- a/crates/kache-service/Cargo.toml +++ b/crates/kache-service/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kache-service" -version = "0.12.0" +version = "0.13.0" edition = "2024" description = "Remote service shell for kache planner endpoints" license = "Apache-2.0" diff --git a/crates/kache-service/src/lib.rs b/crates/kache-service/src/lib.rs index 82fc413b..9ccb518b 100644 --- a/crates/kache-service/src/lib.rs +++ b/crates/kache-service/src/lib.rs @@ -495,10 +495,10 @@ mod tests { namespaces: HashMap::new(), history: HashMap::from([( "serde".to_string(), - vec![PrefetchCandidate { - cache_key: "serde-key".to_string(), - crate_name: "serde".to_string(), - }], + vec![PrefetchCandidate::new( + "serde-key".to_string(), + "serde".to_string(), + )], )]), key_cache: HashMap::new(), }) diff --git a/crates/kache-service/src/state.rs b/crates/kache-service/src/state.rs index 58cb865f..a88852a3 100644 --- a/crates/kache-service/src/state.rs +++ b/crates/kache-service/src/state.rs @@ -3,7 +3,7 @@ use std::path::Path; use anyhow::{Context, Result}; use async_trait::async_trait; -use kache_core::{PlannerDataSource, PrefetchCandidate}; +use kache_core::{CandidateSource, PlannerDataSource, PrefetchCandidate}; use serde::{Deserialize, Serialize}; use surrealdb::{ Surreal, @@ -86,10 +86,7 @@ impl SurrealPlannerRepository { for cache_key in cache_keys { self.upsert_crate_artifact( &crate_name, - &PrefetchCandidate { - cache_key, - crate_name: crate_name.clone(), - }, + &PrefetchCandidate::new(cache_key, crate_name.clone()), ) .await?; } @@ -224,10 +221,10 @@ ORDER BY last_seen_at DESC; for (cache_key, crate_name) in cache_keys.into_iter().zip(crate_names) { if seen.insert(cache_key.clone()) { - candidates.push(PrefetchCandidate { - cache_key, - crate_name, - }); + candidates.push( + PrefetchCandidate::new(cache_key, crate_name) + .with_source(CandidateSource::Shard), + ); } } } @@ -265,10 +262,10 @@ ORDER BY last_seen_at DESC; for (cache_key, row_crate_name) in cache_keys.into_iter().zip(crate_names) { if seen.insert(cache_key.clone()) { - candidates.push(PrefetchCandidate { - cache_key, - crate_name: row_crate_name, - }); + candidates.push( + PrefetchCandidate::new(cache_key, row_crate_name) + .with_source(CandidateSource::History), + ); } } } @@ -365,17 +362,17 @@ mod tests { deps: HashMap::from([ ( "a@1".to_string(), - vec![PrefetchCandidate { - cache_key: "shared-key".to_string(), - crate_name: "shared".to_string(), - }], + vec![PrefetchCandidate::new( + "shared-key".to_string(), + "shared".to_string(), + )], ), ( "b@1".to_string(), - vec![PrefetchCandidate { - cache_key: "shared-key".to_string(), - crate_name: "shared".to_string(), - }], + vec![PrefetchCandidate::new( + "shared-key".to_string(), + "shared".to_string(), + )], ), ]), }, @@ -452,10 +449,10 @@ mod tests { NamespaceState { deps: HashMap::from([( "serde@1.0.0".to_string(), - vec![PrefetchCandidate { - cache_key: "serde-key".to_string(), - crate_name: "serde".to_string(), - }], + vec![PrefetchCandidate::new( + "serde-key".to_string(), + "serde".to_string(), + )], )]), }, )]), @@ -489,10 +486,10 @@ mod tests { namespaces: HashMap::new(), history: HashMap::from([( "serde".to_string(), - vec![PrefetchCandidate { - cache_key: "serde-key".to_string(), - crate_name: "serde".to_string(), - }], + vec![PrefetchCandidate::new( + "serde-key".to_string(), + "serde".to_string(), + )], )]), key_cache: HashMap::from([("tokio".to_string(), vec!["tokio-key".to_string()])]), }) diff --git a/notes/design/prefetch-budgets-and-ranking.md b/notes/design/prefetch-budgets-and-ranking.md index ba1e4e60..be6da203 100644 --- a/notes/design/prefetch-budgets-and-ranking.md +++ b/notes/design/prefetch-budgets-and-ranking.md @@ -15,14 +15,22 @@ change would be hard to review and hard to revert. It splits cleanly because **enforcement does not need the metadata**, and that is also the half that carries the safety risk: -1. **Daemon hard budgets** (this stage). Keys, bytes, deadline, enforced daemon-side on the - existing request shape. Bounds the pathology today. No wire change. +1. **Daemon hard budgets.** Keys, bytes, deadline, enforced daemon-side on the existing + request shape. Bounds the pathology today. No wire change. *(Shipped: #633.)* 2. **Bound the low-confidence source.** Per-crate and plan-wide caps on key-cache expansion, plus a per-crate cap on local history. 3. **Candidate metadata on the wire.** `compile_time_ms`, size, source, demand index, all `Option`, plus shard writers persisting what `ManifestEntry` already carries. 4. **Cost-aware ranking** using that metadata, behind a versioned policy. +**Stages 2 to 4 were then landed together, revising this split.** The file sets made the +subdivision counterproductive: all three rewrite `build_prefetch_plan`, the +`PlannerDataSource` surface, and `fallback_planner.rs`, so three PRs meant changing the same +functions three times. Worse, stage 3 alone would land wire fields no code reads, and a +reviewer cannot judge whether the fields are right without seeing the ranking that consumes +them. The separation that carried its weight was enforcement (stage 1) from selection; going +finer than that did not. + ## Decisions that hold across the stages ### Unknown must neither win nor lose diff --git a/src/cli.rs b/src/cli.rs index 5ed44d1f..2eaa222c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -3384,10 +3384,13 @@ async fn upload_shards( let deps = crate::shards::parse_cargo_lock(lock_path)?; let shard_set = crate::shards::compute_shards(namespace, &deps); - // Build a lookup from crate_name -> cache_key (keep the first match per crate) - let mut crate_to_key = std::collections::HashMap::<&str, &str>::new(); + // crate_name -> its manifest entry (keep the first match per crate). The + // whole entry, not just the cache key: shards now persist compile cost and + // artifact size so the planner can rank by them (kunobi-ninja/kache#617). + let mut crate_to_entry = + std::collections::HashMap::<&str, &crate::remote::ManifestEntry>::new(); for e in entries { - crate_to_key.entry(&e.crate_name).or_insert(&e.cache_key); + crate_to_entry.entry(&e.crate_name).or_insert(e); } // Build Shard objects, skipping crates that have no build event @@ -3396,11 +3399,13 @@ async fn upload_shards( let shard_entries: Vec = shard_deps .iter() .filter_map(|(name, _version)| { - crate_to_key + crate_to_entry .get(name.as_str()) - .map(|&cache_key| crate::remote::ShardEntry { - cache_key: cache_key.to_string(), + .map(|&entry| crate::remote::ShardEntry { + cache_key: entry.cache_key.clone(), crate_name: name.clone(), + compile_time_ms: Some(entry.compile_time_ms), + artifact_size: Some(entry.artifact_size), }) }) .collect(); diff --git a/src/daemon.rs b/src/daemon.rs index bcd862f8..91259e50 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -8796,20 +8796,11 @@ mod tests { planner: Some("fallback".into()), disposition: PrefetchDisposition::Execute, candidates: vec![ - kache_core::PrefetchCandidate { - cache_key: valid_key.clone(), - crate_name: "serde".into(), - }, + kache_core::PrefetchCandidate::new(valid_key.clone(), "serde".into()), // Malformed key from an untrusted planner: must be dropped. - kache_core::PrefetchCandidate { - cache_key: "../../../etc/passwd".into(), - crate_name: "serde".into(), - }, + kache_core::PrefetchCandidate::new("../../../etc/passwd".into(), "serde".into()), // Valid key but path-escaping crate name: must be dropped. - kache_core::PrefetchCandidate { - cache_key: valid_key.clone(), - crate_name: "../evil".into(), - }, + kache_core::PrefetchCandidate::new(valid_key.clone(), "../evil".into()), ], }; diff --git a/src/fallback_planner.rs b/src/fallback_planner.rs index cd7ab510..d53fdcc7 100644 --- a/src/fallback_planner.rs +++ b/src/fallback_planner.rs @@ -4,10 +4,7 @@ use std::sync::Arc; use anyhow::Result; use async_trait::async_trait; use futures::future::join_all; -use kache_core::{ - BuildIntent, PlannerDataSource, PrefetchCandidate, PrefetchPlan, - build_prefetch_plan as build_core_prefetch_plan, -}; +use kache_core::{BuildIntent, PlannerDataSource, PrefetchCandidate, PrefetchPlan}; use crate::daemon::Daemon; @@ -15,7 +12,41 @@ pub async fn build_prefetch_plan( daemon: &Arc, intent: &BuildIntent, ) -> Result { - build_core_prefetch_plan(&LocalPlannerSource { daemon }, intent, "fallback").await + let (plan, composition) = kache_core::build_prefetch_plan_with_limits( + &LocalPlannerSource { daemon }, + intent, + "fallback", + kache_core::PlanLimits::default(), + ) + .await?; + + // Never silently truncate: a plan trimmed by a composition cap must be + // distinguishable from one that had nothing more to offer (#616). + if should_report_composition(&composition) { + tracing::info!( + candidates = plan.candidates.len(), + from_shards = composition.from_shards, + from_history = composition.from_history, + from_key_cache = composition.from_key_cache, + dropped_history_per_crate = composition.dropped_history_per_crate, + dropped_key_cache_per_crate = composition.dropped_key_cache_per_crate, + dropped_key_cache_total = composition.dropped_key_cache_total, + "fallback planner: composition caps trimmed the plan" + ); + } + + Ok(plan) +} + +/// Report a plan's composition only when a cap actually dropped something +/// (kunobi-ninja/kache#616). +/// +/// A separate predicate rather than an inline condition so the "only when +/// something was dropped" rule is testable: every plan logging its full +/// composition would drown the interesting case, and never logging would make +/// a truncated plan indistinguishable from an exhausted one. +fn should_report_composition(composition: &kache_core::PlanComposition) -> bool { + composition.dropped_total() > 0 } struct LocalPlannerSource<'a> { @@ -60,6 +91,10 @@ impl PlannerDataSource for LocalPlannerSource<'_> { candidates.push(PrefetchCandidate { cache_key: entry.cache_key, crate_name: entry.crate_name, + compile_time_ms: entry.compile_time_ms, + size_bytes: entry.artifact_size, + source: kache_core::CandidateSource::Shard, + demand_index: None, }); } } @@ -84,9 +119,13 @@ impl PlannerDataSource for LocalPlannerSource<'_> { .with_store(|store| store.keys_for_crates(crate_names))?; Ok(entries .into_iter() - .map(|(cache_key, crate_name, _entry_dir)| PrefetchCandidate { - cache_key, - crate_name, + .map(|entry| PrefetchCandidate { + cache_key: entry.cache_key, + crate_name: entry.crate_name, + compile_time_ms: entry.compile_time_ms, + size_bytes: entry.size_bytes, + source: kache_core::CandidateSource::History, + demand_index: None, }) .collect()) } @@ -107,6 +146,31 @@ impl PlannerDataSource for LocalPlannerSource<'_> { #[cfg(test)] mod tests { use super::*; + + /// Composition is reported only when a cap actually dropped something + /// (#616): always logging would drown the interesting case, never logging + /// would hide a truncated plan. + #[test] + fn test_should_report_composition_only_when_something_dropped() { + let mut composition = kache_core::PlanComposition::default(); + assert!( + !should_report_composition(&composition), + "a plan that dropped nothing is not worth reporting" + ); + + composition.from_shards = 40; + composition.from_history = 5; + assert!( + !should_report_composition(&composition), + "candidates admitted is not a reason to report" + ); + + composition.dropped_key_cache_per_crate = 1; + assert!( + should_report_composition(&composition), + "a single drop is enough to report" + ); + } use crate::config::{Config, DEFAULT_DAEMON_IDLE_TIMEOUT_SECS, DEFAULT_S3_POOL_IDLE_SECS}; use crate::store::Store; diff --git a/src/planner_client.rs b/src/planner_client.rs index 6cf0d9b2..16f80830 100644 --- a/src/planner_client.rs +++ b/src/planner_client.rs @@ -149,10 +149,7 @@ mod tests { plan_id: Some("plan-1".into()), planner: Some("test".into()), disposition: PrefetchDisposition::Execute, - candidates: vec![PrefetchCandidate { - cache_key: "abc".into(), - crate_name: "serde".into(), - }], + candidates: vec![PrefetchCandidate::new("abc".into(), "serde".into())], }) .unwrap(); let endpoint = spawn_response_server(body, Some("token-123"), "HTTP/1.1 200 OK").await; diff --git a/src/remote.rs b/src/remote.rs index 47d35bc9..b2568143 100644 --- a/src/remote.rs +++ b/src/remote.rs @@ -71,6 +71,17 @@ pub struct BuildManifest { pub struct ShardEntry { pub cache_key: String, pub crate_name: String, + /// What a miss costs to rebuild, and how big the artifact is + /// (kunobi-ninja/kache#617). Both `Option`: shards written before + /// cost-aware planning carry neither, and a missing value must read as + /// "unknown" rather than "free and worthless". + /// + /// `save-manifest` already has both from `ManifestEntry`; it just was not + /// persisting them into the shard. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub compile_time_ms: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub artifact_size: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -230,10 +241,15 @@ mod tests { ShardEntry { cache_key: "abc123".to_string(), crate_name: "serde".to_string(), + compile_time_ms: Some(4200), + artifact_size: Some(9000), }, + // A pre-#617 shard entry: no cost, no size. ShardEntry { cache_key: "def456".to_string(), crate_name: "syn".to_string(), + compile_time_ms: None, + artifact_size: None, }, ], }; @@ -305,6 +321,8 @@ mod tests { entries: vec![ShardEntry { cache_key: "k1".to_string(), crate_name: "tokio".to_string(), + compile_time_ms: None, + artifact_size: None, }], }; let backend = crate::remote_backend::memory_backend(); diff --git a/src/store.rs b/src/store.rs index dddc4e75..0335a0e2 100644 --- a/src/store.rs +++ b/src/store.rs @@ -1119,6 +1119,26 @@ pub struct RebuildStats { pub blobs_registered: usize, } +/// One prior build of a crate on this machine, from the local store's index +/// (kunobi-ninja/kache#617). Replaces a bare `(key, crate, dir)` tuple so the +/// planner can rank by rebuild cost and size. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CrateHistoryEntry { + pub cache_key: String, + pub crate_name: String, + pub entry_dir: PathBuf, + /// `None` when the index has no value: these columns default to 0 for rows + /// predating their migrations, and a 0 read as a real measurement would + /// rank an un-backfilled entry as worthless. + pub compile_time_ms: Option, + pub size_bytes: Option, +} + +/// A non-positive SQLite column value means "not recorded", not "zero". +fn positive_or_none(value: i64) -> Option { + (value > 0).then_some(value as u64) +} + impl Store { pub fn open(config: &Config) -> Result { fs::create_dir_all(&config.cache_dir) @@ -1909,16 +1929,13 @@ impl Store { } /// Look up cache keys for the given crate names (most recent per crate). - pub fn keys_for_crates( - &self, - crate_names: &[String], - ) -> Result> { + pub fn keys_for_crates(&self, crate_names: &[String]) -> Result> { if crate_names.is_empty() { return Ok(Vec::new()); } let placeholders: Vec<&str> = crate_names.iter().map(|_| "?").collect(); let sql = format!( - "SELECT cache_key, crate_name FROM entries WHERE committed = 1 AND crate_name IN ({}) ORDER BY last_accessed DESC", + "SELECT cache_key, crate_name, compile_time_ms, size FROM entries WHERE committed = 1 AND crate_name IN ({}) ORDER BY last_accessed DESC", placeholders.join(",") ); let mut stmt = self.db.prepare(&sql)?; @@ -1929,13 +1946,24 @@ impl Store { let rows = stmt.query_map(params.as_slice(), |row| { let key: String = row.get(0)?; let cn: String = row.get(1)?; - Ok((key, cn)) + let compile_time_ms: i64 = row.get(2)?; + let size: i64 = row.get(3)?; + Ok((key, cn, compile_time_ms, size)) })?; let mut results = Vec::new(); for row in rows { - let (key, cn) = row?; - let entry_dir = self.entry_dir(&key); - results.push((key, cn, entry_dir)); + let (cache_key, crate_name, compile_time_ms, size) = row?; + let entry_dir = self.entry_dir(&cache_key); + results.push(CrateHistoryEntry { + cache_key, + crate_name, + entry_dir, + // Both columns default to 0 for rows written before their + // migrations, so 0 has to mean "unknown" rather than "free to + // fetch and worthless to have" (kunobi-ninja/kache#617). + compile_time_ms: positive_or_none(compile_time_ms), + size_bytes: positive_or_none(size), + }); } Ok(results) } @@ -2908,6 +2936,25 @@ pub struct EntryInfo { #[cfg(test)] mod tests { + + /// `0` and negatives mean "not recorded", not a measured zero + /// (kunobi-ninja/kache#617). Load-bearing: the `size` and + /// `compile_time_ms` columns default to 0 for rows written before their + /// migrations, and a 0 read as a measurement would rank an un-backfilled + /// entry as free to fetch and worthless to have. + #[test] + fn test_positive_or_none_treats_non_positive_as_unknown() { + assert_eq!(positive_or_none(0), None, "0 is unknown, not Some(0)"); + assert_eq!(positive_or_none(-1), None, "a negative is unknown"); + assert_eq!( + positive_or_none(1), + Some(1), + "the smallest real value survives" + ); + assert_eq!(positive_or_none(4200), Some(4200)); + assert_eq!(positive_or_none(i64::MAX), Some(i64::MAX as u64)); + } + use super::*; use crate::eviction::EvictionPolicy as _; @@ -5733,7 +5780,7 @@ mod tests { let result = store.keys_for_crates(&["serde".to_string()]).unwrap(); assert_eq!(result.len(), 1); - assert_eq!(result[0].1, "serde"); + assert_eq!(result[0].crate_name, "serde"); let result = store .keys_for_crates(&["serde".to_string(), "tokio".to_string()])