refactor(job-queue): build DAGs by naming nodes, not counting them
Every template built a `Vec<NodeSpec>` whose edges and parents were positional indices into that vector, so a shape was expressed as arithmetic: `base + 1`, `stop_root + 2`, `sfu + 1`, and a `reconcile_index()` helper that read the emitted vector's length to find out where its own last node had landed. `concat_subgraphs` existed solely to rebase one per-agent subgraph's indices onto another's. Templates now declare into a `hive_jobq::JobBuilder` and hold the handles they get back, so an edge names the node it waits on. The arithmetic is gone, and with it: - `NodeSpec` and the job-queue's own index-based `Dep`. - `insert_group`'s index resolution — it wraps `Scheduler::insert_job`. - `concat_subgraphs` — per-agent chains share one builder and each keeps its own root, so independence is structural rather than computed. - `reconcile_index` and `dep_index`. - `templates::validate` and its petgraph toposort. It rejected dangling deps and cycles; both are now unrepresentable, since a handle only exists for an already-declared node and every edge therefore points backwards. (petgraph stays in the tree for `agent_config::topology`.) `NodeOutput.append_subgraph` becomes `Vec<Job>`: an executor cannot reach the queue, so it hands back declarations and the scheduler inserts them under its own lock. That is what the in-DAG growth path always wanted — a transferable declaration, not a vector of specs. Resource declaration is unchanged in behaviour: the `templates::node` helper applies `NodeKind::resource_deps()` at the construction site, so every node still declares what its kind needs. Moving that declaration to the call sites is #2818's job; this leaves it one place to delete. Three tests went with the guard they covered — they hand-built malformed specs out of indices, which is the representation that made those shapes possible. Two more now read a DAG's shape off the queue rather than out of a spec vector, which is where it is observable. The remaining 45 job-queue tests are unchanged and still pass: lease serialization, roll-up, cancel-cascade, in-DAG growth and per-agent concurrency all behave as before.
This commit is contained in:
parent
9be7731c5e
commit
e7c3cf5a3d
9 changed files with 528 additions and 696 deletions
|
|
@ -6,9 +6,7 @@
|
|||
//! scheduler's async loop is a thin claim/complete pump over the same
|
||||
//! methods exercised here.
|
||||
|
||||
use hive_jobq::DepWhen;
|
||||
|
||||
use super::model::{Dep, NodeKind, NodeSpec};
|
||||
use super::model::NodeKind;
|
||||
use super::*;
|
||||
|
||||
fn submit(q: &JobQueue, spec: DagSpec) -> u64 {
|
||||
|
|
@ -141,71 +139,22 @@ fn resubmit_while_running_is_new_dag() {
|
|||
assert_eq!(q.snapshot().len(), 2);
|
||||
}
|
||||
|
||||
// ---- cycle rejection ----
|
||||
|
||||
#[test]
|
||||
fn cyclic_dag_is_rejected_at_submit() {
|
||||
let q = JobQueue::new(1);
|
||||
let mut spec = rebuild("agent-a", "cyclic");
|
||||
// 0 → 1 → 0 cycle.
|
||||
spec.nodes = vec![
|
||||
NodeSpec {
|
||||
kind: NodeKind::StopForUpdate {
|
||||
agent: "agent-a".to_owned(),
|
||||
},
|
||||
deps: vec![Dep {
|
||||
on: 1,
|
||||
when: DepWhen::AFTER_OK,
|
||||
}],
|
||||
parent: None,
|
||||
},
|
||||
NodeSpec {
|
||||
kind: NodeKind::Reconcile {
|
||||
agent: "agent-a".to_owned(),
|
||||
},
|
||||
deps: vec![Dep {
|
||||
on: 0,
|
||||
when: DepWhen::AFTER_OK,
|
||||
}],
|
||||
parent: None,
|
||||
},
|
||||
];
|
||||
assert!(q.submit(spec).is_err(), "cyclic spec must be refused");
|
||||
assert!(q.snapshot().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_dep_is_rejected_at_submit() {
|
||||
let q = JobQueue::new(1);
|
||||
let mut spec = rebuild("agent-a", "bad dep");
|
||||
spec.nodes = vec![NodeSpec {
|
||||
kind: NodeKind::Reconcile {
|
||||
agent: "agent-a".to_owned(),
|
||||
},
|
||||
deps: vec![Dep {
|
||||
on: 9,
|
||||
when: DepWhen::AFTER_OK,
|
||||
}],
|
||||
parent: None,
|
||||
}];
|
||||
assert!(q.submit(spec).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_parent_is_rejected_at_submit() {
|
||||
let q = JobQueue::new(1);
|
||||
let mut spec = rebuild("agent-a", "bad parent");
|
||||
// A forward/out-of-bounds parent index must be refused at validate, not
|
||||
// panic in `insert_group`.
|
||||
spec.nodes = vec![NodeSpec {
|
||||
kind: NodeKind::Reconcile {
|
||||
agent: "agent-a".to_owned(),
|
||||
},
|
||||
deps: Vec::new(),
|
||||
parent: Some(3),
|
||||
}];
|
||||
assert!(q.submit(spec).is_err());
|
||||
}
|
||||
// ---- malformed specs: no longer expressible ----
|
||||
//
|
||||
// Three tests lived here — a dependency cycle, a dependency on a node that
|
||||
// does not exist, and an out-of-range parent index — each asserting that
|
||||
// `submit` refused the spec. All three built their spec by hand out of
|
||||
// positional indices, which is exactly the representation that made those
|
||||
// shapes possible: an index can name a node that isn't there, or one that
|
||||
// comes later.
|
||||
//
|
||||
// A job is now declared against handles that only exist for nodes already
|
||||
// declared, so there is no index to put out of range, and every edge points
|
||||
// backwards — a cycle needs a forward edge. The guard those tests covered was
|
||||
// deleted along with the failure mode. What remains — a handle used against a
|
||||
// builder that never issued it — is `hive_jobq`'s to reject, and its builder
|
||||
// tests cover it (`a_forward_edge_is_rejected_by_name`,
|
||||
// `a_forward_parent_is_rejected_by_name`, `graph_rejection_surfaces_as_is`).
|
||||
|
||||
// ---- dependency order within a DAG ----
|
||||
|
||||
|
|
@ -243,20 +192,24 @@ fn rebuild_chain_claims_in_dep_order() {
|
|||
#[test]
|
||||
fn graceful_rebuild_chain_drains_before_stopping() {
|
||||
let q = JobQueue::new(1);
|
||||
let spec = DagSpec {
|
||||
source: Source::AutoUpdate,
|
||||
reason: "sweep".to_owned(),
|
||||
|
||||
nodes: templates::rebuild_nodes(
|
||||
"agent-a",
|
||||
templates::RebuildOpts {
|
||||
relock: true,
|
||||
graceful: true,
|
||||
},
|
||||
0,
|
||||
),
|
||||
};
|
||||
let id = submit(&q, spec);
|
||||
let job = Job::new();
|
||||
templates::rebuild_nodes(
|
||||
&job,
|
||||
"agent-a",
|
||||
templates::RebuildOpts {
|
||||
relock: true,
|
||||
graceful: true,
|
||||
},
|
||||
None,
|
||||
);
|
||||
let id = submit(
|
||||
&q,
|
||||
DagSpec {
|
||||
source: Source::AutoUpdate,
|
||||
reason: "sweep".to_owned(),
|
||||
job,
|
||||
},
|
||||
);
|
||||
for expected in [
|
||||
"meta_sync",
|
||||
"prebuild",
|
||||
|
|
@ -284,17 +237,34 @@ fn graceful_rebuild_chain_drains_before_stopping() {
|
|||
/// drain window, so `StopForUpdate` still hangs straight off `Prebuild`.
|
||||
#[test]
|
||||
fn non_graceful_rebuild_has_no_signal_or_drain() {
|
||||
let kinds: Vec<String> = templates::rebuild_nodes(
|
||||
// Read the shape off the queue rather than out of a node list: a declared
|
||||
// job keeps its nodes to itself and inserts them, so what it built is
|
||||
// observable where it matters — in what the scheduler runs.
|
||||
let q = JobQueue::new(1);
|
||||
let job = Job::new();
|
||||
templates::rebuild_nodes(
|
||||
&job,
|
||||
"agent-a",
|
||||
templates::RebuildOpts {
|
||||
relock: true,
|
||||
graceful: false,
|
||||
},
|
||||
0,
|
||||
)
|
||||
.iter()
|
||||
.map(|n| n.kind.as_str().to_owned())
|
||||
.collect();
|
||||
None,
|
||||
);
|
||||
let id = submit(
|
||||
&q,
|
||||
DagSpec {
|
||||
source: Source::Manual,
|
||||
reason: "manual".to_owned(),
|
||||
job,
|
||||
},
|
||||
);
|
||||
let mut kinds = Vec::new();
|
||||
for _ in 0..6 {
|
||||
let c = claim_one(&q);
|
||||
kinds.push(c.kind.as_str().to_owned());
|
||||
q.complete_node(c.node_id, Ok(()));
|
||||
}
|
||||
assert_eq!(
|
||||
kinds,
|
||||
vec![
|
||||
|
|
@ -306,6 +276,8 @@ fn non_graceful_rebuild_has_no_signal_or_drain() {
|
|||
"reconcile"
|
||||
]
|
||||
);
|
||||
// Settled after exactly those six — nothing else was declared.
|
||||
assert_eq!(state_of(&q, id), State::Done);
|
||||
}
|
||||
|
||||
/// A cleanly-finished DAG leaves the snapshot even though its not-taken
|
||||
|
|
@ -720,19 +692,19 @@ fn append_subgraph_roots_on_emitter_and_rebases_local_deps() {
|
|||
// subgraph per stale agent into its OWN DAG. Each subgraph is rooted on
|
||||
// the emitter and its LOCAL 0-based deps are rebased onto the DAG.
|
||||
let q = JobQueue::new(4);
|
||||
let job = Job::new();
|
||||
let _lock = templates::node(
|
||||
&job,
|
||||
NodeKind::MetaLock {
|
||||
sweep: true,
|
||||
fanout: None,
|
||||
inputs: Vec::new(),
|
||||
},
|
||||
);
|
||||
let spec = DagSpec {
|
||||
source: Source::AutoUpdate,
|
||||
reason: "sweep".to_owned(),
|
||||
|
||||
nodes: vec![NodeSpec {
|
||||
kind: NodeKind::MetaLock {
|
||||
sweep: true,
|
||||
fanout: None,
|
||||
inputs: Vec::new(),
|
||||
},
|
||||
deps: Vec::new(),
|
||||
parent: None,
|
||||
}],
|
||||
job,
|
||||
};
|
||||
let id = submit(&q, spec);
|
||||
let emitter = claim_one(&q);
|
||||
|
|
@ -742,18 +714,21 @@ fn append_subgraph_roots_on_emitter_and_rebases_local_deps() {
|
|||
// StopForUpdate → Swap → Reconcile, local 0-based deps. `graceful` must
|
||||
// match the sweep arm of `run_meta_lock` or this stops tracking production.
|
||||
let subgraph = |agent: &str| {
|
||||
let job = Job::new();
|
||||
templates::rebuild_nodes(
|
||||
&job,
|
||||
agent,
|
||||
templates::RebuildOpts {
|
||||
relock: true,
|
||||
graceful: true,
|
||||
},
|
||||
0,
|
||||
)
|
||||
None,
|
||||
);
|
||||
job
|
||||
};
|
||||
// Must append BEFORE completing the emitter (the documented contract).
|
||||
q.append_subgraph(id, &subgraph("a"), emitter.node_id);
|
||||
q.append_subgraph(id, &subgraph("b"), emitter.node_id);
|
||||
q.append_subgraph(id, subgraph("a"), emitter.node_id);
|
||||
q.append_subgraph(id, subgraph("b"), emitter.node_id);
|
||||
q.complete_node(emitter.node_id, Ok(()));
|
||||
// Still ONE DAG; both subgraph roots become ready once the emitter is
|
||||
// Done (rooted on it), each on its own agent lease. Their `MetaSync` heads
|
||||
|
|
@ -845,18 +820,17 @@ fn meta_update_grows_cascade_in_dag() {
|
|||
// Simulate the executor growing the cascade in-DAG (`relock = false` — a
|
||||
// cascade child must not re-lock and revert the parent's bump).
|
||||
for agent in ["alice", "bob"] {
|
||||
q.append_subgraph(
|
||||
id,
|
||||
&templates::rebuild_nodes(
|
||||
agent,
|
||||
templates::RebuildOpts {
|
||||
relock: false,
|
||||
graceful: false,
|
||||
},
|
||||
0,
|
||||
),
|
||||
meta_lock.node_id,
|
||||
let job = Job::new();
|
||||
templates::rebuild_nodes(
|
||||
&job,
|
||||
agent,
|
||||
templates::RebuildOpts {
|
||||
relock: false,
|
||||
graceful: false,
|
||||
},
|
||||
None,
|
||||
);
|
||||
q.append_subgraph(id, job, meta_lock.node_id);
|
||||
}
|
||||
q.complete_node(meta_lock.node_id, Ok(()));
|
||||
// Still ONE DAG — no child DAGs — and both cascade rebuild subgraphs root
|
||||
|
|
@ -1119,15 +1093,21 @@ fn cancelled_power_op_runs_no_compensating_node() {
|
|||
),
|
||||
];
|
||||
for (name, writes_intent, spec) in cases {
|
||||
let q = JobQueue::new(1);
|
||||
let id = submit(&q, spec);
|
||||
// Read the intent head off the submitted DAG rather than out of
|
||||
// the spec: a declared job holds its own nodes and inserts them.
|
||||
assert_eq!(
|
||||
spec.nodes
|
||||
q.snapshot()
|
||||
.iter()
|
||||
.any(|n| matches!(n.kind, NodeKind::SetWanted { .. })),
|
||||
.find(|d| d.id == id)
|
||||
.expect("submitted dag")
|
||||
.nodes
|
||||
.iter()
|
||||
.any(|n| n.kind == "set_wanted"),
|
||||
writes_intent,
|
||||
"{name} intent head (graceful={graceful}, running={running})"
|
||||
);
|
||||
let q = JobQueue::new(1);
|
||||
let id = submit(&q, spec);
|
||||
assert!(q.cancel(id), "cancelled while queued");
|
||||
assert_eq!(state_of(&q, id), State::Cancelled);
|
||||
assert!(
|
||||
|
|
@ -1272,7 +1252,7 @@ fn deploy_apply_grows_rebuild_subgraph_and_finalizes_after_it() {
|
|||
// gate immediately and letting the deploy "finish" before it had built.
|
||||
let grown = q.append_subgraph(
|
||||
id,
|
||||
&templates::deploy_rebuild_nodes("agent-a", 11),
|
||||
templates::deploy_rebuild_nodes("agent-a", 11),
|
||||
apply.node_id,
|
||||
);
|
||||
assert!(!grown.is_empty(), "subgraph grafted onto the apply node");
|
||||
|
|
@ -1330,7 +1310,7 @@ fn deploy_dag_skips_finalize_but_still_tails_a_failed_graft() {
|
|||
let apply = claim_one(&q);
|
||||
q.append_subgraph(
|
||||
id,
|
||||
&templates::deploy_rebuild_nodes("agent-a", 13),
|
||||
templates::deploy_rebuild_nodes("agent-a", 13),
|
||||
apply.node_id,
|
||||
);
|
||||
q.complete_node(apply.node_id, Ok(()));
|
||||
|
|
|
|||
Loading…
Reference in a new issue