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

job_queue: the reconcile fan-out declares from templates

Browse source 
exec.rs's Reconcile arm was the one construction site declaring a
resource inline in an executor rather than in templates.rs. It now calls
templates::fanned_out_mechanical, which is where every other declaration
lives -- construction sites state their own holdings.

That also fixes a test which could not fail. The old one claimed a
Reconcile and then re-declared the fan-out itself, commented "same two
calls the scheduler makes, in the same order" -- a copy of production
inside the test. Had exec.rs stopped declaring the lease, it would have
kept passing. The replacement calls the real function and asserts the
declaration, with no DAG run at all.

The other half of the old test -- that a descendant re-enters its
ancestor's grant instead of taking a second unit of a cap-1 lease -- is
hive-jobq's, tested there by
child_borrows_ancestor_grant_released_when_subtree_done and
nested_borrowers_never_deadlock.
This commit is contained in:
atlas 2026-08-02 20:53:13 +02:00 • committed by mara
commit 53cd010a12
3 changed files with 61 additions and 73 deletions
Download patch file Download diff file Expand all files Collapse all files

9
hive-c0re/src/job_queue/exec.rs
View file

@ -13,7 +13,6 @@ use anyhow::{Context as _, Result};
use hive_jobq::{NodeId, TerminalState};
use super::model::NodeKind;
use super::resource::Resource;
use crate::coordinator::Coordinator;
use crate::power::{ReconcileAction, reconcile_action};
@ -80,13 +79,7 @@ pub(super) async fn run_node(
}),
NodeKind::Reconcile { .. } => run_reconcile(coord, agent).await.map(|sub| {
if let Some(kind) = sub {
// `Start` / `Stop` declare the lease they run under. This node
// is their parent and holds it, so the declaration is a
// re-entrant borrow — no second unit, no deadlock. It exists so
// the requirement belongs to the node rather than to the fact
// that a `Reconcile` happens to fan it out.
let lease = Resource::Agent(kind.agent().to_owned());
let _ = job.node(kind).needs(lease);
super::templates::fanned_out_mechanical(&job, kind);
}
}),
NodeKind::Start { .. } => run_start(coord, agent).await,

17
hive-c0re/src/job_queue/templates.rs
View file

@ -80,6 +80,23 @@ fn resolve_approval_tails(b: &Job, approval_id: i64, root: Handle<'_>) {
}
}
/// Declare the mechanical node a [`NodeKind::Reconcile`] planner fans out
/// (`Start` / `Stop`) onto the builder it was handed while running.
///
/// `Start` / `Stop` declare the agent lease they run under. Their `Reconcile`
/// parent is holding it already, so the declaration is a **re-entrant borrow**
/// — no second unit, no deadlock. It exists so the requirement belongs to the
/// node rather than to the fact that a `Reconcile` happens to fan it out.
///
/// Lives here rather than inline in `exec.rs` for the same reason every other
/// declaration does: this is the one construction site that was hiding in an
/// executor, which meant the only test of it had to re-declare the same two
/// calls itself and would have kept passing if the executor changed.
pub(crate) fn fanned_out_mechanical(b: &Job, kind: NodeKind) {
let lease = Resource::Agent(kind.agent().to_owned());
let _ = b.node(kind).needs(lease);
}
/// Knobs for [`rebuild_nodes`]. A struct rather than two positional `bool`s so
/// a call site cannot silently swap them.
#[derive(Debug, Clone, Copy)]

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

@ -807,71 +807,6 @@ fn offline_agents_skip_mechanical_nodes_but_keep_reconcile() {
);
}
#[test]
fn a_fanned_out_start_declares_the_lease_and_re_enters_its_reconciles_grant() {
// `Start` / `Stop` / `PostSwap` were lease-exempt *as kinds*, which was only
// safe because every construction site fans them out from inside a
// lease-holding ancestor. Now they declare the lease themselves.
//
// The contract says that costs nothing — a descendant re-enters the
// ancestor's grant instead of taking a fresh unit. That is exactly the sort
// of claim that is true until a node is used from a second site, so it is
// pinned here rather than argued: the fanned-out `Start` must (a) actually
// carry the declaration, (b) still run under its parent's grant, and
// (c) not have consumed a second unit of a cap-1 lease.
let q = JobQueue::new(4);
let id = submit(
&q,
templates::reconcile_only("agent-a", Source::Manual, "converge".to_owned()),
);
// A competing DAG on the same agent, to prove the lease is genuinely held
// (and held *once*) across the fan-out.
let rival = submit(
&q,
templates::reconcile_only("agent-a", Source::Manual, "rival".to_owned()),
);
let reconcile = claim_one(&q);
assert_eq!(reconcile.dag_id, id);
assert_eq!(reconcile.kind.as_str(), "reconcile");
// What `run_reconcile` does on observing a down container with wanted=Up:
// declare into the builder it was handed, then hand it back with the
// completion. Same two calls the scheduler makes, in the same order.
let grown = q.new_job();
let kind = NodeKind::Start {
agent: "agent-a".to_owned(),
};
let lease = Resource::Agent(kind.agent().to_owned());
let _ = grown.node(kind).needs(lease);
q.complete_node_growing(reconcile.node_id, Ok(()), grown);
// (a) + (b): the child runs, under the parent that parked in `Finishing`.
let start = claim_one(&q);
assert_eq!(start.kind.as_str(), "start");
assert_eq!(
declared_resources(&q, start.node_id),
vec![Resource::Agent("agent-a".to_owned())],
"a fanned-out Start declares the lease it runs under"
);
// (c): one unit, not two. `claim_one` above already asserted the rival did
// not come back in the same pass; make the reason explicit.
assert!(
q.claim_ready().is_empty(),
"the rival DAG's Reconcile must still be blocked — the appended Start \
borrowed the grant rather than acquiring a second unit"
);
q.complete_node(start.node_id, Ok(()));
// Subtree terminal → the grant releases and the rival finally runs.
let rival_reconcile = claim_one(&q);
assert_eq!(rival_reconcile.dag_id, rival);
q.complete_node(rival_reconcile.node_id, Ok(()));
assert_eq!(state_of(&q, id), State::Done);
assert_eq!(state_of(&q, rival), State::Done);
}
#[test]
fn boot_sweep_nodes_declare_their_own_resources() {
// Regression, and the reason it needs its own test: `workers::auto_update`
@ -1212,6 +1147,49 @@ fn rebuild_reconcile_waits_for_the_whole_build_subtree() {
// `cancelled_power_op_runs_no_compensating_node`, which checks the DAG has no
// pending nodes left at all.
/// `Start` / `Stop` were lease-exempt *as kinds*, which was only safe because
/// every construction site fans them out from inside a lease-holding ancestor.
/// They declare the lease themselves now, and this pins that they do.
///
/// Was `a_fanned_out_start_declares_the_lease_and_re_enters_its_reconciles_
/// grant`, which submitted a `Reconcile`, claimed it, and then **re-declared
/// the fan-out inline** — *"same two calls the scheduler makes"*. That is a
/// copy of production in a test: had `exec.rs` stopped declaring the lease, it
/// would have kept passing. The declaration now lives in `templates::
/// fanned_out_mechanical`, so this calls the real thing.
///
/// The other half of the old test — that a descendant *re-enters* its
/// ancestor's grant rather than taking a second unit of a cap-1 lease — is
/// `hive_jobq`'s, and is tested there by
/// `child_borrows_ancestor_grant_released_when_subtree_done` and
/// `nested_borrowers_never_deadlock`.
#[test]
fn a_fanned_out_mechanical_node_declares_its_agent_lease() {
let q = JobQueue::new(4);
let id = submit(
&q,
DagSpec {
source: Source::Manual,
reason: "fan-out".to_owned(),
declare: Box::new(|b: &Job| {
templates::fanned_out_mechanical(
b,
NodeKind::Start {
agent: "agent-a".to_owned(),
},
);
}),
},
);
assert_eq!(declared_shape(&q, id), vec![row("start", None, &[])]);
assert_eq!(
declared_resources(&q, node_of(&q, id, "start")),
vec![Resource::Agent("agent-a".to_owned())],
"the fanned-out node carries the lease itself, rather than relying on \
whoever happened to fan it out"
);
}
// ---- cancel ----
#[test]