vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

c0re's queue tests no longer drive the scheduler

Browse source 
The last two claim-driven tests were both arranging node states to observe
something that never needed a run:

`settled_dag_leaves_the_snapshot_despite_its_skipped_branch` completed all
seven nodes of a rebuild to assert the DAG left the snapshot. That is one
predicate over a list of states. `shown_on_wire` is it, split out of
`dag_view`, and the cases can now be named rather than arranged — including
the empty set, the one input where "any" and "all" disagree. It takes
states rather than projected nodes so the caller skips projecting what it
is about to discard; a `NodeView` costs a `build_logs` lookup.

`failed_node_cancels_downstream_but_afterany_reconcile_runs` asserted three
unrelated things from one arranged failure: the cascade (hive_jobq's, and
already tested there), the wire filter (now `shown_on_wire`), and the
roll-up. `DagView::rollup_state` lives in hive-host-sock, which had no
tests at all — it does now, next to the invariant, covering the ordering
its own doc comment says has silently disagreed with the frontend before.

With nothing left claiming, `Claimed` / `ClaimReady` / `CompleteNode` /
`claim_one` / `settle_rebuild_tail` are deleted. Claim/complete sites in
`job_queue/tests.rs`: 109 -> 0.

jobq narrows to match: `settle` is gone (it was a `claim_one` loop
returning a Vec, and its only callers were tests — it lives in the test
module now), `claim_one` is private, and `complete_growing` is
`pub(crate)`. `claim_next` is the whole run-loop surface.

`complete` stays `pub` for one caller, noted at the definition: `submit`
completes a group root with no logic of its own so it parks in `Finishing`
and its children unblock. That is a statement about the node, not an event
to report, and it wants to be expressible at insert time.
This commit is contained in:
atlas 2026-08-02 21:44:33 +02:00 • committed by mara
commit e646656c92
4 changed files with 334 additions and 266 deletions
Download patch file Download diff file Expand all files Collapse all files

61
hive-c0re/src/job_queue/mod.rs
View file

@ -404,32 +404,27 @@ fn dag_meta(sched: &Sched, container: NodeId) -> Option<DagMeta> {
/// aged out).
fn dag_view(sched: &Sched, container: NodeId) -> Option<DagView> {
let meta = dag_meta(sched, container)?;
let mut nodes = Vec::new();
// Whether anything in this DAG still has an outcome worth showing.
// Kept separate from `nodes` being non-empty: skipped nodes ride the
// wire so the dashboard can mark the branches that weren't taken, but
// they must not by themselves hold a finished DAG in the snapshot.
let mut any_unsettled = false;
let all: Vec<_> = sched.graph().descendants(container).collect();
// DAG-level timestamps are taken over *all* subtree nodes (including the
// `Done` ones excluded from the wire) — the client can't derive them
// from a `Done`-filtered node set, so the host computes them here.
let mut started: Vec<DateTime<Utc>> = Vec::new();
let mut finished: Vec<DateTime<Utc>> = Vec::new();
for node in sched.graph().descendants(container) {
let id = node.id;
for node in &all {
if let Some(s) = node.started_at {
started.push(s);
}
if let Some(f) = node.finished_at {
finished.push(f);
}
// `Done` nodes drop off the wire — a finished step isn't
// interesting. `Skipped` ones stay: which branch a run *didn't*
// take is the readable half of an outcome-branched DAG.
if matches!(node.state, State::Done) {
continue;
}
any_unsettled |= !matches!(node.state, State::Skipped);
}
// Decide which nodes ride the wire *before* projecting any of them: a
// `NodeView` costs a `build_logs` lookup, so building one for a node
// that's about to be dropped would be a query per finished step.
let shown = shown_on_wire(&all.iter().map(|n| n.state).collect::<Vec<_>>())?;
let mut nodes = Vec::new();
for node in shown.into_iter().map(|i| all[i]) {
let id = node.id;
let deps: Vec<u64> = node
.deps
.iter()
@ -480,9 +475,6 @@ fn dag_view(sched: &Sched, container: NodeId) -> Option<DagView> {
parent,
});
}
if !any_unsettled {
return None;
}
let is_terminal = sched.graph().is_settled(container) == Some(true);
Some(DagView {
id: container.get(),
@ -495,6 +487,39 @@ fn dag_view(sched: &Sched, container: NodeId) -> Option<DagView> {
})
}
/// Which of a DAG's work nodes ride the wire, by index into `states` — or
/// `None` when the DAG has nothing left worth showing and drops out of the
/// snapshot entirely.
///
/// Two separate decisions, and conflating them pins every completed deploy in
/// the queue view forever:
/// - **`Done` drops off the wire.** A finished step isn't interesting.
/// `Skipped` stays: which branch a run *didn't* take is the readable half of
/// an outcome-branched DAG.
/// - **`Skipped` alone doesn't hold a DAG in the snapshot.** So "the node list
/// is non-empty" and "there's still something here worth showing" are
/// different questions, and only the second one may drop the DAG.
///
/// Takes states rather than projected nodes so the caller can skip the work of
/// projecting what it's about to discard, and so this is testable without a
/// graph — the states it keys on are ones only a run can produce.
fn shown_on_wire(states: &[State]) -> Option<Vec<usize>> {
let worth_showing = states
.iter()
.any(|s| !matches!(s, State::Done | State::Skipped));
if !worth_showing {
return None;
}
Some(
states
.iter()
.enumerate()
.filter(|(_, s)| !matches!(s, State::Done))
.map(|(i, _)| i)
.collect(),
)
}
/// When a DAG's work node finishes on `finished_at` — the max over its
/// subtree (read off the graph `Node`, as unix seconds), for the history
/// cap ordering.

245
hive-c0re/src/job_queue/tests.rs
View file

@ -1,10 +1,18 @@
//! Queue-core unit tests: submit / no-dedup, cycle rejection, resource
//! serialization (build slots / per-agent leases), lease-exempt
//! overlap, FIFO fairness, cancel semantics, `AfterAny` failure
//! routing, in-DAG subgraph growth, and history retention. All
//! synchronous — the
//! scheduler's async loop is a thin claim/complete pump over the same
//! methods exercised here.
//! Queue-core unit tests: what c0re's templates **declare** — node kinds,
//! parent nesting, dep edges with the outcomes that satisfy them, and the
//! resources each construction site states it holds — plus the read layer over
//! that graph (wire projection, history retention, error truncation).
//!
//! **Nothing here runs a node.** Everything a template declares is in the graph
//! the moment `submit` returns, so the assertions read it there. Whether the
//! scheduler then honours those declarations — cascade, roll-up, grant
//! borrow/release, fairness, the `Finishing` gate — is `hive_jobq`'s property
//! and is tested in `hive_jobq`, against its own primitives rather than through
//! this module's templates.
//!
//! That split is why this file can't claim or complete: those are not part of
//! c0re's surface. A test helper that reached for them was reaching across the
//! boundary the two crates exist to draw.
use super::model::NodeKind;
use super::*;
@ -45,66 +53,13 @@ fn stop_online(
submit::stop_spec(&targets, graceful, Source::Manual, reason.to_owned())
}
/// What a test observes about a node the settle loop just started: its id, its
/// DAG, and its payload.
///
/// **Test-only, and deliberately not a production type.** `exec::run_node`
/// takes `(NodeId, &NodeKind)` and derives the DAG id on the two arms that
/// actually want it — nothing in production needs a claim snapshot to exist.
/// The assertions here are about *which* node the graph let run, which does
/// need the payload next to the id.
#[derive(Debug, Clone)]
struct Claimed {
node_id: NodeId,
kind: NodeKind,
}
/// Drive one settle wave and report every node that started.
///
/// An **extension trait rather than a method on [`JobQueue`]**: production
/// claims one node at a time ([`hive_jobq::scheduler::Scheduler::claim_next`])
/// and has no use for a whole wave, so this must not be reachable from
/// non-test code. `settle()` is that same claim primitive in a loop, so a test
/// driving it here exercises the production path.
trait ClaimReady {
fn claim_ready(&self) -> Vec<Claimed>;
}
impl ClaimReady for JobQueue {
fn claim_ready(&self) -> Vec<Claimed> {
let mut sched = self.sched().lock().expect("job_queue mutex poisoned");
let started = sched.settle();
started
.into_iter()
.filter_map(|node_id| {
let kind = sched.graph().node(node_id)?.payload.clone();
Some(Claimed { node_id, kind })
})
.collect()
}
}
/// Drive a node terminal by hand.
///
/// Also an extension trait, for the same reason as [`ClaimReady`]: production
/// completes a node **inside** the future
/// [`hive_jobq::scheduler::Scheduler::claim_next`] hands back, so "run the node,
/// then remember to complete it" is not an expressible sequence there — which
/// was the whole point of the seam. These tests need to express it, because
/// they exercise the graph without running any executor.
trait CompleteNode {
fn complete_node(&self, node_id: NodeId, result: Result<(), String>);
}
impl CompleteNode for JobQueue {
fn complete_node(&self, node_id: NodeId, result: Result<(), String>) {
self.sched()
.lock()
.expect("job_queue mutex poisoned")
.complete(node_id, outcome_of(result));
self.notify.notify_one();
}
}
// `Claimed` / `ClaimReady` / `CompleteNode` lived here: a claim snapshot type
// and two extension traits that let this module start and finish nodes by
// hand. Nothing in this file drives the scheduler any more, so they are gone
// — which is the point. Production completes a node **inside** the future
// `hive_jobq::scheduler::Scheduler::claim_next` hands back, so "run the node,
// then remember to complete it" is not an expressible sequence there. A test
// helper that re-expressed it was a hole in exactly the seam it was testing.
/// One node's **declared** shape: what it is, what it hangs under, and what it
/// waits for — all by kind, since ids are not stable across runs.
@ -273,29 +228,6 @@ fn row(
}
}
/// Claim helper asserting exactly one node comes back.
fn claim_one(q: &JobQueue) -> Claimed {
let mut claims = q.claim_ready();
assert_eq!(
claims.len(),
1,
"expected exactly one claim, got {claims:?}"
);
claims.pop().expect("one claim")
}
/// Claim the `EmitRebuilt` tail the graph let run and assert it's the `ok` one
/// expected.
fn settle_rebuild_tail(q: &JobQueue, agent: &str, expect_ok: bool) {
let tail = claim_one(q);
assert!(
matches!(&tail.kind, NodeKind::EmitRebuilt { agent: a, ok } if a == agent && *ok == expect_ok),
"expected the ok={expect_ok} EmitRebuilt tail for {agent}, got {:?}",
tail.kind
);
q.complete_node(tail.node_id, Ok(()));
}
/// The resources a node **declared**, read off its graph edges.
///
/// The declaration is the thing under test now that construction sites state
@ -580,25 +512,48 @@ fn non_graceful_rebuild_has_no_signal_or_drain() {
);
}
/// A cleanly-finished DAG leaves the snapshot even though its not-taken
/// failure branch is still in the graph as `Skipped`. Skipped nodes ride the
/// wire so the dashboard can mark them, which makes "the node list is empty"
/// and "nothing here is still worth showing" two different questions — only
/// the second one may drop the DAG. Conflating them pins every completed
/// deploy in the queue view forever.
/// Which nodes ride the wire, and whether a DAG is still worth showing.
///
/// This used to submit a rebuild, claim and complete all seven of its nodes,
/// then assert the DAG was absent from `snapshot()` — the scheduler, the
/// roll-up and the whole projection standing in for one predicate over a list
/// of states. `shown_on_wire` is that predicate, so the cases can be named
/// instead of arranged.
#[test]
fn settled_dag_leaves_the_snapshot_despite_its_skipped_branch() {
let q = JobQueue::new(1);
let id = submit(&q, rebuild("agent-a", "r"));
for _ in 0..6 {
let c = claim_one(&q);
q.complete_node(c.node_id, Ok(()));
}
settle_rebuild_tail(&q, "agent-a", true);
assert!(
q.snapshot().iter().all(|d| d.id != id),
"a fully settled DAG drops out of the snapshot"
fn skipped_nodes_ride_the_wire_but_do_not_hold_a_settled_dag_in_the_snapshot() {
// Nothing left worth showing → the DAG drops out of the snapshot, and its
// absence is what signals completion.
assert_eq!(shown_on_wire(&[State::Done, State::Done]), None);
// The case the old test was built around: a clean run whose not-taken
// failure branch is still in the graph as `Skipped`. "The node list is
// non-empty" and "there's something here worth showing" are different
// questions, and conflating them pins every completed deploy in the queue
// view forever.
assert_eq!(
shown_on_wire(&[State::Done, State::Skipped, State::Done]),
None,
"a skipped branch does not keep a finished DAG alive"
);
// A `Failed` DAG lingers — and takes its skipped branches with it, which is
// how an operator sees which steps the run never reached.
assert_eq!(
shown_on_wire(&[State::Done, State::Failed, State::Skipped, State::Done]),
Some(vec![1, 2]),
"done nodes drop off, the failure and what it ruled out stay"
);
// Live DAGs keep everything but their finished steps.
assert_eq!(
shown_on_wire(&[State::Done, State::Running, State::Pending]),
Some(vec![1, 2])
);
assert_eq!(
shown_on_wire(&[State::Cancelled, State::Skipped]),
Some(vec![0, 1]),
"a cancelled DAG is still worth showing to whoever cancelled it"
);
// An empty DAG has nothing worth showing either — no special case needed,
// but it is the one input where "any" and "all" disagree, so it is pinned.
assert_eq!(shown_on_wire(&[]), None);
}
// ---- build slots ----
@ -608,8 +563,8 @@ 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:
// there at all — its claim primitive 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,
@ -900,58 +855,26 @@ fn rebuild_chain_nodes_suppress_crash_watch() {
}
// ---- failure: cancel-downstream + AfterAny ----
#[test]
fn failed_node_cancels_downstream_but_afterany_reconcile_runs() {
let q = JobQueue::new(1);
let id = submit(&q, rebuild("agent-a", "r"));
let meta_sync = claim_one(&q);
assert_eq!(meta_sync.kind.as_str(), "meta_sync");
q.complete_node(meta_sync.node_id, Ok(()));
let prebuild = claim_one(&q);
assert_eq!(prebuild.kind.as_str(), "prebuild");
q.complete_node(prebuild.node_id, Err("nix build exploded".to_owned()));
// StopForUpdate + Swap are cancelled (AfterOk on a failed chain);
// the AfterAny Reconcile still runs once Swap is terminal.
let reconcile = claim_one(&q);
assert_eq!(reconcile.kind.as_str(), "reconcile");
q.complete_node(reconcile.node_id, Ok(()));
let snap = q.snapshot();
let dag = snap.iter().find(|d| d.id == id).expect("dag");
assert_eq!(dag.rollup_state(), State::Failed, "roll-up failed");
let by_kind = |k: &str| {
dag.nodes
.iter()
.find(|n| n.kind == k)
.expect("node present")
.state
};
assert_eq!(by_kind("prebuild"), State::Failed);
// `StopForUpdate` / `Swap` / `PostSwap` were *ruled out* by the failed
// `Prebuild`. They ride the wire as `Skipped` so an operator can see which
// steps the run never reached, without them reading as failures of their
// own — the roll-up ignores `Skipped` entirely.
for ruled_out in ["stop_for_update", "swap", "post_swap"] {
assert_eq!(
by_kind(ruled_out),
State::Skipped,
"{ruled_out} was ruled out, so it is on the wire as skipped"
);
}
// The AfterAny reconcile ran (claimed + completed Ok above) → it's `Done`,
// and `Done` nodes are excluded from the wire, so it's absent here.
assert!(
dag.nodes.iter().all(|n| n.kind != "reconcile"),
"the completed (Done) reconcile is filtered off the wire"
);
assert_eq!(
dag.nodes
.iter()
.find(|n| n.kind == "prebuild")
.and_then(|n| n.error.as_deref()),
Some("nix build exploded")
);
}
//
// `failed_node_cancels_downstream_but_afterany_reconcile_runs` lived here. It
// drove a rebuild to a failed `Prebuild` and then asserted three unrelated
// things at once, which is why it needed a running scheduler at all:
//
// 1. the cascade — a failed node cancels its `AfterOk` dependants while the
// `AfterAny` reconcile still runs. That is hive_jobq's rule, and it owns
// the test: `failed_after_ok_dep_cancels_dependents_but_after_any_runs`.
// c0re's declaration of *which* edge is which is asserted in
// `rebuild_chain_is_declared_serial` — the reconcile's accepted outcome
// set is right there in the shape table.
// 2. the wire projection — `Done` off, `Skipped` on. That is
// `shown_on_wire`, tested directly above.
// 3. the roll-up — a DAG with a failed node reads `Failed`, and `Skipped`
// contributes nothing. That is `DagView::rollup_state`, which lives in
// hive-host-sock and is now tested there, next to the invariant.
//
// Reconstructing all three from one arranged run made none of them
// individually legible, and the arrangement was the only reason this module
// needed to claim and complete nodes.
/// The swap-success path: `Swap` ok → the `AfterOk` `PostSwap` (bookkeeping
/// tail) runs, and only then does `Reconcile` fire — serialized behind