refactor(job-queue): let resource_deps say (resource, units)
It never produced a `Dep::Node`, so returning `Vec<Dep<Resource>>` made every caller match a variant that cannot occur. `running_transients` paid for it with a two-arm match to pull the agent out of a lease edge. `Vec<(Resource, u32)>` says the same thing in the type, and is what the job builder's `.needs_units(name, count)` takes — the insertion path wraps it back into a `Dep::Resource` at the one place that still speaks in edges.
This commit is contained in:
parent
f161f8e40f
commit
9be7731c5e
2 changed files with 18 additions and 28 deletions
|
|
@ -170,7 +170,11 @@ fn insert_group(
|
||||||
let mut ids: Vec<NodeId> = Vec::with_capacity(nodes.len());
|
let mut ids: Vec<NodeId> = Vec::with_capacity(nodes.len());
|
||||||
for ns in nodes {
|
for ns in nodes {
|
||||||
let payload = ns.kind.clone();
|
let payload = ns.kind.clone();
|
||||||
let mut deps = payload.resource_deps();
|
let mut deps: Vec<Dep<Resource>> = payload
|
||||||
|
.resource_deps()
|
||||||
|
.into_iter()
|
||||||
|
.map(|(name, count)| Dep::Resource { name, count })
|
||||||
|
.collect();
|
||||||
for d in &ns.deps {
|
for d in &ns.deps {
|
||||||
deps.push(Dep::Node {
|
deps.push(Dep::Node {
|
||||||
id: ids[dep_index(d.on)],
|
id: ids[dep_index(d.on)],
|
||||||
|
|
@ -450,17 +454,14 @@ impl JobQueue {
|
||||||
.nodes()
|
.nodes()
|
||||||
.filter(|n| matches!(n.state, State::Running))
|
.filter(|n| matches!(n.state, State::Running))
|
||||||
.filter_map(|n| {
|
.filter_map(|n| {
|
||||||
let agent = n
|
let agent =
|
||||||
.payload
|
n.payload
|
||||||
.resource_deps()
|
.resource_deps()
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.find_map(|d| match d {
|
.find_map(|(name, _)| match name {
|
||||||
Dep::Resource {
|
Resource::Agent(a) => Some(a),
|
||||||
name: Resource::Agent(a),
|
_ => None,
|
||||||
..
|
})?;
|
||||||
} => Some(a),
|
|
||||||
_ => None,
|
|
||||||
})?;
|
|
||||||
Some(RunningTransient {
|
Some(RunningTransient {
|
||||||
agent,
|
agent,
|
||||||
label: n.payload.as_str().to_owned(),
|
label: n.payload.as_str().to_owned(),
|
||||||
|
|
|
||||||
|
|
@ -4,8 +4,6 @@
|
||||||
//! payload `N`; here `R` is [`Resource`] and `N` is [`NodeKind`] directly (each
|
//! payload `N`; here `R` is [`Resource`] and `N` is [`NodeKind`] directly (each
|
||||||
//! variant carries the agent it targets).
|
//! variant carries the agent it targets).
|
||||||
|
|
||||||
use hive_jobq::Dep;
|
|
||||||
|
|
||||||
use super::model::NodeKind;
|
use super::model::NodeKind;
|
||||||
|
|
||||||
/// The two resource classes the queue gates concurrency on, as the crate's
|
/// The two resource classes the queue gates concurrency on, as the crate's
|
||||||
|
|
@ -36,7 +34,7 @@ pub enum Resource {
|
||||||
}
|
}
|
||||||
|
|
||||||
impl NodeKind {
|
impl NodeKind {
|
||||||
/// The [`Dep::Resource`] edges this node must acquire to run, derived from
|
/// The resources this node must acquire to run — `(name, units)` — derived from
|
||||||
/// its kind + agent: a build slot for nix-heavy kinds
|
/// its kind + agent: a build slot for nix-heavy kinds
|
||||||
/// ([`NodeKind::needs_build_slot`]) and the agent lease for
|
/// ([`NodeKind::needs_build_slot`]) and the agent lease for
|
||||||
/// container-affecting kinds ([`NodeKind::needs_lease`]). Lease-exempt
|
/// container-affecting kinds ([`NodeKind::needs_lease`]). Lease-exempt
|
||||||
|
|
@ -50,25 +48,16 @@ impl NodeKind {
|
||||||
/// (`try_acquire_all`) — a node never holds one resource while waiting on
|
/// (`try_acquire_all`) — a node never holds one resource while waiting on
|
||||||
/// another, so the multi-resource kinds (a `MetaLock` wants a build slot
|
/// another, so the multi-resource kinds (a `MetaLock` wants a build slot
|
||||||
/// *and* the meta window) cannot deadlock against each other.
|
/// *and* the meta window) cannot deadlock against each other.
|
||||||
pub fn resource_deps(&self) -> Vec<Dep<Resource>> {
|
pub fn resource_deps(&self) -> Vec<(Resource, u32)> {
|
||||||
let mut deps = Vec::new();
|
let mut deps = Vec::new();
|
||||||
if self.needs_build_slot() {
|
if self.needs_build_slot() {
|
||||||
deps.push(Dep::Resource {
|
deps.push((Resource::BuildSlot, 1));
|
||||||
name: Resource::BuildSlot,
|
|
||||||
count: 1,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
if self.needs_lease() {
|
if self.needs_lease() {
|
||||||
deps.push(Dep::Resource {
|
deps.push((Resource::Agent(self.agent().to_owned()), 1));
|
||||||
name: Resource::Agent(self.agent().to_owned()),
|
|
||||||
count: 1,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
if self.needs_meta_window() {
|
if self.needs_meta_window() {
|
||||||
deps.push(Dep::Resource {
|
deps.push((Resource::MetaWindow, 1));
|
||||||
name: Resource::MetaWindow,
|
|
||||||
count: 1,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
deps
|
deps
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue