diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index 25b5734b..df1a15cf 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -633,34 +633,49 @@ fn two_build_slots_run_two_prebuilds() { } #[test] -fn fifo_fairness_for_the_slot() { +fn rebuild_chain_declares_the_slot_where_the_nix_work_is() { + // Was `fifo_fairness_for_the_slot`, which submitted three rebuilds and + // drove one to completion to watch the freed slot go to the earlier + // waiter. **That fairness guarantee is hive_jobq's**, and it had no test + // there at all — `claim_one` scans nodes in insertion order and takes the + // first satisfiable one, and nothing pinned that. It does now: + // `a_contended_resource_goes_to_the_oldest_waiter`. + // + // What is c0re's is *which* nodes contend for the slot in the first place, + // and that is a declaration. "Uniform hold across the chain" then follows + // from the parent nesting asserted in `rebuild_chain_is_declared_serial`: + // a resource unit is held for the acquirer's whole subtree, so the slot + // `Prebuild` takes covers `StopForUpdate` → `Swap` → `PostSwap` beneath it. let q = JobQueue::new(1); - let a = submit(&q, rebuild("agent-a", "r")); - let b = submit(&q, rebuild("agent-b", "r")); - let c = submit(&q, rebuild("agent-c", "r")); - let first = claim_one(&q); - assert_eq!(first.dag_id, a, "submit order wins the slot"); - q.complete_node(first.node_id, Ok(())); - // Uniform hold: the slot stays with agent-a until its Swap (the last - // slot-needer) completes. Drive a's chain; the moment its slot frees, - // submit order (b before c) wins it. - let mut freed_to = None; - for _ in 0..6 { - let claims = q.claim_ready(); - if let Some(nb) = claims.iter().find(|cl| cl.dag_id == b || cl.dag_id == c) { - freed_to = Some(nb.dag_id); - break; - } - for cl in claims { - if cl.dag_id == a { - q.complete_node(cl.node_id, Ok(())); - } - } - } + let id = submit(&q, rebuild("agent-a", "r")); + let res = |kind: &str| declared_resources(&q, node_of(&q, id, kind)); + + let agent = || Resource::Agent("agent-a".to_owned()); assert_eq!( - freed_to, - Some(b), - "b's prebuild wins the freed slot before c's" + [ + res("meta_sync"), + res("prebuild"), + res("stop_for_update"), + res("swap"), + res("reconcile"), + ], + [ + // The meta preamble takes the global window and *nothing else* — + // no slot (it does no nix work) and no lease. + vec![Resource::MetaWindow], + // The nix build is the slot-needer, and takes **no lease**. That is + // what lets a prebuild overlap another DAG on the same agent: the + // container is still up and untouched while it builds. + vec![Resource::BuildSlot], + // The lease starts here — the first node that touches the + // container — and not one node earlier. + vec![agent()], + // Swap needs both. It re-enters the slot its `Prebuild` ancestor + // holds rather than acquiring a second unit. + vec![Resource::BuildSlot, agent()], + vec![agent()], + ], + "the slot follows the nix work and the lease follows the container" ); } diff --git a/hive-jobq/src/scheduler.rs b/hive-jobq/src/scheduler.rs index d86a9c21..62f9dcab 100644 --- a/hive-jobq/src/scheduler.rs +++ b/hive-jobq/src/scheduler.rs @@ -767,6 +767,33 @@ mod tests { assert_eq!(s.graph().node(n).unwrap().state, State::Failed); } + /// A contended resource goes to the oldest waiter. + /// + /// [`Scheduler::claim_one`] scans [`Graph::nodes`] — insertion order — and + /// takes the first node whose deps are satisfied and whose resources it can + /// acquire. That *is* the fairness guarantee: there is no queue, no + /// priority, just the scan order. + /// + /// Load-bearing for any host that submits work over time, because without + /// it a steady arrival rate could starve the earliest waiter indefinitely. + /// It was previously only covered downstream, by a host test driving its own + /// templates — which meant the property this crate provides was asserted + /// everywhere except in this crate. + #[test] + fn a_contended_resource_goes_to_the_oldest_waiter() { + let mut s = scheduler_with_slots(1); + let a = s.append("a", res_dep("build-slot"), None).expect("a"); + let b = s.append("b", res_dep("build-slot"), None).expect("b"); + let c = s.append("c", res_dep("build-slot"), None).expect("c"); + + assert_eq!(s.settle(), vec![a], "cap 1: only the first can start"); + s.complete(a, Outcome::Done); + // b and c are both satisfiable now; b was inserted first. + assert_eq!(s.settle(), vec![b], "the freed unit goes to b, not c"); + s.complete(b, Outcome::Done); + assert_eq!(s.settle(), vec![c]); + } + #[test] fn leaf_owner_goes_done_directly_and_releases() { let mut s = scheduler_with_slots(1);