Files
ScadaBridge/docs/plans/2026-08-17-dcl-connection-arbitration-decision.md
T
Joseph Doherty 7c1268d85c feat(dcl+siteruntime): reconfigure a LIVE data connection in place — register row 97
`DataConnectionManagerActor.HandleCreateConnection` is create-only (warns and
returns for a name it already holds), so `EnsureDclConnection`'s documented
"a changed config re-issues the create so the DCL adopts it" was never true on
ANY node: a corrected endpoint/credential/backup/failover-count took effect only
when the connection was next built from scratch (node restart, singleton
failover, remove-then-re-add), while the DM's hash cache recorded it as applied.
Rows 89/90 made that honest with a Warning; this closes it.

New `ReconfigureConnectionCommand` (Commons, same shape as the create):

- `DeploymentManagerActor.EnsureDclConnection` now has three outcomes off its one
  per-name hash slot — unchanged -> nothing at all, new -> create, CHANGED ->
  reconfigure — and logs "Reconfigured DCL connection {Connection}
  (protocol={Protocol}) in place" at Information in place of the old Warning. The
  winner-resolution / arbitration / tombstone logic is untouched; only what
  happens after a change is detected changed.
- `DataConnectionManagerActor` FORWARDS the command to the owning connection
  actor, falling back to a create for a name it does not hold, so the command is
  safe whatever the manager's state.
- `DataConnectionActor` handles it in ALL THREE states and never stashes it — a
  connection pointed at a wrong or dead endpoint lives in Connecting/Reconnecting,
  which is exactly when a correction arrives; stashed it would wait on the broken
  configuration forever. Handling replaces protocol/primary/backup/failover-count,
  re-arms endpoint selection to Primary (the new config's primary is the intent,
  whatever the old config had failed over to), and calls the new shared
  `SwapAdapter` helper — now used by both failover sites too, so the swap exists
  once: unhook + fire-and-forget dispose -> _factory.Create -> rewire -> bump
  _adapterGeneration LAST. `BecomeReconnecting(afterReconfigure: true)` then
  reconnects immediately (StartSingleTimer replaces any armed reconnect, so no
  racing attempts) with the unstable-disconnect accounting and failover check
  suppressed — the link was dropped on purpose, and counting it as an unstable
  cycle could trip a failover onto the NEW backup before the new primary had been
  tried once. ReSubscribeAll/ReSubscribeAllAlarms then re-issue everything from
  the untouched subscription registries. Idempotent: a reconfigure carrying the
  configuration already in force is a no-op.

Stop-and-recreate was rejected as destructive: subscriptions live inside
DataConnectionActor and InstanceActor cancels its subscribe-retry timer on first
success, so a rebuilt actor would have no subscribers and nothing would ever
re-subscribe.

Tests: new DataConnectionReconfigureTests (7; 5 fail / 2 pass against a
behavioural revert). DclConnectionArbitrationTests gains two row-97 tests and its
CDC-arrival test now asserts the second push is a reconfigure; its ping-pong
guard also asserts zero reconfigures, since a flip would now show up there.
SiteRuntime-010 renamed to ..._ReissuesReconfigureCommand. 3 fail against a
reverted EnsureDclConnection. DataConnectionLayer.Tests 327/327,
SiteRuntime.Tests 643/643, solution build 0 warnings.

Docs: Component-DataConnectionLayer.md gains an "In-place reconfiguration"
section under Endpoint Redundancy; Component-SiteRuntime.md, and the
docs/components/ mirrors, updated; the arbitration decision memo gains an
appended §9 postscript resolving its residual 1 (residuals 2 and 3 stay open).
2026-08-17 06:19:13 -04:00

390 lines
23 KiB
Markdown

# DCL connection arbitration — which config source owns a live connection
**Date:** 2026-08-17
**Scope:** Site Runtime (`DeploymentManagerActor`), deferred-work-register rows **89** and **90**
**Status:** implemented
**Predecessor:** `docs/plans/2026-08-15-instance-sweep-self-heal-design.md` §7.4, which designed a
store-level connection sweep, **rejected** it, and recorded rows 89/90 as the residual — explicitly
blocked on "the arbitration decision above (which source wins), not another sweep pass".
---
## 1. Verified topology — and two premise corrections
Both register rows are worded in terms of "the peer node's DCL". That framing is wrong, and the
correction changes what the fix has to do, so it comes first.
### 1.1 What actually runs where
| Actor | Placement | Verified at |
|---|---|---|
| `DeploymentManagerActor` | **Cluster singleton**, role-scoped to `site-{SiteId}` (`SingletonRegistrar.Start(…, "deployment-manager", …, role: siteRole)`) | `Host/Actors/AkkaHostedService.cs` |
| `DataConnectionManagerActor` (`/user/dcl-manager`) | **Every site node**, unconditional `ActorOf` — *not* a singleton | `Host/Actors/AkkaHostedService.cs` |
| `CertStoreActor` | Every site node (documented; re-confirmed) | same file |
`CreateConnectionCommand` and `RemoveConnectionCommand` have exactly **one** sender in the whole
repo — `DeploymentManagerActor` — and it sends them to the `_dclManager` ref it was constructed
with, which is *its own node's* local `dcl-manager` (the Props are built per node, so whichever
node materialises the singleton uses that node's manager).
### 1.2 Premise correction 1 — the standby's DCL is EMPTY, not stale
Because the singleton is the only sender, **the standby node's `dcl-manager` holds no connection
actors at all**. It is not "serving the old endpoint"; it is serving nothing, and nothing subscribes
through it. So there is no per-node divergence to reconcile with a cross-node message.
The real failure mode is one layer over:
> `_createdConnections` — the per-name hash cache that records what the DM last pushed — is
> **in-memory and per singleton incarnation**. When the singleton restarts (failover, or an
> ordinary node restart), the new incarnation starts with an empty cache, loads
> `deployed_configurations`, and rebuilds every connection **from the `Connections` snapshot frozen
> into each instance's flattened config at that instance's deploy time**. Nothing has ever read
> `data_connection_definitions` — that table had **no production reader** — so a newer,
> connection-only artifact deploy is silently discarded and the site goes back to the old endpoint.
This also means **row 89 is not replication-specific**. Replication only puts the definition row on
both nodes, which is if anything helpful. The identical loss happens on an unreplicated single-node
restart. The row-89 fix is therefore a *create-time arbitration* fix, not a *message-delivery* fix.
Row 90 is the same shape: the delete is durable in the store, but the flattened configs still name
the connection, so the next instance start re-creates it.
### 1.3 Premise correction 2 — a changed config never reached a LIVE connection either
`EnsureDclConnection`'s contract, as documented in the code before this change, was that a changed
endpoint/credential *"re-issues a `CreateConnectionCommand` so the DCL adopts the new configuration
instead of keeping a stale connection"*. **That is false.**
`DataConnectionManagerActor.HandleCreateConnection` is create-only:
```csharp
if (_connectionActors.ContainsKey(command.ConnectionName))
{
_log.Warning("Connection {0} already exists", command.ConnectionName);
return;
}
```
So the system-wide artifact deploy's "make it live immediately" push has never worked on the
deploy-handling node, and the DM's hash cache recorded it as applied regardless. This is a
**separate, pre-existing defect** discovered while validating rows 89/90; see §6.
---
## 2. The arbitration rule
> **The definitions store wins wherever it has an opinion. The flattened config is authoritative
> only for names the store has never owned.**
1. **A name with a row in `data_connection_definitions`** — that row's config configures the live
DCL connection actor. The identically-named entry in an instance's flattened `Connections` map
is a deploy-time snapshot and never overrides it.
2. **A name with no definition row** — the flattened config is the source, exactly as before. This
is not a fallback; it is the *correct* answer for a deploy-only site, where the definitions
table is legitimately empty because nothing has ever sent that site a system-wide artifact
deploy while every connection that matters lives inline in the instance configs. §7.4 of the
predecessor memo identified this as the reason a blanket store-level sweep is wrong, and it
remains the reason rule 2 exists.
3. **Removal** semantics on the deploy-handling node are unchanged: the actor is stopped even if
deployed instances still reference it, and their attributes go bad quality until redeploy. What
is *added* is a tombstone (§4).
### 2.1 The overlay is an arbiter, not a creation source
`_connectionDefinitions` decides **which configuration** a connection the runtime already wants is
built from. It never creates a connection on its own: a definition row naming a connection no
deployed instance references produces no actor, because none is wanted. This is what makes it safe
to consult on every sweep tick, and it is why re-adding a centrally deleted connection brings the
actor back at the *next instance start* rather than instantly (pinned by
`ReAddedDefinition_ClearsTheTombstone_SoTheNextInstanceStartRecreatesIt`).
---
## 3. The ping-pong, and why keying the hash on the winner makes it unrepresentable
The predecessor memo rejected a store-level sweep partly because of this:
> `_createdConnections` holds one hash per name. The flattened-config hash and the
> artifact-definition hash for the same connection can legitimately differ (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.**
That analysis is correct, and it is a property of *where the hash is computed*, not of the sweep.
The fix is one line of ordering discipline:
> **Every ensure path resolves the winning configuration BEFORE hashing or pushing.**
`EnsureDclConnections(configJson)` no longer hands the flattened `ConnectionConfig` straight to
`EnsureDclConnection`. It first asks the overlay whether the store owns that name; if it does, the
*definition's* protocol/config/backup/failover fields are what get hashed and pushed. The
artifact-deploy path pushes those same fields. Therefore, for any given name, **only one
configuration can ever be hashed**, no matter which path calls — the two sources cannot occupy the
slot alternately because there is only ever one candidate. The flip is not guarded against; it is
unrepresentable.
The guard test `DisagreeingSources_DoNotPingPongTheConnection_AcrossManySweeps` pins exactly this
with the two sources deliberately disagreeing: one create, zero removes, across many ticks. It
passes before *and* after the fix — before, because nothing read the definitions at all — which is
the point: it is a guard against the **rejected naive design**, not a fail-before test.
### 3.1 A second flip source, closed in the same way
`HandleApplyArtifactDataConnectionsToDcl` now writes the artifact's rows into the overlay in the
same pass it pushes them. Without that there is a window between an artifact deploy and the next
sweep read in which an instance start would resolve the name to its flattened snapshot while the
artifact path had already pushed the definition — the same flip, arriving through a different door.
---
## 4. Removal and the tombstone
`_centrallyRemovedConnections` is a `HashSet<string>` of names this singleton incarnation has seen
removed centrally. It is populated by both removal paths, which share one helper
(`RemoveConnectionCentrally`) so they cannot drift:
- the artifact reconcile's `RemovedConnectionNames` (the existing path); and
- **the sweep's disappearance detection** (new — §5).
`EnsureDclConnections` skips a tombstoned name. Without the tombstone the removal does not survive
one tick: the removal evicts the hash slot, and the next instance start whose flattened config still
names the connection re-creates it. That is not hypothetical — the sweep starts instances — and it
is a pre-existing bug this change also closes
(`ArtifactReconcileRemoval_IsNotResurrectedByALaterInstanceStart` fails on the old code).
**Tombstones are deliberately in-memory only.** Persisting them was rejected: a durable "this name
is gone" marker is a second divergent authority over the same names, and the flattened configs
already resurrect a removed connection across a restart today. That pre-existing semantic is
accepted and unchanged — see the residual in §6.
A tombstone is cleared the moment a definition row for the name reappears (a definition row *is*
the store re-asserting ownership), so a centrally re-added connection is creatable again.
---
## 5. The mechanism: the definitions table as an overlay on the existing reads
No new message, no new actor, no new cross-node path. The two durable tables the DM arbitrates over
are read in **one off-thread task**, `ReadStoreSnapshotAsync`, used by both existing reads:
```
PreStart → LoadDeployedConfigs ─┐
├─ ReadStoreSnapshotAsync (off-thread, one task)
InstanceSweepTick ──────────────┘ deployed_configurations
data_connection_definitions
│
▼ piped back
StartupConfigsLoaded / InstanceSweepLoaded (actor thread)
│
ApplyConnectionDefinitionOverlay
│
┌───────────────────┴───────────────────┐
row present row disappeared
adopt + push if live tombstone + stop actor
```
Nothing blocks the actor thread on SQLite; the overlay mutation, the hash cache and the tombstone
set are all actor-thread-confined, matching the existing `StartupConfigsLoaded` /
`ApplyArtifactDataConnectionsToDcl` patterns.
**Ordering is load-bearing in both handlers.** The overlay is applied *before* the boot batch
creates any Instance Actor and *before* the sweep's repair loop, so a connection is created from the
winning config on its **first** push rather than created stale and corrected a tick later. On
failover that is the difference between a clean takeover and a needless recreate on every one.
### 5.1 The two reads fail differently, on purpose
A failed **config** read propagates and re-arms the whole load (the existing S5 path, unchanged). A
failed **definitions** read is swallowed into a `null` result, and `null` means *keep the overlay
exactly as it is* — deliberately distinct from an empty list, which is a real answer (a deploy-only
site, or a site after central deletes every data connection). Clearing the overlay on a transient
read failure would hand arbitration back to the flattened snapshots for one pass and take it away
on the next successful read: the §3 ping-pong, reintroduced through the error path.
### 5.2 Disappearance detection, and precisely how far it reaches
A name that **was** in the overlay and now has no row is a removal this incarnation can *prove*: the
store owned it a moment ago and does not now. It is tombstoned and its connection actor stopped.
A name that was **never** in the overlay is indistinguishable from one the store has never owned —
a fresh incarnation cannot tell "central deleted this" from "this site keeps its connections
inline". It is left to the flattened config. That asymmetry is deliberate; the alternative (infer
deletion from "the definitions table is non-empty, therefore it is the complete set") would stop
live connections on an inference, and is recorded as a candidate follow-up in §6.
### 5.3 Cost
One extra `SELECT` over `data_connection_definitions` per sweep tick (default 60 s) and once at
boot, on the same task as the existing deployed-configs read. Steady state is otherwise silent: the
hash guard means an unchanged connection produces no message and no log.
---
## 6. Residuals — what this does NOT close
These are precise, and each is a candidate register row.
1. **A live connection actor is still not reconfigured in place** (premise correction 1.3). The DCL
manager's `HandleCreateConnection` is create-only, so a changed configuration for a name whose
actor already exists takes effect only when the connection is next created from scratch — a node
restart, a singleton failover, or the connection being removed and re-added. This change makes
that **honest** rather than silent: `EnsureDclConnection` now logs a Warning naming exactly that,
once per changed configuration (the hash guard suppresses repeats, so a healthy site stays
silent). It is not *fixed* here because the obvious fix is destructive: stopping and re-creating
the connection actor drops every Instance Actor's tag subscription, and `InstanceActor` cancels
its per-connection subscribe-retry timer on the first success, so **nothing re-subscribes** —
trading "stale endpoint, data flowing" for "correct endpoint, no data until redeploy". The
non-destructive fix is a `ReconfigureConnectionCommand` handled *inside* `DataConnectionActor`,
reusing the adapter-swap machinery the primary→backup failover path already has
(`CountFailureAndMaybeFailover`: dispose adapter, `_factory.Create` the new one, bump
`_adapterGeneration`, re-enter the connect cycle, which re-issues everything from
`_subscriptionsByInstance`). That is a Data Connection Layer change and is out of rows 89/90's
scope.
2. **A removal is still not durable across a restart.** Tombstones are in-memory, so a connection
deleted centrally and then resurrected by an instance's flattened config on the next node restart
behaves as it always has. Closing this needs a durable marker or — better — flattened configs
that stop carrying connection bodies at all.
3. **A delete that arrives while this node has never adopted the row is undetectable** (§5.2). The
narrow case that remains open: the singleton moves to a node whose file already has the row
deleted, and some instance's flattened config still names the connection. That node re-creates
it. A stronger rule — "a non-empty definitions table is the complete set, so absence means
deleted" — would close it, at the cost of stopping live connections on an inference about what
central shipped; deliberately not taken.
4. **Dual-active (a real partition under `auto-down`)** is the only routine way a definition change
reaches a singleton that did not process the deploy itself. It is handled correctly by §5, but it
is worth naming that this is not the common path — the common path is failover/restart (§1.2).
---
## 7. Tests
`tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DclConnectionArbitrationTests.cs` — 8 tests,
asserting on the `CreateConnectionCommand` / `RemoveConnectionCommand` messages the DM sends to a
recording stand-in for the DCL manager, and on the **endpoint inside** those commands (which is what
distinguishes the two config sources).
Every test gates on `DeploymentManagerStartupProbe` for the same reason the row-78/85 suites do: a
row written before the boot load reads storage is consumed by the boot load, and an ungated test
would silently prove the boot load works instead of the path it names.
Fail-before evidence (behavioral revert: `ApplyConnectionDefinitionOverlay` made inert and the
tombstone check disabled — i.e. the exact pre-fix arbitration) — **5 fail, 3 pass**:
| Test | Pre-fix |
|---|---|
| `DefinitionRow_WinsOverAnOlderFlattenedSnapshot_WhenTheSingletonBoots` | **FAIL** — the row-89 symptom in its failover shape (create carries the old endpoint) |
| `DefinitionArrivingAfterBoot_IsAdoptedByTheNextSweepTick` | **FAIL** — the CDC-delivered change is never read |
| `DefinitionRowDisappearing_StopsTheConnection_AndItIsNotResurrected` | **FAIL** — the row-90 symptom (orphan actor stays live) |
| `ArtifactReconcileRemoval_IsNotResurrectedByALaterInstanceStart` | **FAIL** — removal undone by the next instance start |
| `ReAddedDefinition_ClearsTheTombstone_SoTheNextInstanceStartRecreatesIt` | **FAIL** |
| `DisagreeingSources_DoNotPingPongTheConnection_AcrossManySweeps` | pass (guard against the rejected naive design) |
| `NoDefinitionRow_LeavesTheFlattenedConfigAuthoritative_AtBoot` | pass (deploy-only regression guard) |
| `NoDefinitionRow_LeavesTheFlattenedConfigAuthoritative_OnTheSweepPath` | pass (deploy-only regression guard) |
The three passers are the point of passing both before and after: rule 2 must be byte-for-byte
unchanged for a deploy-only site, and the ping-pong must never appear.
---
## 8. Configuration
**None added.** The overlay refresh rides `ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds`
(default 60; `0` disables the sweep and therefore the post-boot half of this convergence — the
boot-time arbitration still applies, because it hangs off the startup load, not the sweep timer).
---
## 9. Postscript — residual 1 RESOLVED (register row 97, 2026-08-17)
*Appended; §6 above is left exactly as it was written.*
§6 residual 1 — "a live connection actor is still not reconfigured in place" — is closed, along
the sketch §6 recorded, in a Data Connection Layer change that leaves everything above untouched.
**The arbitration rule, the winner-resolution ordering and the tombstones are unchanged. Only what
happens AFTER a change is detected changed.**
### 9.1 The third command
`EnsureDclConnection` had two outcomes off its single per-name hash slot; it now has three:
| Hash slot | Command sent | Log |
|---|---|---|
| equal | **nothing** | none — what keeps a healthy site silent every sweep tick |
| absent | `CreateConnectionCommand` | `Created DCL connection {Connection} …` (Information) |
| **different** | **`ReconfigureConnectionCommand`** | `Reconfigured DCL connection {Connection} (protocol={Protocol}) in place …` (Information) |
The Warning §6 added to make the gap honest ("…is not reconfigured in place — it keeps serving its
current configuration until it is recreated") is **gone**, because the sentence is no longer true.
`DataConnectionManagerActor.HandleReconfigureConnection` **forwards** the command to the connection
actor that owns the name, and **falls back to a create** for a name it does not hold — so the
command is safe whatever the manager's state, and a sender whose view of which connections exist is
stale (a fresh incarnation, a node that has just taken over) still converges in one message.
`HandleCreateConnection` itself is untouched and stays create-only.
### 9.2 Inside the connection actor
`DataConnectionActor` handles the command in **all three** states, and **never stashes it**. That is
load-bearing rather than tidy: a connection pointed at a wrong or dead endpoint spends its life in
`Connecting` or `Reconnecting` — exactly when a corrected configuration is sent — so a stashed
reconfigure would wait for the *broken* configuration to succeed first.
The swap itself is the failover machinery, not a copy of it: the two failover sites and the
reconfigure now share one `SwapAdapter(newConfig)` helper (unhook + fire-and-forget dispose →
`_factory.Create` → rewire → bump `_adapterGeneration` **last**, so everything in flight on the
outgoing adapter is recognised as stale). Then `BecomeReconnecting(afterReconfigure: true)`:
- reconnects **immediately** rather than after `ReconnectInterval` — there is nothing to wait for,
and `StartSingleTimer` replaces any reconnect already armed, so a reconfigure landing mid-backoff
cannot leave two attempts racing;
- **suppresses the unstable-disconnect accounting and the failover check** — the link was dropped on
purpose, and counting it as an unstable cycle could trip a failover onto the *new* backup before
the new primary had been tried once;
- logs the site event at Info ("Connection reconfigured — reconnecting on the new configuration")
instead of the Warning "Connection lost".
`ReSubscribeAll` / `ReSubscribeAllAlarms` then re-issue everything from `_subscriptionsByInstance`
and `_alarmSourceSubscribers`, which the swap never touched. **Subscribers do nothing and know
nothing**: tags go bad quality across the swap and come back Good, exactly as on a failover.
Two edge cases §6 did not name:
1. **Endpoint selection is re-armed from the top.** Whatever endpoint the previous configuration had
failed over to, the new configuration's *primary* is the operator's intent — so `ActiveEndpoint`
and both failure counters reset. Without this, a connection sitting on its old backup would apply
a new configuration and immediately connect to the new *backup*.
2. **The command is idempotent.** A reconfigure carrying the configuration already in force is a
no-op inside the actor. The sender is hash-gated and does not send one, but an adapter swap drops
the link and re-subscribes every tag, so a duplicate must never be able to cost that.
A **protocol change** is supported for free: `_protocolType` became mutable and the factory keys off
it.
### 9.3 What this cost the ping-pong guard
§3's guard test asserted "one create, zero removes, across many ticks". A flip would now show up as
a stream of **reconfigures**, not creates — each one dropping the link and re-subscribing every tag.
The guard asserts on both counts now; asserting only on creates would have left it silently
guarding the wrong thing.
### 9.4 Tests
`tests/…/DataConnectionLayer.Tests/Actors/DataConnectionReconfigureTests.cs` — 7 tests. Fail-before
evidence (behavioral revert: the three `ReconfigureConnectionCommand` cases removed from the state
machine and the manager's forward replaced by the create-only path — i.e. the exact pre-fix
behaviour) — **5 fail, 2 pass**: the swap-and-re-subscribe from `Connected`, the generation bump,
the honoured-not-stashed cases in `Connecting` and in `Reconnecting`, and the manager's forward all
fail; the identical-config no-op and the unknown-name fallback-to-create pass, as guards.
`DclConnectionArbitrationTests` gained two (`ChangedConfigurationForALiveConnection_IsSentAsA
Reconfigure_NotASecondCreate`, `UnchangedConfiguration_SendsNeitherACreateNorAReconfigure`) and
`DefinitionArrivingAfterBoot_IsAdoptedByTheNextSweepTick` now asserts the second push is a
reconfigure. With `EnsureDclConnection` reverted to sending a create on the change edge, **3 fail**
across `DclConnectionArbitrationTests` + `DeploymentManagerMediumFindingsTests` (the two changed-edge
tests plus SiteRuntime-010, renamed `…_ReissuesReconfigureCommand`).
### 9.5 Residuals after this
§6 residuals **2** (a removal is not durable across a restart) and **3** (a delete is undetectable on
a node that never adopted the row) are unchanged and still open. §6 residual **4** was never a gap.