jobq: pin the fairness guarantee where it is made
fifo_fairness_for_the_slot lived in hive-c0re and submitted three rebuilds, driving one to completion to watch the freed build slot go to the earlier waiter. The guarantee it was checking is this crate's: claim_one scans nodes in insertion order and takes the first satisfiable one. Nothing here tested it -- the property hive-jobq provides was asserted only downstream, through a host's templates. a_contended_resource_goes_to_the_oldest_waiter tests it directly. Mutation-checked: reversing the scan order fails it. What is hive-c0re's is which nodes contend for the slot at all, and that is a declaration, so its half is now a declared_resources table with nothing running. The measured shape corrected an assumption on the way: MetaSync takes the meta window only, and the agent lease starts at StopForUpdate -- the first node that touches the container -- not at the head of the chain. Prebuild deliberately holds no lease, which is what lets it overlap another DAG on the same agent while the container is still up. "Uniform hold across the chain" needs no test of its own: a resource is held for the acquirer's whole subtree, and the parent nesting is already asserted in rebuild_chain_is_declared_serial.
This commit is contained in:
parent
b9f86e415d
commit
3ebfed1226
2 changed files with 68 additions and 26 deletions
|
|
@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in a new issue