refactor(#3034): one quiesce builder, shared by the rebuild and the stop chain

The `Signal` -> `Drain` pair was built in three places, in three different
shapes: siblings under `SetWanted` in `stop_chain`, `Drain` nested under a
lease-holding `Signal` in `restart_chain`, and — as of this branch — a hybrid in
`rebuild_subtree` that was brace-held like the first and nested like the second.

`templates::quiesce(builder, agent, brace)` is now the one definition, returning
the `Drain` handle a caller edges its stop onto. Both nodes hang off the brace as
dep-ordered siblings and declare nothing, borrowing the lease it already holds.

That also fixes an inconsistency this branch introduced: the PR argued that a
brace makes nesting unnecessary and used it to flatten `StopForUpdate` off
`Signal`, then left `Drain` nested under `Signal` two lines away. Nesting is only
load-bearing where `Signal` is itself the lease holder.

`stop_chain`'s pair loses its own `Agent` declaration as a result — `SetWanted`
holds the lease for the subtree, so those were redundant re-entrant borrows.

`restart_chain` is left alone and says why in place: it has no brace, so `Signal`
holds the lease and the nesting under it is what keeps the grant continuous.
Giving it one would unify all three sites at the cost of an extra no-op node on
every graceful restart, which an operator would see — not something to change as
a side effect of a rebuild-shape PR.
This commit is contained in:
atlas 2026-08-04 15:04:26 +02:00
commit c2eafa7548
3 changed files with 56 additions and 25 deletions

View file

@ -81,15 +81,10 @@ fn stop_chain(builder: &JobBuilder, agent: &str, graceful: bool, running: bool)
// Declaration order is dependency order: the quiesce steps come first so
// the `Reconcile` that waits on them can name them.
if graceful && running {
let signal = builder
.node(NodeKind::Signal { agent: a() })
.needs(Resource::Agent(a()))
.part_of(wanted);
let drain = builder
.node(NodeKind::Drain { agent: a() })
.needs(Resource::Agent(a()))
.part_of(wanted)
.after_ok(signal);
// `SetWanted` is the brace here, so the quiesce pair borrows its grant
// rather than declaring the lease itself — same shape the rebuild
// template uses, one definition.
let drain = super::templates::quiesce(builder, agent, wanted);
let _ = builder
.node(NodeKind::Reconcile { agent: a() })
.needs(Resource::Agent(a()))
@ -155,6 +150,14 @@ fn restart_chain(builder: &JobBuilder, agent: &str, graceful: bool, running: boo
// `Reconcile` gates on the last mechanical step. For a non-graceful bounce
// that step *is* the root, and the parent gate already orders it — a child
// must NOT dep on its own parent (dep-scope), so it takes no sibling edge.
//
// ⚠️ This is the one quiesce site that does NOT use `templates::quiesce`.
// The helper needs a brace holding the lease above the pair; here `Signal`
// *is* the holder, and the nesting under it is what keeps the grant
// continuous across the bounce. Giving this chain its own brace would
// unify all three sites — at the cost of one extra no-op node on every
// graceful restart, which an operator would see. Deliberately not done as
// a side effect of a rebuild-shape change.
if graceful {
let signal = builder
.node(NodeKind::Signal { agent: a() })

View file

@ -128,6 +128,33 @@ pub(crate) fn fanned_out_mechanical(builder: &JobBuilder, kind: NodeKind) {
let _ = builder.node(kind).needs(lease);
}
/// 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.
///
/// 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.
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);
builder
.node(NodeKind::Drain { agent: a() })
.part_of(brace)
.after_ok(signal)
}
/// The group-roots a [`rebuild_nodes`] subgraph exposes to its caller: what a
/// tail node edges onto, and what a follow-up node waits for.
///
@ -217,14 +244,7 @@ fn rebuild_subtree<'a>(
.node(NodeKind::Prebuild { agent: a() })
.part_of(agent_window);
let drain = graceful.then(|| {
let signal = builder
.node(NodeKind::Signal { agent: a() })
.part_of(agent_window);
// `Drain` is a *child* of `Signal`, so the parent gate already orders
// it — a child must not dep on its own parent (dep-scope).
builder.node(NodeKind::Drain { agent: a() }).part_of(signal)
});
let drain = graceful.then(|| quiesce(builder, agent, agent_window));
// The container goes down here, not earlier: `AfterOk` the build so a
// failed build never stops a healthy container, and `AfterOk` the drain so

View file

@ -494,7 +494,9 @@ fn graceful_rebuild_chain_drains_before_stopping() {
// the original defect (up to GRACEFUL_STOP_TIMEOUT hidden behind
// every agent's build, on every boot sweep).
row("signal", Some("agent_window"), &[]),
row("drain", Some("signal"), &[]),
// Siblings under the brace, dep-ordered — not nested. Nesting is
// only needed where `Signal` itself holds the lease.
row("drain", Some("agent_window"), &[("signal", "done")]),
// The container still goes down only when *both* are ready: the
// build succeeded and the agent has checkpointed. Running the drain
// early is the win; stopping early would just be downtime.
@ -1490,18 +1492,24 @@ fn graceful_stop_shape_signal_drain_reconcile() {
row("reconcile", Some("set_wanted"), &[("drain", "done")]),
]
);
// Neither half of the graceful window takes a build slot. That is what lets
// a whole-hive graceful stop overlap every agent's drain even at
// `buildSlots = 1` while a rebuild hogs the slot — the cost ceiling is one
// `GRACEFUL_STOP_TIMEOUT` in total, not one per agent.
let agent = Resource::Agent("agent-a".to_owned());
// The quiesce pair declares **nothing**: `set_wanted` is the brace and holds
// the lease for the whole subtree, so both borrow that grant. Same shape,
// same helper (`templates::quiesce`) as the rebuild's window.
//
// ⚠️ In particular neither takes a build slot — which is what lets a
// whole-hive graceful *stop* overlap every agent's drain even at
// `buildSlots = 1`: the ceiling is one `GRACEFUL_STOP_TIMEOUT` in total,
// not one per agent. That holds here because nothing in a stop chain is
// slot-needing. It does **not** hold for the boot *sweep*, whose rebuilds
// do hold the slot across their drains — see the note on the sweep in
// `exec.rs`.
assert_eq!(
[
declared_resources(&q, node_of(&q, id, "signal")),
declared_resources(&q, node_of(&q, id, "drain")),
],
[vec![agent.clone()], vec![agent]],
"signal and drain hold the lease but never a build slot"
[vec![], vec![]],
"the quiesce pair borrows the brace's lease and declares nothing"
);
}