job_queue: filter jobq graph snapshot by per-node state
graph_snapshot previously filtered which whole roots got projected based on the root node's own state, so a group root that was still Running but had already-Done internal steps couldn't be filtered down to just its live nodes, and a filtered-out root hid its entire subtree even when a descendant still matched. Apply the states filter after GraphWire::wire_snapshot instead, over every node in the flattened tree, not just roots. The jobq-graph client already handles an orphaned node (parent filtered out) by promoting it to a rendered root, so this is safe on the client side with no changes needed there. Fixes hyperhive#3210
This commit is contained in:
parent
0b6b3b755d
commit
861a1f8f26
2 changed files with 98 additions and 37 deletions
|
|
@ -333,25 +333,32 @@ impl JobQueue {
|
|||
/// visible rather than vanishing from the payload, which is what makes a
|
||||
/// fast rebuild render as a single node).
|
||||
///
|
||||
/// `states`, when given, keeps only the **root** groups whose own state
|
||||
/// is named — a root's state is already its subtree's rolled-up answer
|
||||
/// (see `hive_jobq_wire`'s doc), so filtering the root filters the whole
|
||||
/// group without needing to also filter descendants. `None` (or the
|
||||
/// full state set) is the unfiltered call, matching prior behaviour.
|
||||
/// A slice rather than a set: `State` (`hive_jobq`'s) derives `Eq` but
|
||||
/// not `Hash`, and the whole vocabulary is 7 variants — a linear
|
||||
/// membership check per root costs nothing at that size, so there is no
|
||||
/// reason to widen `hive_jobq::State`'s derive list just for this.
|
||||
/// `states`, when given, keeps every **individual node** (root or
|
||||
/// descendant) whose own state is named — not just whole root groups.
|
||||
/// A still-live group (root not yet terminal) can otherwise hold any
|
||||
/// number of already-finished steps inside it; filtering only at the
|
||||
/// root leaves every one of those visible regardless of the ask, which
|
||||
/// is exactly the clutter a state filter exists to remove. `None` (or
|
||||
/// the full state set) is the unfiltered call, matching prior
|
||||
/// behaviour. A slice rather than a set: `State` derives `Eq` but not
|
||||
/// `Hash`, and the vocabulary is 7 variants — a linear check per node
|
||||
/// costs nothing at that size.
|
||||
///
|
||||
/// A node whose *parent* got filtered out still rides with its original
|
||||
/// `parent` id — `<hive-jobq-graph>` (the one consumer) already treats
|
||||
/// an unresolvable parent as a new root (`buildTree`'s fallback), so a
|
||||
/// filtered-out ancestor surfaces a still-matching descendant one level
|
||||
/// higher rather than hiding or orphaning it.
|
||||
///
|
||||
/// The projection itself is [`hive_jobq_wire`]'s; all this layer supplies
|
||||
/// is *which* groups to show — see [`visible_roots`] for why the graph
|
||||
/// can't decide that for itself.
|
||||
/// is *which* nodes to show — see [`visible_roots`] for why the graph
|
||||
/// can't decide the root-visibility half of that for itself.
|
||||
#[must_use]
|
||||
pub fn graph_snapshot(&self, states: Option<&[State]>) -> Vec<GraphNode> {
|
||||
let inner = self.lock();
|
||||
let roots = visible_roots(&inner);
|
||||
let roots = filter_roots_by_state(&inner, roots, states);
|
||||
inner.graph().wire_snapshot(roots)
|
||||
let nodes = inner.graph().wire_snapshot(roots);
|
||||
filter_nodes_by_state(nodes, states)
|
||||
}
|
||||
|
||||
/// Per-state counts over the **same** groups [`Queue::graph_snapshot`]
|
||||
|
|
@ -406,30 +413,23 @@ fn find_node(sched: &Sched, id: u64) -> Option<NodeId> {
|
|||
.find_map(|n| (n.id.get() == id).then_some(n.id))
|
||||
}
|
||||
|
||||
/// [`Queue::graph_snapshot`]'s `states` ask, applied to an already-bounded
|
||||
/// root set: keeps only the roots whose own `state` is named.
|
||||
/// [`Queue::graph_snapshot`]'s `states` ask, applied to the already-
|
||||
/// projected node list: keeps every node — root or descendant — whose own
|
||||
/// `state` is named.
|
||||
///
|
||||
/// Applied *after* [`visible_roots`] rather than folded into it — the
|
||||
/// history cap bounds how much settled work is retained at all, which is a
|
||||
/// different question from which of the retained (and live) groups the
|
||||
/// caller wants shown right now. `None` (or an unrecognised/absent query)
|
||||
/// is the identity filter.
|
||||
fn filter_roots_by_state(
|
||||
sched: &Sched,
|
||||
roots: Vec<NodeId>,
|
||||
states: Option<&[State]>,
|
||||
) -> Vec<NodeId> {
|
||||
/// Applied *after* [`GraphWire::wire_snapshot`] rather than as a root
|
||||
/// pre-filter — narrowing which roots are visible at all is
|
||||
/// [`visible_roots`]'s job (a different question: how much settled work is
|
||||
/// retained, full stop); this is "of what's retained and live, which
|
||||
/// individual nodes does the caller want shown right now." `None` (or an
|
||||
/// unrecognised/absent query) is the identity filter.
|
||||
fn filter_nodes_by_state(nodes: Vec<GraphNode>, states: Option<&[State]>) -> Vec<GraphNode> {
|
||||
let Some(states) = states else {
|
||||
return roots;
|
||||
return nodes;
|
||||
};
|
||||
roots
|
||||
nodes
|
||||
.into_iter()
|
||||
.filter(|&id| {
|
||||
sched
|
||||
.graph()
|
||||
.node(id)
|
||||
.is_some_and(|n| states.contains(&n.state))
|
||||
})
|
||||
.filter(|n| states.contains(&n.state))
|
||||
.collect()
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue