refactor(#3034): rename PostSwap to RebuildBookkeeping
It reads as a swap thing but is post-*rebuild* bookkeeping: rev marker, forge and matrix sync, manager kick, container rescan, meta-inputs snapshot. Wire label follows (`post_swap` -> `rebuild_bookkeeping`); the graph view renders whatever label it is sent, so nothing keys on the old string. Also corrects a doc the previous commit falsified: this node's comment still said it declares the agent lease as a re-entrant borrow, after that declaration moved to the brace. Clippy and the whole suite pass over a stale doc comment, so it took reading the file to find. Verified by grepping the new name for places it has no business being, which caught the sed rewriting a *historical* test name in a `// Replaces ...` comment - reverted, since prose about the past must keep its old spelling.
This commit is contained in:
parent
31a1853a45
commit
a0790e4e49
5 changed files with 56 additions and 38 deletions
|
|
@ -867,7 +867,7 @@ async fn prepare_applied_target(
|
|||
/// [`run_deploy_tail`] must not roll `main` back. Ordering that ahead of the tag
|
||||
/// plant is what makes the tail's `deployed/<id>` cross-check a second line of
|
||||
/// defence rather than the only one. No agent kick — the rebuild's own
|
||||
/// `PostSwap` already did it.
|
||||
/// `RebuildBookkeeping` already did it.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
|
|
|
|||
|
|
@ -63,7 +63,7 @@ pub(super) async fn run_node(
|
|||
NodeKind::MetaSync { relock, .. } => run_meta_sync(coord, agent, *relock).await,
|
||||
NodeKind::Prebuild { .. } => run_prebuild(agent, id).await,
|
||||
NodeKind::Swap { .. } => run_swap(coord, agent, id).await,
|
||||
NodeKind::PostSwap { .. } => run_post_swap(coord, agent).await,
|
||||
NodeKind::RebuildBookkeeping { .. } => run_rebuild_bookkeeping(coord, agent).await,
|
||||
NodeKind::Provision { .. } => run_provision(coord, agent).await,
|
||||
NodeKind::Create { .. } => run_create(agent).await,
|
||||
NodeKind::MetaLock {
|
||||
|
|
@ -263,9 +263,9 @@ async fn run_swap(coord: &Arc<Coordinator>, name: &str, id: NodeId) -> Result<()
|
|||
let paths = Coordinator::agent_paths(name, agent_dir);
|
||||
let result = crate::lifecycle::swap_update(name, &hive, &paths, Some(id.get())).await;
|
||||
// On success the Ok-only bookkeeping tail (rev marker, forge/matrix
|
||||
// sync, kick, rescan, snapshot) runs in the sibling `PostSwap` node,
|
||||
// which deps `AfterOk(Swap)`. On failure `PostSwap` is cancel-cascaded
|
||||
// and the tail `Reconcile` (`AfterAny(PostSwap)`) handles recovery; here
|
||||
// sync, kick, rescan, snapshot) runs in the sibling `RebuildBookkeeping` node,
|
||||
// which deps `AfterOk(Swap)`. On failure `RebuildBookkeeping` is cancel-cascaded
|
||||
// and the tail `Reconcile` (`AfterAny(RebuildBookkeeping)`) handles recovery; here
|
||||
// we only refresh the observed state so dashboards reflect the failed
|
||||
// swap immediately. The `Rebuilt { ok: false }` manager event is emitted by
|
||||
// the DAG's `EmitRebuilt` tail (any node may be the one that failed).
|
||||
|
|
@ -280,7 +280,7 @@ async fn run_swap(coord: &Arc<Coordinator>, name: &str, id: NodeId) -> Result<()
|
|||
/// means the profile swap succeeded. Store/forge/matrix work only — no nix
|
||||
/// build (build-slot-exempt); the agent lease taken at `Swap` is still held
|
||||
/// (the whole chain up to `Reconcile` is one agent's subgraph).
|
||||
async fn run_post_swap(coord: &Arc<Coordinator>, name: &str) -> Result<()> {
|
||||
async fn run_rebuild_bookkeeping(coord: &Arc<Coordinator>, name: &str) -> Result<()> {
|
||||
if let Some(rev) = crate::auto_update::current_flake_rev(&coord.hyperhive_flake)
|
||||
&& let Err(e) = std::fs::write(crate::paths::applied_rev_marker(name), rev)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -53,20 +53,22 @@ pub enum NodeKind {
|
|||
/// `nixos-container update` profile-swap (requires the container
|
||||
/// stopped). Re-applies nspawn flags + resource limits first —
|
||||
/// rebuild is the reconcile verb. The post-rebuild bookkeeping tail
|
||||
/// lives in the sibling `PostSwap` node.
|
||||
/// lives in the sibling `RebuildBookkeeping` node.
|
||||
Swap { agent: String },
|
||||
/// The post-`Swap` bookkeeping tail as a first-class node: rev marker,
|
||||
/// forge + matrix sync, manager kick, container rescan, meta-inputs
|
||||
/// snapshot. Split out of `Swap` for dashboard visibility + retry
|
||||
/// granularity. Deps `AfterOk(Swap)`, so it runs only when the profile
|
||||
/// swap succeeded; the tail `Reconcile` deps `AfterAny(PostSwap)`, so on
|
||||
/// swap succeeded; the tail `Reconcile` deps `AfterAny(RebuildBookkeeping)`, so on
|
||||
/// swap failure this node is cancel-cascaded (a terminal state) and
|
||||
/// recovery still runs. Store/forge/matrix work only — no nix build, so
|
||||
/// build-slot-exempt. It *does* declare the agent lease: an ancestor in the
|
||||
/// stop chain already holds it, so this is a re-entrant borrow rather than a
|
||||
/// second unit — declaring it keeps the requirement true of this node rather
|
||||
/// than of the one DAG shape it happens to be used in.
|
||||
PostSwap { agent: String },
|
||||
/// recovery still runs. Store/forge/matrix work only — no nix build.
|
||||
///
|
||||
/// Declares **no resources**: it is a coordinated child of
|
||||
/// [`NodeKind::AgentWindow`], which holds the agent lease (and the build
|
||||
/// slot) for the whole rebuild subtree. See `templates.rs`'s module doc for
|
||||
/// why the brace is the one place a resource is declared on behalf of
|
||||
/// others.
|
||||
RebuildBookkeeping { agent: String },
|
||||
/// First-spawn pre-create provisioning: proposed/applied repos,
|
||||
/// state subvolume, and meta registration (`sync_agents`). Runs
|
||||
/// ahead of `Create` so the `nixos-container create --flake
|
||||
|
|
@ -360,7 +362,7 @@ impl NodeKind {
|
|||
NodeKind::MetaSync { .. } => "meta_sync",
|
||||
NodeKind::Prebuild { .. } => "prebuild",
|
||||
NodeKind::Swap { .. } => "swap",
|
||||
NodeKind::PostSwap { .. } => "post_swap",
|
||||
NodeKind::RebuildBookkeeping { .. } => "rebuild_bookkeeping",
|
||||
NodeKind::Provision { .. } => "provision",
|
||||
NodeKind::Create { .. } => "create",
|
||||
NodeKind::MetaLock { .. } => "meta_lock",
|
||||
|
|
@ -396,7 +398,7 @@ impl NodeKind {
|
|||
NodeKind::MetaSync { agent, .. }
|
||||
| NodeKind::Prebuild { agent }
|
||||
| NodeKind::Swap { agent }
|
||||
| NodeKind::PostSwap { agent }
|
||||
| NodeKind::RebuildBookkeeping { agent }
|
||||
| NodeKind::Provision { agent }
|
||||
| NodeKind::Create { agent }
|
||||
| NodeKind::Reconcile { agent }
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@
|
|||
//! shape says "this subtree is coordinated, one holder speaks for it".
|
||||
//!
|
||||
//! ```text
|
||||
//! rebuild(a): MetaSync(a) → AgentWindow(a){ Prebuild(a) → StopForUpdate(a) → Swap(a) →(ok) PostSwap(a) } →(any) Reconcile(a)
|
||||
//! 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»
|
||||
|
|
@ -149,7 +149,7 @@ pub(crate) struct RebuildRoots<'a> {
|
|||
pub meta_sync: Handle<'a>,
|
||||
/// The brace holding the agent lease and the build slot — its roll-up
|
||||
/// carries the whole mechanical subtree (`Prebuild`, the quiesce chain,
|
||||
/// `StopForUpdate` → `Swap` → `PostSwap`).
|
||||
/// `StopForUpdate` → `Swap` → `RebuildBookkeeping`).
|
||||
pub agent_window: Handle<'a>,
|
||||
/// The recovery/convergence tail root.
|
||||
pub reconcile: Handle<'a>,
|
||||
|
|
@ -186,7 +186,7 @@ impl<'a> RebuildRoots<'a> {
|
|||
/// `Drain`. Waiting on the build is deliberate — running the drain window
|
||||
/// early is the win, taking the container *down* early would be pure
|
||||
/// downtime. This is the node that actually stops the container.
|
||||
/// - `Swap` (child of `StopForUpdate`), then `PostSwap` (`AfterOk` its sibling
|
||||
/// - `Swap` (child of `StopForUpdate`), then `RebuildBookkeeping` (`AfterOk` its sibling
|
||||
/// `Swap`): the Ok-only bookkeeping tail (rev marker, forge/matrix sync,
|
||||
/// kick, rescan).
|
||||
/// - `Reconcile` (**last, root**): `AfterAny` `AgentWindow`, which rolls up
|
||||
|
|
@ -260,8 +260,8 @@ fn rebuild_subtree<'a>(
|
|||
let swap = builder
|
||||
.node(NodeKind::Swap { agent: a() })
|
||||
.part_of(stop_for_update);
|
||||
let _post_swap = builder
|
||||
.node(NodeKind::PostSwap { agent: a() })
|
||||
let _rebuild_bookkeeping = builder
|
||||
.node(NodeKind::RebuildBookkeeping { agent: a() })
|
||||
.part_of(stop_for_update)
|
||||
.after_ok(swap);
|
||||
|
||||
|
|
@ -317,7 +317,7 @@ pub(crate) fn graceful_rebuild_nodes<'a>(
|
|||
/// `FinalizeDeploy` waits on **two** roots, which together reproduce the gate
|
||||
/// the old fused node had around its inline `rebuild_no_meta` call:
|
||||
/// - `AfterOk` `Prebuild` — a parent's state is its roll-up, so this is `Done`
|
||||
/// only once `StopForUpdate` → `Swap` → `PostSwap` all are (a failed *or*
|
||||
/// only once `StopForUpdate` → `Swap` → `RebuildBookkeeping` all are (a failed *or*
|
||||
/// cancelled child rolls the parent up `Failed`). That's the old
|
||||
/// `build_result`.
|
||||
/// - `AfterOk` `Reconcile` — the old call passed `deferred_start = false` on
|
||||
|
|
@ -350,7 +350,7 @@ pub(crate) fn deploy_rebuild_nodes(builder: &JobBuilder, agent: &str, approval_i
|
|||
///
|
||||
/// Closed by an [`NodeKind::EmitRebuilt`] tail edged onto all three group-roots
|
||||
/// (`MetaSync`, `Prebuild`, `Reconcile`) — `Prebuild`'s roll-up carries the
|
||||
/// whole `StopForUpdate`→`Swap`→`PostSwap` subtree, so those three cover every
|
||||
/// whole `StopForUpdate`→`Swap`→`RebuildBookkeeping` subtree, so those three cover every
|
||||
/// node. Edging `Reconcile` alone would not do: it is `AfterAny` `Prebuild`, so
|
||||
/// it reaches `Done` even after a failed swap and the tail would report success.
|
||||
pub fn rebuild(builder: &JobBuilder, agent: &str, relock: bool) {
|
||||
|
|
|
|||
|
|
@ -418,7 +418,11 @@ fn rebuild_chain_is_declared_serial() {
|
|||
&[("prebuild", "done")]
|
||||
),
|
||||
row("swap", Some("stop_for_update"), &[]),
|
||||
row("post_swap", Some("stop_for_update"), &[("swap", "done")]),
|
||||
row(
|
||||
"rebuild_bookkeeping",
|
||||
Some("stop_for_update"),
|
||||
&[("swap", "done")]
|
||||
),
|
||||
row(
|
||||
"reconcile",
|
||||
None,
|
||||
|
|
@ -500,7 +504,11 @@ fn graceful_rebuild_chain_drains_before_stopping() {
|
|||
&[("prebuild", "done"), ("drain", "done")]
|
||||
),
|
||||
row("swap", Some("stop_for_update"), &[]),
|
||||
row("post_swap", Some("stop_for_update"), &[("swap", "done")]),
|
||||
row(
|
||||
"rebuild_bookkeeping",
|
||||
Some("stop_for_update"),
|
||||
&[("swap", "done")]
|
||||
),
|
||||
row(
|
||||
"reconcile",
|
||||
None,
|
||||
|
|
@ -534,7 +542,7 @@ fn non_graceful_rebuild_has_no_signal_or_drain() {
|
|||
"prebuild",
|
||||
"stop_for_update",
|
||||
"swap",
|
||||
"post_swap",
|
||||
"rebuild_bookkeeping",
|
||||
"reconcile"
|
||||
],
|
||||
"exactly seven nodes, and none of them is signal or drain"
|
||||
|
|
@ -713,7 +721,11 @@ fn multi_agent_start_one_dag_folds_per_agent_stale_rebuild() {
|
|||
&[("prebuild", "done")]
|
||||
),
|
||||
row("swap", Some("stop_for_update"), &[]),
|
||||
row("post_swap", Some("stop_for_update"), &[("swap", "done")]),
|
||||
row(
|
||||
"rebuild_bookkeeping",
|
||||
Some("stop_for_update"),
|
||||
&[("swap", "done")]
|
||||
),
|
||||
row(
|
||||
"reconcile",
|
||||
None,
|
||||
|
|
@ -869,21 +881,21 @@ fn rebuild_chain_nodes_suppress_crash_watch() {
|
|||
// individually legible, and the arrangement was the only reason this module
|
||||
// needed to claim and complete nodes.
|
||||
|
||||
/// The swap-success path: `Swap` ok → the `AfterOk` `PostSwap` (bookkeeping
|
||||
/// The swap-success path: `Swap` ok → the `AfterOk` `RebuildBookkeeping` (bookkeeping
|
||||
/// tail) runs, and only then does `Reconcile` fire — serialized behind
|
||||
/// `PostSwap` (not racing it) because `Reconcile` deps `AfterAny(PostSwap)`.
|
||||
/// `RebuildBookkeeping` (not racing it) because `Reconcile` deps `AfterAny(RebuildBookkeeping)`.
|
||||
#[test]
|
||||
fn rebuild_reconcile_waits_for_the_whole_build_subtree() {
|
||||
// Replaces `swap_ok_runs_post_swap_before_reconcile` and
|
||||
// `swap_failure_still_runs_reconcile`, which walked the same DAG with the
|
||||
// swap succeeding in one and failing in the other.
|
||||
//
|
||||
// The interesting claim was "Reconcile must wait for PostSwap, not race
|
||||
// The interesting claim was "Reconcile must wait for RebuildBookkeeping, not race
|
||||
// it" — and it does *not* come from an edge between them. `reconcile` deps
|
||||
// `AfterAny(agent_window)`, while `post_swap` sits inside the brace's
|
||||
// subtree (post_swap → stop_for_update → agent_window). A parent is not
|
||||
// `AfterAny(agent_window)`, while `rebuild_bookkeeping` sits inside the brace's
|
||||
// subtree (rebuild_bookkeeping → stop_for_update → agent_window). A parent is not
|
||||
// terminal until its subtree is, so the brace cannot satisfy that edge while
|
||||
// post_swap is outstanding. **The ordering is the parent chain, not a
|
||||
// rebuild_bookkeeping is outstanding. **The ordering is the parent chain, not a
|
||||
// dependency.**
|
||||
//
|
||||
// Both facts are asserted in `rebuild_chain_is_declared_serial`; this test
|
||||
|
|
@ -907,7 +919,7 @@ fn rebuild_reconcile_waits_for_the_whole_build_subtree() {
|
|||
.unwrap_or_else(|| panic!("{kind} node"))
|
||||
.parent
|
||||
};
|
||||
assert_eq!(parent_of("post_swap"), Some("stop_for_update"));
|
||||
assert_eq!(parent_of("rebuild_bookkeeping"), Some("stop_for_update"));
|
||||
assert_eq!(parent_of("stop_for_update"), Some("agent_window"));
|
||||
assert_eq!(
|
||||
shape
|
||||
|
|
@ -917,13 +929,13 @@ fn rebuild_reconcile_waits_for_the_whole_build_subtree() {
|
|||
.after,
|
||||
vec![("agent_window", "done|failed|skipped".to_owned())],
|
||||
"reconcile gates on the brace's roll-up, which covers the whole \
|
||||
subtree — including post_swap — and runs on failure too"
|
||||
subtree — including rebuild_bookkeeping — and runs on failure too"
|
||||
);
|
||||
}
|
||||
|
||||
// `swap_failure_still_runs_reconcile` lived here.
|
||||
//
|
||||
// It asserted that a failed swap leaves `post_swap` `Skipped` and `swap`
|
||||
// It asserted that a failed swap leaves `rebuild_bookkeeping` `Skipped` and `swap`
|
||||
// `Failed`, and that reconcile still runs. All three are hive_jobq's cascade
|
||||
// (`failed_after_ok_dep_cancels_dependents_but_after_any_still_runs`), and the
|
||||
// "says so on the wire" half turned out to be nothing: `snapshot` fills
|
||||
|
|
@ -1300,7 +1312,11 @@ fn deploy_apply_grows_rebuild_subgraph_and_finalizes_after_it() {
|
|||
&[("prebuild", "done")]
|
||||
),
|
||||
row("swap", Some("stop_for_update"), &[]),
|
||||
row("post_swap", Some("stop_for_update"), &[("swap", "done")]),
|
||||
row(
|
||||
"rebuild_bookkeeping",
|
||||
Some("stop_for_update"),
|
||||
&[("swap", "done")]
|
||||
),
|
||||
row(
|
||||
"reconcile",
|
||||
None,
|
||||
|
|
@ -1539,7 +1555,7 @@ fn perm_change_shape_prefixes_rebuild_chain() {
|
|||
"prebuild",
|
||||
"stop_for_update",
|
||||
"swap",
|
||||
"post_swap",
|
||||
"rebuild_bookkeeping",
|
||||
"reconcile",
|
||||
// the ok / !ok tail pair
|
||||
"emit_rebuilt",
|
||||
|
|
|
|||
Loading…
Reference in a new issue