fix(siteruntime): instance reconcile sweep closes config/runtime divergence (register rows 78, 72)

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 nothing surfaced a disagreement.

Row 78 (availability, proven live 2026-08-15) -- deployed_configurations is one of
LocalDb's ten replicated tables, so on a replicated pair a config can appear in a
node's file with no local actor involved at all: the CDC sync applier writes the row
directly. Live sequence: site-a-b's reconcile fetched 4 configs at its solo boot; the
pair was then restarted with a fresh site-a-a file and snapshot replication seeded
site-a-a's table within ~5s -- AFTER its boot-time load had read an empty table, and
invisibly to its own reconcile (central rightly reported "0 fetched", because central's
view of the site was correct). Enabled instances sat dead with their configs present
until yet another restart. Row 67's EnsureInstanceStartedCommand only covers configs
the reconcile itself FETCHES; this is the same gap through a side door.

Row 72 (silent divergence) -- for a STALE ReconcileGapItem the pass writes the newer
config via StoreDeployedConfigIfNewerAsync, a pure file write, while the running
Instance Actor keeps executing the previously compiled config until the next restart
or redeploy. Nothing said "stored revision != running revision".

Both are closed by one periodic sweep owned by the Deployment Manager singleton
(ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds, default 60, validated >= 0;
0 disables). Each tick re-reads the durable rows off-thread and, on the actor thread,
for every ENABLED stored config either starts a missing Instance Actor (row 78) or --
when the running one's revision no longer matches the stored one -- re-applies the
stored config through the ordinary redeploy path (row 72).

A sweep was chosen over a LocalDb change hook deliberately: a hook covers only the
replication arrival path, and only on the half of the fleet running with replication
enabled, whereas the sweep is arrival-path-agnostic (replication seeding, post-boot
snapshot resync, a hand-restored file, any future path). It is also not available --
ILocalDb 0.3.0 exposes no change-notification or applied-rows callback, so a hook would
mean a new upstream capability in scadaproj plus an in-process bus into this singleton,
a cross-repo change to close a site-local gap.

Row 72's decision is to CONVERGE, not merely warn. The stored row is what central
successfully deployed; an actor serving the config central believes was replaced is the
actual fault, and a surface-only warning leaves the operator to issue by hand exactly
the redeploy the sweep would have performed. Reusing HandleDeploy means the compile
gate, warm-then-gate ordering, redeploy-supersede, the static-override / native-alarm
reset and all-or-nothing-per-instance are the production ones, not a second
implementation -- and a stop still does NOT cancel an in-flight script run, exactly as
for a central redeploy.

Guards, shared with row 67 via the extracted InstanceIsOwnedByADeployPipeline predicate
and the shared StartStoredInstance helper:
- a DISABLED instance is never started (config row, no actor is its correct state);
- the sweep defers to any deploy pipeline owning the instance (compile warm in flight,
  predecessor still terminating, deployment awaiting its persistence/init join), so it
  can never interleave with a real redeploy -- that window has the config stored and no
  actor, which an unguarded sweep would answer by creating a child whose actor name is
  still registered;
- inert until boot-time Instance Actor creation has finished, so a tick between
  staggered batches does not "heal" instances the boot load is about to create;
- ONE repair attempt per (instance, revision) pair -- this is what stops a periodic
  self-heal becoming a restart loop for an actor that dies during init, or for a stored
  config the compile gate rejects (a rejection leaves the running instance untouched:
  the gate returns before anything is stopped);
- reads never overlap, so a slow read cannot stack ticks into duplicate repairs.

A sweep that finds nothing is completely silent -- no log above Trace, no site event.
A repair emits the same Warning `deployment` site-event pattern as row 67 with the
trigger named ("deployment-manager sweep self-heal") so it is distinguishable from a
startup-reconcile heal; a stale-revision restart names BOTH revisions and a rejected
one adds an Error event. The synthetic redeploy replies to the singleton itself rather
than NoSender, because dead letters are a monitored site health metric.

CreateInstanceActor gains a revisionHash parameter and is the single creation funnel
that records it, so the stale check covers every path that can start an Instance Actor;
a missing entry reads as "unknown", never "stale", so a future path bypassing the funnel
degrades to no-op rather than to spurious restarts.

Fail-before by behavioral revert (sweep timer left unarmed): 5 of 10 new tests fail,
including both live symptoms -- the row-78 repro ("A replication-seeded config for an
enabled instance left no running Instance Actor") and the row-72 repro, whose snapshot
still shows the rev-1 value 111 while the store holds rev-2's 222. The 5 negative
guards (disabled / in-flight redeploy / matching revisions / silent-when-healthy /
interval-zero) are green in both states, as they must be.

SiteRuntime 626/626, Host 505/505, AuditLog 383/383.

Design note: docs/plans/2026-08-15-instance-sweep-self-heal-design.md
This commit is contained in:
Joseph Doherty
2026-08-16 04:16:06 -04:00
parent 3dc38fb674
commit e606b24a55
7 changed files with 1186 additions and 16 deletions
@@ -98,6 +98,16 @@ flowchart TD
> **Startup reconcile heals the RUNTIME, not just the config file (register row 67)**: `SiteReconciliationActor` runs on **every** site node (not a singleton) and performs one best-effort pass shortly after boot — it reports the node's local deployed inventory to central and fetches + stores any config central reports as a gap. That pass fires **after** step 1's deployed-config read has already run, so storing a config started nothing: Instance Actors are created only by the boot-time load above and by an explicit deploy/enable. An instance whose config was missing at boot (lost volume, first boot after a restore, rehomed data dir) therefore stayed **dead until a second restart**, whose load finally picked it up — silently, with nothing surfacing "config stored, actor not running". The pass now raises a fire-and-forget `EnsureInstanceStartedCommand` for every **enabled** instance it healed, routed through the **deployment-manager singleton proxy** so Instance Actors are only ever created under the singleton regardless of which node's pass found the gap. The handler is deliberately advisory: it **no-ops when the actor is already running** (both nodes legitimately raise it for the same instance), **re-reads the durable row** — which is the authority on both the config body and enabled/disabled state, so a disable landing after the fetch still wins and the instance stays stopped — **defers to any in-flight deploy/redeploy** for that instance, preserving the deploy gate's ordering guarantees and the all-or-nothing-per-instance rule (it never applies a config, only starts one already fully persisted), and treats a missing row as a quiet no-op (the normal case on an **unreplicated** site whose peer node healed its own file). A successful reconcile start is **surfaced**, not silent: a Warning log line plus an operational `deployment` site event, because reaching it means the node booted without a config for an instance that should have been running.
> **Instance reconcile sweep — the store and the runtime are kept in agreement, and disagreement is surfaced (register rows 78 + 72)**: the reconcile self-heal above covers configs the reconcile pass *itself* fetched. Two sibling divergences were not covered, and neither surfaced anything.
> - **Row 78 — a config that arrives by replication starts nothing.** `deployed_configurations` is one of LocalDb's ten replicated tables, so on a replicated pair a config can appear in a node's file with **no local actor involved at all** — the CDC sync applier writes the row directly. Proven live 2026-08-15: a fresh site-a-a was seeded from its peer by snapshot replication within ~5 s of boot, i.e. **after** its boot-time load had read an empty table and **invisibly** to its own reconcile (central rightly reported no gap, because central's view of the site was correct). Enabled instances sat dead with their configs present until another restart.
> - **Row 72 — a refreshed config does not reach the running actor.** For a *stale* `ReconcileGapItem` the pass 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.
>
> Both are closed by one **periodic sweep owned by the Deployment Manager singleton** (`ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds`, default 60, validated `>= 0`; `0` disables it). Each tick re-reads the durable rows off-thread and, on the actor thread, for every **enabled** stored config: starts the Instance Actor if none is running (row 78), or — if one is running at a revision the store has since replaced — **re-applies the stored config through the exact per-instance apply/restart path a central redeploy uses** (row 72). A sweep is chosen over a LocalDb change hook deliberately: a hook would cover only the replication arrival path, only on replicated sites, and `ILocalDb` exposes no change-notification API at all, whereas the sweep is arrival-path-agnostic (replication seeding, post-boot snapshot resync, a hand-restored file, any future path). See `docs/plans/2026-08-15-instance-sweep-self-heal-design.md`.
>
> The **row-72 decision is to converge, not merely warn**: the stored row *is* what central successfully deployed, so an actor serving the config central believes was replaced is the actual fault. Reusing `HandleDeploy` means the compile gate, warm-then-gate ordering, redeploy-supersede, the static-override / native-alarm reset and all-or-nothing-per-instance are the production ones — and a stop still does **not** cancel an in-flight script run, exactly as for a central redeploy.
>
> Guards, all shared with the row-67 handler: a **disabled** instance is never started; the sweep **defers to any deploy pipeline that owns the instance** (compile warm in flight, predecessor still terminating, deployment awaiting its persistence/init join) so it can never interleave with a real redeploy; it is **inert until boot-time Instance Actor creation has finished**, so a tick between staggered batches does not "heal" instances the boot load is about to create; it makes **one repair attempt per (instance, revision) pair**, which is what stops a periodic self-heal becoming a restart loop for an actor that dies during init or a stored config the compile gate rejects (a rejection leaves the running instance untouched — the gate returns before anything is stopped); and reads never overlap. A sweep that finds nothing is **completely silent**. A repair emits the same Warning `deployment` site-event pattern as row 67, naming the trigger (`deployment-manager sweep self-heal`) so it is distinguishable from a startup-reconcile heal, and a stale-revision restart names **both** revisions; a rejected restart adds an Error event. The synthetic redeploy replies to the singleton itself rather than to `NoSender`, because dead letters are a monitored site health metric.
> **Per-instance compilation during staggered startup — CLOSED (WP3.1)**: this was a deferred optimization (P6) affecting failover time-to-recover; each Instance Actor Roslyn-compiled its own scripts inside its own start, serialising a site's whole recovery behind compilation. Each staggered startup batch now runs a **pre-warm step**: before the batch's Instance Actors are created, the Deployment Manager compiles that batch's distinct script bodies and trigger expressions on a background task and pipes `BatchCompileWarmed` back, so every `PreStart` compile is a cache hit. Instance Actor start still calls the compilation service — it remains the correctness backstop — but in every warmed path those calls are memoised lookups. Cost: one extra message per batch.
> **The Roslyn `ScriptOptions` are process-static, and must stay that way**: the reference set backing every site-side compile is built once per process, never per compile. `ScriptOptions.WithReferences(Assembly[])` resolves each assembly through `MetadataReference.CreateFromFile`, which does **not** cache — each call mints a fresh `MetadataReference` owning an `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock`, an unmanaged copy of the assembly metadata that nothing disposes. Building the options per compile leaks native memory permanently: no GC reclaims it, and it is invisible to gcdump and to the managed allocation counters, so the node's working set grows without the GC heap growing. This was a live defect (a Site node at 2,885 MB working set with 150 MB of live GC heap, ~2,469 MB of it on the default process heap across ~6,700 undisposed `AssemblyMetadata` instances). Note this is orthogonal to the compile cache above — the cache dedupes *identical* script bodies, so it bounds nothing when the bodies differ. (It no longer clears wholesale on overflow: WP3.1 replaced that latency cliff with approximate-LRU batch eviction, see **Compile cache eviction** below.) Pinned by a reference-equality regression test rather than by watching memory.
@@ -127,6 +137,7 @@ flowchart TD
- **Enable**: Creates a new Instance Actor from the stored configuration (same as startup).
- **Delete**: Stops the Instance Actor and its children, removes the deployed configuration from local SQLite. Does **not** clear store-and-forward messages.
- **Ensure Started** (`EnsureInstanceStartedCommand`, site-internal — never sent by central): advisory, idempotent request to make sure an already-persisted, enabled instance is actually running. Raised by the startup reconcile self-heal; see the note under **Startup Behavior**. Unlike the three commands above it has **no reply** and applies no configuration — it only starts an Instance Actor for a config that is already fully stored, and no-ops when the actor is running, the stored row says disabled, the row is absent, or a deploy for that instance is in flight.
- **Instance reconcile sweep** (internal timer, not a command): the periodic singleton-owned check that the running Instance Actors still agree with the durable config rows — starting an enabled instance that has none, and re-applying a stored config a running instance has fallen behind. See the note under **Startup Behavior**; the start half shares its implementation (and therefore its bookkeeping and site events) with **Ensure Started**, and the restart half goes through the ordinary redeploy path.
### Debug Snapshot Routing
- Receives `DebugSnapshotRequest` from the Communication Layer and forwards to the Instance Actor by unique name (same lookup as `SubscribeDebugViewRequest`).