Merge rows 78+72: DeploymentManager singleton instance sweep — starts replication-seeded enabled instances and converges stale revisions through the real HandleDeploy path; one repair attempt per (instance, revision)
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,203 @@
|
||||
# Instance reconcile sweep — closing the config/runtime divergence gaps
|
||||
|
||||
**Date:** 2026-08-15
|
||||
**Scope:** Site Runtime (`DeploymentManagerActor`), deferred-work-register rows **78** and **72**
|
||||
**Status:** implemented
|
||||
|
||||
---
|
||||
|
||||
## 1. The problem
|
||||
|
||||
The site's `deployed_configurations` rows are the durable truth about what should be running.
|
||||
The set of live Instance Actors is what *is* running. Nothing kept the two in agreement after
|
||||
boot, and there was no signal when they disagreed.
|
||||
|
||||
Before this change, an Instance Actor could be created from exactly three places:
|
||||
|
||||
1. the boot-time SQLite load in `DeploymentManagerActor.PreStart`;
|
||||
2. an explicit deploy / enable command from central;
|
||||
3. (register row 67, commit `482165cb`) `SiteReconciliationActor`'s `EnsureInstanceStartedCommand`,
|
||||
raised for enabled instances **the reconcile pass itself fetched and stored**.
|
||||
|
||||
### Row 78 — configs that arrive by replication start nothing
|
||||
|
||||
*(Medium-Low, availability; proven live 2026-08-15.)*
|
||||
|
||||
`deployed_configurations` is one of LocalDb's ten replicated tables. On a replicated site pair,
|
||||
a config can therefore appear in a node's file with **no local actor involved at all**: the CDC
|
||||
sync applier writes the row directly.
|
||||
|
||||
The live sequence was:
|
||||
|
||||
- site-a-b booted solo; its startup reconcile fetched 4 configs from central and stored them
|
||||
(row 67's machinery started their actors on that node — correct).
|
||||
- The pair was then restarted with a **fresh** site-a-a file. CDC snapshot replication seeded
|
||||
site-a-a's `deployed_configurations` within ~5 s of boot.
|
||||
- That was **after** site-a-a's boot-time load had already read an empty table, and **invisible**
|
||||
to site-a-a's own reconcile pass: central rightly reported *no gap* ("0 fetched"), because
|
||||
central's view of the site was correct — the site really did have the configs.
|
||||
- Result: enabled instances sat dead with their configs present (`debug snapshot` →
|
||||
`instanceNotFound`) until yet another restart.
|
||||
|
||||
Row 67 covers configs the reconcile *fetches*. This is the same gap through a side door.
|
||||
|
||||
### Row 72 — a refreshed config does not reach the running actor
|
||||
|
||||
*(Medium-Low, same silent-divergence class.)*
|
||||
|
||||
`ReconcileGapItem` covers both *missing* and *stale* configs. For a **stale** item the reconcile
|
||||
writes the newer config via `StoreDeployedConfigIfNewerAsync` — a pure file write. The running
|
||||
Instance Actor keeps executing the **previously compiled** config until the next restart or
|
||||
redeploy, and nothing anywhere surfaced "stored revision ≠ running revision".
|
||||
|
||||
Row 72 was deliberately deferred out of row 67's scope because restarting a *live* instance
|
||||
carries redeploy semantics that needed their own decision. That decision is section 3.
|
||||
|
||||
---
|
||||
|
||||
## 2. The mechanism: one periodic sweep in the Deployment Manager singleton
|
||||
|
||||
A single periodic tick on the Deployment Manager singleton re-reads every durable
|
||||
`deployed_configurations` row and repairs both divergences.
|
||||
|
||||
```
|
||||
InstanceSweepTick (every InstanceSweepIntervalSeconds, default 60)
|
||||
└── off-thread read of all deployed configs
|
||||
└── InstanceSweepLoaded → actor thread
|
||||
for each ENABLED stored config, unless a deploy pipeline owns the instance:
|
||||
no running actor → start it (row 78)
|
||||
running, revision ≠ stored → re-apply stored (row 72)
|
||||
```
|
||||
|
||||
**Ownership.** The Deployment Manager is a cluster singleton scoped to the site-specific role, so
|
||||
the sweep runs on exactly one node — the active one — and Instance Actors stay under the
|
||||
singleton, which is the same invariant row 67's singleton-proxy routing preserved.
|
||||
|
||||
### 2.1 Why a sweep and not a LocalDb change hook
|
||||
|
||||
The register suggested either. A sweep was chosen:
|
||||
|
||||
- **Coverage.** A replication-apply hook covers exactly one arrival path, and only on the half of
|
||||
the fleet that runs with replication enabled (rig posture: site-a replicated, site-b/site-c
|
||||
deliberately not). The sweep is arrival-path-agnostic — replication seeding, a post-boot
|
||||
snapshot resync, an operator restoring a file by hand, and any future path all reduce to the
|
||||
same question: *does the store agree with the runtime?*
|
||||
- **Availability.** `ILocalDb` exposes **no** change-notification, applied-rows callback or
|
||||
observer API — verified against `ZB.MOM.WW.LocalDb` 0.3.0. A hook would require a new upstream
|
||||
capability in the `scadaproj` library plus an in-process bus from the sync applier into this
|
||||
singleton, i.e. a cross-repo change to close a site-local gap.
|
||||
- **Reuse.** The sweep inherits row 67's guards verbatim; the shared `StartStoredInstance` helper
|
||||
means the reconcile heal and the sweep heal produce identical bookkeeping and identical
|
||||
operator-facing events, differing only in the trigger they name.
|
||||
|
||||
Cost is one SQLite read per minute per site — the same read `HandleDeploymentStateQuery` already
|
||||
performs per central query.
|
||||
|
||||
### 2.2 Guards (all inherited or shared with row 67)
|
||||
|
||||
- **Disabled instances are never started.** A disabled instance legitimately has a config row and
|
||||
no actor; that is its correct steady state.
|
||||
- **The sweep defers to any deploy pipeline that owns the instance** — a compile warm in flight
|
||||
(`_deployWarms`), a predecessor still terminating (`_terminatingActorsByName`), or a deployment
|
||||
awaiting its persistence/init join (`_inFlightDeployments`). This is the same
|
||||
`InstanceIsOwnedByADeployPipeline` predicate row 67's `ShouldAttemptEnsureStart` is now built
|
||||
from. It matters: a redeploy passes through a window where the predecessor has been stopped and
|
||||
removed from routing but the replacement does not exist yet, which an unguarded sweep would read
|
||||
as "enabled config, no actor" and answer by creating a child whose actor name is still
|
||||
registered.
|
||||
- **Inert until boot-time creation finishes.** A tick landing between two staggered startup
|
||||
batches would "heal" instances the boot load is already about to create, announcing a fault on
|
||||
a node that is merely still starting.
|
||||
- **One repair attempt per (instance, revision) pair.** This is what keeps a *periodic* self-heal
|
||||
from becoming a restart loop: an Instance Actor that dies during init leaves its config row in
|
||||
place, and a stored config the site compile gate rejects can never apply. Without the claim,
|
||||
either would be re-attempted — and re-announced — every tick forever. A genuinely new revision
|
||||
is a different key and is always attempted.
|
||||
- **No overlapping reads.** A slow storage read cannot let ticks stack up into duplicate repairs.
|
||||
- **Silent when healthy.** A sweep that finds nothing emits no log line above Trace and no site
|
||||
event. This runs on every tick of every healthy site, forever.
|
||||
|
||||
---
|
||||
|
||||
## 3. The row-72 decision: converge, do not merely warn
|
||||
|
||||
**Decision.** When the sweep finds a running instance whose revision no longer matches the stored
|
||||
row, it **applies the stored config through the same per-instance apply/restart path a central
|
||||
redeploy uses** (`HandleDeploy` → compile gate → stop/watch/recreate → persist), and announces
|
||||
both revisions in a Warning `deployment` site event.
|
||||
|
||||
**Rationale.**
|
||||
|
||||
- The stored row **is** what central successfully deployed. The running actor is executing a
|
||||
config central believes was replaced. Leaving it running is the actual fault — a site quietly
|
||||
serving superseded logic is worse than a controlled restart, and the operator has no way to
|
||||
discover it.
|
||||
- The alternative (surface-only) leaves an operator with a Warning they can only act on by
|
||||
issuing a redeploy — the exact operation the sweep would have performed, minus the delay and
|
||||
minus the chance they see it.
|
||||
- Reusing `HandleDeploy` rather than writing a second apply path means the compile gate, the
|
||||
warm-then-gate ordering, the redeploy-supersede semantics, the static-override and
|
||||
native-alarm reset, and all-or-nothing-per-instance are the ones already in production. No new
|
||||
semantics were invented. In particular a stop still does **not** cancel an in-flight script
|
||||
run, exactly as for a central redeploy.
|
||||
|
||||
**Why this is safe against the ordering hazards the register worried about.**
|
||||
|
||||
- The sweep never acts on an instance a deploy pipeline owns, so it cannot interleave with a real
|
||||
redeploy; and if a deploy arrives *after* the sweep's own deploy is queued, it is ordinary
|
||||
last-write-wins on the same mailbox, which is a case the deploy path already handles.
|
||||
- A rejected restart is inert, not partial: `RunDeployGateAndProceed` returns **before** anything
|
||||
is stopped, so the running instance keeps serving its old config. It is announced once
|
||||
(Warning for the attempt, Error for the failure) and never retried for that revision.
|
||||
- The synthetic `DeployInstanceCommand` replies to `Self`, not to `NoSender` — dead letters are a
|
||||
monitored site health metric, and a self-heal must not manufacture them.
|
||||
|
||||
**Row 72's "nothing surfaces it" is false either way.** Even in the failure path the divergence
|
||||
produces two operator-visible `deployment` site events naming both revisions.
|
||||
|
||||
---
|
||||
|
||||
## 4. Configuration
|
||||
|
||||
`ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds` — default **60**, validated `>= 0`
|
||||
(`SiteRuntimeOptionsValidator`). **`0` disables the sweep**, which reopens both gaps; the row-67
|
||||
`EnsureInstanceStartedCommand` path is unaffected either way. No config change is required on any
|
||||
node to get the fix — the default is live.
|
||||
|
||||
## 5. Observability
|
||||
|
||||
| Condition | Log | Site event |
|
||||
|---|---|---|
|
||||
| Nothing to repair | — | — |
|
||||
| Row 78 start | Warning naming the instance and trigger | `deployment` / **Warning** — "started by deployment-manager sweep self-heal" |
|
||||
| Row 72 restart attempt | Warning naming both revisions | `deployment` / **Warning** — "restarted by deployment-manager sweep self-heal — running revision X is stale, applying stored revision Y" |
|
||||
| Row 72 restart applied | Information | — (the deploy path's own Info event covers it) |
|
||||
| Row 72 restart rejected | Warning with the gate's errors | `deployment` / **Error** — "stale-revision deployment-manager sweep self-heal failed" |
|
||||
| Storage read failed | Warning | — |
|
||||
|
||||
The trigger is named in the message so a sweep heal is distinguishable from row 67's
|
||||
"startup reconcile self-heal".
|
||||
|
||||
## 6. Tests
|
||||
|
||||
`tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceSweepSelfHealTests.cs` — 10 tests.
|
||||
|
||||
Fail-before evidence (behavioral revert: the periodic timer left unarmed) — **5 fail, 5 pass**:
|
||||
|
||||
| Test | Pre-fix |
|
||||
|---|---|
|
||||
| `Sweep_StartsEnabledInstance_WhoseConfigArrivedWithoutADeployOrReconcile` | **FAIL** — the live row-78 symptom |
|
||||
| `SweepStartedInstance_EmitsAWarningSiteEvent_NamingTheSweepTrigger` | **FAIL** |
|
||||
| `Sweep_StartsTheInstanceExactlyOnce_AcrossManySweeps` | **FAIL** |
|
||||
| `Sweep_RestartsInstance_ServingAStaleRevision` | **FAIL** — the row-72 symptom |
|
||||
| `Sweep_StaleRestartRejectedByTheCompileGate_IsAnnouncedOnceAndNotRetried` | **FAIL** |
|
||||
| `Sweep_NeverStarts_ADisabledInstance` | pass (negative guard) |
|
||||
| `Sweep_DefersToAnInFlightRedeploy` | pass (negative guard) |
|
||||
| `Sweep_DoesNotRestart_WhenStoredAndRunningRevisionsMatch` | pass (negative guard) |
|
||||
| `Sweep_IsSilent_WhenThereIsNothingToHeal` | pass (negative guard) |
|
||||
| `Sweep_DoesNothing_WhenTheIntervalIsZero` | pass (negative guard) |
|
||||
|
||||
Every test gates on the Deployment Manager's first instance-count report before writing a config
|
||||
row, because a row written before the boot-time load has read storage is picked up by that load —
|
||||
which is precisely the path row 78 is about *not* having. Without the gate a test would silently
|
||||
prove the boot load works.
|
||||
Reference in New Issue
Block a user