From 9e91bf7813560f95092125bc2bb11b416f6bd2f5 Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 2 Aug 2026 18:30:32 +0200 Subject: [PATCH] refactor(#2949): claim_one is the primitive, settle is it in a loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One-at-a-time claiming is what lets a caller choose between claiming again immediately and backing off — a batch return cannot express that choice, and the choice is the point: the run loop wants to know there was work before it decides whether to wait. `settle()` keeps its exact meaning as `while let Some(id) = claim_one()`. A node started by an earlier iteration is `Running`, not terminal, so it cannot satisfy another node's dependency in the same sweep; it only consumes resources. The crate's ~45 existing `settle()` assertions — which cover resource borrowing, cap-1 serialisation, roll-up and cancellation — are what verify that equivalence, so it is checked rather than argued. `None` means "nothing runnable right now", which is deliberately a different statement from "nothing pending": a node can be pending and unrunnable because its resources are held elsewhere. Cost stated rather than left to be found: each `claim_one` rescans the pending set, so `settle` is O(n^2) in nodes claimed where the single-pass version was O(n). The graph is bounded by history retention. --- hive-jobq/src/scheduler.rs | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/hive-jobq/src/scheduler.rs b/hive-jobq/src/scheduler.rs index ccdd017e..3fd46c3d 100644 --- a/hive-jobq/src/scheduler.rs +++ b/hive-jobq/src/scheduler.rs @@ -138,25 +138,42 @@ impl Scheduler { }) } - /// Claim every currently-runnable pending node and start it: node-deps + /// Claim **one** currently-runnable pending node and start it: node-deps /// satisfied and all resource-deps acquired atomically (all-or-nothing). - /// Each claimed node is marked `Running`, its acquired units recorded, and - /// its id returned for the runner to execute. A single pass suffices — a - /// node started here is `Running`, not terminal, so it cannot satisfy another - /// node's dependency in the same pass; it only consumes resources. + /// The node is marked `Running`, its acquired units recorded, and its id + /// returned for the caller to execute. `None` means nothing is runnable + /// right now — which is a different statement from "nothing is pending". + /// + /// One-at-a-time is the primitive on purpose: it lets the caller decide + /// between claiming again immediately and backing off, a choice a batch + /// return can't express. [`Self::settle`] is this in a loop. #[must_use] - pub fn settle(&mut self) -> Vec { + pub fn claim_one(&mut self) -> Option { let pending: Vec = self .graph .nodes() .filter(|n| n.state == State::Pending) .map(|n| n.id) .collect(); + pending + .into_iter() + .find(|&id| self.node_deps_satisfied(id) && self.try_start(id)) + } + + /// Claim every currently-runnable pending node. Equivalent to calling + /// [`Self::claim_one`] until it yields `None`: a node started by an earlier + /// iteration is `Running`, not terminal, so it cannot satisfy another + /// node's dependency here — it only consumes resources. + /// + /// ⚠️ Each iteration rescans the pending set, so this is O(n²) in the + /// number of nodes claimed where the old single-pass version was O(n). The + /// graph is bounded by history retention, so that is affordable; it is + /// stated rather than left to be discovered. + #[must_use] + pub fn settle(&mut self) -> Vec { let mut started = Vec::new(); - for id in pending { - if self.node_deps_satisfied(id) && self.try_start(id) { - started.push(id); - } + while let Some(id) = self.claim_one() { + started.push(id); } started }