diff --git a/docs/Redundancy.md b/docs/Redundancy.md index 1349ee036..150c15cb5 100644 --- a/docs/Redundancy.md +++ b/docs/Redundancy.md @@ -679,8 +679,18 @@ deployment green fleet-wide. Now: **That row stays `Applying` until the outcome is known** (lmxopcua#868): it used to be written `Applied`/clean the moment the revision advanced, and `ConfigPublishCoordinator`'s recovery counts any non-`Applying` row as an ack, so a coordinator restart inside the ~105 s outcome window sealed the deployment clean while the node - was about to report degraded. `/fleet` therefore shows the node *Applying* until its outcome lands; a restart - of the node in that window replays the orphan `Applying` row (re-applies) instead of restoring it. + was about to report degraded. `/fleet` therefore shows the node *Applying* until its outcome lands. + A restart of the node inside that window depends on how it boots: + - a `Direct` node that can read the ConfigDb finds the orphan `Applying` row and **replays** the apply, which + acks the real outcome; + - a node that boots **from its LocalDb cache** (every `FetchAndCache` driver-only node, or a `Direct` node + whose ConfigDb is unreachable) restores the cached revision — the cache is written before the outcome — and + sends **no ack**, so central times that deployment out even though the node runs it. The restored revision + is marked *unconfirmed*, so **re-deploying it re-applies** and acks the real outcome rather than + short-circuiting a clean `Applied`. +- **Upgrade note (lmxopcua#868): do not deploy while the central pair is mid rolling-upgrade across this + change.** An old-build fused central node still writes its own row `Applied` before the outcome, which a + new-build coordinator's recovery would count as a clean ack. Finish upgrading both central nodes, then deploy. - **Mixed-version fleets.** An old node never sets the flag, so its acks are treated exactly as before (clean). A new node acking an old central: the old coordinator ignores the unknown field and seals as it always did — no worse than today. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/FailureReasons.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/FailureReasons.cs new file mode 100644 index 000000000..5a1dac84e --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/FailureReasons.cs @@ -0,0 +1,20 @@ +namespace ZB.MOM.WW.OtOpcUa.Configuration; + +/// +/// The FailureReason column bound shared by Deployment and NodeDeploymentState +/// (nvarchar(2048), ). Every writer of either column goes through +/// : a reason built from free text (an exception message, a node's reported reason) that +/// overflows it makes SQL Server refuse the whole SaveChanges, and a write that can never succeed wedges +/// whatever is retrying it (lmxopcua#868). +/// +public static class FailureReasons +{ + /// The column's maximum length. + public const int MaxLength = 2048; + + /// Bounds a reason to (ellipsis-terminated when cut); null stays null. + /// The reason, or null. + /// The reason, fitting the column. + public static string? Truncate(string? reason) => + reason is null || reason.Length <= MaxLength ? reason : reason[..(MaxLength - 1)] + "…"; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs index 3e0cb2c17..898c6e282 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs @@ -109,6 +109,12 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU private readonly Dictionary _unpersisted = new(NodeIdComparer); private string? _lastPersistError; + /// When the current deadline was first deferred on an unpersisted ack (bounds the deferral). + private DateTime? _deadlineDeferredSinceUtc; + + /// How long a deadline may wait on unpersisted acks before deciding from memory (10 × the retry ceiling). + private TimeSpan MaxDeadlineDeferral => TimeSpan.FromTicks(_loadRetryMax.Ticks * 10); + /// /// Where central→node commands are handed off (per-cluster mesh Phase 2), or /// to publish on the mediator directly. @@ -419,11 +425,15 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU _log.Warning( "Coordinator recovered deployment {Id} with NO expected-ack rows — the dispatch was lost " + "before any node was expected (coordinator restart or singleton hand-over mid-dispatch); " + - "re-dispatching with {Remaining} of the apply deadline left", + "re-dispatching with a fresh apply deadline ({Remaining} of the original was left)", _current, remaining); - // The REMAINING time (review L4): CreatedAtUtc never moves, so a re-drive must not restart the clock. + // A FULL deadline (third review #3, superseding the second review's "remaining time"): a node needs its + // whole outcome timeout (DeployTimings.ApplyDeadline - ApplyOutcomeSafetyMargin) plus that margin to + // report — i.e. the full ApplyDeadline — so ANY remaining time shorter than that let the nodes apply + // and the deployment time out moments later. Bounded anyway: there is no dispatched-at column, so a + // SECOND restart recomputes from CreatedAtUtc and, past the deadline by then, closes the row instead. HandleDispatch(new DispatchDeployment( - _current.Value, RevisionHash.Parse(inflight.RevisionHash), CorrelationId.NewId()), remaining); + _current.Value, RevisionHash.Parse(inflight.RevisionHash), CorrelationId.NewId())); } else { @@ -468,11 +478,7 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU _current, _acks.Count, _expectedAcks.Count); } - private void HandleDispatch(DispatchDeployment msg) => HandleDispatch(msg, deadline: null); - - /// The dispatch. - /// The apply deadline to arm; null = the full . - private void HandleDispatch(DispatchDeployment msg, TimeSpan? deadline) + private void HandleDispatch(DispatchDeployment msg) { // A duplicate of the deployment already in flight with a seeded expected set (a dispatch queued while the // recovery load ran, for the very row that load re-drove) must not reset the acks already gathered. @@ -567,7 +573,7 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU // directly, so this actor no longer knows which transport is in force. if (_meshRouter is not null) _meshRouter.Tell(new MeshCommand(DeploymentsTopic, msg)); else DistributedPubSub.Get(Context.System).Mediator.Tell(new Publish(DeploymentsTopic, msg)); - Timers.StartSingleTimer(DeadlineTimerKey, new DeadlineElapsed(msg.DeploymentId), deadline ?? _applyDeadline); + Timers.StartSingleTimer(DeadlineTimerKey, new DeadlineElapsed(msg.DeploymentId), _applyDeadline); if (_expectedAcks.Count == 0) { @@ -621,6 +627,19 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU { PersistNodeAck(msg); } + catch (Exception ex) when (IsPoison(ex)) + { + // Third review #1: a write the DB REJECTS (constraint, truncation, …) fails identically on every retry; + // retrying it forever wedged the deployment (the deadline deferred on it) and every later deploy. The + // node DID ack, so count it from memory and say loudly that its row is not recorded. + _log.Error(ex, + "The ApplyAck from {Node} for {Id} ({Outcome}) was REJECTED by the ConfigDb and will not be retried; " + + "counting it from memory. Its NodeDeploymentState row stays unrecorded", + msg.NodeId, msg.DeploymentId, msg.Outcome); + DropUnpersisted(msg.NodeId); + CountAck(msg); + return true; + } catch (Exception ex) { var attempts = _unpersisted.TryGetValue(msg.NodeId, out var prior) ? prior.Attempts + 1 : 1; @@ -639,17 +658,42 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU return false; } - if (_unpersisted.Remove(msg.NodeId)) - { - Timers.Cancel(AckRetryTimerKey(msg.NodeId)); - if (_unpersisted.Count == 0) _lastPersistError = null; - ReportPersistBacklog(); - } + DropUnpersisted(msg.NodeId); + CountAck(msg); + return true; + } + private void CountAck(ApplyAck msg) + { _acks[msg.NodeId] = msg.Outcome; // An old node never sets Degraded, so its ack takes the clean branch exactly as before. if (IsDegradedAck(msg)) _degraded.Add(msg.NodeId); else _degraded.Remove(msg.NodeId); + } + + private void DropUnpersisted(NodeId node) + { + if (!_unpersisted.Remove(node)) return; + Timers.Cancel(AckRetryTimerKey(node)); + if (_unpersisted.Count == 0) _lastPersistError = null; + ReportPersistBacklog(); + } + + /// + /// A non-transient rejection of the write itself: an EF (not a concurrency + /// conflict) with no transient database error, timeout or socket failure underneath. Connectivity failures + /// surface as other exception types (or as transient s) and stay + /// retryable. + /// + internal static bool IsPoison(Exception ex) + { + if (ex is not DbUpdateException || ex is DbUpdateConcurrencyException) return false; + for (var e = ex.InnerException; e is not null; e = e.InnerException) + { + if (e is System.Data.Common.DbException { IsTransient: true } + or TimeoutException or System.Net.Sockets.SocketException or System.IO.IOException) + return false; + } return true; } @@ -697,18 +741,17 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU ? Truncate(DegradedReasonPrefix + (string.IsNullOrWhiteSpace(msg.DegradedReason) ? "address-space apply degraded (no reason given)" : msg.DegradedReason)) - : msg.FailureReason; + // Third review #1: a Failed ack's reason is the node's free text (often an exception message) and was + // written UNBOUNDED — on SQL Server an overflow refuses the SaveChanges forever. + : FailureReasons.Truncate(msg.FailureReason); db.SaveChanges(); } private static bool IsDegradedAck(ApplyAck msg) => msg.Outcome == ApplyAckOutcome.Applied && msg.Degraded; // FailureReason is nvarchar(2048) on both tables (OtOpcUaConfigDbContext); a reason built from a node's - // free text must never fail the SaveChanges that records the ack. - private const int FailureReasonMaxLength = 2048; - - private static string Truncate(string s) => - s.Length <= FailureReasonMaxLength ? s : s[..(FailureReasonMaxLength - 1)] + "…"; + // free text must never fail the SaveChanges that records the ack. One shared bound (FailureReasons). + private static string Truncate(string s) => FailureReasons.Truncate(s)!; private void SealDeployment() { @@ -763,6 +806,21 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU if (_unpersisted.Count > 0) { foreach (var pending in _unpersisted.Values.ToList()) TryPersistAck(pending.Ack); + _deadlineDeferredSinceUtc ??= DateTime.UtcNow; + if (_unpersisted.Count > 0 && DateTime.UtcNow - _deadlineDeferredSinceUtc.Value >= MaxDeadlineDeferral) + { + // Third review #1: bounded. Past the cap the outcome is decided from what was RECEIVED — the nodes did + // ack — rather than waiting on rows that may never be writable. + _log.Error( + "Deployment {Id}: {Count} received ack(s) still could not be persisted after deferring the deadline " + + "{Cap}; deciding the outcome from the acks held in memory (last error: {Error})", + _current.Value, _unpersisted.Count, MaxDeadlineDeferral, _lastPersistError); + foreach (var pending in _unpersisted.Values.ToList()) + { + DropUnpersisted(pending.Ack.NodeId); + CountAck(pending.Ack); + } + } if (_unpersisted.Count > 0) { var delay = Backoff(_unpersisted.Values.Min(p => p.Attempts)); @@ -841,6 +899,7 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers, IWithU private void ResetForNext() { Timers.Cancel(DeadlineTimerKey); + _deadlineDeferredSinceUtc = null; foreach (var node in _unpersisted.Keys) Timers.Cancel(AckRetryTimerKey(node)); if (_unpersisted.Count > 0) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index 56ea2ad3b..b16f68634 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -295,7 +295,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// idempotent "already at this revision" short-circuit: the short-circuit would ack a clean Applied /// for an address space known to be partial, and the re-apply is what repairs it (a degraded /// non-rebuild apply leaves the publish actor's composition un-advanced, so it re-diffs the missing - /// nodes). Reset by every clean outcome and by every boot path that restores a revision. + /// nodes). Reset by every clean outcome and by the ConfigDb boot path (an Applied row is a final outcome); + /// SET by the cache boot paths, whose cached revision may never have reported an outcome (lmxopcua#868). /// private bool _currentRevisionDegraded; @@ -1434,6 +1435,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS } _currentRevision = revision; + // Unconfirmed (lmxopcua#868 third review #2): the cache is written BEFORE the address-space outcome, so a + // restart in that window boots a revision central never got an ack for. Marking it unconfirmed makes a + // re-dispatch of it RE-APPLY (and ack the real outcome) instead of short-circuiting a clean Applied. + _currentRevisionDegraded = true; BecomeSteady(); _log.Info( @@ -1514,6 +1519,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS // Stale would have started does not run here, so recovery rides on the next dispatch — // matching how a normally-booted node behaves. _currentRevision = revision; + // Unconfirmed, as in BootstrapFromCache (third review #2): a re-dispatch of it re-applies. + _currentRevisionDegraded = true; BecomeSteady(); _isRunningFromCache = true; @@ -4336,6 +4343,9 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS private void UpsertNodeDeploymentState(DeploymentId deploymentId, NodeDeploymentStatus status, string? failureReason) { + // nvarchar(2048): an exception message or a degraded reason can exceed it, and SQL Server then refuses the + // write — the row would stay Applying forever (lmxopcua#868 third review #1). + failureReason = FailureReasons.Truncate(failureReason); // Per-cluster mesh Phase 4: a driver-only node has NO ConfigDb. Central is the ack system of // record — ConfigPublishCoordinator.PersistNodeAck already writes NodeDeploymentState from every // ApplyAck it receives (and seeds the Applying rows at dispatch) — so this node's own upsert is diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigPublishCoordinatorDbOutageTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigPublishCoordinatorDbOutageTests.cs index bb3045a67..202969bc7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigPublishCoordinatorDbOutageTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigPublishCoordinatorDbOutageTests.cs @@ -402,6 +402,115 @@ public sealed class ConfigPublishCoordinatorDbOutageTests : ControlPlaneActorTes }, duration: PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); } + /// + /// Third review #1: a Failed ack's reason (node free text) was written unbounded; on SQL Server the overflow + /// refuses the SaveChanges forever and the deployment wedged. It is truncated to the column. + /// + [Fact] + public void An_oversized_failure_reason_is_truncated_not_retried_forever() + { + var (db, deploymentId, coordinator) = DispatchedOnConstraintDb(NoTimeout); + + coordinator.Tell(new ApplyAck(deploymentId, NodeId.Parse(Node), ApplyAckOutcome.Failed, + new string('x', 5000), CorrelationId.NewId())); + + AwaitAssert(() => + { + using var ctx = db.CreateDbContext(); + ctx.Deployments.Single().Status.ShouldBe(DeploymentStatus.PartiallyFailed); + var row = ctx.NodeDeploymentStates.Single(); + row.Status.ShouldBe(NodeDeploymentStatus.Failed); + row.FailureReason!.Length.ShouldBe(FailureReasons.MaxLength); + }, duration: PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + } + + /// + /// Third review #1: a write the DB REJECTS (non-transient DbUpdateException) is not retried forever — the ack is + /// counted from memory and the deployment completes. + /// + [Fact] + public void A_rejected_ack_write_is_counted_from_memory_not_retried_forever() + { + var (db, deploymentId, coordinator) = DispatchedOnConstraintDb(NoTimeout); + db.PoisonNodeStateUpdates = true; + + coordinator.Tell(new ApplyAck(deploymentId, NodeId.Parse(Node), ApplyAckOutcome.Applied, null, CorrelationId.NewId())); + + AwaitAssert(() => + { + using var ctx = db.CreateDbContext(); + ctx.Deployments.Single().Status.ShouldBe(DeploymentStatus.Sealed); + }, duration: PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + } + + /// + /// Third review #1: a write that keeps failing transiently defers the deadline only up to a cap + /// (10 × the retry ceiling); past it the outcome is decided from the acks held in memory. + /// + [Fact] + public void Deadline_deferral_on_unpersisted_acks_is_bounded() + { + var (db, deploymentId, coordinator) = DispatchedOnConstraintDb(TimeSpan.FromSeconds(1)); + db.FailNodeStateUpdatesTransiently = true; + + coordinator.Tell(new ApplyAck(deploymentId, NodeId.Parse(Node), ApplyAckOutcome.Applied, null, CorrelationId.NewId())); + + // Deadline 1 s + cap 10 × 200 ms: decided within a few seconds, never left in flight. + AwaitAssert(() => + { + using var ctx = db.CreateDbContext(); + ctx.Deployments.Single().Status.ShouldBe(DeploymentStatus.Sealed); + }, duration: PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + } + + /// + /// Third review #3: a lost dispatch recovered with only a sliver of its deadline left used to be re-driven with + /// that sliver — the nodes applied and the deployment timed out moments later. It gets a full deadline. + /// + [Fact] + public void A_lost_dispatch_redriven_near_its_deadline_gets_a_full_deadline() + { + var dbFactory = NewInMemoryDbFactory(); + SeedNodes(dbFactory, Node); + var deadline = TimeSpan.FromSeconds(3); + var deploymentId = SeedDeployment(dbFactory, DeploymentStatus.Dispatching, DateTime.UtcNow - TimeSpan.FromSeconds(2.5)); + + var coordinator = Sys.ActorOf(ConfigPublishCoordinator.Props(dbFactory, deadline)); + AwaitAssert(() => + { + using var db = dbFactory.CreateDbContext(); + db.Deployments.Single().Status.ShouldBe(DeploymentStatus.AwaitingApplyAcks); + }, duration: PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + + // Well past the 0.5 s that was left, still inside a fresh 3 s deadline. + Thread.Sleep(TimeSpan.FromSeconds(1.5)); + using (var db = dbFactory.CreateDbContext()) + db.Deployments.Single().Status.ShouldBe(DeploymentStatus.AwaitingApplyAcks); + + coordinator.Tell(new ApplyAck(deploymentId, NodeId.Parse(Node), ApplyAckOutcome.Applied, null, CorrelationId.NewId())); + AwaitAssert(() => + { + using var db = dbFactory.CreateDbContext(); + db.Deployments.Single().Status.ShouldBe(DeploymentStatus.Sealed); + }, duration: PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + } + + private (ConstraintEnforcingDbFactory Db, DeploymentId Id, IActorRef Coordinator) DispatchedOnConstraintDb(TimeSpan applyDeadline) + { + var db = new ConstraintEnforcingDbFactory(Guid.NewGuid().ToString("N")); + SeedNodes(db, Node); + var deploymentId = SeedDeployment(db, DeploymentStatus.Dispatching, DateTime.UtcNow); + var coordinator = Sys.ActorOf(ConfigPublishCoordinator.Props( + db, applyDeadline, loadRetryMin: RetryMin, loadRetryMax: RetryMax)); + coordinator.Tell(new DispatchDeployment(deploymentId, TestRevision, CorrelationId.NewId())); + AwaitAssert(() => + { + using var ctx = db.CreateDbContext(); + ctx.Deployments.Single().Status.ShouldBe(DeploymentStatus.AwaitingApplyAcks); + }, duration: PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + return (db, deploymentId, coordinator); + } + /// A single-node deployment dispatched and AwaitingApplyAcks through an outage-capable factory. private (IDbContextFactory Direct, OutageDbFactory Outage, DeploymentId Id, IActorRef Coordinator, ConfigPublishCoordinatorReadiness Readiness) DispatchedSingleNode(TimeSpan applyDeadline) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Harness/ConstraintEnforcingDbFactory.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Harness/ConstraintEnforcingDbFactory.cs new file mode 100644 index 000000000..a3d29603b --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Harness/ConstraintEnforcingDbFactory.cs @@ -0,0 +1,69 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; + +namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Harness; + +/// +/// An in-memory ConfigDb that behaves like SQL Server where the InMemory provider does not (lmxopcua#868 third +/// review): a FailureReason longer than the column refuses the whole SaveChanges with a +/// . It can also reject (, a non-transient +/// ) or fail transiently () every +/// update of an existing NodeDeploymentState row. +/// +public sealed class ConstraintEnforcingDbFactory(string dbName) : IDbContextFactory +{ + private readonly Interceptor _interceptor = new(); + + /// Every update of an existing node row is rejected as a constraint violation. + public bool PoisonNodeStateUpdates { get => _interceptor.Poison; set => _interceptor.Poison = value; } + + /// Every update of an existing node row fails as if the connection dropped. + public bool FailNodeStateUpdatesTransiently { get => _interceptor.Transient; set => _interceptor.Transient = value; } + + /// + public OtOpcUaConfigDbContext CreateDbContext() => new(new DbContextOptionsBuilder() + .UseInMemoryDatabase(dbName) + .AddInterceptors(_interceptor) + .Options); + + private sealed class Interceptor : SaveChangesInterceptor + { + public volatile bool Poison; + public volatile bool Transient; + + public override InterceptionResult SavingChanges(DbContextEventData eventData, InterceptionResult result) + { + Check(eventData.Context!); + return result; + } + + public override ValueTask> SavingChangesAsync( + DbContextEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + Check(eventData.Context!); + return ValueTask.FromResult(result); + } + + private void Check(DbContext ctx) + { + foreach (var e in ctx.ChangeTracker.Entries()) + { + var reason = e.Entity switch + { + NodeDeploymentState n => n.FailureReason, + Deployment d => d.FailureReason, + _ => null, + }; + if (reason is { Length: > FailureReasons.MaxLength }) + throw new DbUpdateException("String or binary data would be truncated in column 'FailureReason'."); + if (e.Entity is NodeDeploymentState && e.State == EntityState.Modified) + { + if (Poison) throw new DbUpdateException("The UPDATE statement conflicted with a CHECK constraint."); + if (Transient) throw new InvalidOperationException("A network-related error occurred (simulated)."); + } + } + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBootFromCacheTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBootFromCacheTests.cs index 23c2d4195..033c22934 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBootFromCacheTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBootFromCacheTests.cs @@ -56,6 +56,31 @@ public sealed class DriverHostActorBootFromCacheTests : RuntimeActorTestBase cache.UnkeyedReads.ShouldBe(1); } + /// + /// lmxopcua#868 third review #2: the cache is written before the address-space outcome, so a cache-booted + /// revision may never have been acked. A re-dispatch of it must RE-APPLY (here: fail honestly, since the + /// ConfigDb is unreachable) — never short-circuit a clean Applied for an outcome nobody confirmed. + /// + [Fact] + public void A_cache_booted_revision_is_unconfirmed_so_a_redispatch_reapplies_instead_of_acking_clean() + { + var cached = new CachedDeploymentArtifact( + DeploymentId.NewId().ToString(), CachedRev.Value, EmptyArtifact(), DateTimeOffset.UtcNow.AddMinutes(-1)); + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + new ThrowingDbFactory(), TestNode, ackRouter: coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: new StubArtifactCache(cached))); + AskDiagnostics(actor).CurrentRevision.ShouldBe(CachedRev); + + actor.Tell(new ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy.DispatchDeployment( + DeploymentId.NewId(), CachedRev, CorrelationId.NewId())); + + var ack = coordinator.ExpectMsg( + PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + ack.Outcome.ShouldNotBe(ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy.ApplyAckOutcome.Applied); + } + [Fact] public void BootFromCache_HandsTheCachedArtifactToTheAddressSpaceRebuild() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorDegradedAckTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorDegradedAckTests.cs index 99f261ee1..0875d1755 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorDegradedAckTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorDegradedAckTests.cs @@ -238,6 +238,58 @@ public sealed class DriverHostActorDegradedAckTests : RuntimeActorTestBase }, PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); } + [Fact] + public void An_oversized_degraded_reason_is_truncated_so_the_local_row_still_records_the_outcome() + { + // lmxopcua#868 third review #1: the row's FailureReason is nvarchar(2048). An unbounded reason made SQL Server + // refuse the write (logged and swallowed), so the row stayed Applying forever. A DB that enforces the column. + var db = new LengthEnforcingDbFactory(Guid.NewGuid().ToString("N")); + var deploymentId = V3ArtifactSeeder.SeedV3Deployment(db, RevA, + rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, + unsRefs: new[] { (Area: "filling", Line: "line1", Equip: "station1", BackingTag: "speed", Effective: (string?)null) }); + var coordinator = CreateTestProbe(); + var publish = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, localRoles: new HashSet { "driver" }, + opcUaPublishActor: publish.Ref)); + + actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId())); + var rebuild = publish.FishForMessage(_ => true, PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + publish.Reply(new OpcUaPublishActor.RebuildCompleted(rebuild.Correlation, rebuild.DeploymentId, true, new string('f', 5000))); + coordinator.ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken).Degraded.ShouldBeTrue(); + + AwaitAssert(() => + { + using var ctx = db.CreateDbContext(); + var row = ctx.NodeDeploymentStates.Single(r => r.NodeId == TestNode.Value && r.DeploymentId == deploymentId.Value); + row.Status.ShouldBe(ZB.MOM.WW.OtOpcUa.Configuration.Enums.NodeDeploymentStatus.Applied); + row.FailureReason!.Length.ShouldBe(FailureReasons.MaxLength); + row.FailureReason.ShouldStartWith(ApplyAck.DegradedReasonPrefix); + }, PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + } + + /// In-memory ConfigDb that refuses an over-length FailureReason, as SQL Server does. + private sealed class LengthEnforcingDbFactory(string dbName) : IDbContextFactory + { + public OtOpcUaConfigDbContext CreateDbContext() => new(new DbContextOptionsBuilder() + .UseInMemoryDatabase(dbName) + .AddInterceptors(new Enforcer()) + .Options); + + private sealed class Enforcer : Microsoft.EntityFrameworkCore.Diagnostics.SaveChangesInterceptor + { + public override Microsoft.EntityFrameworkCore.Diagnostics.InterceptionResult SavingChanges( + Microsoft.EntityFrameworkCore.Diagnostics.DbContextEventData eventData, + Microsoft.EntityFrameworkCore.Diagnostics.InterceptionResult result) + { + foreach (var e in eventData.Context!.ChangeTracker.Entries()) + if (e.Entity.FailureReason is { Length: > FailureReasons.MaxLength }) + throw new DbUpdateException("String or binary data would be truncated in column 'FailureReason'."); + return result; + } + } + } + [Fact] public void The_default_outcome_timeout_is_derived_from_centrals_apply_deadline_minus_a_margin() {