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).
This commit is contained in:
+117
-8
@@ -16,8 +16,8 @@ using RemoveConnectionCommand = ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Actors
|
||||
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors;
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for deferred register rows <b>89</b> and <b>90</b> — <b>which configuration
|
||||
/// source owns a live DCL connection</b>.
|
||||
/// Regression tests for deferred register rows <b>89</b>, <b>90</b> and <b>97</b> — <b>which
|
||||
/// configuration source owns a live DCL connection, and how a change reaches it</b>.
|
||||
///
|
||||
/// <para>
|
||||
/// <b>The arbitration rule under test.</b> The <c>data_connection_definitions</c> store wins
|
||||
@@ -39,6 +39,14 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors;
|
||||
/// flattened snapshots and never consults the newer definition rows. Design record:
|
||||
/// <c>docs/plans/2026-08-17-dcl-connection-arbitration-decision.md</c>.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Row 97 — the third command.</b> Resolving the winner decides WHAT to push; it took a
|
||||
/// <see cref="ReconfigureConnectionCommand"/> to make a push against an ALREADY-LIVE connection
|
||||
/// mean anything at all, because the DCL manager's create handler warns and returns for a name it
|
||||
/// already holds. So a name the hash cache has never held gets a create, a changed one gets a
|
||||
/// reconfigure, and an unchanged one gets nothing.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
{
|
||||
@@ -56,6 +64,7 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
private readonly TestLocalDb _localDb;
|
||||
|
||||
private readonly ConcurrentQueue<CreateConnectionCommand> _creates = new();
|
||||
private readonly ConcurrentQueue<ReconfigureConnectionCommand> _reconfigures = new();
|
||||
private readonly ConcurrentQueue<RemoveConnectionCommand> _removes = new();
|
||||
|
||||
public DclConnectionArbitrationTests()
|
||||
@@ -93,7 +102,8 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
/// </summary>
|
||||
private IActorRef CreateBootedDeploymentManager(int sweepSeconds = SweepSeconds)
|
||||
{
|
||||
var dclSpy = ActorOf(Props.Create(() => new DclConnectionSpy(_creates, _removes)));
|
||||
var dclSpy = ActorOf(Props.Create(() =>
|
||||
new DclConnectionSpy(_creates, _reconfigures, _removes)));
|
||||
var health = new DeploymentManagerStartupProbe();
|
||||
|
||||
var dm = ActorOf(Props.Create(() => new DeploymentManagerActor(
|
||||
@@ -150,12 +160,19 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
private IReadOnlyList<CreateConnectionCommand> CreatesFor(string connectionName) =>
|
||||
_creates.Where(c => c.ConnectionName == connectionName).ToArray();
|
||||
|
||||
private IReadOnlyList<ReconfigureConnectionCommand> ReconfiguresFor(string connectionName) =>
|
||||
_reconfigures.Where(c => c.ConnectionName == connectionName).ToArray();
|
||||
|
||||
/// <summary>
|
||||
/// The endpoint the DCL was actually told to connect to — the whole point of the arbitration.
|
||||
/// </summary>
|
||||
private static string EndpointOf(CreateConnectionCommand command) =>
|
||||
command.PrimaryConnectionDetails["endpoint"];
|
||||
|
||||
/// <inheritdoc cref="EndpointOf(CreateConnectionCommand)"/>
|
||||
private static string EndpointOf(ReconfigureConnectionCommand command) =>
|
||||
command.PrimaryConnectionDetails["endpoint"];
|
||||
|
||||
private async Task AwaitCreateCountAsync(int count, string because) =>
|
||||
await AwaitAssertAsync(() =>
|
||||
{
|
||||
@@ -163,6 +180,13 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
return Task.CompletedTask;
|
||||
}, TimeSpan.FromSeconds(20));
|
||||
|
||||
private async Task AwaitReconfigureCountAsync(int count, string because) =>
|
||||
await AwaitAssertAsync(() =>
|
||||
{
|
||||
Assert.True(ReconfiguresFor(ConnectionName).Count >= count, because);
|
||||
return Task.CompletedTask;
|
||||
}, TimeSpan.FromSeconds(20));
|
||||
|
||||
// ── Rule 1: a definition row outranks an older flattened snapshot ──
|
||||
|
||||
/// <summary>
|
||||
@@ -224,6 +248,10 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
await Task.Delay(SeveralSweeps);
|
||||
|
||||
Assert.Single(CreatesFor(ConnectionName));
|
||||
// Row 97 moved the cost of a flip: a ping-pong now shows up as a stream of RECONFIGURES,
|
||||
// each of which drops the link and re-subscribes every tag. Assert on both, or the guard
|
||||
// silently stops guarding the thing that would actually happen.
|
||||
Assert.Empty(ReconfiguresFor(ConnectionName));
|
||||
Assert.Empty(_removes);
|
||||
}
|
||||
|
||||
@@ -283,8 +311,15 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
/// pushes the new configuration for the connection the runtime already holds.
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Fails before the fix:</b> exactly one create is ever issued, carrying <c>old-plc</c>,
|
||||
/// and no sweep tick ever looks at the definitions table.
|
||||
/// <b>Fails before the fix:</b> nothing is ever issued for the change at all, and no sweep
|
||||
/// tick ever looks at the definitions table.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Register row 97 changed the SHAPE of the second push.</b> The connection already exists
|
||||
/// at this point, so the change goes out as a
|
||||
/// <see cref="ReconfigureConnectionCommand"/> — a second create would be dropped by the DCL
|
||||
/// manager's create-only guard and change nothing.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
@@ -302,15 +337,87 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
// Now the definition row lands — a bare storage write, the CDC applier's only side effect.
|
||||
await StoreDefinitionAsync(DefinitionEndpoint);
|
||||
|
||||
await AwaitCreateCountAsync(2,
|
||||
await AwaitReconfigureCountAsync(1,
|
||||
"a definition row that arrived by replication was never picked up: the live DCL kept "
|
||||
+ "the endpoint frozen into an instance's flattened config (register row 89)");
|
||||
|
||||
Assert.Equal(DefinitionEndpoint, EndpointOf(CreatesFor(ConnectionName).Last()));
|
||||
Assert.Equal(DefinitionEndpoint, EndpointOf(ReconfiguresFor(ConnectionName).Last()));
|
||||
|
||||
// And it settles: the overlay now owns the name, so further ticks are silent.
|
||||
await Task.Delay(SeveralSweeps);
|
||||
Assert.Equal(2, CreatesFor(ConnectionName).Count);
|
||||
Assert.Single(CreatesFor(ConnectionName));
|
||||
Assert.Single(ReconfiguresFor(ConnectionName));
|
||||
}
|
||||
|
||||
// ── Row 97: a changed configuration reaches the LIVE connection ──
|
||||
|
||||
/// <summary>
|
||||
/// <b>THE row-97 reproduction on the sender side.</b> A connection the DCL already holds gets
|
||||
/// a new endpoint. What must go out is a <see cref="ReconfigureConnectionCommand"/>, which the
|
||||
/// live connection actor applies in place by swapping its adapter and re-issuing its
|
||||
/// subscriptions.
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Fails before the fix:</b> the Deployment Manager re-issued a
|
||||
/// <see cref="CreateConnectionCommand"/> and logged a Warning saying it would not take effect
|
||||
/// — which was accurate, because <c>DataConnectionManagerActor.HandleCreateConnection</c>
|
||||
/// warns and returns for a name it already holds. The hash cache recorded the change as
|
||||
/// applied regardless, so a corrected endpoint sat unapplied until a node restart, a singleton
|
||||
/// failover, or a remove-then-add.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task ChangedConfigurationForALiveConnection_IsSentAsAReconfigure_NotASecondCreate()
|
||||
{
|
||||
const string instanceName = "ReconfiguredPump";
|
||||
CreateBootedDeploymentManager();
|
||||
|
||||
await _storage.StoreDeployedConfigAsync(
|
||||
instanceName, MakeConfigJson(instanceName), "dep-rc1", "rev-rc1", true);
|
||||
await AwaitCreateCountAsync(1, "the sweep never created the connection");
|
||||
|
||||
// The connection-only artifact deploy that corrects the endpoint.
|
||||
await StoreDefinitionAsync(DefinitionEndpoint);
|
||||
|
||||
await AwaitReconfigureCountAsync(1,
|
||||
"a changed configuration for an ALREADY-LIVE connection was not sent as a "
|
||||
+ "ReconfigureConnectionCommand, so the live connection actor keeps serving the old "
|
||||
+ "endpoint until it is rebuilt from scratch (register row 97)");
|
||||
|
||||
Assert.Equal(DefinitionEndpoint, EndpointOf(ReconfiguresFor(ConnectionName).Single()));
|
||||
|
||||
// A create would have been dropped by the manager's create-only guard — so the change must
|
||||
// NOT have been shipped as one.
|
||||
Assert.Single(CreatesFor(ConnectionName));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The hash guard is unchanged by row 97: an UNCHANGED configuration still sends nothing at
|
||||
/// all. This is what keeps a healthy site silent across every sweep tick, and it is the reason
|
||||
/// a reconfigure — which drops the link and re-subscribes every tag — is safe to send on the
|
||||
/// change edge.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task UnchangedConfiguration_SendsNeitherACreateNorAReconfigure()
|
||||
{
|
||||
const string instanceName = "SteadyPump";
|
||||
|
||||
await StoreDefinitionAsync(DefinitionEndpoint);
|
||||
await _storage.StoreDeployedConfigAsync(
|
||||
instanceName, MakeConfigJson(instanceName), "dep-st", "rev-st", true);
|
||||
|
||||
CreateBootedDeploymentManager();
|
||||
await AwaitCreateCountAsync(1, "the boot load never created the connection");
|
||||
|
||||
// Re-write the SAME definition row several sweeps running, and start a second instance
|
||||
// naming the same connection: neither is a configuration change.
|
||||
await StoreDefinitionAsync(DefinitionEndpoint);
|
||||
await _storage.StoreDeployedConfigAsync(
|
||||
"SteadyPumpTwo", MakeConfigJson("SteadyPumpTwo"), "dep-st2", "rev-st2", true);
|
||||
await Task.Delay(SeveralSweeps);
|
||||
|
||||
Assert.Single(CreatesFor(ConnectionName));
|
||||
Assert.Empty(ReconfiguresFor(ConnectionName));
|
||||
}
|
||||
|
||||
// ── Row 90: removal, and not being resurrected by a flattened snapshot ──
|
||||
@@ -469,9 +576,11 @@ public class DclConnectionArbitrationTests : TestKit, IDisposable
|
||||
{
|
||||
public DclConnectionSpy(
|
||||
ConcurrentQueue<CreateConnectionCommand> creates,
|
||||
ConcurrentQueue<ReconfigureConnectionCommand> reconfigures,
|
||||
ConcurrentQueue<RemoveConnectionCommand> removes)
|
||||
{
|
||||
Receive<CreateConnectionCommand>(creates.Enqueue);
|
||||
Receive<ReconfigureConnectionCommand>(reconfigures.Enqueue);
|
||||
Receive<RemoveConnectionCommand>(removes.Enqueue);
|
||||
ReceiveAny(_ => { });
|
||||
}
|
||||
|
||||
+16
-8
@@ -166,12 +166,19 @@ public class DeploymentManagerMediumFindingsTests : TestKit, IDisposable
|
||||
|
||||
/// <summary>
|
||||
/// SiteRuntime-010: when a redeployment changes a connection's configuration
|
||||
/// (here the failover retry count and endpoint), the Deployment Manager must
|
||||
/// re-issue a <see cref="ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection.CreateConnectionCommand"/>
|
||||
/// so the DCL adopts the new configuration rather than keeping the stale one.
|
||||
/// (here the failover retry count and endpoint), the Deployment Manager must push the new
|
||||
/// configuration to the DCL rather than leaving it serving the stale one.
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Register row 97 changed WHICH command that is.</b> It used to be a re-issued
|
||||
/// <c>CreateConnectionCommand</c> — which the create-only manager silently dropped for a name
|
||||
/// it already held, so this test proved a message was sent and nothing more. It is now a
|
||||
/// <see cref="ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection.ReconfigureConnectionCommand"/>,
|
||||
/// which the live connection actor actually applies in place.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task EnsureDclConnections_ConnectionConfigChanged_ReissuesCreateCommand()
|
||||
public async Task EnsureDclConnections_ConnectionConfigChanged_ReissuesReconfigureCommand()
|
||||
{
|
||||
var storage = NewStorage(_localDb.Db);
|
||||
await storage.InitializeAsync();
|
||||
@@ -198,11 +205,12 @@ public class DeploymentManagerMediumFindingsTests : TestKit, IDisposable
|
||||
MakeConfigJsonWithConnection("ConnPump", "opc.tcp://host-b:4840", 7),
|
||||
"admin", DateTimeOffset.UtcNow));
|
||||
|
||||
// The DCL must receive a fresh create command reflecting the new config.
|
||||
var secondCreate = dcl.ExpectMsg<ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection.CreateConnectionCommand>(
|
||||
// The DCL must receive a RECONFIGURE reflecting the new config — not a second create,
|
||||
// which the create-only manager would drop for a name it already holds.
|
||||
var reconfigure = dcl.ExpectMsg<ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection.ReconfigureConnectionCommand>(
|
||||
TimeSpan.FromSeconds(10));
|
||||
Assert.Equal("Conn1", secondCreate.ConnectionName);
|
||||
Assert.Equal(7, secondCreate.FailoverRetryCount);
|
||||
Assert.Equal("Conn1", reconfigure.ConnectionName);
|
||||
Assert.Equal(7, reconfigure.FailoverRetryCount);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
Reference in New Issue
Block a user