diff --git a/docs/requirements/Component-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index 7b6345c5..020fb832 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -232,6 +232,10 @@ flowchart TD - **SetAttribute** (for attributes with data source reference): Sends a write request to the Data Connection Layer. The DCL writes to the physical device. If the write fails (connection down, device rejection, timeout), the error is returned synchronously to the calling script for handling. On success, the existing subscription picks up the confirmed value from the device and sends it back as a value update, which then updates the in-memory value. The in-memory value is **not** optimistically updated. - **SetAttribute** (for static attributes): Updates the in-memory value and **persists the override to local SQLite**. On restart (or failover), the Instance Actor loads persisted overrides on top of the deployed configuration, preserving runtime-modified values. A redeployment of the instance resets all persisted overrides to the new deployed configuration values. +> **Loaded overrides are republished to the children and the stream (review 2026-10-21 01/SR-U1).** The override load is asynchronous and lands *after* `PreStart` has already created the Script/Alarm Actors, whose trigger snapshots are seeded from the attribute set at that moment — the deploy **defaults**. Static attributes never re-emit a change on their own, so before this fix every trigger on an instance that restarted, failed over, was enabled or was started by the reconcile sweep evaluated an overridden attribute at its default indefinitely (an Expression alarm on `Mode == "Auto"` never raised), and the Debug View live stream showed the default, while `GetAttribute` answered correctly. The load now republishes each override that changed the in-memory value or quality as an ordinary `AttributeValueChanged` (quality from the load, no `SourceExecutionId`) through the same fan-out every change uses, with waiter evaluation **off** — nothing can legitimately be waiting on a replay. Consequence, accepted deliberately: a ValueChange/Conditional script keyed on an overridden static attribute fires once as the actor starts, exactly as one keyed on a data-sourced attribute already fires on that attribute's first DCL value. A static write that lands before the load is newer than the stored row, so the load skips that attribute rather than rolling it back. Overrides for attributes the deployed config no longer defines are still applied (unchanged) but not republished. + +> **Stopping an instance answers its pending waits (review 2026-10-21 01/SR-M3).** `PostStop` cancels every registered `WaitForAttribute` timeout and replies `Matched=false, TimedOut=false, ErrorMessage="… abandoned: instance stopped"`, so a script parked in `Attributes.WaitAsync` across a redeploy/disable/delete learns why immediately instead of burning its whole wait plus the Ask timeout and reporting a time-out, and no `WaitForAttributeTimeout` lands in dead letters (a monitored health metric). + ### Debug View Support - On request from central (via Communication Layer), the Instance Actor provides a **snapshot** of all current attribute values and alarm states. - Subsequent changes are delivered via the **SiteStreamManager** → **SiteStreamGrpcServer** → gRPC stream to central. The Instance Actor publishes attribute value and alarm state changes to the SiteStreamManager; it does not forward events directly to the Communication Layer. diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs index 96bb7d45..f17c271f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs @@ -132,6 +132,16 @@ public class InstanceActor : ReceiveActor private Task _staticOverridePersistTail = Task.CompletedTask; + /// + /// Review 2026-10-21 01/SR-U1: false until the PreStart static-override load has landed + /// (successfully or not). While false, every static write records its attribute in + /// so the load cannot roll a newer in-memory + /// value back to the older stored one — a race that only mattered once the load started + /// republishing what it applies. + /// + private bool _overridesLoaded; + private readonly HashSet _staticWritesBeforeOverrideLoad = new(); + internal sealed record StaticOverridesFlushed(IActorRef Actor, Exception? Error); /// @@ -439,6 +449,20 @@ public class InstanceActor : ReceiveActor state.Timer?.Cancel(); _tagSubscribeStates.Clear(); + // Review 2026-10-21 01/SR-M3: answer every pending WaitForAttribute now. Left alone, each + // waiter's scheduled WaitForAttributeTimeout dead-letters later (dead letters are a + // monitored site-health metric) and the waiting script burns its full wait timeout plus + // the Ask timeout, then reports "timed out" — indistinguishable from a device that never + // answered, when the truth is that the instance went away (redeploy/disable/delete). + foreach (var (correlationId, pending) in _attributeWaiters) + { + pending.Timeout.Cancel(); + pending.Replyer.Tell(new WaitForAttributeResponse( + correlationId, Matched: false, null, null, TimedOut: false, + ErrorMessage: $"Wait for '{pending.AttributeName}' abandoned: instance stopped")); + } + _attributeWaiters.Clear(); + // SQLite's async APIs may block synchronously. Persist on the ordered // thread-pool chain and acknowledge completion separately from Terminated. _staticOverrideFlushTimer?.Cancel(); @@ -606,6 +630,9 @@ public class InstanceActor : ReceiveActor _attributes[command.AttributeName] = command.Value; } + if (!_overridesLoaded) + _staticWritesBeforeOverrideLoad.Add(command.AttributeName); + // Publish attribute change to stream and notify children var changed = new AttributeValueChanged( _instanceUniqueName, @@ -1798,10 +1825,34 @@ public class InstanceActor : ReceiveActor } /// - /// Applies static overrides loaded from SQLite on top of default values. + /// Applies static overrides loaded from SQLite on top of default values, then republishes + /// every value that actually changed (review 2026-10-21 01/SR-U1). + /// + /// + /// The republish is the point of the fix. runs synchronously + /// in , BEFORE this load result arrives, so every Script/Alarm Actor's + /// trigger snapshot was seeded with the deploy DEFAULT; and a static attribute never re-emits + /// a change on its own, so after a restart, failover, enable or sweep start the children + /// evaluated against the default for the actor's whole lifetime while GetAttribute + /// answered with the override. Routing each changed value through + /// re-seeds every child snapshot and the site stream in + /// one place. Consequence, deliberately accepted: a ValueChange/Conditional script keyed on an + /// overridden static attribute fires once as the actor starts — exactly as one keyed on a + /// data-sourced attribute already fires on that attribute's first DCL value. + /// + /// + /// Waiters are NOT evaluated: none can be waiting on a value the instance already held, and a + /// replay must never satisfy a WaitAsync registered against the live value. An + /// attribute a static write touched before this load landed is skipped — the write is newer + /// than the stored row and stays authoritative. + /// /// private void HandleOverridesLoaded(LoadOverridesResult result) { + _overridesLoaded = true; + var writtenBeforeLoad = new HashSet(_staticWritesBeforeOverrideLoad); + _staticWritesBeforeOverrideLoad.Clear(); + if (result.Error != null) { _logger.LogWarning( @@ -1810,8 +1861,15 @@ public class InstanceActor : ReceiveActor return; } + var changedAttributes = new List(); foreach (var kvp in result.Overrides) { + if (writtenBeforeLoad.Contains(kvp.Key)) + continue; + + _attributes.TryGetValue(kvp.Key, out var previousValue); + _attributeQualities.TryGetValue(kvp.Key, out var previousQuality); + // Persisted override values are canonical strings — a JSON array // string for List attributes, a plain string for scalars. Decode List // overrides to a typed list (matching the config-default load), set @@ -1852,11 +1910,31 @@ public class InstanceActor : ReceiveActor { _attributes[kvp.Key] = kvp.Value; } + + // Only attributes the deployed config defines are republished: an orphan override row + // (an attribute since removed from the template) has no child interested in it. + if (_resolvedAttributeByName.ContainsKey(kvp.Key) && + (!string.Equals(AttributeValueCodec.Encode(previousValue), + AttributeValueCodec.Encode(_attributes[kvp.Key]), StringComparison.Ordinal) || + !string.Equals(previousQuality, _attributeQualities.GetValueOrDefault(kvp.Key), + StringComparison.Ordinal))) + { + changedAttributes.Add(kvp.Key); + } + } + + foreach (var name in changedAttributes) + { + PublishAndNotifyChildren( + new AttributeValueChanged( + _instanceUniqueName, name, name, _attributes[name], + _attributeQualities.GetValueOrDefault(name) ?? "Good", DateTimeOffset.UtcNow), + evaluateWaiters: false); } _logger.LogDebug( - "Loaded {Count} static overrides for {Instance}", - result.Overrides.Count, _instanceUniqueName); + "Loaded {Count} static overrides for {Instance} ({Changed} republished)", + result.Overrides.Count, _instanceUniqueName, changedAttributes.Count); } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorOverrideRepublishTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorOverrideRepublishTests.cs new file mode 100644 index 00000000..6a33b840 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorOverrideRepublishTests.cs @@ -0,0 +1,175 @@ +using System.Text.Json; +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Instance; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// Review 2026-10-21 01/SR-U1: persisted static overrides were applied to the Instance Actor's +/// own _attributes but never republished. The Script/Alarm children are created in +/// PreStart BEFORE the async override load lands, so their snapshots were seeded with the +/// deploy default — and static attributes never re-emit a change — so after a restart, failover, +/// enable or sweep start every trigger evaluated against the default, indefinitely. +/// +public class InstanceActorOverrideRepublishTests : TestKit, IDisposable +{ + private readonly SiteStorageService _storage; + private readonly ScriptCompilationService _compilationService; + private readonly SharedScriptLibrary _sharedScriptLibrary; + private readonly TestLocalDb _localDb; + + /// Creates a temporary LocalDb-backed site store per test. + public InstanceActorOverrideRepublishTests() + { + _localDb = TestLocalDb.CreateTemp("instance-override-republish"); + _storage = new SiteStorageService(_localDb.Db, NullLogger.Instance); + _storage.InitializeAsync().GetAwaiter().GetResult(); + _compilationService = new ScriptCompilationService(NullLogger.Instance); + _sharedScriptLibrary = new SharedScriptLibrary( + _compilationService, NullLogger.Instance); + } + + /// + void IDisposable.Dispose() + { + Shutdown(); + var path = _localDb.Path; + _localDb.Dispose(); + TestLocalDb.DeleteFiles(path); + } + + /// + /// Mode defaults to Manual; the runtime override is Auto. An Expression + /// alarm on Mode == "Auto" && Temp > 100. + /// + private static FlattenedConfiguration ModeAlarmConfig(string instanceName) => new() + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute { CanonicalName = "Mode", Value = "Manual", DataType = "String" }, + new ResolvedAttribute { CanonicalName = "Temp", Value = "50", DataType = "Int32" } + ], + Alarms = + [ + new ResolvedAlarm + { + CanonicalName = "AutoOverTemp", + TriggerType = "Expression", + TriggerConfiguration = JsonSerializer.Serialize(new + { + expression = "\"Auto\".Equals(Attributes[\"Mode\"]) && int.Parse(Attributes[\"Temp\"].ToString()) > 100" + }), + PriorityLevel = 500 + } + ] + }; + + private IActorRef StartInstance(string instanceName, FlattenedConfiguration config) => + ActorOf(Props.Create(() => new InstanceActor( + instanceName, JsonSerializer.Serialize(config), _storage, _compilationService, + _sharedScriptLibrary, null, new SiteRuntimeOptions(), NullLogger.Instance))); + + private AlarmState AlarmStateOf(IActorRef instance, string instanceName, string alarm) + { + instance.Tell(new GetAlarmSnapshotRequest("snap", instanceName, DateTimeOffset.UtcNow)); + var snapshot = ExpectMsg(TimeSpan.FromSeconds(5)); + return snapshot.Alarms.Single(a => a.AlarmName == alarm).State; + } + + /// + /// The finding's regression: deploy → override → restart → the Expression alarm sees the + /// override once another attribute changes. Before the fix the child snapshot still held + /// Manual, so the alarm never raised. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task PersistedOverride_ReachesExpressionAlarm_AfterActorRestart() + { + const string name = "PumpU1"; + await _storage.SetStaticOverrideAsync(name, "Mode", "Auto"); + + var instance = StartInstance(name, ModeAlarmConfig(name)); + + // The override load is async; GetAttribute is the authoritative view. Generous timeouts: + // a cold Roslyn compile of the trigger expression runs inside PreStart. + AwaitAssert(() => + { + instance.Tell(new GetAttributeRequest("g", name, "Mode", DateTimeOffset.UtcNow)); + Assert.Equal("Auto", ExpectMsg(TimeSpan.FromSeconds(15)).Value?.ToString()); + }, TimeSpan.FromSeconds(30)); + + instance.Tell(new SetStaticAttributeCommand("w", name, "Temp", "150", DateTimeOffset.UtcNow)); + Assert.True(ExpectMsg().Success); + + AwaitAssert(() => Assert.Equal(AlarmState.Active, AlarmStateOf(instance, name, "AutoOverTemp")), + TimeSpan.FromSeconds(10), TimeSpan.FromMilliseconds(100)); + } + + /// + /// A static write that lands BEFORE the async override load is newer than the stored + /// override, so the load must not overwrite (or republish over) it. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task WriteBeforeOverrideLoad_IsNotOverwrittenByTheOlderStoredValue() + { + const string name = "PumpU1Race"; + await _storage.SetStaticOverrideAsync(name, "Mode", "Auto"); + var gate = new TaskCompletionSource(); + + // A long flush window keeps the write dirty in memory, so the store still holds the + // OLDER "Auto" when the (gated) load finally reads it. + var flushWindow = TimeSpan.FromMinutes(1); + var configJson = JsonSerializer.Serialize(ModeAlarmConfig(name)); + var instance = ActorOf(Props.Create(() => new InstanceActor( + name, configJson, _storage, _compilationService, + _sharedScriptLibrary, null, new SiteRuntimeOptions(), NullLogger.Instance, + null, null, null, flushWindow, gate.Task))); + + instance.Tell(new SetStaticAttributeCommand("w", name, "Mode", "Maintenance", DateTimeOffset.UtcNow)); + Assert.True(ExpectMsg(TimeSpan.FromSeconds(15)).Success); + + gate.SetResult(); + await Task.Delay(500); + + instance.Tell(new GetAttributeRequest("g", name, "Mode", DateTimeOffset.UtcNow)); + Assert.Equal("Maintenance", ExpectMsg().Value?.ToString()); + } + + /// + /// Review 2026-10-21 01/SR-M3: stopping the instance answers every pending + /// WaitAsync at once ("instance stopped") instead of leaving the script to burn its + /// whole wait timeout plus the Ask timeout, and leaves no timeout self-message to dead-letter. + /// + [Fact] + public void Stop_AnswersPendingAttributeWaiters_Immediately() + { + const string name = "PumpM3"; + var instance = StartInstance(name, ModeAlarmConfig(name)); + var waiter = CreateTestProbe(); + + instance.Tell(new WaitForAttributeRequest( + "wait-1", name, "Mode", "\"NeverReached\"", null, TimeSpan.FromMinutes(5), DateTimeOffset.UtcNow), + waiter.Ref); + // A round trip proves the waiter is registered before the stop. + instance.Tell(new GetAttributeRequest("g", name, "Mode", DateTimeOffset.UtcNow)); + ExpectMsg(TimeSpan.FromSeconds(15)); + + Sys.Stop(instance); + + var response = waiter.ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal("wait-1", response.CorrelationId); + Assert.False(response.Matched); + Assert.False(response.TimedOut); + Assert.Contains("instance stopped", response.ErrorMessage); + } +}