Skip to content

Commit dd13972

Browse files
committed
doc comments explaining new arg
1 parent e1713b1 commit dd13972

File tree

2 files changed

+32
-2
lines changed

2 files changed

+32
-2
lines changed

nexus/reconfigurator/preparation/src/lib.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,38 @@ pub struct PlanningInputFromDb<'a> {
9696
}
9797

9898
impl PlanningInputFromDb<'_> {
99-
// TODO-john docs
99+
/// Read the current set of database state needed to assemble a new
100+
/// [`PlanningInput`].
101+
///
102+
/// The caller is required to pass `parent_blueprint` in so that we
103+
/// statically enforce that any `PlanningInput` information we read from the
104+
/// database is _at least as new_ as the parent blueprint on which the
105+
/// planner will operate. This is particularly important for the planner's
106+
/// "garbage collection" phase where it drops expunged items for which all
107+
/// cleanup is complete: if the `PlanningInput` is read before the parent
108+
/// blueprint, some of the "is cleanup complete" information that doesn't
109+
/// have explicit generation numbers (or equivalent) could show a state
110+
/// where the planner believes cleanup is complete when actually it hasn't
111+
/// yet had a chance to run; e.g..
112+
///
113+
/// * Nexus A constructs a `PlanningInput` that has no record of zone Z.
114+
/// * Nexus A goes out to lunch.
115+
/// * Nexus B constructs a new blueprint that adds zone Z, makes it the
116+
/// target, and executes it.
117+
/// * Zone Z runs, and inserts some records that we'd see in `PlanningInput`
118+
/// indicating it is not yet cleaned up.
119+
/// * Nexus B constructs a new blueprint that expunges zone Z, makes it the
120+
/// target, and executes it.
121+
/// * At this point zone Z is expunged and ready for cleanup, but cleanup is
122+
/// not yet complete - we still have records related to zone Z in the db.
123+
/// * Nexus A resumes. It loads the current target blueprint (which shows
124+
/// zone Z is expunged) and has a `PlanningInput` with no records
125+
/// indicating zone Z is _not_ ready for cleanup, so incorrectly garbage
126+
/// collects zone Z.
127+
///
128+
/// We could enforce this instead by loading `parent_blueprint` ourselves
129+
/// with a note that it must come first, but in practice our caller has
130+
/// already loaded it.
100131
pub async fn assemble(
101132
opctx: &OpContext,
102133
datastore: &DataStore,

nexus/types/src/deployment/planning_input.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1601,7 +1601,6 @@ pub struct PlanningInputBuilder {
16011601
}
16021602

16031603
impl PlanningInputBuilder {
1604-
// TODO-john docs
16051604
pub fn new(
16061605
parent_blueprint: Arc<Blueprint>,
16071606
policy: Policy,

0 commit comments

Comments
 (0)