docs(#3034): give the brace one home instead of five
Measured after mara's "2/3 of this is docs, most of it duplicated": 243 of 380 added .rs lines were comments. The brace rationale was written out in full in `model.rs`, the `templates.rs` module header, `quiesce`, `rebuild_subtree` and `docs/coordinator.md` — five copies of one argument, which is why four docs needed correcting earlier in this branch. Correcting every copy preserves the thing that made them go stale. `docs/coordinator.md` (_Braces_) is now the single home. The rest state what a node *is* and point there. Also drops the per-operation DAG diagram from the `templates.rs` header, which the same doc already carries, and cuts `rebuild_subtree`'s node-by-node walkthrough down to the three choices a reader would otherwise undo — the code below it is the source of truth for the shape. Comments -59 lines, no behaviour change, 317 tests unchanged.
This commit is contained in:
atlas
parent
c2eafa7548
commit
0a14055a33
4 changed files with 45 additions and 116 deletions
|
|
@ -130,11 +130,8 @@ pub(super) async fn run_node(
|
|||
// reach `Finishing` so the nodes under it start.
|
||||
// - `Dag`: pure grouping container. The DAG's terminal side effect, if
|
||||
// any, is its own tail node in the graph.
|
||||
// - `DeployWindow`: pure resource holder — the meta window, agent lease
|
||||
// and build slot it declares stay held until its subtree settles.
|
||||
// - `AgentWindow`: pure resource holder for one agent's rebuild — holds
|
||||
// the lease so the build and the graceful-stop window can run as
|
||||
// siblings rather than one nested under the other.
|
||||
// - `DeployWindow` / `AgentWindow`: pure resource holders (braces) —
|
||||
// what they declare stays held until their subtree settles.
|
||||
NodeKind::Dag { .. } | NodeKind::DeployWindow { .. } | NodeKind::AgentWindow { .. } => {
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -159,36 +159,14 @@ pub enum NodeKind {
|
|||
/// those phases does — the id is the node's own payload, not something a
|
||||
/// DAG-level catch-all hands down.
|
||||
DeployWindow { agent: String, approval_id: i64 },
|
||||
/// Group root of a rebuild subtree, and the node that **owns the agent
|
||||
/// lease** for every phase below it. Performs no work of its own — same
|
||||
/// pure-resource-holder shape as [`NodeKind::DeployWindow`], scoped to one
|
||||
/// agent instead of a whole deploy.
|
||||
/// Group root of a rebuild subtree — the **brace**: declares the agent
|
||||
/// lease and the build slot, holds both for the whole subtree, and performs
|
||||
/// no work of its own. Same pure-resource-holder shape as
|
||||
/// [`NodeKind::DeployWindow`], scoped to one agent.
|
||||
///
|
||||
/// It exists so the lease is held *continuously* across the build, the
|
||||
/// graceful-stop window and the swap. That is what lets `Prebuild` and the
|
||||
/// `Signal` → `Drain` stop window run **concurrently**: they contend for
|
||||
/// different resources (a build slot vs. the agent), and without a brace
|
||||
/// the only way to order the stop after the build was to nest it under
|
||||
/// `Prebuild` — which hid the whole graceful-stop timeout behind the nix
|
||||
/// build, per agent, on every sweep.
|
||||
///
|
||||
/// ⚠️ Its children deliberately **do not declare
|
||||
/// [`Resource::Agent`](super::resource::Resource::Agent)**. Declaring a
|
||||
/// resource means "I need this exclusively", and the lease is single-unit —
|
||||
/// two siblings that both declared it could never run in parallel, which is
|
||||
/// the entire point of the brace. Holding it on the parent and omitting it
|
||||
/// on coordinated children is the opt-in "this subtree knows what it is
|
||||
/// doing" shape.
|
||||
///
|
||||
/// This costs nothing in observability: `running_transients` keys off the
|
||||
/// node's **payload** agent, not off a declared lease edge, so every child
|
||||
/// still lights its own pill and still reports its own
|
||||
/// [`NodeKind::takes_container_down`] to the crash watcher.
|
||||
///
|
||||
/// Sits *after* `MetaSync` rather than under it — a parent holds its
|
||||
/// resources for its whole subtree, so nesting this inside `MetaSync` would
|
||||
/// pin the **global** meta window across every agent's build and serialise
|
||||
/// the sweep.
|
||||
/// Its children declare no resources and borrow these grants, which is what
|
||||
/// lets `Prebuild` run beside the `Signal` → `Drain` window. Why braces
|
||||
/// exist and what they cost: `docs/coordinator.md`, _Braces_.
|
||||
AgentWindow { agent: String },
|
||||
/// Deploy phase 1 — **verify only, mutates nothing.** Drift-gate the
|
||||
/// approval's PR head, fetch it into the applied repo, and eval-verify the
|
||||
|
|
@ -460,13 +438,10 @@ impl NodeKind {
|
|||
// - `Reconcile` is a planner; it fans out `Start` / `Stop`, which carry
|
||||
// their own answer.
|
||||
// - `DeployWindow` brackets a deploy without itself stopping anything.
|
||||
// - `AgentWindow` likewise. It is the one that looks wrong: it *parents*
|
||||
// `Signal` / `Drain` / `StopForUpdate` / `Swap`, which all answer
|
||||
// `true`. But this is per-node, not per-subtree, and every one of
|
||||
// those children is in `running_transients` on its own — so the
|
||||
// suppression window is exactly the span where a child that really
|
||||
// takes the container down is running, not the whole rebuild. Saying
|
||||
// `true` here would widen it to cover the build and the tail, where a
|
||||
// vanished container is still a real crash.
|
||||
// - `AgentWindow` likewise, though it *parents* nodes that answer
|
||||
// `true`. This is per-node, not per-subtree, and each of those
|
||||
// children reports for itself — so `true` here would only widen
|
||||
// suppression over the build and tail, where a vanished container is
|
||||
// still a real crash.
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,18 +7,9 @@
|
|||
//!
|
||||
//! **The one sanctioned exception is a brace** — a pure-resource-holder root
|
||||
//! ([`NodeKind::AgentWindow`], [`NodeKind::DeployWindow`]) declaring for a
|
||||
//! coordinated subtree whose members then declare nothing. Forced by the lease
|
||||
//! being single-unit: two siblings that both declared it could never run
|
||||
//! concurrently. Why this is not a relapse: `docs/coordinator.md`.
|
||||
//!
|
||||
//! ```text
|
||||
//! rebuild(a): MetaSync(a) → AgentWindow(a){ Prebuild(a) → StopForUpdate(a) → Swap(a) →(ok) RebuildBookkeeping(a) } →(any) Reconcile(a)
|
||||
//! rebuild(a) graceful: … AgentWindow(a){ Prebuild(a) ∥ Signal(a) → Drain(a); both →(ok) StopForUpdate(a) → … } [boot sweep only]
|
||||
//! spawn(a): Provision(a) → Create(a) → WriteDropin(a) → Reconcile(a) [wanted=Up at approve]
|
||||
//! perm-change(a): WritePermFile(a) → «rebuild subgraph»
|
||||
//! meta-update(inp): MetaLock(inp) →«in-DAG rebuild subgraph per affected a»
|
||||
//! reparent(moves): Reparent(moves) [no rebuild — topology.json is read live]
|
||||
//! ```
|
||||
//! coordinated subtree whose members then declare nothing. See
|
||||
//! `docs/coordinator.md`, _Braces_ — which also carries the per-operation DAG
|
||||
//! shapes, so they are not restated here.
|
||||
//!
|
||||
//! Nodes are **named, not counted** — a template holds the handle
|
||||
//! [`JobBuilder::node`] hands back, so an edge says which node it waits on. Why that
|
||||
|
|
@ -130,22 +121,13 @@ pub(crate) fn fanned_out_mechanical(builder: &JobBuilder, kind: NodeKind) {
|
|||
|
||||
/// The graceful quiesce pair — `Signal` then `Drain`: ask the agent to run its
|
||||
/// stop-checkpoint turn, then wait for it to go quiet. Returns the `Drain`
|
||||
/// handle, which is what a caller edges its stop onto.
|
||||
/// handle, which is what a caller edges its stop onto. The container stays
|
||||
/// **up** throughout; the actual stop is the caller's next node.
|
||||
///
|
||||
/// The container is still **up** throughout; this only reaches a point where
|
||||
/// stopping is safe. The actual stop is the caller's next node.
|
||||
///
|
||||
/// `brace` must already hold [`Resource::Agent`] for its whole subtree. The two
|
||||
/// nodes therefore declare **nothing** and borrow that grant — which is what
|
||||
/// lets them run *beside* the brace's other children (a nix build, typically)
|
||||
/// instead of serialising against them. Declaring the lease here would make
|
||||
/// them mutually exclusive with any sibling that also declared it, since the
|
||||
/// lease is single-unit; see this module's header for the general rule.
|
||||
///
|
||||
/// They are **siblings under the brace, dep-ordered**, not nested. Nesting
|
||||
/// `Drain` under `Signal` is only necessary where `Signal` is itself the lease
|
||||
/// holder and the nesting is what keeps the grant continuous — with a brace
|
||||
/// above, that reason is gone.
|
||||
/// `brace` must already hold [`Resource::Agent`] for its subtree; the pair
|
||||
/// declares nothing and borrows it (see the module header). They are
|
||||
/// dep-ordered **siblings**, not nested — nesting is only load-bearing where
|
||||
/// `Signal` itself holds the lease, as in `submit.rs`'s `restart_chain`.
|
||||
pub(crate) fn quiesce<'a>(builder: &'a JobBuilder, agent: &str, brace: Handle<'a>) -> Handle<'a> {
|
||||
let a = || agent.to_owned();
|
||||
let signal = builder.node(NodeKind::Signal { agent: a() }).part_of(brace);
|
||||
|
|
@ -179,36 +161,22 @@ impl<'a> RebuildRoots<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
/// The rebuild node subtree (three group roots). `after`, when given, is the
|
||||
/// node this subgraph chains behind. Structure:
|
||||
/// - `MetaSync` (**root**): the meta-repo preamble (dir prep, agent sync,
|
||||
/// optional relock). Owns the global `MetaWindow` — and *only* for its own
|
||||
/// short duration, which is why it is a sibling root rather than the brace's
|
||||
/// parent: a resource is held across the holder's whole subtree, so parenting
|
||||
/// the rebuild under it would extend a hive-global window over every
|
||||
/// rebuild's nix build.
|
||||
/// - `AgentWindow` (**root**): `AfterOk` `MetaSync`. The brace — declares the
|
||||
/// build slot *and* the agent lease, atomically, and holds both for the whole
|
||||
/// subtree. Everything below it declares **nothing** and re-enters these
|
||||
/// grants.
|
||||
/// - `Prebuild` and the **quiesce chain** (`Signal` → `Drain`, graceful only)
|
||||
/// are siblings under the brace and run **concurrently** — they contend for
|
||||
/// different resources, so nesting the stop under the build (as this template
|
||||
/// did) only hid the graceful-stop timeout behind the nix build. The container
|
||||
/// is still up throughout the quiesce.
|
||||
/// - `StopForUpdate` (child of the brace): `AfterOk` **both** `Prebuild` and
|
||||
/// `Drain`. Waiting on the build is deliberate — running the drain early is
|
||||
/// the win, taking the container *down* early would be pure downtime.
|
||||
/// - `Swap` (child of `StopForUpdate`), then `RebuildBookkeeping` (`AfterOk` its
|
||||
/// sibling `Swap`): the Ok-only bookkeeping tail.
|
||||
/// - `Reconcile` (**last, root**): `AfterAny` `AgentWindow`, which rolls up
|
||||
/// terminal only once its whole subtree has settled — so it runs after the
|
||||
/// swap regardless of outcome, and as a top-level root it survives the
|
||||
/// cancel-cascade of a failed brace (recovery-start invariant). Fresh lease;
|
||||
/// the tiny gap is harmless, `Reconcile` converges idempotently.
|
||||
/// The rebuild node subtree — three group roots (`MetaSync`, the `AgentWindow`
|
||||
/// brace, `Reconcile`). `after`, when given, is the node it chains behind. The
|
||||
/// shape itself is in `docs/coordinator.md`; the code below is the source of
|
||||
/// truth for it, so only the three choices a reader would otherwise undo are
|
||||
/// called out here:
|
||||
///
|
||||
/// Why flattening the stop chain is safe, and why the lease-continuity argument
|
||||
/// that used to justify nesting it is satisfied by the brace: `docs/coordinator.md`.
|
||||
/// - **`MetaSync` is a sibling root, not the brace's parent** — it owns the
|
||||
/// *global* `MetaWindow`, and a resource is held across the holder's whole
|
||||
/// subtree, so parenting the rebuild under it would serialise every agent's
|
||||
/// nix build behind one hive-wide window.
|
||||
/// - **`StopForUpdate` waits on `Prebuild` as well as `Drain`** — running the
|
||||
/// drain early is the win; taking the container *down* early is pure downtime.
|
||||
/// - **`Reconcile` is a top-level root, not a child** — so it survives the
|
||||
/// cancel-cascade of a failed brace and still converges the container
|
||||
/// (recovery-start invariant). It takes a fresh lease; the gap is harmless
|
||||
/// because it is idempotent.
|
||||
fn rebuild_subtree<'a>(
|
||||
builder: &'a JobBuilder,
|
||||
agent: &str,
|
||||
|
|
|
|||
|
|
@ -558,16 +558,10 @@ fn rebuild_chain_declares_its_resources_on_the_brace() {
|
|||
// Was `rebuild_chain_declares_the_slot_where_the_nix_work_is` (and before
|
||||
// that `fifo_fairness_for_the_slot`).
|
||||
//
|
||||
// ⚠️ **This rename is a reversal, not tidying.** The old assertion read
|
||||
// "the slot follows the nix work and the lease follows the container": each
|
||||
// node declared the resource it personally needed, deliberately, so that the
|
||||
// requirement belonged to the node rather than to one DAG shape. The brace
|
||||
// inverts that for a coordinated subtree, and the reason is forced:
|
||||
// declaring a resource means *"I need this exclusively"* and the lease is
|
||||
// single-unit, so `Prebuild` and the quiesce chain — which exist to run
|
||||
// **concurrently** — could never overlap if both declared it. One holder
|
||||
// above them speaks for the subtree. `templates.rs`'s module doc carries the
|
||||
// general rule and states this as its one sanctioned exception.
|
||||
// ⚠️ **The rename is a reversal, not tidying.** The old name asserted "the
|
||||
// slot follows the nix work and the lease follows the container" — each node
|
||||
// declaring what it personally needed. The brace inverts that for a
|
||||
// coordinated subtree; see `docs/coordinator.md`, _Braces_.
|
||||
//
|
||||
// (`hive_jobq` owns slot *fairness*, pinned there by
|
||||
// `a_contended_resource_goes_to_the_oldest_waiter`. What is c0re's is
|
||||
|
|
@ -903,14 +897,9 @@ fn rebuild_reconcile_waits_for_the_whole_build_subtree() {
|
|||
// Both facts are asserted in `rebuild_chain_is_declared_serial`; this test
|
||||
// states the derived property explicitly because the indirection is the
|
||||
// easy thing to break — someone flattening the chain would keep every edge
|
||||
// and still lose the guarantee.
|
||||
//
|
||||
// 🎯 That warning earned itself: introducing `AgentWindow` **did** flatten
|
||||
// this chain (`stop_for_update` moved off `prebuild` and onto the brace),
|
||||
// and the guarantee survives only because the roll-up point moved with it.
|
||||
// The same roll-up rule is also what makes the obvious-looking alternative
|
||||
// — `stop_for_update` `AfterOk` `prebuild` *with* `swap` still nested under
|
||||
// `prebuild` — a **cycle**: the two would wait on each other.
|
||||
// and still lose the guarantee. That warning earned itself: `AgentWindow`
|
||||
// **did** flatten this chain, and the guarantee survives only because the
|
||||
// roll-up point moved with it.
|
||||
let q = JobQueue::new(1);
|
||||
let id = submit(&q, "r", |builder| rebuild(builder, "agent-a"));
|
||||
let shape = declared_shape(&q, id);
|
||||
|
|
|
|||
Loading…
Reference in a new issue