fix(dcl): queue a create whose actor name belongs to a still-terminating child instead of refusing it (review 2026-10-21 follow-up L2)

docs/requirements/Component-DataConnectionLayer.md

@@ -371,7 +371,7 @@ 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, and the DCL has no independent notion of "the connections this site ought to have" — the site's `data_connection_definitions` table is read by the **Deployment Manager**, never here (see the arbitration note below; it had no reader at all until register rows 89/90). 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.
+> **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, and the DCL has no independent notion of "the connections this site ought to have" — the site's `data_connection_definitions` table is read by the **Deployment Manager**, never here (see the arbitration note below; it had no reader at all until register rows 89/90). 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; a create whose actor name still belongs to a child the manager has just **stopped** (a Remove→Create rebuild inside the old actor's dispose window — Akka reserves the name until `Terminated`) is **queued** and run on that child's `Terminated` (latest command wins; a Remove cancels it), and only a collision with a *live* child is refused (review 2026-10-21 follow-up L2) — 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.
 >
 > **The two residual gaps of that family are now closed — but not where the register said they were (rows 89 + 90, 2026-08-17)**: the peer node's DCL was never "serving the old endpoint". Only the node running the `site-{SiteId}` Deployment Manager singleton ever receives a `CreateConnectionCommand`, so the standby's manager is **empty**. The divergence surfaces when the singleton's *next* incarnation (failover, or an ordinary restart — no replication required) rebuilds every connection from the `Connections` snapshots frozen into the instances' flattened configs, because the per-name hash cache that recorded the artifact deploy's change is in-memory and dies with the incarnation. The fix lives entirely in the Site Runtime, as an **arbitration rule** — the definitions store wins wherever it has an opinion, resolved *before* the hash is computed so the two sources cannot fight over the same per-name slot — plus in-memory tombstones so a centrally removed connection is not resurrected by a flattened config. Nothing changes here. See `Component-SiteRuntime.md` → *DCL connection arbitration* and `docs/plans/2026-08-17-dcl-connection-arbitration-decision.md`.
 >

src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionManagerActor.cs

@@ -32,6 +32,15 @@ public class DataConnectionManagerActor : ReceiveActor
     private readonly OpcUaGlobalOptions _opcUaGlobalOptions;
     private readonly Dictionary<string, IActorRef> _connectionActors = new();
 
+    // Children this manager has STOPPED (RemoveConnectionCommand) but whose Terminated has not
+    // arrived yet, keyed by actor name. Akka keeps a stopping child's name reserved until it has
+    // fully terminated, so a create of the same name inside that dispose window must wait for
+    // Terminated rather than be refused as a collision (review 2026-10-21 follow-up L2).
+    private readonly Dictionary<string, IActorRef> _stoppingChildren = new();
+
+    // Creates queued behind a stopping child of the same actor name; the latest command wins.
+    private readonly Dictionary<string, CreateConnectionCommand> _queuedCreates = new();
+
     /// <summary>
     /// Initializes a new <see cref="DataConnectionManagerActor"/> with the required dependencies.
     /// </summary>
@@ -71,6 +80,7 @@ public class DataConnectionManagerActor : ReceiveActor
         Receive<SearchAddressSpaceCommand>(HandleSearch);
         Receive<ReadTagValuesCommand>(HandleReadTagValues);
         Receive<VerifyEndpointCommand>(HandleVerifyEndpoint);
+        Receive<Terminated>(HandleChildTerminated);
     }
 
     private void HandleCreateConnection(CreateConnectionCommand command)
@@ -81,6 +91,28 @@ public class DataConnectionManagerActor : ReceiveActor
             return;
         }
 
+        var actorName = ConnectionActorNames.Sanitize(command.ConnectionName);
+        if (_stoppingChildren.ContainsKey(actorName))
+        {
+            // The previous actor of this name is still terminating (a Remove→Create redeploy):
+            // its name is reserved until Terminated, so queue the create rather than refuse it —
+            // a refusal is permanent, because the deployment manager has already recorded the
+            // config hash and will not resend (review 2026-10-21 follow-up L2).
+            if (_queuedCreates.TryGetValue(actorName, out var queued)
+                && !string.Equals(queued.ConnectionName, command.ConnectionName, StringComparison.Ordinal))
+            {
+                RefuseCollision(command, actorName,
+                    $"actor name [{actorName}] is already claimed by queued connection '{queued.ConnectionName}'");
+                return;
+            }
+
+            _queuedCreates[actorName] = command;
+            _log.Info(
+                "Connection {0} queued: the previous actor '{1}' is still stopping; it is created on Terminated",
+                command.ConnectionName, actorName);
+            return;
+        }
+
         // Factory creates the correct adapter based on protocol type
         var adapter = _factory.Create(command.ProtocolType, command.PrimaryConnectionDetails);
 
@@ -97,10 +129,11 @@ public class DataConnectionManagerActor : ReceiveActor
         // ("PLC A" / "PLC-A"), and ActorOf throws synchronously on that (or on any name Akka
         // refuses): that must fail THIS connection only, never escape to the supervisor and
         // restart the whole manager with every live connection (review 2026-10-21, lane A M2).
-        var actorName = ConnectionActorNames.Sanitize(command.ConnectionName);
         IActorRef actorRef;
         try
         {
+            // Only a LIVE child of this name is a collision; a child this manager stopped was
+            // handled above by queueing.
             if (!Context.Child(actorName).IsNobody())
                 throw new InvalidActorNameException(
                     $"actor name [{actorName}] is already used by another connection");
@@ -108,17 +141,7 @@ public class DataConnectionManagerActor : ReceiveActor
         }
         catch (InvalidActorNameException ex)
         {
-            _log.Error(
-                "Connection {0} not created: its actor name '{1}' is invalid or already used by another " +
-                "connection ({2}). Rename the connection so its name differs in more than spaces/symbols.",
-                command.ConnectionName, actorName, ex.Message);
-            if (_siteEventLogger != null)
-            {
-                _ = _siteEventLogger.LogEventAsync(
-                    "connection", "Error", null, command.ConnectionName,
-                    $"Connection not created — runtime actor name '{actorName}' is invalid or already in use",
-                    ex.Message);
-            }
+            RefuseCollision(command, actorName, ex.Message);
             adapter.DisposeAsync().AsTask().ContinueWith(_ => { });
             return;
         }
@@ -362,10 +385,66 @@ public class DataConnectionManagerActor : ReceiveActor
             .PipeTo(Sender);
     }
 
+    private void RefuseCollision(CreateConnectionCommand command, string actorName, string reason)
+    {
+        _log.Error(
+            "Connection {0} not created: its actor name '{1}' is invalid or already used by another " +
+            "connection ({2}). Rename the connection so its name differs in more than spaces/symbols.",
+            command.ConnectionName, actorName, reason);
+        if (_siteEventLogger != null)
+        {
+            _ = _siteEventLogger.LogEventAsync(
+                "connection", "Error", null, command.ConnectionName,
+                $"Connection not created — runtime actor name '{actorName}' is invalid or already in use",
+                reason);
+        }
+    }
+
+    /// <summary>
+    /// A child this manager stopped has fully terminated, releasing its actor name: run any
+    /// create that was queued behind it (review 2026-10-21 follow-up L2).
+    /// </summary>
+    private void HandleChildTerminated(Terminated terminated)
+    {
+        string? actorName = null;
+        foreach (var (name, child) in _stoppingChildren)
+        {
+            if (child.Equals(terminated.ActorRef))
+            {
+                actorName = name;
+                break;
+            }
+        }
+
+        if (actorName is null)
+            return;
+
+        _stoppingChildren.Remove(actorName);
+        if (_queuedCreates.Remove(actorName, out var queued))
+        {
+            _log.Info("Previous actor '{0}' terminated — creating queued connection {1}",
+                actorName, queued.ConnectionName);
+            HandleCreateConnection(queued);
+        }
+    }
+
     private void HandleRemoveConnection(RemoveConnectionCommand command)
     {
+        // A Remove for a connection whose create is still queued behind a stopping child
+        // cancels that create — the connection is meant to be gone.
+        var removedActorName = ConnectionActorNames.Sanitize(command.ConnectionName);
+        if (_queuedCreates.TryGetValue(removedActorName, out var queued)
+            && string.Equals(queued.ConnectionName, command.ConnectionName, StringComparison.Ordinal))
+        {
+            _queuedCreates.Remove(removedActorName);
+            _log.Info("Queued create for connection {0} cancelled by remove", command.ConnectionName);
+        }
+
         if (_connectionActors.TryGetValue(command.ConnectionName, out var actor))
         {
+            // Watch before stopping so the Terminated that frees the actor name reaches us.
+            Context.Watch(actor);
+            _stoppingChildren[actor.Path.Name] = actor;
             Context.Stop(actor);
             _connectionActors.Remove(command.ConnectionName);
             _healthCollector.RemoveConnection(command.ConnectionName);

tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Actors/ConnectionActorNameTests.cs

@@ -20,6 +20,7 @@ public class ConnectionActorNameTests : TestKit
 {
     private readonly IDataConnectionFactory _factory = Substitute.For<IDataConnectionFactory>();
     private readonly ISiteHealthCollector _health = Substitute.For<ISiteHealthCollector>();
+    private volatile bool _slowDispose;
     private readonly DataConnectionOptions _options = new()
     {
         ReconnectInterval = TimeSpan.FromMilliseconds(100),
@@ -37,6 +38,13 @@ public class ConnectionActorNameTests : TestKit
             adapter.Status.Returns(ConnectionHealth.Connected);
             adapter.SubscribeAsync(Arg.Any<string>(), Arg.Any<SubscriptionCallback>(), Arg.Any<CancellationToken>())
                 .Returns("sub-1");
+            // A synchronous delay inside DisposeAsync holds the stopping child in PostStop, which
+            // keeps its actor name reserved — the dispose window the L2 tests exercise.
+            adapter.DisposeAsync().Returns(_ =>
+            {
+                if (_slowDispose) Thread.Sleep(400);
+                return ValueTask.CompletedTask;
+            });
             return adapter;
         });
     }
@@ -89,4 +97,45 @@ public class ConnectionActorNameTests : TestKit
         Assert.Contains("Unknown connection", Subscribe(manager, "PLC-A").ErrorMessage);
         Assert.DoesNotContain("Unknown connection", Subscribe(manager, "PLC A").ErrorMessage ?? "");
     }
+
+    /// <summary>
+    /// Follow-up L2: a Remove immediately followed by a Create of the SAME name (a redeploy that
+    /// rebuilds the connection) lands while the stopped child is still terminating, so its actor
+    /// name is still reserved. The create used to be refused permanently as a "collision" — and
+    /// the deployment manager, having recorded the config hash, never resent it. The manager now
+    /// queues the create until the old child's Terminated arrives.
+    /// </summary>
+    [Fact]
+    public void RemoveThenCreateSameName_InsideDisposeWindow_IsCreatedOnceTheOldChildTerminates()
+    {
+        _slowDispose = true;
+        var manager = CreateManager();
+        manager.Tell(new CreateConnectionCommand("PLC1", "OpcUa", new Dictionary<string, string>(), null, 3));
+        manager.Tell(new RemoveConnectionCommand("PLC1"));
+        manager.Tell(new CreateConnectionCommand("PLC1", "OpcUa", new Dictionary<string, string>(), null, 3));
+
+        AwaitAssert(
+            () => Assert.DoesNotContain("Unknown connection", Subscribe(manager, "PLC1").ErrorMessage ?? ""),
+            TimeSpan.FromSeconds(10),
+            TimeSpan.FromMilliseconds(100));
+    }
+
+    /// <summary>
+    /// Follow-up L2: a Remove that arrives while a create is still queued behind a terminating
+    /// child cancels the queued create — the connection stays removed.
+    /// </summary>
+    [Fact]
+    public void RemoveWhileCreateQueued_CancelsTheQueuedCreate()
+    {
+        _slowDispose = true;
+        var manager = CreateManager();
+        manager.Tell(new CreateConnectionCommand("PLC2", "OpcUa", new Dictionary<string, string>(), null, 3));
+        manager.Tell(new RemoveConnectionCommand("PLC2"));
+        manager.Tell(new CreateConnectionCommand("PLC2", "OpcUa", new Dictionary<string, string>(), null, 3));
+        manager.Tell(new RemoveConnectionCommand("PLC2"));
+
+        // Let the old child terminate (and any wrongly-queued create run) before asserting.
+        Thread.Sleep(1000);
+        Assert.Contains("Unknown connection", Subscribe(manager, "PLC2").ErrorMessage);
+    }
 }