diff --git a/docs/components/SiteRuntime.md b/docs/components/SiteRuntime.md index 3d846c1d..703ceb35 100644 --- a/docs/components/SiteRuntime.md +++ b/docs/components/SiteRuntime.md @@ -27,6 +27,15 @@ The component code lives in `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/`: The singleton reads all deployed configurations from SQLite in `PreStart`, compiles shared scripts off-thread, and then creates `InstanceActor` children in batches. The default batch size is 20, with a 100 ms delay between batches (`StartupBatchSize` and `StartupBatchDelayMs` in `SiteRuntimeOptions`). Without staggering, 500 instances each subscribing to OPC UA tags simultaneously would produce a reconnection storm that overwhelms the OPC UA server. +### Instance reconcile sweep + +The staggered startup above reads storage exactly once. Anything that writes a `deployed_configurations` row afterwards without going through a deploy command therefore changed the store but not the runtime — and nothing said so. Two real cases (deferred register rows **78** and **72**): + +- a config seeded by **LocalDb CDC replication** (a fresh standby filled in from its peer, or a snapshot resync) landed after the boot load and was invisible to the startup reconcile, so its enabled instances never started; +- a config the startup reconcile **refreshed** because it was stale left the running `InstanceActor` serving the previously compiled revision. + +`DeploymentManagerActor` therefore runs a periodic sweep (`InstanceSweepIntervalSeconds`, default 60; `0` disables) that re-reads the durable rows off-thread and, on the actor thread, for every **enabled** stored config either starts a missing `InstanceActor` or — when the running one's revision no longer matches the stored one — re-applies the stored config **through the ordinary redeploy path**, so nothing about redeploy semantics is re-implemented. It defers to any in-flight deploy/warm/termination, never starts a disabled instance, stays inert until boot-time creation has finished, makes one repair attempt per (instance, revision) pair so it can never become a restart loop, and is completely silent when there is nothing to repair. Repairs emit Warning `deployment` site events naming the trigger (and, for a stale-revision restart, both revisions). Design note: `docs/plans/2026-08-15-instance-sweep-self-heal-design.md`. + ### Actor supervision and lifetimes | Actor | Kind | Supervises children with | On exception | @@ -265,6 +274,7 @@ All options live in the `ScadaBridge:SiteRuntime` section, bound to `SiteRuntime | `MaxConcurrentRunsPerScript` | `4` | Runs in flight (queued or executing) per script/alarm on-trigger script before the newest is shed (WP3.1) | | `MirroredAlarmCapPerSource` | `1000` | Maximum mirrored conditions per `NativeAlarmActor` source binding before oldest is dropped and logged | | `NativeAlarmRetryIntervalMs` | `5000` | Milliseconds before retrying a failed native alarm subscription | +| `InstanceSweepIntervalSeconds` | `60` | Interval of the singleton's instance reconcile sweep (register rows 78 + 72); `0` disables it — see Instance reconcile sweep | The SQLite connection string is passed directly to `AddSiteRuntime(connectionString)` in the host composition root and is not part of `SiteRuntimeOptions`. diff --git a/docs/plans/2026-08-15-instance-sweep-self-heal-design.md b/docs/plans/2026-08-15-instance-sweep-self-heal-design.md new file mode 100644 index 00000000..b95e0634 --- /dev/null +++ b/docs/plans/2026-08-15-instance-sweep-self-heal-design.md @@ -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. diff --git a/docs/requirements/Component-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index ea305300..3d25a06d 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -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`). diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index eb6ba472..b498eb8d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -159,6 +159,58 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// private readonly ScriptExecutionScheduler? _scriptScheduler; + /// + /// Deferred register row 72: the config revision hash each RUNNING Instance Actor was + /// created from, keyed by instance name. Written by — + /// the single creation funnel — and compared against the stored row by the instance + /// reconcile sweep to detect an actor still serving a config the durable store has since + /// replaced. + /// + /// Reads are always guarded by _instanceActors.ContainsKey, so a leftover entry for + /// an instance that has stopped is never consulted, and a stop-then-recreate overwrites it. + /// An instance with no entry (only reachable if a future creation path bypasses the funnel) + /// is treated as "revision unknown" and is deliberately never restarted. + /// + /// + private readonly Dictionary _runningRevisions = new(); + + /// + /// The stored revision hash the instance reconcile sweep has already ACTED on for an + /// instance, keyed by instance name — one repair attempt per (instance, revision) + /// pair, for both the row-78 start and the row-72 restart. + /// + /// This is what keeps a periodic self-heal from becoming a restart loop. An instance whose + /// Instance Actor dies during init leaves its config row in place, so without this the + /// sweep would recreate it — and re-announce it — every tick forever; likewise a stored + /// config the site compile gate rejects would be re-attempted forever. The divergence is + /// still surfaced either way: the attempt itself emits a Warning site event, and a failed + /// restart emits a second one. A genuinely NEW revision (the normal case: central redeploys + /// something better) is a different key and is always attempted. + /// + /// + private readonly Dictionary _sweepAttemptedRevisions = new(); + + /// + /// Interval of the instance reconcile sweep (rows 78 + 72); + /// disables it. See + /// . + /// + private readonly TimeSpan _instanceSweepInterval; + + /// + /// True once boot-time Instance Actor creation has finished (every staggered batch created, + /// or there were no enabled configs to create). The sweep is inert until then: a tick landing + /// between two staggered batches would otherwise "heal" instances the boot load is already + /// about to create, emitting a Warning site event for a node that is merely still starting. + /// + private bool _startupInstanceCreationComplete; + + /// + /// True while a sweep's off-thread storage read is outstanding. A slow read must not let + /// ticks stack up into overlapping reads (and duplicate repairs) on a large site. + /// + private bool _instanceSweepInFlight; + /// Akka timer scheduler injected by the framework via . public ITimerScheduler Timers { get; set; } = null!; @@ -224,6 +276,9 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers _serviceProvider = serviceProvider; _configFetcher = configFetcher; _startupLoadRetryInterval = startupLoadRetryInterval ?? TimeSpan.FromSeconds(5); + _instanceSweepInterval = options.InstanceSweepIntervalSeconds > 0 + ? TimeSpan.FromSeconds(options.InstanceSweepIntervalSeconds) + : TimeSpan.Zero; _configLoader = configLoader ?? (() => _storage.GetAllDeployedConfigsAsync()); _logger = logger; // Reuse a single logger factory for all Instance Actors. @@ -262,6 +317,18 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers Receive(HandleEnsureInstanceStarted); Receive(HandleEnsureInstanceStartedResult); + // Instance reconcile sweep (deferred register rows 78 + 72). Periodically re-reads the + // durable rows and repairs the two ways the runtime can silently diverge from them: + // an enabled config with no running actor (row 78), and a running actor whose revision + // no longer matches the stored one (row 72). + Receive(_ => HandleInstanceSweepTick()); + Receive(HandleInstanceSweepLoaded); + // Terminal reply of a sweep-driven stale-revision restart. Every OTHER deploy path + // replies to central (or to a test probe); only the sweep makes this actor its own + // deployer, so a response landing here is always its own and is surfaced, never + // dead-lettered. + Receive(HandleSweepRestartResult); + // Query-the-site-before-redeploy idempotency. // Central asks for the instance's currently-applied deployment identity // before re-sending a deployment whose prior record is stuck InProgress @@ -380,6 +447,16 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers typeof(ClusterEvent.MemberUp)); } + // Instance reconcile sweep (rows 78 + 72). Armed here rather than after the startup + // load so it survives a load that never succeeds; the handler itself is inert until + // boot-time Instance Actor creation has finished. + if (_instanceSweepInterval > TimeSpan.Zero) + { + Timers.StartPeriodicTimer( + "instance-sweep", InstanceSweepTick.Instance, + _instanceSweepInterval, _instanceSweepInterval); + } + LoadDeployedConfigs(); } @@ -491,7 +568,14 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers msg.CompiledCount, msg.TotalCount); if (msg.EnabledConfigs.Count == 0) + { + // Nothing to create, so boot-time creation is trivially complete and the instance + // reconcile sweep may run. This is exactly the row-78 live case: the node booted + // with an empty deployed_configurations table and CDC replication seeds it moments + // later, with no boot load left to notice. + _startupInstanceCreationComplete = true; return; + } // Start the first batch immediately var batchState = new BatchState(msg.EnabledConfigs, 0); @@ -563,7 +647,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers { var config = state.Configs[i]; EnsureDclConnections(config.ConfigJson); - CreateInstanceActor(config.InstanceUniqueName, config.ConfigJson); + CreateInstanceActor(config.InstanceUniqueName, config.ConfigJson, config.RevisionHash); } UpdateInstanceCounts(); @@ -581,6 +665,9 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers { _logger.LogInformation( "All {Count} Instance Actors created", state.Configs.Count); + // Boot-time creation is done — the instance reconcile sweep may now run without + // racing the staggered batches it would otherwise mistake for a divergence. + _startupInstanceCreationComplete = true; } } @@ -993,7 +1080,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers EnsureDclConnections(command.FlattenedConfigurationJson); // Create the Instance Actor immediately - CreateInstanceActor(instanceName, command.FlattenedConfigurationJson); + CreateInstanceActor(instanceName, command.FlattenedConfigurationJson, command.RevisionHash); if (!isRedeploy) _deployedInstanceNames.Add(instanceName); UpdateInstanceCounts(); @@ -1277,7 +1364,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers if (!_instanceActors.ContainsKey(instanceName)) { - CreateInstanceActor(instanceName, result.Config.ConfigJson); + CreateInstanceActor(instanceName, result.Config.ConfigJson, result.Config.RevisionHash); } UpdateInstanceCounts(); @@ -1335,6 +1422,19 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers .PipeTo(Self); } + /// + /// True when a deploy/redeploy/delete pipeline currently OWNS the instance — a compile warm + /// is in flight, a predecessor is still terminating, or a deployment is awaiting its + /// persistence/init join. Every advisory self-heal (row 67's ensure-started and the row + /// 78/72 sweep) defers to those paths: they are authoritative, they will create or replace + /// the actor themselves, and stepping in front of them would break the deploy gate's + /// ordering guarantees and the all-or-nothing-per-instance rule. + /// + private bool InstanceIsOwnedByADeployPipeline(string instanceName) => + _deployWarms.ContainsKey(instanceName) || + _terminatingActorsByName.ContainsKey(instanceName) || + _inFlightDeployments.ContainsKey(instanceName); + /// /// Shared guard for the two halves of the ensure-started round trip. Re-evaluated after /// the storage read because the instance's state can change while that read is in flight @@ -1342,9 +1442,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// private bool ShouldAttemptEnsureStart(string instanceName) => !_instanceActors.ContainsKey(instanceName) && - !_deployWarms.ContainsKey(instanceName) && - !_terminatingActorsByName.ContainsKey(instanceName) && - !_inFlightDeployments.ContainsKey(instanceName); + !InstanceIsOwnedByADeployPipeline(instanceName); /// /// Actor-thread half of the reconcile self-heal: creates the Instance Actor for a config @@ -1396,21 +1494,261 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers return; } - CreateInstanceActor(instanceName, result.Config.ConfigJson); + StartStoredInstance(result.Config, reason, + "Config was missing at node boot and was restored by reconciliation; " + + "the Instance Actor was not running and has now been started."); + } + + /// + /// The one place an advisory self-heal turns a stored, enabled config into a running + /// Instance Actor — shared by the row-67 ensure-started handler and the row-78 sweep so + /// both produce identical bookkeeping and identical operator-facing surfacing. Callers + /// are responsible for the guards; this method assumes they have passed. + /// + /// The durable row to start from — the authority on config and revision. + /// Provenance, carried verbatim into the log line and the site event. + /// Longer explanation attached to the site event. + private void StartStoredInstance(DeployedInstance config, string reason, string details) + { + var instanceName = config.InstanceUniqueName; + + CreateInstanceActor(instanceName, config.ConfigJson, config.RevisionHash); _deployedInstanceNames.Add(instanceName); UpdateInstanceCounts(); - // Surface the heal. Warning (not Info) on purpose: reaching here means the node - // booted WITHOUT a config for an enabled, successfully-deployed instance — the - // condition is repaired, but it is abnormal and worth an operator's attention. + // Surface the heal. Warning (not Info) on purpose: reaching here means an enabled, + // successfully-deployed instance was NOT running while its config sat in the store — + // the condition is repaired, but it is abnormal and worth an operator's attention. _logger.LogWarning( - "Instance {Instance} started by {Reason} self-heal — its config was missing at boot " - + "and has been restored; the instance was NOT running until now", + "Instance {Instance} started by {Reason} self-heal — its config is present in the " + + "deployed-config store but no Instance Actor was running until now", instanceName, reason); LogDeploymentEvent("Warning", instanceName, - $"Instance {instanceName} started by {reason} self-heal", - "Config was missing at node boot and was restored by reconciliation; " - + "the Instance Actor was not running and has now been started."); + $"Instance {instanceName} started by {reason} self-heal", details); + } + + /// Provenance string carried on every repair the instance reconcile sweep makes. + private const string SweepReason = "deployment-manager sweep"; + + /// + /// Instance reconcile sweep, tick half (deferred register rows 78 + 72). Kicks off an + /// off-thread read of the durable deployed_configurations rows; the comparison and + /// every repair happen on the actor thread in + /// . + /// + /// + /// Why a periodic sweep and not a LocalDb change hook. Row 78's live symptom was a + /// config that arrived by CDC replication: the sync applier writes + /// deployed_configurations rows straight into the file, which is invisible to the + /// boot-time load (already run) and to the startup reconcile (central reports no gap, + /// because central's view is correct — the site really does have the config). A hook on the + /// replication apply would cover exactly that one arrival path, and only for the half of + /// the fleet that runs with replication enabled. It is also not available: ILocalDb + /// exposes no change-notification or applied-rows callback at all, so a hook would mean a + /// new upstream library capability plus an in-process bus from the sync applier into this + /// singleton. A sweep over the durable rows is the arrival-path-agnostic form of the same + /// check — replication seeding, a post-boot snapshot resync, an operator restoring a file, + /// and any future path all converge on "the store says X, the runtime says Y" — and it + /// inherits row 67's guards unchanged. + /// + /// + /// + /// Inert until boot-time creation finishes. A tick landing between two staggered + /// startup batches would see instances the boot load is about to create and "heal" them, + /// announcing a fault on a node that is merely still starting. + /// + /// + private void HandleInstanceSweepTick() + { + if (!_startupInstanceCreationComplete || _instanceSweepInFlight) + return; + + _instanceSweepInFlight = true; + + _configLoader().ContinueWith(t => t.IsCompletedSuccessfully + ? new InstanceSweepLoaded(t.Result, null) + : new InstanceSweepLoaded([], t.Exception?.GetBaseException().Message)) + .PipeTo(Self); + } + + /// + /// Instance reconcile sweep, actor-thread half: compares the durable rows against the + /// running Instance Actors and repairs the two divergences the register recorded. + /// + /// + /// A sweep that finds nothing is completely silent — no log line above Trace, no site + /// event — because that is the case on every tick of a healthy site. + /// + /// + private void HandleInstanceSweepLoaded(InstanceSweepLoaded msg) + { + _instanceSweepInFlight = false; + + if (msg.Error != null) + { + _logger.LogWarning( + "Instance reconcile sweep could not read the deployed configs: {Error}", msg.Error); + return; + } + + foreach (var config in msg.Configs) + { + var instanceName = config.InstanceUniqueName; + + // A disabled instance legitimately has a config row and no running actor; that is + // its correct steady state, not a divergence. + if (!config.IsEnabled) + continue; + + // Defer to any deploy pipeline that owns the instance right now. It is + // authoritative and will create or replace the actor itself. + if (InstanceIsOwnedByADeployPipeline(instanceName)) + continue; + + if (!_instanceActors.ContainsKey(instanceName)) + { + // Row 78 — an enabled config with no actor. + if (!TryClaimSweepAttempt(instanceName, config.RevisionHash)) + continue; + + StartStoredInstance(config, SweepReason, + "An enabled instance had a deployed config in the site store but no running " + + "Instance Actor. Configs that arrive by LocalDb replication (a standby " + + "seeded from its peer, or a snapshot resync) are written after the " + + "boot-time load and are invisible to the startup reconcile, so nothing " + + "else would have started it."); + continue; + } + + // Row 72 — a running actor serving a revision the store has replaced. + TryRestartStaleInstance(config); + } + } + + /// + /// Claims the single sweep repair attempt for an (instance, revision) pair. Returns + /// when this exact revision has already been attempted — see + /// for why a periodic self-heal must not retry the + /// same repair forever. + /// + private bool TryClaimSweepAttempt(string instanceName, string revisionHash) + { + if (_sweepAttemptedRevisions.TryGetValue(instanceName, out var attempted) && + string.Equals(attempted, revisionHash, StringComparison.Ordinal)) + { + return false; + } + + _sweepAttemptedRevisions[instanceName] = revisionHash; + return true; + } + + /// + /// Deferred register row 72 — the running Instance Actor is serving a config the durable + /// store has replaced. + /// + /// + /// The startup reconcile refreshes a STALE row via StoreDeployedConfigIfNewerAsync + /// (and a CDC-replicated deploy lands the same way), which heals the file while the running + /// actor keeps executing the previously compiled config until the next restart or redeploy. + /// Nothing surfaced the difference. + /// + /// + /// + /// The decision (2026-08-15, docs/plans/2026-08-15-instance-sweep-self-heal-design.md): + /// converge, do not merely warn. The stored row is what central successfully deployed; + /// the running actor is executing a config central believes was replaced. Serving it + /// indefinitely is the actual fault. The stored config is therefore applied through the + /// EXACTLY the same path a central redeploy uses — , so the + /// compile gate, the warm-then-gate ordering, the stop/watch/recreate handshake, the + /// static-override and native-alarm reset, and all-or-nothing-per-instance are the ones + /// already in production, not a second implementation. A stop still does not cancel an + /// in-flight script run, exactly as for a central redeploy. + /// + /// + /// + /// Safety. The restart is attempted once per stored revision, so a config the + /// compile gate rejects (which leaves the running actor untouched — the gate returns before + /// anything is stopped) is announced but never looped on. The sweep is also skipped + /// entirely while a deploy pipeline owns the instance, so it can never interleave with a + /// real redeploy. + /// + /// + private void TryRestartStaleInstance(DeployedInstance config) + { + var instanceName = config.InstanceUniqueName; + var storedRevision = config.RevisionHash; + + // No stored revision to compare against — nothing can be concluded, so do nothing. + if (string.IsNullOrEmpty(storedRevision)) + return; + + // No recorded running revision. Unreachable while CreateInstanceActor is the single + // creation funnel; treated as "unknown", never as "stale", so a future path that + // bypasses the funnel degrades to no-op rather than to spurious restarts. + if (!_runningRevisions.TryGetValue(instanceName, out var runningRevision) || + string.IsNullOrEmpty(runningRevision)) + { + return; + } + + if (string.Equals(runningRevision, storedRevision, StringComparison.Ordinal)) + return; + + if (!TryClaimSweepAttempt(instanceName, storedRevision)) + return; + + _logger.LogWarning( + "Instance {Instance} is running stale revision {RunningRevision} while the deployed-config " + + "store holds {StoredRevision} — applying the stored config through the redeploy path " + + "({Reason} self-heal)", + instanceName, runningRevision, storedRevision, SweepReason); + LogDeploymentEvent("Warning", instanceName, + $"Instance {instanceName} restarted by {SweepReason} self-heal — " + + $"running revision {runningRevision} is stale, applying stored revision {storedRevision}", + "The site's deployed-config store was refreshed out of band (startup reconcile " + + "refreshing a stale row, or a replicated deploy) while the Instance Actor kept " + + "serving the previously compiled config. The stored config is what central " + + "successfully deployed, so it is applied through the normal redeploy path."); + + // Reply to Self: this actor is its own deployer here, and HandleSweepRestartResult + // surfaces the outcome. Replying to NoSender would dead-letter it, and dead letters are + // a monitored site health metric. + HandleDeploy( + new DeployInstanceCommand( + config.DeploymentId, + instanceName, + storedRevision, + config.ConfigJson, + $"{SweepReason} self-heal", + DateTimeOffset.UtcNow), + Self); + } + + /// + /// Terminal outcome of a sweep-driven stale-revision restart. Only the sweep makes this + /// actor its own deployer, so any arriving here is + /// its own. A failure is surfaced and NOT retried (see + /// ); the instance keeps serving whatever config it + /// had, which for a compile-gate rejection is the old one, untouched. + /// + private void HandleSweepRestartResult(DeploymentStatusResponse response) + { + if (response.Status == DeploymentStatus.Success) + { + _logger.LogInformation( + "Instance {Instance} re-applied its stored config after a stale-revision " + + "{Reason} self-heal (deploymentId={DeploymentId})", + response.InstanceUniqueName, SweepReason, response.DeploymentId); + return; + } + + _logger.LogWarning( + "Stale-revision {Reason} self-heal for {Instance} did not apply: {Error}", + SweepReason, response.InstanceUniqueName, response.ErrorMessage); + LogDeploymentEvent("Error", response.InstanceUniqueName, + $"Instance {response.InstanceUniqueName} stale-revision {SweepReason} self-heal failed", + response.ErrorMessage ?? "no error detail"); } /// @@ -1462,6 +1800,11 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers // removes nothing and leaves the count unchanged. Supersedes the earlier // map-presence check, which leaked the count on disabled-instance deletes. _deployedInstanceNames.Remove(instanceName); + // Delete is the one lifecycle event after which the instance name never comes back on + // its own, so drop the sweep's per-instance bookkeeping with it rather than letting it + // accumulate for the process lifetime. + _runningRevisions.Remove(instanceName); + _sweepAttemptedRevisions.Remove(instanceName); UpdateInstanceCounts(); var sender = replyTo; @@ -2373,7 +2716,14 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// /// The unique name of the instance to create an actor for. /// The JSON-serialized flattened configuration for the instance. - internal void CreateInstanceActor(string instanceName, string configJson) + /// + /// Revision hash of , recorded in + /// so the instance reconcile sweep can tell an actor serving the CURRENT stored config from + /// one still serving a revision the store has replaced (deferred register row 72). This is + /// the single creation funnel, so recording it here covers every path that can ever start an + /// Instance Actor. + /// + internal void CreateInstanceActor(string instanceName, string configJson, string revisionHash) { if (_instanceActors.ContainsKey(instanceName)) { @@ -2398,6 +2748,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers var actorRef = Context.ActorOf(props, instanceName); _instanceActors[instanceName] = actorRef; + _runningRevisions[instanceName] = revisionHash; // Watch every Instance Actor so an unexpected termination (init failure or // crash) delivers a Terminated signal that evicts the dead ref from routing // (S6). Redeploy already double-watches; a double Watch is idempotent in @@ -2557,6 +2908,24 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// internal record EnsureInstanceStartedResult( EnsureInstanceStartedCommand Command, DeployedInstance? Config, string? Error); + + /// + /// Periodic instance reconcile-sweep tick (deferred register rows 78 + 72). Interval comes + /// from . + /// + internal sealed record InstanceSweepTick + { + /// The singleton tick instance — the message carries no state. + public static readonly InstanceSweepTick Instance = new(); + private InstanceSweepTick() { } + } + + /// + /// Carries one sweep's off-thread read of the durable deployed-config rows back onto the + /// actor thread, where every comparison and repair happens. + /// + internal record InstanceSweepLoaded(List Configs, string? Error); + internal record DeployPersistenceResult( string DeploymentId, string InstanceName, bool Success, string? Error, IActorRef OriginalSender, bool IsRedeploy); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs index e63d1bcc..598a58b8 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs @@ -181,4 +181,25 @@ public class SiteRuntimeOptions /// script run under the legacy per-run behaviour. /// public int ScriptRunSummaryIntervalSeconds { get; set; } = 300; + + /// + /// Interval (seconds) of the Deployment Manager singleton's instance reconcile sweep + /// (deferred register rows 78 + 72). Every tick the singleton re-reads the durable + /// deployed_configurations rows and repairs two silent divergences between the + /// stored configs and the running Instance Actors: + /// + /// an ENABLED instance with a stored config and NO running actor is started + /// (row 78 — configs that arrive by LocalDb CDC replication are written straight into the + /// file by the sync applier, so they are invisible to both the boot-time load, which has + /// already run, and the startup reconcile, which reports no gap because central's view is + /// correct); + /// a running instance whose stored revision hash no longer matches the revision it + /// was started from is re-applied through the ordinary redeploy path (row 72 — the startup + /// reconcile refreshes a stale config row but leaves the running actor serving the config + /// central believes it replaced). + /// + /// The sweep is silent when there is nothing to repair. 0 disables it entirely + /// (the row-67 EnsureInstanceStartedCommand path is unaffected). Default: 60. + /// + public int InstanceSweepIntervalSeconds { get; set; } = 60; } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs index 2fd1739b..53e81c69 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs @@ -91,5 +91,12 @@ public sealed class SiteRuntimeOptionsValidator : OptionsValidatorBase= 0 " + $"(was {options.ScriptRunSummaryIntervalSeconds}); it is the ScriptRunSummaryRecorder " + "flush interval and a negative value throws inside its delay. 0 disables the flush."); + + builder.RequireThat(options.InstanceSweepIntervalSeconds >= 0, + $"ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds must be >= 0 " + + $"(was {options.InstanceSweepIntervalSeconds}); it is the Deployment Manager's " + + "instance reconcile-sweep interval and a negative value throws inside the Akka timer. " + + "0 disables the sweep, which reopens the replication-seeded-config and stale-revision " + + "self-heals (deferred register rows 78 and 72)."); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceSweepSelfHealTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceSweepSelfHealTests.cs new file mode 100644 index 00000000..032c4048 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceSweepSelfHealTests.cs @@ -0,0 +1,549 @@ +using System.Text.Json; +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.HealthMonitoring; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; +using ZB.MOM.WW.ScadaBridge.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// Regression tests for deferred register rows 78 and 72 — the two ways the site +/// runtime silently diverges from its own durable deployed_configurations rows. +/// +/// +/// Row 78 (availability). Instance Actors were created from three paths: the boot-time +/// SQLite load, an explicit deploy/enable command, and (row 67) the startup reconcile's +/// for configs the reconcile ITSELF fetched. A config +/// that arrives by LocalDb CDC replication hits none of them: the sync applier writes the +/// row straight into the file, AFTER the boot load has read storage and invisibly to the +/// reconcile, which correctly reports no gap because central's view of the site is right. Proven +/// live 2026-08-15 — a fresh site-a-a was seeded from its peer within ~5 s of boot and its +/// enabled instances sat dead, with their configs present, until another restart. +/// +/// +/// +/// Row 72 (silent divergence). The reconcile refreshes a STALE row via +/// StoreDeployedConfigIfNewerAsync, but the running Instance Actor keeps serving the OLD +/// compiled config until the next restart or redeploy, and nothing surfaced +/// "stored revision != running revision". +/// +/// +/// +/// Both are closed by one periodic sweep owned by the Deployment Manager singleton. Design note: +/// docs/plans/2026-08-15-instance-sweep-self-heal-design.md. +/// +/// +public class InstanceSweepSelfHealTests : TestKit, IDisposable +{ + private readonly SiteStorageService _storage; + private readonly ScriptCompilationService _compilationService; + private readonly SharedScriptLibrary _sharedScriptLibrary; + private readonly TestLocalDb _localDb; + + public InstanceSweepSelfHealTests() + { + _localDb = TestLocalDb.CreateTemp("instance-sweep-test"); + _storage = new SiteStorageService(_localDb.Db, NullLogger.Instance); + _storage.InitializeAsync().GetAwaiter().GetResult(); + _compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedScriptLibrary = new SharedScriptLibrary( + _compilationService, NullLogger.Instance); + } + + void IDisposable.Dispose() + { + // Shut the actors down BEFORE the ILocalDb they write through is disposed. + Shutdown(); + var path = _localDb.Path; + _localDb.Dispose(); + TestLocalDb.DeleteFiles(path); + } + + /// + /// Sweep interval used by these tests. One second is the shortest the (whole-second) option + /// allows; every assertion below therefore budgets several seconds so it is waiting on the + /// behaviour, not on the clock. + /// + private const int SweepSeconds = 1; + + /// Long enough for at least three sweep ticks to have come and gone. + private static readonly TimeSpan SeveralSweeps = TimeSpan.FromSeconds(4); + + /// + /// Starts a Deployment Manager and blocks until its boot-time config load has PROVABLY read + /// storage (its first instance-count report). Load-bearing for every test here: a row written + /// before that read is picked up by the boot load itself, which is precisely the path row 78 + /// is about NOT having. Without this gate a test would silently prove the boot load works. + /// + private IActorRef CreateBootedDeploymentManager( + FakeSiteEventLogger? siteLog = null, + int sweepSeconds = SweepSeconds) + { + var health = new StartupProbe(); + var dm = ActorOf(Props.Create(() => new DeploymentManagerActor( + _storage, + _compilationService, + _sharedScriptLibrary, + null, + new SiteRuntimeOptions { InstanceSweepIntervalSeconds = sweepSeconds }, + NullLogger.Instance, + dclManager: null, + healthCollector: health, + serviceProvider: siteLog == null ? null : new SingleServiceProvider(siteLog), + loggerFactory: null, + configFetcher: null))); + + AwaitCondition(() => health.ReportCount > 0, TimeSpan.FromSeconds(20), + TimeSpan.FromMilliseconds(25)); + return dm; + } + + private static string MakeConfigJson(string instanceName, string attributeValue = "42") => + JsonSerializer.Serialize(new FlattenedConfiguration + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute + { + CanonicalName = "TestAttr", Value = attributeValue, DataType = "Int32" + } + ] + }); + + /// + /// A config carrying a real script, so its compile gate / compile warm takes long enough for + /// the deploy pipeline to hold the instance across a sweep tick. + /// + private static string MakeScriptedConfigJson(string instanceName, string scriptCode) => + JsonSerializer.Serialize(new FlattenedConfiguration + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute + { + CanonicalName = "TestAttr", Value = "42", DataType = "Int32" + } + ], + Scripts = + [ + new ResolvedScript + { + CanonicalName = "Worker", Code = scriptCode, TriggerType = "Call" + } + ] + }); + + /// + /// Asks the Deployment Manager for a debug snapshot — the exact probe that answered + /// instanceNotFound on the live rig. + /// + private async Task SnapshotAsync(IActorRef dm, string instanceName) => + await dm.Ask( + new SubscribeDebugViewRequest(instanceName, Guid.NewGuid().ToString()), + TimeSpan.FromSeconds(5)); + + private async Task InstanceIsRunningAsync(IActorRef dm, string instanceName) => + !(await SnapshotAsync(dm, instanceName)).InstanceNotFound; + + private async Task ResolveInstanceChildAsync(IActorRef dm, string instanceName) => + await Sys.ActorSelection(dm.Path / instanceName).ResolveOne(TimeSpan.FromSeconds(5)); + + /// + /// Site events announcing a sweep REPAIR (a start or a stale-revision restart). The terminal + /// "self-heal failed" event is deliberately excluded — it reports the outcome of a repair + /// already counted here, so counting both would double-count a single attempt. + /// + private static IReadOnlyList SelfHealEvents( + FakeSiteEventLogger log, string instanceName) => + log.OfType("deployment") + .Where(e => e.InstanceId == instanceName && + e.Message.Contains("self-heal", StringComparison.OrdinalIgnoreCase) && + !e.Message.Contains("self-heal failed", StringComparison.OrdinalIgnoreCase)) + .ToArray(); + + /// + /// Deploys an instance the ordinary way and blocks until it is running, so a test can start + /// from a known-good runtime state. + /// + private async Task DeployAndAwaitAsync( + IActorRef dm, string instanceName, string revision, string configJson) + { + dm.Tell(new DeployInstanceCommand( + $"dep-{revision}", instanceName, revision, configJson, "admin", DateTimeOffset.UtcNow)); + var response = ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.Equal(DeploymentStatus.Success, response.Status); + await AwaitAssertAsync(async () => + Assert.True(await InstanceIsRunningAsync(dm, instanceName)), TimeSpan.FromSeconds(10)); + } + + // ── Row 78: a config that arrives with no deploy, no enable and no reconcile ── + + /// + /// THE row-78 reproduction. The node boots with an empty deployed-config store (so the + /// boot-time load starts nothing), and the config for an ENABLED instance then appears in + /// storage on its own — exactly what LocalDb's CDC sync applier does when it seeds a fresh + /// node from its peer. No , no + /// , no reconcile actor: nothing but the row. + /// + /// + /// Fails before the fix at the final assert — the row is in storage and the debug + /// snapshot still answers InstanceNotFound = true, the live symptom verbatim. + /// + /// + [Fact] + public async Task Sweep_StartsEnabledInstance_WhoseConfigArrivedWithoutADeployOrReconcile() + { + const string instanceName = "ReplicatedPump"; + + var dm = CreateBootedDeploymentManager(); + + // Baseline: the node genuinely booted with nothing. + await AwaitAssertAsync(async () => + Assert.False(await InstanceIsRunningAsync(dm, instanceName)), TimeSpan.FromSeconds(5)); + + // Replication seeds the row. This is a bare storage write on purpose — it is the only + // side effect the CDC applier has, and it is invisible to every other start path. + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-replicated", "rev-replicated", true); + + await AwaitAssertAsync(async () => + Assert.True( + await InstanceIsRunningAsync(dm, instanceName), + "A replication-seeded config for an enabled instance left no running Instance " + + "Actor — the instance would stay dead until the next restart (register row 78)"), + TimeSpan.FromSeconds(20)); + } + + /// + /// The heal must be attributable: it emits the same Warning deployment site event + /// pattern as row 67, with the trigger named so an operator can tell a sweep heal from a + /// startup-reconcile heal. + /// + [Fact] + public async Task SweepStartedInstance_EmitsAWarningSiteEvent_NamingTheSweepTrigger() + { + const string instanceName = "SurfacedSweepPump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-s", "rev-s", true); + + await AwaitAssertAsync(() => + { + Assert.Contains(siteLog.OfType("deployment"), e => + e.InstanceId == instanceName && + e.Severity == "Warning" && + e.Source == "DeploymentManagerActor" && + e.Message.Contains("deployment-manager sweep", StringComparison.OrdinalIgnoreCase) && + e.Message.Contains("self-heal", StringComparison.OrdinalIgnoreCase)); + return Task.CompletedTask; + }, TimeSpan.FromSeconds(20)); + } + + // ── Row 78 negative guards ── + + /// + /// A DISABLED instance legitimately has a config row and no running actor. The sweep must + /// never start it — that would silently undo an operator's disable. + /// + [Fact] + public async Task Sweep_NeverStarts_ADisabledInstance() + { + const string instanceName = "DisabledSweepPump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-d", "rev-d", false); + + await Task.Delay(SeveralSweeps); + + Assert.False(await InstanceIsRunningAsync(dm, instanceName)); + Assert.Empty(SelfHealEvents(siteLog, instanceName)); + } + + /// + /// Idempotence: many sweeps, one start. The sweep runs forever on a live node, so a repair + /// that re-fired every tick would be a restart loop plus an unbounded stream of Warning site + /// events. + /// + [Fact] + public async Task Sweep_StartsTheInstanceExactlyOnce_AcrossManySweeps() + { + const string instanceName = "IdempotentSweepPump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-i", "rev-i", true); + + await AwaitAssertAsync(async () => + Assert.True(await InstanceIsRunningAsync(dm, instanceName)), TimeSpan.FromSeconds(20)); + + var afterFirstHeal = await ResolveInstanceChildAsync(dm, instanceName); + + // Let several more sweeps run over the now-healthy state. + await Task.Delay(SeveralSweeps); + + Assert.Equal(afterFirstHeal, await ResolveInstanceChildAsync(dm, instanceName)); + Assert.Single(SelfHealEvents(siteLog, instanceName)); + } + + /// + /// A sweep that finds nothing wrong must be completely silent — no site events at all. This + /// runs on every tick of every healthy site, forever. + /// + [Fact] + public async Task Sweep_IsSilent_WhenThereIsNothingToHeal() + { + const string instanceName = "HealthySweepPump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await DeployAndAwaitAsync(dm, instanceName, "rev-1", MakeConfigJson(instanceName)); + + var eventsAfterDeploy = siteLog.Entries.Count; + await Task.Delay(SeveralSweeps); + + Assert.Equal(eventsAfterDeploy, siteLog.Entries.Count); + Assert.True(await InstanceIsRunningAsync(dm, instanceName)); + } + + /// + /// The sweep is opt-out: InstanceSweepIntervalSeconds = 0 arms no timer at all, so a + /// site that wants the pre-fix behaviour keeps it (and the option's disable path is proven, + /// not merely validated). + /// + [Fact] + public async Task Sweep_DoesNothing_WhenTheIntervalIsZero() + { + const string instanceName = "NoSweepPump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog, sweepSeconds: 0); + + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-z", "rev-z", true); + + await Task.Delay(SeveralSweeps); + + Assert.False(await InstanceIsRunningAsync(dm, instanceName)); + Assert.Empty(SelfHealEvents(siteLog, instanceName)); + } + + /// + /// The sweep must defer to any deploy pipeline that owns the instance. A redeploy passes + /// through a window where the predecessor has been stopped and removed from routing but its + /// replacement does not exist yet — the sweep would read that as "enabled config, no actor" + /// and try to create a child whose actor name is still registered. + /// + /// + /// Sweep ticks are fired continuously across the whole redeploy so the window is genuinely + /// sampled; the assertion is that the redeploy succeeded untouched and the sweep announced + /// nothing. + /// + /// + [Fact] + public async Task Sweep_DefersToAnInFlightRedeploy() + { + const string instanceName = "RedeployingPump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await DeployAndAwaitAsync( + dm, instanceName, "rev-1", + MakeScriptedConfigJson(instanceName, "var a = 1; var b = a + 1; return b;")); + + using var ticking = new CancellationTokenSource(); + var ticker = Task.Run(async () => + { + while (!ticking.IsCancellationRequested) + { + dm.Tell(DeploymentManagerActor.InstanceSweepTick.Instance); + await Task.Delay(5); + } + }); + + dm.Tell(new DeployInstanceCommand( + "dep-rev-2", instanceName, "rev-2", + MakeScriptedConfigJson(instanceName, "var x = 2; var y = x * 3; return y;"), + "admin", DateTimeOffset.UtcNow)); + var response = ExpectMsg(TimeSpan.FromSeconds(20)); + + await ticking.CancelAsync(); + await ticker; + + Assert.Equal(DeploymentStatus.Success, response.Status); + Assert.True(await InstanceIsRunningAsync(dm, instanceName)); + Assert.Empty(SelfHealEvents(siteLog, instanceName)); + } + + // ── Row 72: a running actor serving a revision the store has replaced ── + + /// + /// THE row-72 reproduction. An instance is deployed and running at rev-1; the durable + /// row is then refreshed to rev-2 out of band — what + /// StoreDeployedConfigIfNewerAsync does when the startup reconcile finds a STALE + /// config, and what a replicated deploy does on the standby. + /// + /// + /// Fails before the fix: the actor keeps serving rev-1's config indefinitely + /// (the snapshot still shows the old attribute value) and nothing anywhere says the stored + /// and running revisions differ. + /// + /// + [Fact] + public async Task Sweep_RestartsInstance_ServingAStaleRevision() + { + const string instanceName = "StalePump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await DeployAndAwaitAsync( + dm, instanceName, "rev-1", MakeConfigJson(instanceName, attributeValue: "111")); + + var beforeHeal = await ResolveInstanceChildAsync(dm, instanceName); + + // The reconcile refreshes the stale row. Storage only; the running actor is untouched. + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName, attributeValue: "222"), + "dep-rev-2", "rev-2", true); + + // The runtime must converge on the stored config... + await AwaitAssertAsync(async () => + { + var snapshot = await SnapshotAsync(dm, instanceName); + Assert.False(snapshot.InstanceNotFound); + Assert.Contains(snapshot.AttributeValues, a => + a.AttributeName == "TestAttr" && a.Value?.ToString() == "222"); + }, TimeSpan.FromSeconds(25)); + + // ...through a genuine restart, not an in-place mutation. + Assert.NotEqual(beforeHeal, await ResolveInstanceChildAsync(dm, instanceName)); + + // ...and the divergence must be surfaced, naming BOTH revisions. + Assert.Contains(SelfHealEvents(siteLog, instanceName), e => + e.Severity == "Warning" && + e.Message.Contains("rev-1", StringComparison.Ordinal) && + e.Message.Contains("rev-2", StringComparison.Ordinal)); + } + + /// + /// The stale-revision restart must not fire when the stored and running revisions agree — + /// otherwise every healthy instance on the site would be restarted once a minute. + /// + [Fact] + public async Task Sweep_DoesNotRestart_WhenStoredAndRunningRevisionsMatch() + { + const string instanceName = "MatchingRevisionPump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await DeployAndAwaitAsync(dm, instanceName, "rev-1", MakeConfigJson(instanceName)); + var beforeSweeps = await ResolveInstanceChildAsync(dm, instanceName); + + // Rewrite the SAME revision — a same-revision re-store must not read as divergence. + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-rev-1", "rev-1", true); + + await Task.Delay(SeveralSweeps); + + Assert.Equal(beforeSweeps, await ResolveInstanceChildAsync(dm, instanceName)); + Assert.Empty(SelfHealEvents(siteLog, instanceName)); + } + + /// + /// A stale-revision restart that the site compile gate REJECTS must be announced once and + /// then left alone. Retrying it every tick would be an endless stream of Warning + Error + /// site events for a config that can never apply, and the running instance is unharmed — + /// the gate returns before anything is stopped, so it keeps serving the old config. + /// + [Fact] + public async Task Sweep_StaleRestartRejectedByTheCompileGate_IsAnnouncedOnceAndNotRetried() + { + const string instanceName = "UncompilablePump"; + var siteLog = new FakeSiteEventLogger(); + var dm = CreateBootedDeploymentManager(siteLog); + + await DeployAndAwaitAsync(dm, instanceName, "rev-1", MakeConfigJson(instanceName)); + var beforeHeal = await ResolveInstanceChildAsync(dm, instanceName); + + // The refreshed row carries a script that cannot compile. + await _storage.StoreDeployedConfigAsync( + instanceName, + MakeScriptedConfigJson(instanceName, "this is not valid C# at all ((("), + "dep-rev-bad", "rev-bad", true); + + // The attempt is announced... + await AwaitAssertAsync(() => + { + Assert.Single(SelfHealEvents(siteLog, instanceName)); + return Task.CompletedTask; + }, TimeSpan.FromSeconds(20)); + + // ...its failure is announced... + await AwaitAssertAsync(() => + { + Assert.Contains(siteLog.OfType("deployment"), e => + e.InstanceId == instanceName && + e.Severity == "Error" && + e.Message.Contains("self-heal failed", StringComparison.OrdinalIgnoreCase)); + return Task.CompletedTask; + }, TimeSpan.FromSeconds(20)); + + // ...and several further sweeps add nothing and leave the old actor serving. + await Task.Delay(SeveralSweeps); + + Assert.Single(SelfHealEvents(siteLog, instanceName)); + Assert.Equal(beforeHeal, await ResolveInstanceChildAsync(dm, instanceName)); + Assert.True(await InstanceIsRunningAsync(dm, instanceName)); + } + + /// + /// Minimal whose only job is to expose the observable edge + /// "the Deployment Manager's startup config load has completed". The manager calls + /// unconditionally the moment that load finishes — including + /// for an empty site — so a non-zero replaces a wall-clock sleep. + /// + private sealed class StartupProbe : ISiteHealthCollector + { + private int _reports; + + public int ReportCount => Volatile.Read(ref _reports); + + public void SetInstanceCounts(int deployed, int enabled, int disabled) => + Interlocked.Increment(ref _reports); + + public void IncrementScriptError() { } + public void IncrementAlarmError() { } + public void IncrementDeadLetter() { } + public void IncrementSiteAuditWriteFailures() { } + public void IncrementAuditRedactionFailure() { } + public void UpdateSiteAuditBacklog(Commons.Types.SiteAuditBacklogSnapshot snapshot) { } + public void UpdateConnectionHealth(string connectionName, ConnectionHealth health) { } + public void RemoveConnection(string connectionName) { } + public void UpdateTagResolution(string connectionName, int totalSubscribed, int successfullyResolved) { } + public void UpdateConnectionEndpoint(string connectionName, string endpoint) { } + public void UpdateTagQuality(string connectionName, int good, int bad, int uncertain) { } + public void SetStoreAndForwardDepths(IReadOnlyDictionary depths) { } + public void SetParkedMessageCount(int count) { } + public void SetNodeHostname(string hostname) { } + public void SetClusterNodes(IReadOnlyList nodes) { } + public void SetActiveNode(bool isActive) { } + public bool IsActiveNode => true; + public Commons.Messages.Health.SiteHealthReport CollectReport(string siteId) => + throw new NotSupportedException("Startup probe collects no report."); + } +}