diff --git a/docs/plans/2026-07-21-localdb-followups-live-gate.md b/docs/plans/2026-07-21-localdb-followups-live-gate.md index d2c6ac54..d3cc61a0 100644 --- a/docs/plans/2026-07-21-localdb-followups-live-gate.md +++ b/docs/plans/2026-07-21-localdb-followups-live-gate.md @@ -48,7 +48,7 @@ scenario was believed impossible to recover from — it was a *documented limita gate is what turned the limitation into a reproducible case, and the second defect only existed behind the first. -## Unrelated finding (NOT a follow-up of this work — pre-existing; filed as `lmxopcua` issue #485) +## Unrelated finding (NOT a follow-up of this work — pre-existing; `lmxopcua` issue #485, since FIXED) While SQL was being stopped and started to drive check 4, site-b-1 took a deploy whose artifact load hit a transient ConfigDb error, and **emptied its served address space**: @@ -66,3 +66,13 @@ the node recovered fully on restart (`containers=16`). But a transient ConfigDb served address space instead of holding last-known-good is the same failure *class* as the Phase 1 gate's check-3 defect. Untouched by this work — site-b-1 has replication off entirely — and induced by this gate's own SQL flapping. + +**Fixed** (issue #485): `OpcUaPublishActor.HandleRebuild` now treats an artifact it could not obtain as +*no answer* rather than *an empty configuration*, and abandons the rebuild with the served address space +(and `_lastApplied`) intact. Nothing legitimate produces a zero-length blob — an operator who really +deletes everything still deploys a JSON document with empty arrays — so "no bytes" can only mean the read +failed. The log excerpt above is now impossible: the `PureRemove` never runs, and the loader's own +"the rebuild is abandoned" line is finally true. Covered by +`OpcUaPublishActorRebuildTests.Rebuild_keeps_the_last_known_good_address_space_when_the_artifact_load_fails`, +with a positive control proving a *readable* artifact that genuinely drops an equipment still tears its +subtree down. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs index f19127ab..8cd39d07 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -112,6 +112,10 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers private int _writes; private byte _lastServiceLevel; private bool _publishedAtLeastOnce; + /// True once a real composition has been applied, i.e. there is a served address space worth + /// protecting. Distinguishes "the artifact went missing under a running server" (issue #485 — warn, and + /// hold what is materialised) from "nothing has been deployed here yet" (a quiet no-op at boot). + private bool _hasAppliedComposition; private DbHealthProbeActor.DbHealthStatus? _lastDbHealth; private RedundancyStateChanged? _lastSnapshot; private (bool Ok, DateTime At)? _probeAboutMe; @@ -364,6 +368,28 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers // carry neither. var artifact = msg.Artifact ?? (msg.DeploymentId is { } depId ? LoadArtifact(depId) : LoadLatestArtifact()); + + // Issue #485 — an artifact we could not obtain is NOT an empty configuration. Parsing zero + // bytes yields an empty composition, which the planner diffs against the live one as a + // PureRemove and the applier then faithfully tears down: a transient ConfigDb blip empties the + // served address space. Nothing legitimate produces a zero-length blob (an operator who really + // deletes everything still deploys a JSON document with empty arrays), so the only honest + // reading of "no bytes" is "no answer" — hold the last-known-good address space and let the + // next deploy (or the next boot) supply a real one. Returning here also leaves _lastApplied + // intact, so the retry diffs against what is actually materialised. + if (artifact is null or { Length: 0 }) + { + if (_hasAppliedComposition) + _log.Warning( + "OpcUaPublish: no usable artifact for deployment {Id} (correlation={Correlation}) — KEEPING the currently served address space rather than tearing it down", + msg.DeploymentId, msg.Correlation); + else + _log.Debug( + "OpcUaPublish: no artifact to materialise yet (deployment={Id}, correlation={Correlation})", + msg.DeploymentId, msg.Correlation); + return; + } + var composition = _localNode is { } ln ? DeploymentArtifact.ParseComposition(artifact, ln.Value, inconsistency => _log.Warning("OpcUaPublish {Node}: cross-cluster binding — {Message}", ln, inconsistency)) @@ -379,6 +405,7 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers var outcome = _applier.Apply(plan); _lastApplied = composition; + _hasAppliedComposition = true; // Sum swallowed per-node materialise failures across every pass together with Apply's own // removal-pass tally (archreview 01/S-1). A degraded apply used to vanish into per-node @@ -443,7 +470,8 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } /// Read a specific deployment's artifact blob from ConfigDb (the one just applied, - /// which may not be Sealed yet). Empty array on any failure — parser treats it as "no composition". + /// which may not be Sealed yet). Empty array on any failure — treats that as + /// "no answer" and abandons the rebuild, leaving the served address space untouched (issue #485). private byte[] LoadArtifact(DeploymentId deploymentId) { try @@ -456,13 +484,13 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } catch (Exception ex) { - _log.Warning(ex, "OpcUaPublish: failed to load artifact for deployment {Id}; rebuild becomes no-op", deploymentId); + _log.Warning(ex, "OpcUaPublish: failed to load artifact for deployment {Id}; the rebuild is abandoned", deploymentId); return Array.Empty(); } } /// Read the most recent Sealed deployment's artifact blob from ConfigDb. - /// Empty array on any failure — the parser treats empty blob as "no composition". + /// Empty array on any failure — see for how that is handled. private byte[] LoadLatestArtifact() { try @@ -476,7 +504,7 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } catch (Exception ex) { - _log.Warning(ex, "OpcUaPublish: failed to load latest deployment artifact; rebuild becomes no-op"); + _log.Warning(ex, "OpcUaPublish: failed to load latest deployment artifact; the rebuild is abandoned"); return Array.Empty(); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index be94b075..ba619faf 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -279,6 +279,85 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase sink.Calls.Count(c => c.StartsWith("NA:")).ShouldBe(naAfterFirst); } + /// + /// Issue #485 — a transient ConfigDb error while loading a deploy's artifact must NOT empty the + /// served address space. The loader's own log already claims "rebuild becomes no-op", but an + /// unreadable artifact used to become an EMPTY composition, which the planner then diffed against + /// the live one as a PureRemove and tore every node down. A blip on the way to the database is not + /// evidence that the operator deployed an empty configuration: the last-known-good address space + /// must stand until a real artifact says otherwise. + /// + [Fact] + public void Rebuild_keeps_the_last_known_good_address_space_when_the_artifact_load_fails() + { + var db = new FlakyDbFactory(NewInMemoryDbFactory()); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1"), ("eq-2", "Pump-2")); + + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: TimeSpan.FromSeconds(2)); + var callsAfterFirst = sink.Calls.Count; + + // The next deploy arrives while the ConfigDb is briefly unreachable. + db.Fail = true; + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(Guid.NewGuid()))); + Thread.Sleep(200); + + sink.Calls.ShouldNotContain(c => c.StartsWith("RE:")); // no equipment subtree torn down + sink.RebuildCalls.ShouldBe(0); + sink.Calls.Count.ShouldBe(callsAfterFirst); // the failed load touched nothing at all + + // …and the actor still remembers WHAT is applied: once the database is back, re-applying the same + // composition diffs to an empty plan. A clobbered _lastApplied would re-materialise everything. + db.Fail = false; + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); + Thread.Sleep(200); + sink.Calls.Count.ShouldBe(callsAfterFirst); + } + + /// + /// Positive control for : + /// the guard suppresses teardown ONLY when the artifact could not be read. A real deploy that + /// genuinely drops an equipment still tears its subtree down, so the removal path is not simply dead. + /// + [Fact] + public void Rebuild_still_removes_equipment_a_readable_artifact_genuinely_dropped() + { + var db = new FlakyDbFactory(NewInMemoryDbFactory()); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1"), ("eq-2", "Pump-2")); + + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: TimeSpan.FromSeconds(2)); + + // eq-2 really is gone from the next artifact — that IS a configuration change, so it must apply. + var dep2 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1")); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + + AwaitAssert(() => sink.Calls.ShouldContain("RE:eq-2"), duration: TimeSpan.FromSeconds(2)); + } + + /// An whose CreateDbContext can be made to + /// throw the way a real transient ConfigDb outage does, so the artifact load fails exactly where issue + /// #485 observed it failing. + private sealed class FlakyDbFactory(IDbContextFactory inner) + : IDbContextFactory + { + /// Gets or sets a value indicating whether the next context creation should throw. + public bool Fail { get; set; } + + /// Creates a context, or throws when is set. + /// A new context. + public OtOpcUaConfigDbContext CreateDbContext() => Fail + ? throw new InvalidOperationException( + "An error occurred using the connection to database 'OtOpcUa' on server 'sql,1433'.") + : inner.CreateDbContext(); + } + /// Seal a deployment carrying the given (equipmentId, name) rows under a shared line, returning /// the new DeploymentId so a test can target THAT artifact. Renaming an equipment across two such /// deployments produces a ChangedEquipment delta (Rebuild), while adding a row is a PureAdd.