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

jobq: delete DagView/NodeView, the second projection of one graph

Browse source 
Two views of the same graph existed: the typed `DagView`/`NodeView`
(`/api/state.rebuild_queue`, the `QueueDag` socket request, and the
`RebuildQueueChanged` payload) and `hive-jobq-wire`'s generic
`GraphNode` (`/api/jobq/graph`, `QueueNodes`). Every consumer has moved
to the generic one, so the typed pair is deleted rather than kept in
agreement with it.

What that removes, beyond the types: the `QueueDag` request and
`HostResponse::dags`; `Queue::snapshot`; `dag_view`, `visible_dags`,
`shown_on_wire`, `dag_finished_at` and `containers`; and the
`rebuild_queue` field on `/api/state`. `RebuildQueueChanged` keeps its
seq and loses its payload — nothing read it, and shipping the graph
both on an event and on an endpoint is the duplication this issue is
about. It stays an event rather than becoming a poll because
push-on-change is what every other live surface here does.

Two behaviours came out simpler for a structural reason. `await_dags`
needed two rules — settled means "gone from the snapshot" *or* "present
with every node terminal" — because the typed view evicted finished
groups; the generic view doesn't, so pending is just "some node isn't
terminal". And `state_of` in the tests no longer derives a roll-up at
all: a group root's own state is the scheduler's answer.

That second one found a bug. `cancelled_dag_still_runs_its_approval
tail` asserted the group reads `Cancelled` while the tail it exists to
protect was still pending — `rollup_state` flattened the surviving
child away and called the group settled. The root reads `Finishing`,
which is what the scheduler documents: own logic done, children still
running. The test now asserts that, with the reasoning inline so it
doesn't get "fixed" back.

Kept: `Source`, `State`, `PermPayload` and the `NodeId` alias in
`hive-host-sock::jobs` — shared vocabulary, still used by hivectl.
This commit is contained in:
atlas 2026-08-03 21:07:58 +02:00 • committed by mara
commit f707c60f90
10 changed files with 131 additions and 615 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -284,14 +284,27 @@ fn declared_shape_for(q: &JobQueue, dag: u64, agent: &str) -> Vec<Declared> {
}
fn state_of(q: &JobQueue, dag_id: u64) -> State {
// A DAG whose nodes have all settled `Done` or `Skipped` drops out of the
// snapshot — absence is the completion signal, so map it to `Done`.
// Otherwise derive the roll-up from the node set, exactly as every wire
// consumer does.
q.snapshot()
// A group root's own state *is* its subtree's roll-up — that is the
// scheduler's contract (`Finishing` until children settle, then the
// rolled-up outcome), so there is nothing to derive here any more.
// A root aged out of the retained history reads `Done`: it settled, or
// it would still be live.
q.graph_snapshot()
.iter()
.find(|d| d.id == dag_id)
.map_or(State::Done, DagView::rollup_state)
.find(|n| n.id == dag_id)
.map_or(State::Done, |n| n.state)
}
/// How many groups the queue is showing — roots, not nodes.
///
/// `graph_snapshot` is flat (every node under every visible root), so a test
/// asking "how many DAGs" counts the parentless ones. Counting rows would
/// count steps, which is a different number: one rebuild is ~7 nodes.
fn dag_count(q: &JobQueue) -> usize {
q.graph_snapshot()
.iter()
.filter(|n| n.parent.is_none())
.count()
}
// ---- submit (dedup removed — every submit is a fresh DAG) ----
@ -302,7 +315,7 @@ fn submit_assigns_distinct_ids() {
let first = submit(&q, "first", |builder| rebuild(builder, "agent-a"));
let second = submit(&q, "second", |builder| rebuild(builder, "agent-b"));
assert_ne!(first, second);
assert_eq!(q.snapshot().len(), 2);
assert_eq!(dag_count(&q), 2);
}
/// Submit-time dedup was removed with the agent-per-node refactor (a
@ -316,7 +329,7 @@ fn identical_resubmit_is_a_distinct_dag() {
let first = submit(&q, "first", |builder| rebuild(builder, "agent-a"));
let resubmit = submit(&q, "again", |builder| rebuild(builder, "agent-a"));
assert_ne!(first, resubmit, "no dedup: identical resubmit is a new DAG");
assert_eq!(q.snapshot().len(), 2);
assert_eq!(dag_count(&q), 2);
}
#[test]
@ -329,7 +342,7 @@ fn distinct_submits_never_collapse() {
});
assert_ne!(rebuild_a, rebuild_b);
assert_ne!(rebuild_a, restart_a);
assert_eq!(q.snapshot().len(), 3);
assert_eq!(dag_count(&q), 3);
}
#[test]
@ -349,7 +362,7 @@ fn resubmit_while_running_is_new_dag() {
rebuild(builder, "agent-a");
});
assert_ne!(a, again);
assert_eq!(q.snapshot().len(), 2);
assert_eq!(dag_count(&q), 2);
}
// ---- malformed specs: no longer expressible ----
@ -488,50 +501,6 @@ fn non_graceful_rebuild_has_no_signal_or_drain() {
);
}
/// 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 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 ----
#[test]
@ -590,7 +559,7 @@ fn multi_agent_restart_is_one_dag_with_concurrent_per_agent_subgraphs() {
restart_online(builder, &["agent-a", "agent-b"], false);
});
// A hive-wide restart is ONE DAG, not one-per-agent.
assert_eq!(q.snapshot().len(), 1);
assert_eq!(dag_count(&q), 1);
// Each agent's subgraph head (StopForUpdate, since both are running) is a
// group root with no deps, so nothing orders them against each other; and
// each declares only its OWN agent's lease, so nothing makes them contend.
@ -626,7 +595,7 @@ fn multi_agent_stop_is_one_dag_with_concurrent_per_agent_subgraphs() {
stop_online(builder, &["agent-a", "agent-b"], false);
});
// A hive-wide stop is ONE DAG, not one-per-agent.
assert_eq!(q.snapshot().len(), 1);
assert_eq!(dag_count(&q), 1);
// Same declared story as the restart case above: each agent's subgraph head
// is a group root with no node-deps, holding only its own agent's lease.
// Independent roots on disjoint resources is what "concurrently" means at
@ -665,7 +634,7 @@ fn multi_agent_start_one_dag_folds_per_agent_stale_rebuild() {
);
});
// One DAG spanning both agents.
assert_eq!(q.snapshot().len(), 1);
assert_eq!(dag_count(&q), 1);
// The fold is a *declared* difference, readable the moment submit returns:
// both agents get a `SetWanted(Up)` group root, but the fresh agent's
// subgraph ends at the Reconcile behind it while the stale agent's carries
@ -712,13 +681,12 @@ fn offline_agents_skip_mechanical_nodes_but_keep_reconcile() {
submit::restart_nodes(builder, &[("down2".to_owned(), false)], true);
});
let shape = |id: u64| -> Vec<String> {
q.snapshot()
// The group's work nodes: its subtree minus the container itself,
// which the generic view carries as an ordinary node.
q.node_subtrees(&[id])
.iter()
.find(|d| d.id == id)
.expect("dag")
.nodes
.iter()
.map(|n| n.kind.clone())
.filter(|n| n.id != id)
.map(|n| n.payload.label.clone())
.collect()
};
assert_eq!(
@ -827,11 +795,13 @@ fn rebuild_chain_nodes_suppress_crash_watch() {
// 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.
// 2. the wire projection — which nodes ride and which are filtered out.
// **That decision no longer exists:** the generic view serves every node
// under a visible root, terminal ones included, so there is no predicate
// left to test. The `Done`-off/`Skipped`-on filter died with `DagView`.
// 3. the roll-up — a group with a failed node reads `Failed`. That is the
// root node's own `state`, stamped by `hive_jobq`'s settle loop and
// tested there; nothing re-derives it host-side any more.
//
// Reconstructing all three from one arranged run made none of them
// individually legible, and the arrangement was the only reason this module
@ -1035,13 +1005,21 @@ fn cancel_drops_one_agents_branch_leaving_the_rest() {
let id = submit(&q, "r", |builder| {
restart_online(builder, &["agent-a", "agent-b"], false);
});
// Per-agent subgraphs are independent roots; find agent-a's.
let snap = q.snapshot();
let dag = snap.iter().find(|d| d.id == id).expect("dag in snapshot");
let a_root = dag
.nodes
// Per-agent subgraphs hang directly off the container, one per agent.
// ⚠️ `parent` is the *graph* parent here, not a DAG-relative one: what
// the typed view called a parentless group root is a direct child of the
// container node in the generic view.
let snap = q.node_subtrees(&[id]);
let a_root = snap
.iter()
.find(|n| n.agent == "agent-a" && n.parent.is_none())
.find(|n| {
n.parent == Some(id)
&& n.payload
.data
.get("agent")
.and_then(serde_json::Value::as_str)
== Some("agent-a")
})
.expect("agent-a has a group root");
assert!(q.cancel(a_root.id), "an interior/group root cancels alone");
@ -1154,11 +1132,18 @@ fn cancelled_dag_still_runs_its_approval_tail() {
),
"only the cancelled-outcome tail is spared, got {spared:?}"
);
assert_eq!(state_of(&q, id), State::Cancelled);
// An unrelated DAG landing in the same graph doesn't disturb this one's
// roll-up — the snapshot is per-DAG, not a global state machine.
// ⚠️ `Finishing`, not `Cancelled` — and the change is a **fix**, not a
// regression. This used to read the host-side `DagView::rollup_state`,
// which flattened the spared tail away and reported the group settled
// while a node of it was still pending. The root's own state is the
// scheduler's answer: `Finishing` means "own logic done, children still
// running", and the tail this test exists to protect *is* such a child.
// A group that still has work to do does not read terminal.
assert_eq!(state_of(&q, id), State::Finishing);
// An unrelated group landing in the same graph doesn't disturb this one's
// state — a root rolls up its own subtree, not the graph.
let _other = submit(&q, "r", |builder| rebuild(builder, "agent-b"));
assert_eq!(state_of(&q, id), State::Cancelled);
assert_eq!(state_of(&q, id), State::Finishing);
}
// ---- approval deploy subtree ----