jobq: make NodeGuid an actual guid
Third time the operator asked for a guid and got a substitute: first an
i64 index, then a {random job id, per-builder counter} pair. The pair was
defensible in isolation -- a foreign handle misses rather than colliding,
with no new dependency -- but "an equivalent that avoids a dep" is a
counter-proposal, not an implementation.
It is also simpler as a guid, which was the question asked: NodeGuid(Uuid)
drops the `job` field, the `next_seq` counter, `fresh_job_id()` and the
`Cell` import, and halves the type's doc. One random draw per node rather
than one per builder -- noise next to what a node does when it runs.
uuid 1.24 was already in Cargo.lock as a transitive dependency, so this
adds an edge rather than a package.
This commit is contained in:
parent
9c97365f8f
commit
c82853af5a
4 changed files with 15 additions and 40 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -1758,6 +1758,7 @@ dependencies = [
|
||||||
"serde",
|
"serde",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
"thiserror 2.0.18",
|
"thiserror 2.0.18",
|
||||||
|
"uuid",
|
||||||
]
|
]
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
|
|
|
||||||
|
|
@ -64,6 +64,7 @@ hive-sock-client = { path = "hive-sock-client" }
|
||||||
hive-types = { path = "hive-types" }
|
hive-types = { path = "hive-types" }
|
||||||
thiserror = "2"
|
thiserror = "2"
|
||||||
tower-http = { version = "0.7", features = ["fs"] }
|
tower-http = { version = "0.7", features = ["fs"] }
|
||||||
|
uuid = { version = "1", features = ["v4"] }
|
||||||
rmcp = { version = "2", default-features = false, features = [
|
rmcp = { version = "2", default-features = false, features = [
|
||||||
"server",
|
"server",
|
||||||
"macros",
|
"macros",
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,7 @@ chrono = { workspace = true }
|
||||||
enumflags2.workspace = true
|
enumflags2.workspace = true
|
||||||
serde = { workspace = true }
|
serde = { workspace = true }
|
||||||
thiserror = { workspace = true }
|
thiserror = { workspace = true }
|
||||||
|
uuid = { workspace = true }
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
serde_json = { workspace = true }
|
serde_json = { workspace = true }
|
||||||
|
|
|
||||||
|
|
@ -26,7 +26,7 @@
|
||||||
//! silent reorder — a builder that sorted for you would quietly accept a shape
|
//! silent reorder — a builder that sorted for you would quietly accept a shape
|
||||||
//! the graph itself cannot express.
|
//! the graph itself cannot express.
|
||||||
|
|
||||||
use std::cell::{Cell, RefCell};
|
use std::cell::RefCell;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
|
|
||||||
use crate::{Dep, DepWhen, Graph, GraphError, NodeId, TerminalState};
|
use crate::{Dep, DepWhen, Graph, GraphError, NodeId, TerminalState};
|
||||||
|
|
@ -38,32 +38,14 @@ use crate::{Dep, DepWhen, Graph, GraphError, NodeId, TerminalState};
|
||||||
/// insert time and are meaningful system-wide, while this is a builder-local
|
/// insert time and are meaningful system-wide, while this is a builder-local
|
||||||
/// label that stops existing once the job is inserted.
|
/// label that stops existing once the job is inserted.
|
||||||
///
|
///
|
||||||
/// The `job` half is what makes it safe to compare. A [`NodeRef`] converts into
|
/// It is a v4 uuid because it has to survive leaving its builder. A [`NodeRef`]
|
||||||
/// a bare `NodeGuid`, dropping the borrow that ties it to its builder — so a
|
/// converts into a bare `NodeGuid`, dropping the borrow that tied it to the
|
||||||
/// handle *can* be carried to a different job (captured by an inner closure,
|
/// builder — so a handle *can* be carried into a different job (captured by an
|
||||||
/// say). Were the identity a plain per-builder counter, the two jobs' `0`s would
|
/// inner closure, say). Under a per-builder counter the two jobs' `0`s would be
|
||||||
/// be equal and that handle would silently resolve to an unrelated node. With a
|
/// equal and that handle would silently address an unrelated node; drawn at
|
||||||
/// per-builder random half it simply isn't found, and the insert fails by name.
|
/// random per node it simply isn't found, and the insert fails by name.
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||||
pub struct NodeGuid {
|
pub struct NodeGuid(uuid::Uuid);
|
||||||
/// Identifies the builder that issued this handle. Random per builder, so
|
|
||||||
/// two jobs' handles never compare equal.
|
|
||||||
job: u64,
|
|
||||||
/// Position within that builder — the cheap half, since `job` already
|
|
||||||
/// separates one builder's handles from another's.
|
|
||||||
seq: u32,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// A fresh identity for one builder.
|
|
||||||
///
|
|
||||||
/// `RandomState` is seeded from the process's random state and advances per
|
|
||||||
/// instance, so each builder gets a distinct, unguessable value — enough to
|
|
||||||
/// make a foreign handle a miss rather than a collision. No `rand` dependency
|
|
||||||
/// for what is a collision-avoidance job, not a cryptographic one.
|
|
||||||
fn fresh_job_id() -> u64 {
|
|
||||||
use std::hash::BuildHasher as _;
|
|
||||||
std::collections::hash_map::RandomState::new().hash_one(0_u8)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Why a job could not be inserted.
|
/// Why a job could not be inserted.
|
||||||
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
|
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
|
||||||
|
|
@ -117,10 +99,6 @@ struct Pending<N, R> {
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub struct JobBuilder<N, R> {
|
pub struct JobBuilder<N, R> {
|
||||||
nodes: RefCell<Vec<Pending<N, R>>>,
|
nodes: RefCell<Vec<Pending<N, R>>>,
|
||||||
/// Identifies this builder in every handle it issues.
|
|
||||||
job: u64,
|
|
||||||
/// Position within this builder — the `seq` half of a [`NodeGuid`].
|
|
||||||
next_seq: Cell<u32>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Hand-written rather than derived: `#[derive(Default)]` would demand
|
// Hand-written rather than derived: `#[derive(Default)]` would demand
|
||||||
|
|
@ -129,8 +107,6 @@ impl<N, R> Default for JobBuilder<N, R> {
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self {
|
Self {
|
||||||
nodes: RefCell::new(Vec::new()),
|
nodes: RefCell::new(Vec::new()),
|
||||||
job: fresh_job_id(),
|
|
||||||
next_seq: Cell::new(0),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -160,11 +136,7 @@ impl<N, R> JobBuilder<N, R> {
|
||||||
/// The returned handle is where those are declared; it is [`Copy`], so it
|
/// The returned handle is where those are declared; it is [`Copy`], so it
|
||||||
/// can be named as a dependency as many times as needed.
|
/// can be named as a dependency as many times as needed.
|
||||||
pub fn node(&self, payload: N) -> NodeRef<'_, N, R> {
|
pub fn node(&self, payload: N) -> NodeRef<'_, N, R> {
|
||||||
let guid = NodeGuid {
|
let guid = NodeGuid(uuid::Uuid::new_v4());
|
||||||
job: self.job,
|
|
||||||
seq: self.next_seq.get(),
|
|
||||||
};
|
|
||||||
self.next_seq.set(guid.seq + 1);
|
|
||||||
self.nodes.borrow_mut().push(Pending {
|
self.nodes.borrow_mut().push(Pending {
|
||||||
guid,
|
guid,
|
||||||
payload,
|
payload,
|
||||||
|
|
@ -558,9 +530,9 @@ mod tests {
|
||||||
/// job — an inner closure capturing an outer handle, say — must not
|
/// job — an inner closure capturing an outer handle, say — must not
|
||||||
/// silently address whichever node happens to sit at the same position.
|
/// silently address whichever node happens to sit at the same position.
|
||||||
///
|
///
|
||||||
/// This is what the random half of a [`NodeGuid`] buys: with a plain
|
/// This is what a random [`NodeGuid`] buys: with a plain per-builder
|
||||||
/// per-builder counter both jobs' first handles would be equal, and the
|
/// counter both jobs' first handles would be equal, and the edge below
|
||||||
/// edge below would resolve, wrongly, to the second job's own node.
|
/// would resolve, wrongly, to the second job's own node.
|
||||||
#[test]
|
#[test]
|
||||||
fn a_handle_from_another_job_is_not_silently_resolved() {
|
fn a_handle_from_another_job_is_not_silently_resolved() {
|
||||||
let mut g = graph();
|
let mut g = graph();
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue