fix(siteruntime): self-healed and newly-enabled instances now get their DCL connections (register row 85)
An Instance Actor started by a self-heal came up with NO DCL connection actors, so every tag it subscribed was answered "Unknown connection: <name>" by the DCL manager indefinitely. Proven live 2026-08-16 by the row-78 close-out probe: the sweep correctly started the replication-seeded instances, then every subscribe failed for 6+ min until a second pair restart. Root cause is narrower than the register assumed. A DCL connection actor is never created from data_connection_definitions -- that table has no production reader at all (GetAllDataConnectionDefinitionsAsync is called only by a test). It exists only as a side effect of starting an instance, from the Connections map inside that instance's own flattened config, via EnsureDclConnections. That ensure was the CALLER's job and only two of the four callers made it: CreateInstanceActorBatch (boot) ensured OK ApplyDeployment (deploy) ensured OK StartStoredInstance MISSING <- row-67 ensure-started + row-78 sweep HandleEnableResult MISSING <- enabling a disabled instance The two misses are the two paths added after the original pair -- the signature of a convention living at call sites instead of in the funnel. It was never replication-specific: the boot load reads only ENABLED configs, so a node booting with an instance disabled never creates its connections either, and enabling it afterwards hit the identical dead end on an ordinary unreplicated site. Fix: EnsureDclConnections moves into CreateInstanceActor, already documented as the single Instance Actor creation funnel (it is where the running revision is stamped for the same reason), and the two caller-side calls are removed. Placed before the already-exists guard so the two paths that already called it keep their exact previous semantics. No new message, option or DCL change -- it reuses EnsureDclConnection's existing per-name hash guard, so steady state is silent. Structural rather than another guard: a fifth start path cannot forget. Deliberately NOT done, with rationale in the memo: a sweep pass over data_connection_definitions. It would not have fixed row 85 (that table is empty on a site that has only ever received instance deploys), and re-hashing from it would fight the deploy path over the same per-name hash slot, recreating connection actors and dropping live subscriptions on a loop. Two residual sibling gaps (connection-only artifact deploy replicating to a peer that never adopts it, and the mirror-image removal case) are recorded in the memo and the component docs as open, not silently skipped. Tests: SelfHealCreatesDclConnectionTests, 7 tests asserting on the CreateConnectionCommand messages sent to a recording DCL stand-in. Verified fail-before against a behavioral revert (call removed from the funnel, restored at the two original sites): 4 fail / 3 pass, the 3 being steady-state silence plus two refactor guards that must pass on both sides. Docs: sweep design memo gains section 7; Component-SiteRuntime, Component-DataConnectionLayer, docs/components/SiteRuntime and docs/components/DataConnectionLayer updated (the deploy-flow step ordering in docs/components/SiteRuntime was stale as written).
This commit is contained in:
Joseph Doherty
@@ -161,7 +161,7 @@ A separate in-flight set `_subscribesInFlight` tracks the initial `SubscribeAsyn
|
||||
|
||||
### Creating a connection
|
||||
|
||||
The Site Runtime's `DeploymentManagerActor` sends `CreateConnectionCommand` to `DataConnectionManagerActor` during instance deployment:
|
||||
The Site Runtime's `DeploymentManagerActor` sends `CreateConnectionCommand` to `DataConnectionManagerActor` whenever it creates an `InstanceActor` — the config comes from the `Connections` map inside that instance's flattened config, not from the site's `data_connection_definitions` table (which has no production reader). The manager never reads storage and has no independent view of which connections a site should have, so a start path that fails to push is indistinguishable here from a connection that genuinely does not exist: that was register row **85**, where two of the four Instance-Actor start paths omitted the push and every subscribe from the resulting instances was answered `Unknown connection: <name>` forever. The push now lives inside `CreateInstanceActor` on the Site Runtime side. Sends are hash-guarded there, so an unchanged connection produces no message at all:
|
||||
|
||||
```csharp
|
||||
// Commons/Messages/DataConnection/CreateConnectionCommand.cs
|
||||
|
||||
@@ -36,6 +36,8 @@ The staggered startup above reads storage exactly once. Anything that writes a `
|
||||
|
||||
`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`.
|
||||
|
||||
Register row **85** was the same gap one layer down: the sweep started the instance, but the instance came up with **no DCL connection actors**, so every tag subscribe was answered `Unknown connection: <name>`. The connection push (`EnsureDclConnections`) was the *caller's* job and two of the four Instance-Actor start paths omitted it; it now lives inside `CreateInstanceActor`. See *Deployment flow* below and §7 of the design note.
|
||||
|
||||
### Actor supervision and lifetimes
|
||||
|
||||
| Actor | Kind | Supervises children with | On exception |
|
||||
@@ -83,10 +85,11 @@ Prior to WP3.1 (2026-08-15) each invocation spawned a short-lived `ScriptExecuti
|
||||
|
||||
Central sends a `DeployInstanceCommand` carrying a JSON `FlattenedConfiguration` to the site singleton. The singleton:
|
||||
|
||||
1. Calls `EnsureDclConnections` to push any new or changed connection definitions to the DCL manager (hash-guarded: unchanged configs are skipped).
|
||||
2. Calls `CreateInstanceActor`, which does `Context.ActorOf(props, instanceName)`.
|
||||
3. Runs an off-thread `Task` that calls `SiteStorageService.StoreDeployedConfigAsync` and clears static overrides and native alarm state. Nothing is pushed to the peer: as of LocalDb Phase 2 those three tables are replicated, so the writes themselves reach the standby.
|
||||
4. Pipes back a `DeployPersistenceResult`; only on success does it tell the deployer `DeploymentStatus.Success`. If persistence fails, the optimistically-created actor is stopped and the error is returned to central (`SiteRuntime-005`).
|
||||
1. Calls `CreateInstanceActor`, which first calls `EnsureDclConnections` to push any new or changed connection definitions to the DCL manager (hash-guarded: unchanged configs are skipped), then does `Context.ActorOf(props, instanceName)`.
|
||||
2. Runs an off-thread `Task` that calls `SiteStorageService.StoreDeployedConfigAsync` and clears static overrides and native alarm state. Nothing is pushed to the peer: as of LocalDb Phase 2 those three tables are replicated, so the writes themselves reach the standby.
|
||||
3. Pipes back a `DeployPersistenceResult`; only on success does it tell the deployer `DeploymentStatus.Success`. If persistence fails, the optimistically-created actor is stopped and the error is returned to central (`SiteRuntime-005`).
|
||||
|
||||
> **`CreateInstanceActor` is the single funnel for the connection push too (register row 85)**: the `EnsureDclConnections` call moved *into* `CreateInstanceActor` on 2026-08-17. It used to sit at each caller, and only two of the four callers made it — the boot-time batch and this deploy path. `StartStoredInstance` (row-67 ensure-started and the row-78 sweep) and `HandleEnableResult` did not, so a self-healed or newly-enabled instance came up with no connection actors and every tag subscribe was answered `Unknown connection: <name>` indefinitely. See the *Instance reconcile sweep* section and `docs/plans/2026-08-15-instance-sweep-self-heal-design.md` §7.
|
||||
|
||||
For redeployment (instance already running), the existing actor is stopped and watched:
|
||||
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
# Instance reconcile sweep — closing the config/runtime divergence gaps
|
||||
|
||||
**Date:** 2026-08-15
|
||||
**Scope:** Site Runtime (`DeploymentManagerActor`), deferred-work-register rows **78** and **72**
|
||||
**Date:** 2026-08-15 (§7 added 2026-08-17)
|
||||
**Scope:** Site Runtime (`DeploymentManagerActor`), deferred-work-register rows **78**, **72**
|
||||
and **85**
|
||||
**Status:** implemented
|
||||
|
||||
---
|
||||
@@ -201,3 +202,134 @@ Every test gates on the Deployment Manager's first instance-count report before
|
||||
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.
|
||||
|
||||
That gate is now a shared helper,
|
||||
`tests/…/TestSupport/DeploymentManagerStartupProbe.cs`, and **register row 83** applied it to
|
||||
row 67's suite (`ReconcileStartsInstanceTests`), which had the same trap behind a bare
|
||||
`Task.Delay(500)`. Those tests also pin `InstanceSweepIntervalSeconds = 0`: with the sweep armed
|
||||
it would independently start the same instances, and the suite would stop distinguishing the
|
||||
row-67 mechanism from the row-78 one. The remaining `Task.Delay`s in that file are deliberate —
|
||||
they precede assertions that *nothing happened*, which cannot be awaited on.
|
||||
|
||||
---
|
||||
|
||||
## 7. Row 85 — a self-healed instance started with no DCL connection actors
|
||||
|
||||
*(Medium-Low, availability; proven live 2026-08-16 by the row-78 close-out probe. Added
|
||||
2026-08-17.)*
|
||||
|
||||
Row 78 got the Instance Actor running. Row 85 is what was still missing underneath it: the
|
||||
instance came up with **no data connection actors**, so every tag it subscribed was answered
|
||||
`Unknown connection: OPC PLC Simulator` by the DCL manager — for 6+ minutes, until a second pair
|
||||
restart. Same class as row 78, one layer down.
|
||||
|
||||
### 7.1 Where connection actors actually come from
|
||||
|
||||
The register's suggested fix was "ensure a connection actor exists for every stored site data
|
||||
connection". Reading the code first changed the shape of the answer, because the premise is not
|
||||
how the runtime works:
|
||||
|
||||
- **`data_connection_definitions` has no production reader.**
|
||||
`SiteStorageService.GetAllDataConnectionDefinitionsAsync` is called from exactly one place in
|
||||
the repo, and it is a test. Nothing loads that table at boot.
|
||||
- A DCL connection actor is created **only as a side effect of starting an instance**, from the
|
||||
`Connections` map inside that instance's *own flattened config*, via `EnsureDclConnections`
|
||||
(plus the separate system-wide artifact-deploy path, `HandleApplyArtifactDataConnectionsToDcl`).
|
||||
|
||||
So "the connection row sat in the store with no actor" was right about the symptom and wrong
|
||||
about the mechanism: there is no store→actor path to repair. The real defect is that
|
||||
`EnsureDclConnections` was **the caller's job, and only two of the four callers did it**:
|
||||
|
||||
| Path | Creates the Instance Actor | Ensured its connections (pre-fix) |
|
||||
|---|---|---|
|
||||
| Boot-time staggered batch | `CreateInstanceActorBatch` | ✅ |
|
||||
| Central deploy / redeploy | `ApplyDeployment` | ✅ |
|
||||
| **Self-heal start** (row 67 ensure-started **and** row 78 sweep) | `StartStoredInstance` | ❌ |
|
||||
| **Enable** a disabled instance | `HandleEnableResult` | ❌ |
|
||||
|
||||
The two misses are exactly the two paths that were added *after* the original pair, which is the
|
||||
signature of a convention that lives at call sites instead of in the funnel.
|
||||
|
||||
**The enable path proves this was never replication-specific.** The boot-time load reads only
|
||||
ENABLED configs, so a node that boots with an instance disabled never creates that instance's
|
||||
connections; enabling it afterwards hit the identical dead end on an ordinary, unreplicated
|
||||
site. Row 85 was reachable with no LocalDb replication anywhere in the picture — the replication
|
||||
sequence is just how it was found.
|
||||
|
||||
### 7.2 The fix: move the ensure into the single creation funnel
|
||||
|
||||
`CreateInstanceActor` is already documented as *"the single creation funnel, so recording it here
|
||||
covers every path that can ever start an Instance Actor"* — it is where `_runningRevisions` is
|
||||
stamped for exactly that reason. `EnsureDclConnections(configJson)` now sits at the top of it,
|
||||
and the two caller-side calls are removed.
|
||||
|
||||
- **Placed BEFORE the already-exists guard**, so the two paths that already called it keep their
|
||||
exact previous semantics (both called it unconditionally, before `CreateInstanceActor`).
|
||||
- **No new message, no new option, no DCL change.** It reuses `EnsureDclConnection`'s existing
|
||||
`_createdConnections` hash guard, so an unchanged connection is a no-op — no message, no log,
|
||||
nothing on the wire. Steady state is silent, which matters because this now runs on every
|
||||
instance start on every node.
|
||||
- **Structural, not another guard.** A fifth start path cannot forget, because there is nothing
|
||||
left to remember.
|
||||
|
||||
### 7.3 Why the DM and not the DCL manager
|
||||
|
||||
The register offered either. The DCL manager was rejected: it has no storage handle and no notion
|
||||
of "the connections this site ought to have" — it is a router over actors it was told to create.
|
||||
Teaching it to read `data_connection_definitions` would give the DCL a **second, divergent config
|
||||
source** alongside the flattened instance configs the DM already pushes, with no arbiter when the
|
||||
two disagree. The DM owns `_createdConnections`, owns the flattened configs, and is already the
|
||||
only sender of `CreateConnectionCommand`/`RemoveConnectionCommand` — it is a single funnel
|
||||
already, and the fix keeps it that way.
|
||||
|
||||
### 7.4 Why NOT a store-level connection pass in the sweep
|
||||
|
||||
A sweep pass over `data_connection_definitions` was designed and **deliberately dropped**. It is
|
||||
recorded here because it looks obviously right and is not:
|
||||
|
||||
- **It would not have fixed row 85.** That table is written only by the artifact-deploy path, so
|
||||
on a site that has only ever received instance deploys it is empty — while the connections that
|
||||
matter live in the instances' flattened configs.
|
||||
- **The unguarded version is actively harmful.** `_createdConnections` holds one hash per name.
|
||||
The flattened-config hash and the artifact-definition hash for the same connection can
|
||||
legitimately differ (artifact newer than an instance's flattened config — the supported
|
||||
version-skew case). A sweep re-hashing from the definitions table would flip the slot every
|
||||
tick against the deploy path flipping it back, **recreating the connection actor and dropping
|
||||
its live subscriptions on a loop**.
|
||||
- **The guarded version is near-empty.** Restricting it to names absent from `_createdConnections`
|
||||
removes the ping-pong but then covers only "a definition row exists that no instance
|
||||
references" — where no connection actor is wanted anyway.
|
||||
|
||||
**Residual gap, accepted and recorded:** `data_connection_definitions` *is* one of LocalDb's ten
|
||||
replicated tables, so a **connection-only** artifact deploy replicates to the peer node, and only
|
||||
the node that received the deploy runs `HandleApplyArtifactDataConnectionsToDcl` — the peer keeps
|
||||
serving the old endpoint until something restarts it or an instance is redeployed. That is a real
|
||||
sibling of row 85, it is **not** closed here, and closing it needs the arbitration decision above
|
||||
(which source wins), not another sweep pass. Flagged for the deferred-work register.
|
||||
|
||||
**Removal side:** also unchanged and out of scope. `RemoveConnectionCommand` is sent only from the
|
||||
artifact-deploy reconcile; a connection deleted on the peer via replication leaves a live actor on
|
||||
this node. Same root cause, same arbitration question, same register entry.
|
||||
|
||||
### 7.5 Tests
|
||||
|
||||
`tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/SelfHealCreatesDclConnectionTests.cs` —
|
||||
7 tests, asserting on the `CreateConnectionCommand` messages the DM sends to a recording stand-in
|
||||
for the DCL manager (the message whose absence *was* row 85).
|
||||
|
||||
Fail-before evidence (behavioral revert: `EnsureDclConnections` removed from the funnel and
|
||||
restored at the two original call sites — i.e. the exact pre-fix code) — **4 fail, 3 pass**:
|
||||
|
||||
| Test | Pre-fix |
|
||||
|---|---|
|
||||
| `SweepStartedInstance_AlsoCreatesItsDclConnection` | **FAIL** — the live row-85 symptom |
|
||||
| `ReconcileEnsureStartedInstance_AlsoCreatesItsDclConnection` | **FAIL** — row-67 path |
|
||||
| `EnabledInstance_DisabledAtBoot_AlsoCreatesItsDclConnection` | **FAIL** — no replication involved |
|
||||
| `SweepStartedInstance_CreatesTheConnection_ExactlyOnce_AcrossManySweeps` | **FAIL** (0, not 1) |
|
||||
| `Sweep_SendsNoConnectionCommands_WhenThereIsNothingToHeal` | pass (steady-state silence) |
|
||||
| `OrdinaryDeploy_StillCreatesItsDclConnection` | pass (refactor guard) |
|
||||
| `BootLoadedInstance_StillCreatesItsDclConnection` | pass (refactor guard) |
|
||||
|
||||
The two refactor guards are the point of passing both before and after: they pin that moving the
|
||||
call out of `CreateInstanceActorBatch` and `ApplyDeployment` did not regress the paths that
|
||||
already worked.
|
||||
|
||||
@@ -355,6 +355,10 @@ Each value update delivered to an Instance Actor includes:
|
||||
|
||||
## Connection Actor Model
|
||||
|
||||
> **Where connection actors come from — the Site Runtime is the only creator (register row 85)**: the DCL manager creates and removes connection actors **only** when the Deployment Manager tells it to, via `CreateConnectionCommand` / `RemoveConnectionCommand`. It never reads storage; the site's `data_connection_definitions` table has no production reader at all, and the DCL has no independent notion of "the connections this site ought to have". A connection actor's config therefore comes from the `Connections` map inside a deployed instance's **flattened config** (pushed as part of starting that instance) or from the system-wide artifact-deploy path. `HandleCreateConnection` is idempotent on the name — it warns and returns if it already holds that connection — but the Deployment Manager is the side that keeps the traffic quiet, hash-guarding on the last config pushed per name so an unchanged connection sends nothing. Row 85 was the consequence of that ownership split being incompletely honoured on the Site Runtime side: two of the four Instance-Actor start paths omitted the connection push, so instances came up whose every tag subscribe was answered `Unknown connection: <name>` here, forever. The fix is in the Site Runtime (the ensure moved into its single creation funnel) — **not** here; giving the DCL manager its own config source would create a second, divergent authority with no arbiter when the two disagree. See `Component-SiteRuntime.md` and `docs/plans/2026-08-15-instance-sweep-self-heal-design.md` §7.
|
||||
>
|
||||
> Two known residual gaps of the same family, **not** closed by row 85's fix: `data_connection_definitions` is one of LocalDb's ten replicated tables, so a **connection-only** artifact deploy replicates to the peer node — but only the node that received the deploy pushes it to its DCL, so the peer keeps serving the old endpoint until an instance is redeployed or the node restarts. The removal side is the mirror image: a connection deleted on the peer leaves a live connection actor on this node.
|
||||
|
||||
Each data connection is managed by a dedicated connection actor that uses the Akka.NET **Become/Stash** pattern to model its lifecycle as a state machine:
|
||||
|
||||
- **Connecting**: The actor attempts to establish the connection. Subscription requests and write commands received during this phase are **stashed** (buffered in the actor's stash).
|
||||
|
||||
@@ -108,6 +108,8 @@ flowchart TD
|
||||
>
|
||||
> 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.
|
||||
|
||||
> **Starting an Instance Actor also creates its DCL connections — the ensure lives in the creation funnel, not at call sites (register row 85)**: a DCL connection actor is **never** created from the `data_connection_definitions` table (nothing in production reads it). It exists only as a side effect of starting an instance, built from the `Connections` map inside that instance's **own flattened config** via `EnsureDclConnections` — plus the separate system-wide artifact-deploy path. That ensure used to be each *caller's* responsibility, and only two of the four callers made it (the boot-time staggered batch and `ApplyDeployment`). The two that did not were **`StartStoredInstance`** — the path shared by the row-67 ensure-started handler and the row-78 sweep — and **`HandleEnableResult`**. Both therefore produced a running instance whose every tag subscribe was answered `Unknown connection: <name>` by the DCL manager, indefinitely, because nothing else on the node would ever create that connection actor. Proven live 2026-08-16: a replication-seeded node's instances were correctly started by the sweep and then failed every subscribe for 6+ minutes until a second pair restart let the boot load create the connections. **It was never replication-specific** — the boot-time load reads only *enabled* configs, so a node that boots with an instance disabled never creates its connections either, and enabling it afterwards hit the identical dead end on an ordinary unreplicated site. `EnsureDclConnections` now sits at the top of **`CreateInstanceActor`**, already the single Instance Actor creation funnel (it is where the running revision is stamped for the same reason), so all four paths get it and a fifth cannot forget. It is placed *before* the already-exists guard so the two paths that already called it keep their exact previous semantics, and it stays silent in steady state because `EnsureDclConnection` is hash-guarded on the last config pushed per connection name. Deliberately **not** done: a sweep pass over `data_connection_definitions` — it would not have fixed row 85 (that table is empty on a site that has only ever received instance deploys), and re-hashing from it would fight the deploy path over the same per-name hash slot, recreating connection actors and dropping live subscriptions on a loop. See `docs/plans/2026-08-15-instance-sweep-self-heal-design.md` §7.
|
||||
|
||||
> **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.
|
||||
|
||||
Reference in New Issue
Block a user