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

refactor(#2949): the mutex holds the scheduler, not a wrapper

Browse source 
`QueueInner` existed to hold the scheduler *and* a per-node side map. The
map is gone, so it was a struct around one field — and worse, a struct of
a type `hive_jobq` cannot drive: the crate's run-loop seam takes
`&Arc<Mutex<Scheduler<..>>>` specifically.

So `JobQueue` now holds `Arc<Mutex<Sched>>` directly, where `Sched` is
just `Scheduler<NodeKind, Resource>`. Its six methods become free
functions over `&Sched`; all six are `DagView` projections, i.e. the code
the endpoint rework is going to delete anyway, so this does not entrench
them.

This is the precondition for c0re calling `claim_next`, not that switch
itself — `run_worker` still claims through `claim_ready`. Landing it
separately keeps the type change reviewable on its own.
This commit is contained in:
atlas 2026-08-02 18:44:12 +02:00 • committed by mara
commit be1060e52f
2 changed files with 199 additions and 213 deletions
Download patch file Download diff file Expand all files Collapse all files

121
hive-c0re/src/job_queue/mod.rs
View file

@ -36,7 +36,7 @@ pub mod templates;
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
use std::sync::Mutex; use std::sync::{Arc, Mutex};
use chrono::{DateTime, Utc}; use chrono::{DateTime, Utc};
use hive_host_sock::jobs::NodeView; use hive_host_sock::jobs::NodeView;
@ -109,27 +109,29 @@ struct DagMeta {
created_at: DateTime<Utc>, created_at: DateTime<Utc>,
} }
/// The mutable queue state behind the mutex: **just the crate scheduler**. /// The crate scheduler, specialised to this host's node + resource types.
///
/// A **DAG is a single container node** ([`NodeKind::Dag`], `parent = None`) /// A **DAG is a single container node** ([`NodeKind::Dag`], `parent = None`)
/// whose subtree is the DAG's work — so the container's `NodeId` is the DAG id, /// whose subtree is the DAG's work — so the container's `NodeId` is the DAG id,
/// its rolled-up state is the DAG state, and there are no grouping side-tables: /// its rolled-up state is the DAG state, and there are no grouping side-tables:
/// membership + meta are graph queries ([`QueueInner::container`] / /// membership + meta are graph queries ([`container`] / [`dag_meta`] + the
/// [`QueueInner::dag_meta`] + the `hive_jobq::Graph` accessors). One shared /// `hive_jobq::Graph` accessors). One shared crate [`Graph`] holds every DAG.
/// crate [`Graph`] holds every DAG.
/// ///
/// There is deliberately **no per-node side map** any more. The last one held /// There is deliberately **no wrapper struct and no per-node side map**. The
/// the `build_logs` row id; that link now lives on the log row itself /// last map held the `build_logs` row id; that link now lives on the log row
/// (`build_logs.node_id`), so it survives a restart and needs no lock held /// itself (`build_logs.node_id`). With nothing else to guard, the mutex holds
/// alongside the scheduler's — which is what lets the scheduler's own lock be /// the scheduler *directly* — which is what lets `hive_jobq` drive the run loop
/// the only one the run loop takes. /// (it takes `&Arc<Mutex<Scheduler<..>>>`, a type a host-side wrapper could not
struct QueueInner { /// satisfy).
sched: Scheduler<NodeKind, Resource>, type Sched = Scheduler<NodeKind, Resource>;
}
/// The queue. Lives on `Coordinator` (one per hive-c0re process); a single /// The queue. Lives on `Coordinator` (one per hive-c0re process); a single
/// scheduler task ([`scheduler::run_worker`]) drives it. /// scheduler task ([`scheduler::run_worker`]) drives it.
pub struct JobQueue { pub struct JobQueue {
inner: Mutex<QueueInner>, /// The scheduler, held directly rather than behind a host-side wrapper —
/// `hive_jobq`'s run-loop seam takes `&Arc<Mutex<Scheduler<..>>>`, so this
/// *is* the type the crate drives.
sched: Arc<Mutex<Sched>>,
/// Wakes the scheduler when something new arrives or state changed. /// Wakes the scheduler when something new arrives or state changed.
pub(crate) notify: Notify, pub(crate) notify: Notify,
} }
@ -173,12 +175,11 @@ fn outcome_of(result: Result<(), String>) -> Outcome {
/// # Errors /// # Errors
/// Propagates a crate graph-insert error (malformed dep/parent / dep-scope). /// Propagates a crate graph-insert error (malformed dep/parent / dep-scope).
fn insert_group( fn insert_group(
inner: &mut QueueInner, inner: &mut Sched,
declare: impl FnOnce(&Job), declare: impl FnOnce(&Job),
group_parent: Option<NodeId>, group_parent: Option<NodeId>,
) -> anyhow::Result<()> { ) -> anyhow::Result<()> {
inner inner
.sched
.insert_job(group_parent, |b| { .insert_job(group_parent, |b| {
declare(b); declare(b);
// c0re names no handles: a DAG is addressed by its container node, // c0re names no handles: a DAG is addressed by its container node,
@ -199,15 +200,13 @@ impl JobQueue {
u32::try_from(build_slots.max(1)).unwrap_or(u32::MAX), u32::try_from(build_slots.max(1)).unwrap_or(u32::MAX),
); );
Self { Self {
inner: Mutex::new(QueueInner { sched: Arc::new(Mutex::new(Scheduler::new(Graph::new(), table))),
sched: Scheduler::new(Graph::new(), table),
}),
notify: Notify::new(), notify: Notify::new(),
} }
} }
fn lock(&self) -> std::sync::MutexGuard<'_, QueueInner> { fn lock(&self) -> std::sync::MutexGuard<'_, Sched> {
self.inner.lock().expect("job_queue mutex poisoned") self.sched.lock().expect("job_queue mutex poisoned")
} }
/// Submit a DAG: insert a [`NodeKind::Dag`] **container node** carrying the /// Submit a DAG: insert a [`NodeKind::Dag`] **container node** carrying the
@ -225,7 +224,6 @@ impl JobQueue {
pub fn submit<F: FnOnce(&Job)>(&self, spec: DagSpec<F>) -> anyhow::Result<u64> { pub fn submit<F: FnOnce(&Job)>(&self, spec: DagSpec<F>) -> anyhow::Result<u64> {
let mut inner = self.lock(); let mut inner = self.lock();
let container = inner let container = inner
.sched
.append( .append(
NodeKind::Dag { NodeKind::Dag {
source: spec.source, source: spec.source,
@ -241,7 +239,7 @@ impl JobQueue {
// `Finishing` and its children become runnable — it never needs claiming // `Finishing` and its children become runnable — it never needs claiming
// or executing, and stays out of `claim_ready`. It rolls up terminal when // or executing, and stays out of `claim_ready`. It rolls up terminal when
// its whole subtree settles (that's the DAG-done signal). // its whole subtree settles (that's the DAG-done signal).
inner.sched.complete(container, Outcome::Done); inner.complete(container, Outcome::Done);
drop(inner); drop(inner);
self.notify.notify_one(); self.notify.notify_one();
Ok(container.get()) Ok(container.get())
@ -255,15 +253,15 @@ impl JobQueue {
pub fn claim_ready(&self) -> Vec<Claim> { pub fn claim_ready(&self) -> Vec<Claim> {
let mut inner = self.lock(); let mut inner = self.lock();
let inner = &mut *inner; let inner = &mut *inner;
let started = inner.sched.settle(); let started = inner.settle();
let mut claims = Vec::with_capacity(started.len()); let mut claims = Vec::with_capacity(started.len());
for id in started { for id in started {
let Some(node) = inner.sched.graph().node(id) else { let Some(node) = inner.graph().node(id) else {
continue; continue;
}; };
let kind = node.payload.clone(); let kind = node.payload.clone();
let agent = node.payload.agent().to_owned(); let agent = node.payload.agent().to_owned();
let Some(container) = inner.sched.graph().root_of(id) else { let Some(container) = inner.graph().root_of(id) else {
continue; continue;
}; };
claims.push(Claim { claims.push(Claim {
@ -291,7 +289,7 @@ impl JobQueue {
// complete) to express "grew nothing". The shared part is the outcome // complete) to express "grew nothing". The shared part is the outcome
// mapping, and that's a free fn. // mapping, and that's a free fn.
let mut inner = self.lock(); let mut inner = self.lock();
inner.sched.complete(node_id, outcome_of(result)); inner.complete(node_id, outcome_of(result));
drop(inner); drop(inner);
self.notify.notify_one(); self.notify.notify_one();
} }
@ -304,7 +302,7 @@ impl JobQueue {
/// `Job::default()`. /// `Job::default()`.
#[must_use] #[must_use]
pub fn new_job(&self) -> Job { pub fn new_job(&self) -> Job {
self.lock().sched.new_job() self.lock().new_job()
} }
/// [`JobQueue::complete_node`] plus the work the node declared while it ran. /// [`JobQueue::complete_node`] plus the work the node declared while it ran.
@ -318,10 +316,7 @@ impl JobQueue {
// A rejected grown job is logged, not propagated: the node's own work // A rejected grown job is logged, not propagated: the node's own work
// already ran, and refusing to complete it here would both misreport // already ran, and refusing to complete it here would both misreport
// that and wedge the DAG on a node stuck `Running`. // that and wedge the DAG on a node stuck `Running`.
if let Err(e) = inner if let Err(e) = inner.complete_growing(node_id, outcome_of(result), grown) {
.sched
.complete_growing(node_id, outcome_of(result), grown)
{
tracing::error!( tracing::error!(
node = node_id.get(), node = node_id.get(),
error = %e, error = %e,
@ -354,10 +349,10 @@ impl JobQueue {
/// just that branch. Nothing here knows about DAGs. /// just that branch. Nothing here knows about DAGs.
pub fn cancel(&self, id: u64) -> bool { pub fn cancel(&self, id: u64) -> bool {
let mut inner = self.lock(); let mut inner = self.lock();
let Some(node) = inner.sched.graph().resolve_id(id) else { let Some(node) = inner.graph().resolve_id(id) else {
return false; return false;
}; };
if !inner.sched.cancel_node(node) { if !inner.cancel_node(node) {
return false; return false;
} }
drop(inner); drop(inner);
@ -377,12 +372,8 @@ impl JobQueue {
#[must_use] #[must_use]
pub fn first_error(&self, dag_id: u64) -> Option<String> { pub fn first_error(&self, dag_id: u64) -> Option<String> {
let inner = self.lock(); let inner = self.lock();
let container = inner.container(dag_id)?; let container = container(&inner, dag_id)?;
inner inner.graph().first_error(container).map(ToOwned::to_owned)
.sched
.graph()
.first_error(container)
.map(ToOwned::to_owned)
} }
/// `(agent, label, takes_container_down)` for the live transient-pill set, /// `(agent, label, takes_container_down)` for the live transient-pill set,
@ -414,7 +405,6 @@ impl JobQueue {
pub fn running_transients(&self) -> Vec<RunningTransient> { pub fn running_transients(&self) -> Vec<RunningTransient> {
let inner = self.lock(); let inner = self.lock();
inner inner
.sched
.graph() .graph()
.nodes() .nodes()
.filter(|n| matches!(n.state, State::Running)) .filter(|n| matches!(n.state, State::Running))
@ -443,9 +433,11 @@ impl JobQueue {
#[must_use] #[must_use]
pub fn snapshot(&self) -> Vec<DagView> { pub fn snapshot(&self) -> Vec<DagView> {
let inner = self.lock(); let inner = self.lock();
let mut ids = inner.visible_dags(); let mut ids = visible_dags(&inner);
ids.sort_unstable_by_key(|c| c.get()); ids.sort_unstable_by_key(|c| c.get());
ids.into_iter().filter_map(|c| inner.dag_view(c)).collect() ids.into_iter()
.filter_map(|c| dag_view(&inner, c))
.collect()
} }
/// Number of live (non-terminal) DAGs — tests + diagnostics. /// Number of live (non-terminal) DAGs — tests + diagnostics.
@ -453,22 +445,18 @@ impl JobQueue {
#[must_use] #[must_use]
pub fn live_count(&self) -> usize { pub fn live_count(&self) -> usize {
let inner = self.lock(); let inner = self.lock();
inner containers(&inner)
.containers()
.into_iter() .into_iter()
.filter(|&c| inner.sched.graph().is_settled(c) == Some(false)) .filter(|&c| inner.graph().is_settled(c) == Some(false))
.count() .count()
} }
} }
impl QueueInner {
/// The container node of `dag_id` — the `NodeKind::Dag` root whose id equals /// The container node of `dag_id` — the `NodeKind::Dag` root whose id equals
/// `dag_id`. `NodeId` is un-fabricable from a raw `u64`, so this is a search. /// `dag_id`. `NodeId` is un-fabricable from a raw `u64`, so this is a search.
fn container(&self, dag_id: u64) -> Option<NodeId> { fn container(sched: &Sched, dag_id: u64) -> Option<NodeId> {
self.sched.graph().nodes().find_map(|n| { sched.graph().nodes().find_map(|n| {
(n.parent.is_none() (n.parent.is_none() && n.id.get() == dag_id && matches!(n.payload, NodeKind::Dag { .. }))
&& n.id.get() == dag_id
&& matches!(n.payload, NodeKind::Dag { .. }))
.then_some(n.id) .then_some(n.id)
}) })
} }
@ -476,12 +464,12 @@ impl QueueInner {
/// The container's carried domain metadata as an owned read-view. The data /// The container's carried domain metadata as an owned read-view. The data
/// lives solely in the [`NodeKind::Dag`] payload — this is a derived read, /// lives solely in the [`NodeKind::Dag`] payload — this is a derived read,
/// not a stored side-table. /// not a stored side-table.
fn dag_meta(&self, container: NodeId) -> Option<DagMeta> { fn dag_meta(sched: &Sched, container: NodeId) -> Option<DagMeta> {
let NodeKind::Dag { let NodeKind::Dag {
source, source,
reason, reason,
created_at, created_at,
} = &self.sched.graph().node(container)?.payload } = &sched.graph().node(container)?.payload
else { else {
return None; return None;
}; };
@ -501,8 +489,8 @@ impl QueueInner {
/// `None` when every work node is `Done` or `Skipped` — a fully-settled /// `None` when every work node is `Done` or `Skipped` — a fully-settled
/// DAG drops out of the snapshot entirely (a `Failed` one lingers until /// DAG drops out of the snapshot entirely (a `Failed` one lingers until
/// aged out). /// aged out).
fn dag_view(&self, container: NodeId) -> Option<DagView> { fn dag_view(sched: &Sched, container: NodeId) -> Option<DagView> {
let meta = self.dag_meta(container)?; let meta = dag_meta(sched, container)?;
let mut nodes = Vec::new(); let mut nodes = Vec::new();
// Whether anything in this DAG still has an outcome worth showing. // Whether anything in this DAG still has an outcome worth showing.
// Kept separate from `nodes` being non-empty: skipped nodes ride the // Kept separate from `nodes` being non-empty: skipped nodes ride the
@ -514,7 +502,7 @@ impl QueueInner {
// from a `Done`-filtered node set, so the host computes them here. // from a `Done`-filtered node set, so the host computes them here.
let mut started: Vec<DateTime<Utc>> = Vec::new(); let mut started: Vec<DateTime<Utc>> = Vec::new();
let mut finished: Vec<DateTime<Utc>> = Vec::new(); let mut finished: Vec<DateTime<Utc>> = Vec::new();
for node in self.sched.graph().descendants(container) { for node in sched.graph().descendants(container) {
let id = node.id; let id = node.id;
if let Some(s) = node.started_at { if let Some(s) = node.started_at {
started.push(s); started.push(s);
@ -582,7 +570,7 @@ impl QueueInner {
if !any_unsettled { if !any_unsettled {
return None; return None;
} }
let is_terminal = self.sched.graph().is_settled(container) == Some(true); let is_terminal = sched.graph().is_settled(container) == Some(true);
Some(DagView { Some(DagView {
id: container.get(), id: container.get(),
source: meta.source, source: meta.source,
@ -597,8 +585,8 @@ impl QueueInner {
/// When a DAG's work node finishes on `finished_at` — the max over its /// When a DAG's work node finishes on `finished_at` — the max over its
/// subtree (read off the graph `Node`, as unix seconds), for the history /// subtree (read off the graph `Node`, as unix seconds), for the history
/// cap ordering. /// cap ordering.
fn dag_finished_at(&self, container: NodeId) -> i64 { fn dag_finished_at(sched: &Sched, container: NodeId) -> i64 {
self.sched sched
.graph() .graph()
.descendants(container) .descendants(container)
.filter_map(|n| n.finished_at) .filter_map(|n| n.finished_at)
@ -608,8 +596,8 @@ impl QueueInner {
} }
/// Every DAG container node id in the graph. /// Every DAG container node id in the graph.
fn containers(&self) -> Vec<NodeId> { fn containers(sched: &Sched) -> Vec<NodeId> {
self.sched sched
.graph() .graph()
.nodes() .nodes()
.filter(|n| n.parent.is_none() && matches!(n.payload, NodeKind::Dag { .. })) .filter(|n| n.parent.is_none() && matches!(n.payload, NodeKind::Dag { .. }))
@ -621,12 +609,12 @@ impl QueueInner {
/// plus the newest [`MAX_HISTORY_DAGS`] terminal ones. Crate nodes for /// plus the newest [`MAX_HISTORY_DAGS`] terminal ones. Crate nodes for
/// evicted DAGs linger in the graph (bounded-prune is a Stage-C follow-up); /// evicted DAGs linger in the graph (bounded-prune is a Stage-C follow-up);
/// this filter is what bounds what the dashboard sees. /// this filter is what bounds what the dashboard sees.
fn visible_dags(&self) -> Vec<NodeId> { fn visible_dags(sched: &Sched) -> Vec<NodeId> {
let mut live: Vec<NodeId> = Vec::new(); let mut live: Vec<NodeId> = Vec::new();
let mut terminal: Vec<(NodeId, i64)> = Vec::new(); let mut terminal: Vec<(NodeId, i64)> = Vec::new();
for c in self.containers() { for c in containers(sched) {
if self.sched.graph().is_settled(c) == Some(true) { if sched.graph().is_settled(c) == Some(true) {
terminal.push((c, self.dag_finished_at(c))); terminal.push((c, dag_finished_at(sched, c)));
} else { } else {
live.push(c); live.push(c);
} }
@ -638,7 +626,6 @@ impl QueueInner {
kept.extend(terminal.into_iter().map(|(c, _)| c)); kept.extend(terminal.into_iter().map(|(c, _)| c));
kept kept
} }
}
/// Truncate a node error to [`MAX_ERROR_LEN`] on a char boundary, appending `…`. /// Truncate a node error to [`MAX_ERROR_LEN`] on a char boundary, appending `…`.
fn truncate_error(e: &str) -> String { fn truncate_error(e: &str) -> String {

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

@ -117,7 +117,6 @@ fn settle_rebuild_tail(q: &JobQueue, agent: &str, expect_ok: bool) {
fn declared_resources(q: &JobQueue, node_id: hive_jobq::NodeId) -> Vec<Resource> { fn declared_resources(q: &JobQueue, node_id: hive_jobq::NodeId) -> Vec<Resource> {
let inner = q.lock(); let inner = q.lock();
inner inner
.sched
.graph() .graph()
.node(node_id) .node(node_id)
.expect("node exists") .expect("node exists")