Files
ScadaBridge/docs/plans/2026-08-17-dcl-connection-arbitration-decision.md
Joseph Doherty f02e116688 fix(siteruntime): extend the row-101 stale-snapshot guard to the instance-config half
ReadStoreSnapshotAsync's snapshot is read off the actor thread and consumed only
when its piped message lands. Row 101 guarded the DEFINITIONS half of that race;
the CONFIG half was left to machinery whose reach is time-limited, and both legs
of the instance reconcile sweep were reachable through it:

- Row-72 leg: a read dispatched before a deploy's persist, landing after the
  deploy completed AND after the pipeline released ownership, carries the
  revision the deploy replaced. TryRestartStaleInstance then "repairs" the
  instance BACKWARD, re-deploying the snapshot's own superseded ConfigJson.
  InstanceIsOwnedByADeployPipeline covers only in-flight deploys, and the
  per-revision claim has never seen the old revision, so nothing stopped it.
- Row-78 leg: a read dispatched before a delete/disable commits still shows an
  enabled row while the actor is already stopped, so StartStoredInstance
  resurrects it — and delete clears _sweepAttemptedRevisions, freeing the claim.

Both verified real by neutralising the freshness check and re-running the new
tests (the fail-before site event literally reads "running revision rev-2 is
stale, applying stored revision rev-1"; the deleted instance came back).

_definitionsWriteGeneration becomes _storeWriteGeneration, covering both tables
(the snapshot is one consistent read and is discarded whole, so one counter is
the right shape); DefinitionsGenerationAtRead -> StoreGenerationAtRead on both
internal records. The generation is now bumped at BOTH ends of every mutation —
when the off-thread write is dispatched and when its completion message lands —
because the config paths advance actor state before the write, so an
initiation-only bump would let an in-window read look fresh. Disable, delete and
the init-failure rollback have no actor-thread completion message of their own,
so a minimal StoreWriteCompleted self-message carries the bump for them.

CDC-replicated writes, snapshot resyncs, restored files and
SiteReconciliationActor's fetch deliberately do NOT bump: discovering exactly
those is the sweep's reason to exist.

A stale sweep result is now dropped WHOLE (no overlay apply, no start, no
restart). A stale boot load cannot be dropped — startup must finish — so it
re-dispatches LoadDeployedConfigs(); a fifth consecutive superseded read is
applied anyway with a Warning, and a late one arriving after the boot completed
is dropped rather than looping against the S5 retry timer.

Tests: three fail-before regressions (backward restart, resurrection, boot
re-read) plus two guard twins pinning that the legitimate row-72 restart and
row-78 start still fire on a current-generation snapshot. The row-101
definitions tests keep passing; FreshEmptySweepSnapshot's hardcoded generation
moves 1 -> 2 now that one artifact apply bumps at both ends.

Docs: Component-SiteRuntime.md's stale-snapshot paragraph now describes the
whole snapshot (both halves, both consumers, the boot re-dispatch, the
CDC-no-bump rule), the components mirror follows, and the arbitration memo gains
§11 extending §10 to the config half.
2026-08-18 05:38:00 -04:00

33 KiB

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:

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.

10. Postscript — the stale-snapshot race (register row 101, 2026-08-17)

The sweep design read the store off the actor thread (§5) and compared the result against the overlay only when the piped message landed. That gap was a TOCTOU the original memo did not name: a read starting before an artifact apply's store writes could land AFTER HandleApplyArtifactDataConnectionsToDcl populated the overlay, and its stale snapshot would then "prove" the just-applied names disappeared. The disappearance detection (§5) tombstoned them and stopped the just-created live connection actors — and because the removal also evicts the hash slot the overlay's push is gated on, the next fresh sweep clears the tombstone but re-creates nothing: the connection stays dead until an instance redeploy. Surfaced as a one-off test flake (ApplyingArtifacts_RemovedDataConnection_StopsLiveDclConnection received a removal for PlcA where PlcGone was expected — overlay iteration order removing BOTH names off a stale empty snapshot), first suspected to be test-fixture-local; verification showed the mechanism is real in production whenever an artifact deploy lands while a sweep read is in flight.

Fix: a definitions-write generation. _definitionsWriteGeneration is bumped on the actor thread by every artifact apply; each snapshot read captures it at dispatch and a result carrying a stale generation is discarded WHOLE — additions included, since a stale row pushed at a live connection would reconfigure it backward and flip-flop with the next fresh read. Discarding is safe: the apply already updated the overlay in the same pass (§5), and the next tick reads fresh. Applies to both snapshot consumers (boot load and sweep). Deterministic reproduction + a guard test that the legitimate row-90 disappearance still fires on a FRESH empty snapshot: StaleSweepSnapshot_RacingAnArtifactApply_DoesNotRemoveJustAppliedConnections (fails before the fix) and FreshEmptySweepSnapshot_StillRemovesDisappearedConnections.

11. Postscript — the same race on the CONFIG half of the snapshot (register row 101 follow-on, 2026-08-18)

§10 fixed the DEFINITIONS half of ReadStoreSnapshotAsync's snapshot. The instance-config half of the same snapshot has the identical TOCTOU, and it was left protected only by machinery whose reach is time-limited. This section extends §10 rather than revising it; everything §10 says about the definitions half still stands.

11.1 The two hazards, both verified real against the code

Both were reproduced by neutralising the freshness check and re-running the new regression tests (see §11.5), so neither is theoretical.

Hazard 1 — backward restart (row-72 leg). A read dispatched before a deploy persists its new row can land after the deploy has completed and after the pipeline released the instance. The snapshot then carries the revision the deploy replaced while _runningRevisions carries the new one — the exact shape TryRestartStaleInstance repairs, in the wrong direction. It is not a harmless restart: the repair passes the snapshot's own ConfigJson to HandleDeploy, so the superseded config is literally re-deployed. The pre-existing guards do not reach it: InstanceIsOwnedByADeployPipeline covers only _deployWarms / _terminatingActorsByName / _inFlightDeployments, all of which are cleared when the deploy finishes; and TryClaimSweepAttempt does bound restarts to one per (instance, revision), but the old revision has never been claimed, so the claim is available and the instance simply stays reverted. Observed fail-before site event: "Instance BackwardRestartPump restarted by deployment-manager sweep self-heal — running revision rev-2 is stale, applying stored revision rev-1".

Hazard 2 — resurrection (row-78 leg). A read dispatched before a delete or a disable commits still shows an enabled row while the actor is already stopped (both handlers stop the actor synchronously and write off-thread). StartStoredInstance then starts an instance the operator just removed. Delete is the worse of the two because it explicitly clears _sweepAttemptedRevisions for the instance, handing the sweep a fresh repair claim. Fail-before: the deleted instance was running again after the stale snapshot landed.

Hazard 3 — the boot load's other consumers. HandleStartupConfigsLoaded also rebuilds _deployedInstanceNames from the snapshot and hands its enabled configs to the staggered startup batch. A deploy or delete processed while the boot read is in flight therefore skews the deployed-instance count and can hand the batch a superseded config; CreateInstanceActor's already-exists guard blunts the second effect but not the first.

11.2 One generation for both tables

_definitionsWriteGeneration is renamed _storeWriteGeneration and now counts mutations of BOTH snapshot tables; the message parameter DefinitionsGenerationAtRead becomes StoreGenerationAtRead on both internal records (neither is a wire message). One counter is the right shape because the snapshot is a single consistent read and is discarded whole either way — a per-table counter would only buy the ability to apply half a snapshot that the other half's mutation already invalidated.

11.3 Bump at BOTH ends of every mutation

§10 bumped once, in the handler that runs after the write. That is sufficient when the actor's own state advances in that same handler, but the config mutations do not all work that way — ApplyDeployment creates the Instance Actor and stamps the running revision before the persist, and HandleDisable/HandleDelete stop the actor before the write. So the generation is now bumped twice per mutation: once when the off-thread write is dispatched, once when its completion message lands. The pair leaves no window — a read dispatched before the mutation is discarded by the first bump, a read dispatched inside the write window is discarded by the second, and a read dispatched after the second necessarily sees the committed row. Two paths had no actor-thread completion message at all (disable/delete pipe their reply to the caller, and the init-failure rollback pipes nothing), so a minimal internal StoreWriteCompleted(Source) message exists purely to carry the completion bump for them.

Bump sites: ApplyDeployment + HandleDeployPersistenceResult; HandleEnable + HandleEnableResult; HandleDisable + StoreWriteCompleted; HandleDelete + StoreWriteCompleted; RollbackPersistedConfig + StoreWriteCompleted; HandleDeployArtifacts

  • HandleApplyArtifactDataConnectionsToDcl (the §10 bump, kept). The deploy and enable completion bumps are unconditional on success — a failed write may still have committed the row before a later step threw, so "failed" is not "nothing changed". Cost of over-bumping is at most one discarded sweep tick or one extra boot read.

Deliberately NOT bumped: SiteReconciliationActor's StoreDeployedConfigIfNewerAsync (a different actor), LocalDb CDC replication applies, snapshot resyncs, and an operator restoring the file. None passes through this actor, and discovering exactly those divergences is the sweep's entire reason to exist (rows 72 + 78). Bumping for them — were it even reachable — would blind the sweep to its own use case.

11.4 Sweep drops, boot re-reads

The sweep consumer discards a stale result whole (definitions half and configs half, no overlay apply, no start, no restart) at Debug, because there is nothing to complete and the next tick is already a fresh read. The boot consumer cannot do that — a node whose startup read is dropped stays a silent zero-instance node — so it re-dispatches LoadDeployedConfigs(), which captures the current generation. Convergence argument: staleness requires a fresh mutation inside each attempt's window, which is externally driven and finite. The belt-and-braces cap is MaxConsecutiveStaleStartupLoads = 5, after which the snapshot is applied anyway with a Warning (a boot that never finishes is strictly worse than one starting from a slightly superseded view the sweep converges on its next tick). A superseded boot snapshot arriving after _startupLoadCompleted — the S5 RetryStartupLoad timer racing a success — is dropped rather than re-read, so the retry timer and the re-dispatch cannot feed each other.

11.5 Tests

In InstanceSweepSelfHealTests, all hand-delivering crafted InstanceSweepLoaded / StartupConfigsLoaded messages with an explicit generation:

  • StaleSweepSnapshot_CarryingASupersededRevision_DoesNotRestartBackward — fails before.
  • StaleSweepSnapshot_ShowingADeletedInstance_DoesNotResurrectIt — fails before (full deploy-then-delete shape, not the weaker never-started equivalent).
  • StaleBootSnapshot_IsReReadRatherThanApplied — fails before (one read instead of two); a slow first read plus a lifecycle command processed during it makes the race deterministic, and the test also pins that a LATE superseded snapshot is dropped rather than looping.
  • FreshSweepSnapshot_CarryingANewerRevision_StillRestartsTheInstance and FreshSweepSnapshot_ShowingAStoredEnabledInstance_StillStartsIt — the guard twins, pinning that legitimate row-72 and row-78 repairs survive. Both start the instance from the BOOT LOAD rather than a deploy command, precisely so no mutation passes through the actor and generation 0 is genuinely current.

FreshEmptySweepSnapshot_StillRemovesDisappearedConnections (§10) still pins the definitions half; its hardcoded current generation moves from 1 to 2 because one artifact apply now bumps at both ends.