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

fix(#2802): a group whose children were all dropped rolls up Cancelled

Browse source 
The roll-up treated a cancelled child the same as a failed one, so a DAG
the operator cancelled before it started reported `Failed` — it claimed to
have failed at something when nothing under it ever ran.

`Failed` still outranks `Cancelled`: a group where one step broke and the
rest were dropped in response is a failure, and that is the fact worth
surfacing. Only a group with no failed child at all reports the cancel.

This also settles a disagreement. `DagView::rollup_state` on the wire has
always ranked failed over cancelled over the rest; the graph's own roll-up
had no `Cancelled` outcome to rank, so the two described the same DAG
differently depending on which one you asked.
This commit is contained in:
atlas 2026-07-27 21:22:53 +02:00
commit 6fd91ccf6a
1 changed files with 33 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

59
hive-jobq/src/scheduler.rs
View file

@ -233,15 +233,31 @@ impl<N, R: Clone + Eq + Hash> Scheduler<N, R> {
.all(|n| n.parent != Some(id) || n.state.is_terminal())
}
/// Whether any direct child of `id` ended `Failed`/`Cancelled` — the roll-up
/// failure condition for the parent.
/// `Skipped` children are **not** counted: being ruled out by an edge is the
/// expected fate of every branch not taken, so counting it would make any
/// group that branches on outcome roll up failed no matter how the run went.
fn any_child_failed(&self, id: NodeId) -> bool {
self.graph
.nodes()
.any(|n| n.parent == Some(id) && matches!(n.state, State::Failed | State::Cancelled))
/// What `id` rolls up to once all its children are terminal: `Failed` if any
/// child failed, else `Cancelled` if any was cancelled, else `Done`.
///
/// `Failed` outranks `Cancelled` because the failure is the actionable fact —
/// a group where one step broke and the rest were dropped in response is a
/// failure, not a cancellation. A group whose children were *all* dropped
/// never failed at anything, and says so.
///
/// `Skipped` children are ignored: being ruled out by an edge is the expected
/// fate of every branch not taken, so counting it would make any group that
/// branches on outcome roll up non-`Done` however the run went.
fn rolled_up_state(&self, id: NodeId) -> State {
let mut any_cancelled = false;
for child in self.graph.nodes().filter(|n| n.parent == Some(id)) {
match child.state {
State::Failed => return State::Failed,
State::Cancelled => any_cancelled = true,
_ => {}
}
}
if any_cancelled {
State::Cancelled
} else {
State::Done
}
}
/// Transition a node whose own logic just *succeeded* to its resulting state:
@ -254,12 +270,10 @@ impl<N, R: Clone + Eq + Hash> Scheduler<N, R> {
/// its dependency succeeds, and leaving it `Pending` would wedge the subtree
/// non-terminal forever.
fn settle_terminal(&mut self, id: NodeId) {
let state = if !self.all_children_terminal(id) {
State::Finishing
} else if self.any_child_failed(id) {
State::Failed
let state = if self.all_children_terminal(id) {
self.rolled_up_state(id)
} else {
State::Done
State::Finishing
};
self.graph.set_state(id, state);
if state.is_terminal() {
@ -279,11 +293,7 @@ impl<N, R: Clone + Eq + Hash> Scheduler<N, R> {
{
break;
}
let state = if self.any_child_failed(a) {
State::Failed
} else {
State::Done
};
let state = self.rolled_up_state(a);
self.graph.set_state(a, state);
// Any terminal outcome can rule a dependent out — see `settle_terminal`.
self.cascade_cancel(a);
@ -982,13 +992,10 @@ mod tests {
assert!(s.cancel_node(root));
assert_eq!(s.graph().node(a).unwrap().state, State::Cancelled);
assert_eq!(s.graph().node(b).unwrap().state, State::Cancelled);
// With every child terminal the root leaves `Finishing` and rolls up.
// It rolls up `Failed`, not `Cancelled`: the roll-up counts a cancelled
// child as a non-success, and it has no separate "the whole group was
// dropped" outcome. Pre-existing and unchanged here — a caller that
// wants to show a cancel as a cancel derives that from the node states,
// not from the root's.
assert_eq!(s.graph().node(root).unwrap().state, State::Failed);
// With every child terminal the root leaves `Finishing` and rolls up —
// as `Cancelled`, not `Failed`: nothing under it failed at anything, the
// work was dropped.
assert_eq!(s.graph().node(root).unwrap().state, State::Cancelled);
}
/// All-or-nothing: an in-flight node's work is not interruptible, and