docs(#3034): move the brace rationale out of templates.rs into coordinator.md

The pre-push comment-block lint rejected two 40-line doc blocks, correctly: the
module doc and `rebuild_subtree`'s now carry the trigger and a pointer, and the
reasoning lives in a new `#### Braces` section.

That move surfaced a third doc the resource change had falsified. The scheduler's
lease-acquirer list still named `StopForUpdate` / `Swap` / `Signal` / `Drain`,
all of which are now exempt. The list now separates container-affecting nodes
from braces, and says why the rebuild subtree's members are exempt for a
different reason than `MetaSync` / `Prebuild`: they do touch the container, but
their brace holds the lease above them.
This commit is contained in:
atlas 2026-08-04 13:00:07 +02:00
commit 7d26d6017f
2 changed files with 76 additions and 57 deletions

View file

@ -190,17 +190,56 @@ resources are free. Resources:
held by nix-heavy nodes for the node's duration. held by nix-heavy nodes for the node's duration.
2. **Per-agent lifecycle lease** — keyed on the **node's** agent (agent is 2. **Per-agent lifecycle lease** — keyed on the **node's** agent (agent is
per-node; a DAG can span agents) and globally exclusive per agent across per-node; a DAG can span agents) and globally exclusive per agent across
all DAGs: acquired at a container-affecting node (`SetWanted`, all DAGs: acquired either at a container-affecting node (`SetWanted`,
`StopForUpdate`, `Swap`, `Signal`, `Drain`, `Reconcile`, `WriteDropin`, `Reconcile`, `WriteDropin`, `Create`) or at a **brace** (`AgentWindow`,
`Create`, `DeployWindow`), held by the owning DAG until it's terminal, `DeployWindow`) on behalf of a whole coordinated subtree; held by the owning
so two DAGs never interleave container ops on the same agent. A DAG DAG until it's terminal, so two DAGs never interleave container ops on the
touching several agents holds one lease per agent. (`SetWanted` is a store same agent. A DAG touching several agents holds one lease per agent.
write, not a container op, but takes the lease anyway so a power-op DAG's (`SetWanted` is a store write, not a container op, but takes the lease anyway
intent write + reconcile is atomic — two racing ops can't clobber intent so a power-op DAG's intent write + reconcile is atomic — two racing ops can't
before either reconciles.) **Lease-exempt**: `MetaSync`, `Prebuild`, clobber intent before either reconciles.) **Lease-exempt**: `MetaSync`,
`MetaLock`, `WritePermFile`, `Reparent` `Prebuild`, `MetaLock`, `WritePermFile`, `Reparent` — they touch the store /
they touch the store / meta, not the running container, which is exactly meta, not the running container, which is exactly why a stop can land while
why a stop can land while another DAG's prebuild is still building. another DAG's prebuild is still building. Also exempt, for a different
reason, are the rebuild subtree's own members (`StopForUpdate`, `Swap`,
`Signal`, `Drain`, `RebuildBookkeeping`): they genuinely do touch the
container, but their `AgentWindow` brace holds the lease above them — see
_Braces_ below.
#### Braces
Templates otherwise declare a resource on **every** node that needs it, even
when a parent already holds it, so the requirement belongs to the node rather
than to one DAG shape it happens to appear in. A **brace** is the one sanctioned
exception: a pure-resource-holder root that declares on behalf of a subtree
coordinated with itself, whose members then declare nothing.
It is forced rather than stylistic. Declaring a resource means *"I need this
exclusively"*, and the agent lease is single-unit — so **two siblings that both
declared it could never run concurrently.** For a subtree whose whole point is
concurrency (`Prebuild` beside the `Signal``Drain` quiesce window), declaring
the requirement truthfully on every node and running those nodes in parallel are
mutually exclusive. One holder above them speaks for the subtree.
This is the opposite of the failure the declare-your-own rule exists to prevent,
not a relapse into it: there the requirement was *implicit*, inferred from a
node's kind and true only by accident of placement. Here it is explicit, on one
node, with the omission below it documented on the brace itself.
Two consequences worth knowing:
- **Flattening a chain under a brace is safe.** The stop chain used to nest
`Signal` over `Drain` over `StopForUpdate` specifically so the lease stayed
continuous — as independent siblings each would acquire it separately and
leave a gap another DAG could claim the agent in, mid-bounce. A brace supplies
that continuity directly, so the nesting is no longer load-bearing.
- **Observability is unaffected.** `running_transients` keys off a node's
*payload* agent, not off a declared lease edge, so every child still lights its
own dashboard pill and still reports its own `takes_container_down` to the
crash watcher. A brace itself reports `false`: it parents the stopping nodes
but does not stop anything, and claiming otherwise would widen crash
suppression across the build and tail, where a vanished container is still a
real crash.
Among simultaneously-ready nodes competing for a resource, DAG-submit order Among simultaneously-ready nodes competing for a resource, DAG-submit order
wins (FIFO) so bulk operations drain predictably. The scheduler also owns the wins (FIFO) so bulk operations drain predictably. The scheduler also owns the

View file

@ -1,25 +1,15 @@
//! DAG shape builders — every operation as a template over the shared //! DAG shape builders — every operation as a template over the shared node
//! node primitives. Pure (no I/O); each node carries its own `agent` (there is //! primitives. Pure (no I/O); each node carries its own `agent` (there is no
//! no DAG-level agent) and **declares its own resources** with `.needs(…)` //! DAG-level agent) and **declares its own resources** with `.needs(…)` right
//! right where it is constructed, rather than having them derived from its //! where it is constructed, rather than derived from its kind. Deriving made the
//! kind. Deriving made the requirement a property of the *kind*, so a kind that //! requirement a property of the *kind*, so a kind running under an ancestor
//! happened to run under an ancestor already holding the resource could get //! that already held the resource could get away with declaring nothing.
//! away with declaring nothing.
//! //!
//! **The one sanctioned exception is a brace** — a pure-resource-holder root //! **The one sanctioned exception is a brace** — a pure-resource-holder root
//! ([`NodeKind::AgentWindow`], [`NodeKind::DeployWindow`]) that declares for a //! ([`NodeKind::AgentWindow`], [`NodeKind::DeployWindow`]) declaring for a
//! subtree of nodes coordinated with each other, which then declare nothing. //! coordinated subtree whose members then declare nothing. Forced by the lease
//! This is the opposite of the failure above, not a relapse into it: there the //! being single-unit: two siblings that both declared it could never run
//! requirement was *implicit*, inferred from a kind and true only by accident of //! concurrently. Why this is not a relapse: `docs/coordinator.md`.
//! placement; here it is declared explicitly on one node, and the omission below
//! it is deliberate and documented on the brace.
//!
//! It has to work this way, because declaring a resource means *"I need this
//! exclusively"* and the agent lease is single-unit: **two siblings that both
//! declared it could never run concurrently.** So for a subtree whose whole
//! point is concurrency, declaring the requirement truthfully on every node and
//! running those nodes in parallel are mutually exclusive. The brace is how the
//! shape says "this subtree is coordinated, one holder speaks for it".
//! //!
//! ```text //! ```text
//! rebuild(a): MetaSync(a) → AgentWindow(a){ Prebuild(a) → StopForUpdate(a) → Swap(a) →(ok) RebuildBookkeeping(a) } →(any) Reconcile(a) //! rebuild(a): MetaSync(a) → AgentWindow(a){ Prebuild(a) → StopForUpdate(a) → Swap(a) →(ok) RebuildBookkeeping(a) } →(any) Reconcile(a)
@ -37,7 +27,7 @@
//! The hive-wide **power ops** (`stop` / `start` / `restart`) are NOT here: //! The hive-wide **power ops** (`stop` / `start` / `restart`) are NOT here:
//! their per-agent shape depends on live running state (an async //! their per-agent shape depends on live running state (an async
//! `lifecycle::is_running` read), so `submit.rs` assembles them out of the //! `lifecycle::is_running` read), so `submit.rs` assembles them out of the
//! primitives this module exports ([`rebuild_nodes`]) over `JobBuilder::node`. //! primitives this module exports ([`rebuild_nodes`]).
use hive_jobq::TerminalState; use hive_jobq::TerminalState;
@ -174,34 +164,24 @@ impl<'a> RebuildRoots<'a> {
/// build slot *and* the agent lease, atomically, and holds both for the whole /// build slot *and* the agent lease, atomically, and holds both for the whole
/// subtree. Everything below it declares **nothing** and re-enters these /// subtree. Everything below it declares **nothing** and re-enters these
/// grants. /// grants.
/// - `Prebuild` and the quiesce chain are **siblings under the brace, and run /// - `Prebuild` and the **quiesce chain** (`Signal` → `Drain`, graceful only)
/// concurrently.** That is the point of the brace: they contend for different /// are siblings under the brace and run **concurrently** — they contend for
/// resources (slot vs. agent), so nesting the stop under the build — as this /// different resources, so nesting the stop under the build (as this template
/// template did — hid the entire graceful-stop timeout behind the nix build, /// did) only hid the graceful-stop timeout behind the nix build. The container
/// per agent, on every sweep. /// is still up throughout the quiesce.
/// - the **quiesce chain** (graceful only): `Signal` then `Drain` as its child.
/// Asks the agent to checkpoint and waits for it to go quiet; the container
/// is still *up* throughout.
/// - `StopForUpdate` (child of the brace): `AfterOk` **both** `Prebuild` and /// - `StopForUpdate` (child of the brace): `AfterOk` **both** `Prebuild` and
/// `Drain`. Waiting on the build is deliberate — running the drain window /// `Drain`. Waiting on the build is deliberate — running the drain early is
/// early is the win, taking the container *down* early would be pure /// the win, taking the container *down* early would be pure downtime.
/// downtime. This is the node that actually stops the container. /// - `Swap` (child of `StopForUpdate`), then `RebuildBookkeeping` (`AfterOk` its
/// - `Swap` (child of `StopForUpdate`), then `RebuildBookkeeping` (`AfterOk` its sibling /// sibling `Swap`): the Ok-only bookkeeping tail.
/// `Swap`): the Ok-only bookkeeping tail (rev marker, forge/matrix sync,
/// kick, rescan).
/// - `Reconcile` (**last, root**): `AfterAny` `AgentWindow`, which rolls up /// - `Reconcile` (**last, root**): `AfterAny` `AgentWindow`, which rolls up
/// terminal only once its whole subtree has settled — so `Reconcile` runs /// terminal only once its whole subtree has settled — so it runs after the
/// after the swap regardless of outcome, and as a top-level root it survives /// swap regardless of outcome, and as a top-level root it survives the
/// the cancel-cascade of a failed brace (recovery-start invariant, which also /// cancel-cascade of a failed brace (recovery-start invariant). Fresh lease;
/// covers a failed `MetaSync`: that cancel-cascades the brace, i.e. terminal, /// the tiny gap is harmless, `Reconcile` converges idempotently.
/// so the tail still runs). It takes a fresh lease; the tiny gap is harmless —
/// `Reconcile` converges to the persisted `wanted` idempotently.
/// ///
/// The lease-continuity argument that used to justify nesting the stop chain /// Why flattening the stop chain is safe, and why the lease-continuity argument
/// (*"siblings would each take the lease separately and leave a gap another DAG /// that used to justify nesting it is satisfied by the brace: `docs/coordinator.md`.
/// could claim the agent in, mid-bounce"*) is **satisfied by the brace instead**
/// — one holder above them all, so siblings share one continuous grant and
/// there is no gap to claim. That is what makes flattening them safe.
fn rebuild_subtree<'a>( fn rebuild_subtree<'a>(
builder: &'a JobBuilder, builder: &'a JobBuilder,
agent: &str, agent: &str,