From a04196d7257055524fd2d5263eed9d5cb755f625 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:24:04 -0400 Subject: [PATCH 01/20] fix(adminui): evict unconfigured drivers from status stores; isolate throwing subscribers (R04 L1, L5) The driver-status and resilience stores had no removal path, so a deleted driver stayed on /hosts as a frozen row until restart. Both stores gain RetainOnly, which /hosts calls after each successful config load (never on a failed one). The stores and InProcessBroadcaster now raise their events one subscriber at a time, so a throwing handler cannot starve later subscribers or escape onto the bridge actor. The alert/script-log bridges publish inside their try, and the resilience bridge catches a store failure instead of restarting. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Components/Pages/Hosts.razor | 11 ++ .../Hubs/AlertSignalRBridge.cs | 7 +- .../Hubs/DriverResilienceStatusBridge.cs | 9 +- .../Hubs/IDriverResilienceStatusStore.cs | 11 ++ .../Hubs/IDriverStatusSnapshotStore.cs | 11 ++ .../Hubs/IInProcessBroadcaster.cs | 16 +- .../InMemoryDriverResilienceStatusStore.cs | 25 +++- .../Hubs/InMemoryDriverStatusSnapshotStore.cs | 25 +++- .../Hubs/SafeMulticast.cs | 39 +++++ .../Hubs/ScriptLogSignalRBridge.cs | 5 +- .../AdminUiSource.cs | 25 ++++ .../Hubs/StoreEvictionAndIsolationTests.cs | 139 ++++++++++++++++++ .../Telemetry/TelemetryDialDeadlineTests.cs | 2 + .../Telemetry/TelemetryDialSupervisorTests.cs | 2 + 14 files changed, 314 insertions(+), 13 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AdminUiSource.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor index 4c7849fd..d4a1d6a3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor @@ -17,6 +17,7 @@ @using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers @inject ActorSystem ActorSystem @inject IDriverStatusSnapshotStore DriverStore +@inject IDriverResilienceStatusStore ResilienceStore @inject IDbContextFactory DbFactory @inject Microsoft.Extensions.Logging.ILogger Logger @implements IAsyncDisposable @@ -428,6 +429,16 @@ else _instances = await db.DriverInstances.AsNoTracking() .Select(d => new HostsDriverInstanceInfo(d.DriverInstanceId, d.ClusterId, d.Name, d.DriverType, d.Enabled)) .ToListAsync(_cts.Token); + + // Evict snapshots of driver instances that are no longer configured (deleted, or re-created + // under a new id). The stores have no other removal path, so without this a removed driver + // stays on this page as a frozen row until the admin node restarts. Only a SUCCESSFUL load + // reaches here; the failure path below blanks _instances and must never sweep the stores. + var configured = _instances + .Select(i => i.DriverInstanceId) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + DriverStore.RetainOnly(configured); + ResilienceStore.RetainOnly(configured); } catch (Exception ex) when (ex is OperationCanceledException or ObjectDisposedException) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertSignalRBridge.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertSignalRBridge.cs index 16c7271f..f415bd88 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertSignalRBridge.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertSignalRBridge.cs @@ -57,15 +57,16 @@ public sealed class AlertSignalRBridge : ReceiveActor private async Task ForwardAsync(AlarmTransitionEvent msg) { // In-process fan-out first — this is what the Blazor Server Alerts page reads. The hub push - // is kept for any out-of-process (e.g. WASM) SignalR client. - _broadcaster.Publish(msg); + // is kept for any out-of-process (e.g. WASM) SignalR client. Both sit inside the try so a + // broadcast failure cannot restart the bridge (and drop its DPS subscription). try { + _broadcaster.Publish(msg); await _hub.Clients.All.SendAsync(AlertHub.MethodName, msg); } catch (Exception ex) { - _log.Warning(ex, "AlertSignalRBridge: SignalR push failed for {AlarmId}", msg.AlarmId); + _log.Warning(ex, "AlertSignalRBridge: forward failed for {AlarmId}", msg.AlarmId); } } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/DriverResilienceStatusBridge.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/DriverResilienceStatusBridge.cs index 8b488327..25cb09e0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/DriverResilienceStatusBridge.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/DriverResilienceStatusBridge.cs @@ -1,5 +1,6 @@ using Akka.Actor; using Akka.Cluster.Tools.PublishSubscribe; +using Akka.Event; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; @@ -17,6 +18,7 @@ public sealed class DriverResilienceStatusBridge : ReceiveActor public const string TopicName = DriverResilienceStatusChanged.TopicName; private readonly IDriverResilienceStatusStore _store; + private readonly ILoggingAdapter _log = Context.GetLogger(); /// Creates actor props for a . /// Resilience status store updated on every forwarded snapshot. @@ -29,7 +31,12 @@ public sealed class DriverResilienceStatusBridge : ReceiveActor public DriverResilienceStatusBridge(IDriverResilienceStatusStore store) { _store = store; - Receive(msg => _store.Upsert(msg)); + Receive(msg => + { + // A store failure must not restart the bridge (a restart re-subscribes DPS and drops messages). + try { _store.Upsert(msg); } + catch (Exception ex) { _log.Warning(ex, "DriverResilienceStatusBridge: upsert failed for {0}", msg.DriverInstanceId); } + }); Receive(_ => { /* DPS confirmation */ }); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs index 0e503824..631b31be 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs @@ -27,6 +27,17 @@ public interface IDriverResilienceStatusStore /// A read-only collection of the last-known resilience snapshot for each pair. IReadOnlyCollection GetAll(); + /// + /// Evicts every snapshot whose driver instance is not in . + /// The stores have no other removal path, so a deleted (or re-created under a new id) driver would + /// otherwise keep its last snapshot until the process restarts. The /hosts page calls this + /// after each successful config load; a failed load must never call it with an empty set. + /// + /// The configured driver-instance ids to keep. The set's own + /// comparer decides matching. + /// The number of entries removed. + int RetainOnly(IReadOnlySet configuredInstanceIds); + /// /// Raised after every with the just-stored snapshot. Lets the Blazor Server /// driver status panel receive live updates by reading this singleton directly. Handlers run on diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs index b37b7bd4..8cfbd2aa 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs @@ -25,6 +25,17 @@ public interface IDriverStatusSnapshotStore /// A read-only collection of the last-known health snapshot for each driver instance. IReadOnlyCollection GetAll(); + /// + /// Evicts every snapshot whose driver instance is not in . + /// The stores have no other removal path, so a deleted (or re-created under a new id) driver would + /// otherwise keep its last snapshot until the process restarts. The /hosts page calls this + /// after each successful config load; a failed load must never call it with an empty set. + /// + /// The configured driver-instance ids to keep. The set's own + /// comparer decides matching. + /// The number of entries removed. + int RetainOnly(IReadOnlySet configuredInstanceIds); + /// /// Raised after every with the just-stored snapshot. Lets in-process /// consumers (the Blazor Server DriverStatusPanel) receive live updates by reading diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs index 1e9a42ac..3a29265a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs @@ -1,3 +1,5 @@ +using Microsoft.Extensions.Logging; + namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; /// @@ -59,6 +61,11 @@ public sealed class InProcessBroadcaster : IInProcessBroadcaster // render thread, so access must be serialised. private readonly object _connectionLock = new(); private bool _isConnected; + private readonly ILogger>? _logger; + + /// Creates the broadcaster. + /// Optional logger for a subscriber that throws during a raise. + public InProcessBroadcaster(ILogger>? logger = null) => _logger = logger; /// Raised once per with the published item, on the caller's thread. public event Action? Received; @@ -67,8 +74,10 @@ public sealed class InProcessBroadcaster : IInProcessBroadcaster public event Action? ConnectionStateChanged; /// - // Capture-then-invoke (via ?.) so a concurrent unsubscribe can't null the delegate mid-raise. - public void Publish(T item) => Received?.Invoke(item); + // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise; one subscriber + // at a time so a throwing one cannot starve the rest or escape onto the bridge actor. + public void Publish(T item) => + SafeMulticast.Invoke(Received, item, ex => _logger?.LogDebug(ex, "In-process broadcast subscriber threw")); /// public bool IsConnected @@ -99,6 +108,7 @@ public sealed class InProcessBroadcaster : IInProcessBroadcaster handler = ConnectionStateChanged; } - handler?.Invoke(connected); + SafeMulticast.Invoke(handler, connected, + ex => _logger?.LogDebug(ex, "In-process connection-state subscriber threw")); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs index 6283eb5d..36b64bec 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs @@ -1,4 +1,5 @@ using System.Collections.Concurrent; +using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; @@ -10,6 +11,12 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; public sealed class InMemoryDriverResilienceStatusStore : IDriverResilienceStatusStore { private readonly ConcurrentDictionary<(string Instance, string Host), DriverResilienceStatusChanged> _byPair = new(); + private readonly ILogger? _logger; + + /// Creates the store. + /// Optional logger for a subscriber that throws during a raise. + public InMemoryDriverResilienceStatusStore(ILogger? logger = null) + => _logger = logger; /// Raised after every with the just-stored snapshot. public event Action? SnapshotChanged; @@ -18,8 +25,22 @@ public sealed class InMemoryDriverResilienceStatusStore : IDriverResilienceStatu public void Upsert(DriverResilienceStatusChanged snapshot) { _byPair[(snapshot.DriverInstanceId, snapshot.HostName)] = snapshot; - // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise. - SnapshotChanged?.Invoke(snapshot); + // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise; one + // subscriber at a time so a throwing one cannot starve the rest (or escape onto the bridge actor). + SafeMulticast.Invoke(SnapshotChanged, snapshot, + ex => _logger?.LogDebug(ex, "Driver resilience subscriber threw for {DriverInstanceId}", snapshot.DriverInstanceId)); + } + + /// + public int RetainOnly(IReadOnlySet configuredInstanceIds) + { + var removed = 0; + foreach (var key in _byPair.Keys) + { + if (!configuredInstanceIds.Contains(key.Instance) && _byPair.TryRemove(key, out _)) removed++; + } + + return removed; } /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs index 3a74e6be..21089cef 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs @@ -1,4 +1,5 @@ using System.Collections.Concurrent; +using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; @@ -10,6 +11,12 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; public sealed class InMemoryDriverStatusSnapshotStore : IDriverStatusSnapshotStore { private readonly ConcurrentDictionary _byInstance = new(); + private readonly ILogger? _logger; + + /// Creates the store. + /// Optional logger for a subscriber that throws during a raise. + public InMemoryDriverStatusSnapshotStore(ILogger? logger = null) + => _logger = logger; /// Raised whenever a driver status snapshot is stored or updated. public event Action? SnapshotChanged; @@ -18,8 +25,22 @@ public sealed class InMemoryDriverStatusSnapshotStore : IDriverStatusSnapshotSto public void Upsert(DriverHealthChanged snapshot) { _byInstance[snapshot.DriverInstanceId] = snapshot; - // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise. - SnapshotChanged?.Invoke(snapshot); + // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise; one + // subscriber at a time so a throwing one cannot starve the rest (or escape onto the bridge actor). + SafeMulticast.Invoke(SnapshotChanged, snapshot, + ex => _logger?.LogDebug(ex, "Driver status subscriber threw for {DriverInstanceId}", snapshot.DriverInstanceId)); + } + + /// + public int RetainOnly(IReadOnlySet configuredInstanceIds) + { + var removed = 0; + foreach (var id in _byInstance.Keys) + { + if (!configuredInstanceIds.Contains(id) && _byInstance.TryRemove(id, out _)) removed++; + } + + return removed; } /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs new file mode 100644 index 00000000..610f5d4c --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs @@ -0,0 +1,39 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; + +/// +/// Raises a multicast delegate one subscriber at a time, so a subscriber that throws cannot stop +/// delivery to the subscribers after it. The in-process stores and broadcasters raise their events on +/// the bridge actor's thread; a plain handler?.Invoke(x) stops at the first throwing handler (for +/// example a Blazor circuit whose renderer is mid-teardown), so the remaining panels miss the update and +/// the exception escapes into the bridge actor, which restarts it. +/// +internal static class SafeMulticast +{ + /// Invokes every subscriber of with , isolating failures. + /// The event payload type. + /// The captured multicast delegate; null means no subscribers. + /// The payload passed to each subscriber. + /// Optional sink for a subscriber's exception; it must not throw. + /// The number of subscribers that threw. + public static int Invoke(Action? handler, T arg, Action? onError = null) + { + if (handler is null) return 0; + + var failures = 0; + foreach (var subscriber in handler.GetInvocationList()) + { + try + { + ((Action)subscriber)(arg); + } + catch (Exception ex) + { + failures++; + try { onError?.Invoke(ex); } + catch { /* the error sink must never take delivery down with it */ } + } + } + + return failures; + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogSignalRBridge.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogSignalRBridge.cs index 650465ba..ac849446 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogSignalRBridge.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogSignalRBridge.cs @@ -52,10 +52,11 @@ public sealed class ScriptLogSignalRBridge : ReceiveActor private async Task ForwardAsync(ScriptLogEntry msg) { // In-process fan-out first — this is what the Blazor Server Script log page reads. The hub - // push is kept for any out-of-process (e.g. WASM) SignalR client. - _broadcaster.Publish(msg); + // push is kept for any out-of-process (e.g. WASM) SignalR client. Both sit inside the try so a + // broadcast failure cannot restart the bridge (and drop its DPS subscription). try { + _broadcaster.Publish(msg); await _hub.Clients.All.SendAsync(ScriptLogHub.MethodName, msg); } catch (Exception ex) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AdminUiSource.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AdminUiSource.cs new file mode 100644 index 00000000..e78c5785 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AdminUiSource.cs @@ -0,0 +1,25 @@ +using Shouldly; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests; + +/// +/// Reads an AdminUI source file for a source-scan pin. The repo has no bUnit, so Razor wiring that a +/// unit test cannot render is pinned by scanning the markup; it proves the wiring exists, not that it +/// renders correctly (the docker-dev live pass covers that). +/// +internal static class AdminUiSource +{ + /// Reads a file under src/Server/ZB.MOM.WW.OtOpcUa.AdminUI. + /// Path segments below the AdminUI project directory. + /// The file text. + public static string Read(params string[] relativeSegments) + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx"))) + dir = dir.Parent; + dir.ShouldNotBeNull("could not locate the repository root (no ZB.MOM.WW.OtOpcUa.slnx above the test binary)"); + + var parts = new[] { dir!.FullName, "src", "Server", "ZB.MOM.WW.OtOpcUa.AdminUI" }.Concat(relativeSegments).ToArray(); + return File.ReadAllText(Path.Combine(parts)); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs new file mode 100644 index 00000000..4cc857b3 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs @@ -0,0 +1,139 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Hubs; + +/// +/// Review R04 L1 + L5: the in-process stores evict driver instances that are no longer configured +/// (so /hosts stops rendering ghost rows), and a throwing subscriber no longer aborts the +/// multicast for the subscribers after it (nor escapes onto the bridge actor). +/// +public sealed class StoreEvictionAndIsolationTests +{ + private static DriverHealthChanged Health(string instance) => + new("MAIN", instance, "Healthy", null, null, 0, new DateTime(2026, 10, 22, 0, 0, 0, DateTimeKind.Utc)); + + private static DriverResilienceStatusChanged Res(string instance, string host) => + new(instance, host, false, 0, 0, null, + new DateTime(2026, 10, 22, 0, 0, 0, DateTimeKind.Utc), + new DateTime(2026, 10, 22, 0, 0, 0, DateTimeKind.Utc)); + + [Fact] + public void HealthStore_RetainOnly_evicts_unconfigured_instances_and_keeps_configured_ones() + { + var store = new InMemoryDriverStatusSnapshotStore(); + store.Upsert(Health("drv-live")); + store.Upsert(Health("drv-deleted")); + + var removed = store.RetainOnly(new HashSet(["drv-live"], StringComparer.OrdinalIgnoreCase)); + + removed.ShouldBe(1); + store.TryGet("drv-deleted", out _).ShouldBeFalse(); + // Positive control: the configured instance survives the sweep. + store.TryGet("drv-live", out _).ShouldBeTrue(); + store.GetAll().Select(s => s.DriverInstanceId).ShouldBe(["drv-live"]); + } + + [Fact] + public void HealthStore_RetainOnly_matches_ids_case_insensitively() + { + var store = new InMemoryDriverStatusSnapshotStore(); + store.Upsert(Health("DRV-Live")); + + store.RetainOnly(new HashSet(["drv-live"], StringComparer.OrdinalIgnoreCase)).ShouldBe(0); + store.TryGet("DRV-Live", out _).ShouldBeTrue(); + } + + [Fact] + public void ResilienceStore_RetainOnly_evicts_every_host_of_an_unconfigured_instance() + { + var store = new InMemoryDriverResilienceStatusStore(); + store.Upsert(Res("drv-live", "h1")); + store.Upsert(Res("drv-gone", "h1")); + store.Upsert(Res("drv-gone", "h2")); + + var removed = store.RetainOnly(new HashSet(["drv-live"], StringComparer.OrdinalIgnoreCase)); + + removed.ShouldBe(2); + store.GetForInstance("drv-gone").ShouldBeEmpty(); + store.GetForInstance("drv-live").Count.ShouldBe(1); + } + + [Fact] + public void HealthStore_throwing_subscriber_does_not_stop_later_subscribers() + { + var store = new InMemoryDriverStatusSnapshotStore(); + var received = new List(); + store.SnapshotChanged += _ => throw new InvalidOperationException("renderer torn down"); + store.SnapshotChanged += s => received.Add(s.DriverInstanceId); + + Should.NotThrow(() => store.Upsert(Health("drv-1"))); + + received.ShouldBe(["drv-1"]); + store.TryGet("drv-1", out _).ShouldBeTrue(); + } + + [Fact] + public void ResilienceStore_throwing_subscriber_does_not_stop_later_subscribers() + { + var store = new InMemoryDriverResilienceStatusStore(); + var received = 0; + store.SnapshotChanged += _ => throw new InvalidOperationException("boom"); + store.SnapshotChanged += _ => received++; + + Should.NotThrow(() => store.Upsert(Res("drv-1", "h1"))); + + received.ShouldBe(1); + } + + [Fact] + public void Broadcaster_throwing_subscriber_does_not_stop_later_subscribers() + { + var broadcaster = new InProcessBroadcaster(); + var received = new List(); + broadcaster.Received += _ => throw new InvalidOperationException("boom"); + broadcaster.Received += received.Add; + + Should.NotThrow(() => broadcaster.Publish("evt")); + + received.ShouldBe(["evt"]); + } + + [Fact] + public void Broadcaster_throwing_connection_subscriber_does_not_stop_later_subscribers() + { + var broadcaster = new InProcessBroadcaster(); + var raised = new List(); + broadcaster.ConnectionStateChanged += _ => throw new InvalidOperationException("boom"); + broadcaster.ConnectionStateChanged += raised.Add; + + Should.NotThrow(() => broadcaster.SetConnected(true)); + + raised.ShouldBe([true]); + broadcaster.IsConnected.ShouldBeTrue(); + } +} + +/// +/// Source pin (no bUnit): /hosts sweeps both stores after a successful config load, and the +/// sweep sits before the failure catch, so a failed load (which blanks the config) never evicts. +/// +[Trait("Category", "Unit")] +public sealed class HostsEvictionWiringTests +{ + [Fact] + public void Hosts_page_evicts_both_stores_inside_the_successful_load_only() + { + var src = AdminUiSource.Read("Components", "Pages", "Hosts.razor"); + + var load = src.IndexOf("private async Task LoadConfigAsync()", StringComparison.Ordinal); + load.ShouldBeGreaterThan(0); + var firstCatch = src.IndexOf("catch (Exception ex) when (ex is OperationCanceledException", load, StringComparison.Ordinal); + firstCatch.ShouldBeGreaterThan(load); + + src.IndexOf("DriverStore.RetainOnly(", load, StringComparison.Ordinal).ShouldBeInRange(load, firstCatch); + src.IndexOf("ResilienceStore.RetainOnly(", load, StringComparison.Ordinal).ShouldBeInRange(load, firstCatch); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialDeadlineTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialDeadlineTests.cs index d19506ca..ecb952dc 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialDeadlineTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialDeadlineTests.cs @@ -141,6 +141,7 @@ public sealed class TelemetryDialDeadlineTests : TestKit public void Upsert(DriverHealthChanged snapshot) { Upserts.Enqueue(snapshot); SnapshotChanged?.Invoke(snapshot); } public bool TryGet(string driverInstanceId, out DriverHealthChanged snapshot) { snapshot = null!; return false; } public IReadOnlyCollection GetAll() => Upserts.ToArray(); + public int RetainOnly(IReadOnlySet configuredInstanceIds) => 0; } private sealed class NullResilienceStore : IDriverResilienceStatusStore @@ -149,5 +150,6 @@ public sealed class TelemetryDialDeadlineTests : TestKit public void Upsert(DriverResilienceStatusChanged snapshot) => SnapshotChanged?.Invoke(snapshot); public IReadOnlyList GetForInstance(string driverInstanceId) => Array.Empty(); public IReadOnlyCollection GetAll() => Array.Empty(); + public int RetainOnly(IReadOnlySet configuredInstanceIds) => 0; } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialSupervisorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialSupervisorTests.cs index 4de5f16d..b0bf0b9a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialSupervisorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialSupervisorTests.cs @@ -438,6 +438,7 @@ public sealed class TelemetryDialSupervisorTests : TestKit } public IReadOnlyCollection GetAll() => Upserts.ToArray(); + public int RetainOnly(IReadOnlySet configuredInstanceIds) => 0; } private sealed class RecordingResilienceStore : IDriverResilienceStatusStore @@ -456,6 +457,7 @@ public sealed class TelemetryDialSupervisorTests : TestKit Upserts.ToArray(); public IReadOnlyCollection GetAll() => Upserts.ToArray(); + public int RetainOnly(IReadOnlySet configuredInstanceIds) => 0; } private sealed class InMemoryFactory(string name) : IDbContextFactory From 348ffaec1cd6ff97accfe01ab6e069a34b8e5dc0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:28:23 -0400 Subject: [PATCH 02/20] perf(adminui): /hosts rebuilds driver rows on the tick, not per health publish (R04 P4) OnSnapshotChanged used to re-project every cluster group and re-render the whole page for each driver publish, per open tab. It now only marks a CoalescingDirtyFlag; the 5 s tick rebuilds when the flag is set or the config load changed (or evicted) anything. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Components/Pages/Hosts.razor | 35 ++++++++---- .../Hosts/CoalescingDirtyFlag.cs | 20 +++++++ .../Hosts/HostsRenderCoalescingTests.cs | 53 +++++++++++++++++++ .../Hubs/StoreEvictionAndIsolationTests.cs | 2 +- 4 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/CoalescingDirtyFlag.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsRenderCoalescingTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor index d4a1d6a3..67c9be56 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor @@ -347,6 +347,9 @@ else // next one. A manual Refresh landing on top of an in-flight tick is a no-op by the same guard. private int _tickRunning; + // Set by OnSnapshotChanged (bridge actor thread), consumed by TickAsync. + private readonly CoalescingDirtyFlag _driversDirty = new(); + // Page-lifetime cancellation: cancelled in DisposeAsync() so a tick in flight when the circuit // goes away aborts its queries instead of running them against a component that's gone. private readonly CancellationTokenSource _cts = new(); @@ -372,8 +375,11 @@ else try { Refresh(); - await LoadConfigAsync(); - RebuildDriverGroups(); + var configChanged = await LoadConfigAsync(); + // Re-project the driver rows only when a health snapshot or the config actually changed + // since the last tick (R04 P4). The snapshot handler just marks the flag, so N drivers + // publishing every 5 s cost one rebuild per tick per tab, not N rebuilds + N full renders. + if (_driversDirty.TryConsume() | configChanged) RebuildDriverGroups(); StateHasChanged(); } catch (Exception ex) when (ex is ObjectDisposedException or OperationCanceledException) @@ -397,6 +403,8 @@ else await Task.Yield(); Refresh(); await LoadConfigAsync(); + // A manual refresh always rebuilds; clear the flag so the next tick does not rebuild again. + _driversDirty.TryConsume(); RebuildDriverGroups(); } catch (Exception ex) when (ex is ObjectDisposedException or OperationCanceledException) @@ -418,8 +426,11 @@ else // Load the per-cluster node + driver-instance config from the ConfigDB. Kept cheap and // swallow-on-failure so a transient DB hiccup dims the enrichment (rows fall back to id/—) // without crashing the page. The live health feed (the snapshot store) is independent of this. - private async Task LoadConfigAsync() + // Returns whether the loaded config differs from the previous load (records compare by value). + private async Task LoadConfigAsync() { + var previousNodes = _nodes; + var previousInstances = _instances; try { await using var db = await DbFactory.CreateDbContextAsync(_cts.Token); @@ -437,8 +448,12 @@ else var configured = _instances .Select(i => i.DriverInstanceId) .ToHashSet(StringComparer.OrdinalIgnoreCase); - DriverStore.RetainOnly(configured); + var evicted = DriverStore.RetainOnly(configured); ResilienceStore.RetainOnly(configured); + + return evicted > 0 + || !_nodes.SequenceEqual(previousNodes) + || !_instances.SequenceEqual(previousInstances); } catch (Exception ex) when (ex is OperationCanceledException or ObjectDisposedException) { @@ -456,6 +471,7 @@ else Logger.LogWarning(ex, "/hosts: failed to load driver-instance config; rows show ids only."); _nodes = new(); _instances = new(); + return true; } } @@ -483,11 +499,12 @@ else return (null, null); } - // Raised by the snapshot store on the bridge actor's thread for every driver instance. Rebuild - // from the cached config (cheap — no DB hit) and marshal onto the render sync context. A - // brand-new driver shows by id until the next config reload (timer/manual Refresh) enriches it. - private void OnSnapshotChanged(DriverHealthChanged _) - => InvokeAsync(() => { RebuildDriverGroups(); StateHasChanged(); }); + // Raised by the snapshot store on the bridge actor's thread for every driver instance. It only + // marks the rows dirty: the 5 s tick does the rebuild + render (R04 P4). Rendering here cost one + // full re-projection and full-page diff per driver publish per open tab (200 drivers ≈ 40/s). + // A health change therefore shows within one tick; the open Controls dialog's DriverStatusPanel + // subscribes to the store itself and stays live. + private void OnSnapshotChanged(DriverHealthChanged _) => _driversDirty.Mark(); private void Refresh() { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/CoalescingDirtyFlag.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/CoalescingDirtyFlag.cs new file mode 100644 index 00000000..3f5e538b --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/CoalescingDirtyFlag.cs @@ -0,0 +1,20 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hosts; + +/// +/// A thread-safe "something changed since you last looked" flag. A high-rate producer (the driver +/// health store raises once per driver per publish, on the bridge actor's thread) calls +/// , which is cheap and never renders; the page's own periodic tick calls +/// and does the expensive re-projection + render at most once per tick, +/// however many marks arrived in between. +/// +public sealed class CoalescingDirtyFlag +{ + private int _dirty; + + /// Records that the underlying data changed. Safe from any thread. + public void Mark() => Volatile.Write(ref _dirty, 1); + + /// Returns whether anything was marked since the last call, and clears the flag. + /// when at least one happened since the last consume. + public bool TryConsume() => Interlocked.Exchange(ref _dirty, 0) == 1; +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsRenderCoalescingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsRenderCoalescingTests.cs new file mode 100644 index 00000000..7583d508 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsRenderCoalescingTests.cs @@ -0,0 +1,53 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Hosts; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Hosts; + +/// +/// R04 P4: a driver health publish only marks /hosts dirty; the page's 5 s tick rebuilds and +/// renders at most once, however many publishes arrived. +/// +[Trait("Category", "Unit")] +public sealed class HostsRenderCoalescingTests +{ + [Fact] + public void Many_marks_coalesce_into_one_consume() + { + var flag = new CoalescingDirtyFlag(); + for (var i = 0; i < 200; i++) flag.Mark(); + + flag.TryConsume().ShouldBeTrue(); + flag.TryConsume().ShouldBeFalse(); + } + + [Fact] + public void A_fresh_flag_is_clean_and_a_mark_after_consume_is_seen() + { + var flag = new CoalescingDirtyFlag(); + flag.TryConsume().ShouldBeFalse(); + + // Positive control: the flag is not stuck clean. + flag.Mark(); + flag.TryConsume().ShouldBeTrue(); + } + + [Fact] + public void Concurrent_marks_are_never_lost() + { + var flag = new CoalescingDirtyFlag(); + Parallel.For(0, 1000, _ => flag.Mark()); + + flag.TryConsume().ShouldBeTrue(); + } + + /// Source pin (no bUnit): the snapshot handler must not render or rebuild. + [Fact] + public void Snapshot_handler_only_marks_and_the_tick_consumes() + { + var src = AdminUiSource.Read("Components", "Pages", "Hosts.razor"); + + src.ShouldContain("private void OnSnapshotChanged(DriverHealthChanged _) => _driversDirty.Mark();"); + src.ShouldContain("if (_driversDirty.TryConsume() | configChanged) RebuildDriverGroups();"); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs index 4cc857b3..411eeb20 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs @@ -128,7 +128,7 @@ public sealed class HostsEvictionWiringTests { var src = AdminUiSource.Read("Components", "Pages", "Hosts.razor"); - var load = src.IndexOf("private async Task LoadConfigAsync()", StringComparison.Ordinal); + var load = src.IndexOf("private async Task LoadConfigAsync()", StringComparison.Ordinal); load.ShouldBeGreaterThan(0); var firstCatch = src.IndexOf("catch (Exception ex) when (ex is OperationCanceledException", load, StringComparison.Ordinal); firstCatch.ShouldBeGreaterThan(load); From 8e69118200ee198dee658ff03e23eae3f1722a31 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:28:23 -0400 Subject: [PATCH 03/20] perf(adminui): coalesce /script-log and /alerts renders with a ring buffer (R04 P7) Each received event used to Insert(0) into a List and render the full page. LiveTailBuffer queues events from the bridge thread into a bounded newest-first ring; CoalescingRenderScheduler (TimeProvider-driven, drained on dispose) flushes and renders at most once per 200 ms window. ScriptLog's filtered rows are recomputed on flush/filter change instead of three times per render. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Components/Pages/Alerts.razor | 37 ++-- .../Components/Pages/ScriptLog.razor | 78 ++++---- .../LiveTail/CoalescingRenderScheduler.cs | 79 ++++++++ .../LiveTail/LiveTailBuffer.cs | 94 +++++++++ .../LiveTail/LiveTailTests.cs | 185 ++++++++++++++++++ 5 files changed, 426 insertions(+), 47 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/CoalescingRenderScheduler.cs create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/LiveTailBuffer.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/LiveTail/LiveTailTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Alerts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Alerts.razor index 7cee1860..3ead3ee0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Alerts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Alerts.razor @@ -12,6 +12,7 @@ @inject AuthenticationStateProvider AuthState @inject IAuthorizationService AuthorizationService @inject IAdminOperationsClient AdminOps +@using ZB.MOM.WW.OtOpcUa.AdminUI.LiveTail @implements IAsyncDisposable @@ -29,7 +30,7 @@ Shows the most-recent @Capacity entries since the page opened; reload for a fresh window. -@if (_rows.Count == 0) +@if (_tail.Items.Count == 0) {
No alarms in the current window. The table will populate as soon as a @@ -39,7 +40,7 @@ else {
-
Recent transitions (@_rows.Count)
+
Recent transitions (@_tail.Items.Count)
@@ -58,7 +59,7 @@ else - @foreach (var e in _rows) + @foreach (var e in _tail.Items) { @@ -137,7 +138,9 @@ else @code { private const int Capacity = 200; - private readonly List _rows = new(); + // Newest-first ring, filled from the bridge thread and drained once per coalescing window (R04 P7). + private readonly LiveTailBuffer _tail = new(Capacity); + private CoalescingRenderScheduler? _render; private bool _connected; // Authorization — DriverOperator gates the per-row Ack/Shelve/Unshelve controls. @@ -172,6 +175,8 @@ else // Subscribe to the health signal BEFORE reading IsConnected so a state change between the // read and the subscribe can't be missed (TOCTOU) — mirrors DriverStatusPanel's // SnapshotChanged-then-TryGet ordering. + _render = new CoalescingRenderScheduler(CoalescingRenderScheduler.DefaultWindow, + () => InvokeAsync(FlushTail)); Alarms.ConnectionStateChanged += OnConnectionStateChanged; Alarms.Received += OnAlarm; _connected = Alarms.IsConnected; @@ -275,15 +280,18 @@ else null, TimeSpan.FromSeconds(8), Timeout.InfiniteTimeSpan); } - private void OnAlarm(AlarmTransitionEvent evt) => - // Marshal both the mutation and the re-render onto the circuit sync context so this can't - // race ClearAsync (which runs there) over the shared _rows list. - InvokeAsync(() => - { - _rows.Insert(0, evt); - if (_rows.Count > Capacity) _rows.RemoveAt(_rows.Count - 1); - StateHasChanged(); - }); + // Bridge-actor thread: queue only. The first transition of a window arms one render ~200 ms + // later, so an alarm storm costs one render per window per tab instead of one per transition. + private void OnAlarm(AlarmTransitionEvent evt) + { + if (_tail.Add(evt)) _render?.Schedule(); + } + + // Render context: drain the queue into the ring and render once. + private void FlushTail() + { + if (_tail.Flush()) StateHasChanged(); + } // Raised by the broadcaster (on the bridge actor's thread) whenever the upstream DPS // subscription's health flips. Marshal onto the circuit sync context like OnAlarm so the live @@ -293,7 +301,7 @@ else private async Task ClearAsync() { - _rows.Clear(); + _tail.Clear(); await InvokeAsync(StateHasChanged); } @@ -314,5 +322,6 @@ else Alarms.ConnectionStateChanged -= OnConnectionStateChanged; Alarms.Received -= OnAlarm; if (_opResultClearTimer is not null) await _opResultClearTimer.DisposeAsync(); + if (_render is not null) await _render.DisposeAsync(); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptLog.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptLog.razor index ff2893a1..65dd7551 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptLog.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptLog.razor @@ -6,13 +6,14 @@ @using ZB.MOM.WW.OtOpcUa.AdminUI.Hubs @using ZB.MOM.WW.OtOpcUa.Commons.Messages.Logging @inject IInProcessBroadcaster ScriptLogs -@implements IDisposable +@using ZB.MOM.WW.OtOpcUa.AdminUI.LiveTail +@implements IAsyncDisposable

Script log

- @@ -21,7 +22,7 @@ + placeholder="Filter script ID…" @bind="_scriptFilter" @bind:event="oninput" @bind:after="RecomputeVisible" /> @(_connected ? "live" : "disconnected") @@ -29,23 +30,23 @@
-@if (VisibleRows.Count == 0) +@if (_visible.Count == 0) {
- @if (_rows.Count == 0) + @if (_tail.Items.Count == 0) { No script-log entries yet. } else { - No entries match the current filter (@_rows.Count entries available). + No entries match the current filter (@_tail.Items.Count entries available). }
} else {
-
Showing @VisibleRows.Count of @_rows.Count
+
Showing @_visible.Count of @_tail.Items.Count
@e.TimestampUtc.ToString("HH:mm:ss.fff")
@@ -58,7 +59,7 @@ else - @foreach (var e in VisibleRows) + @foreach (var e in _visible) { @@ -82,7 +83,11 @@ else @code { private const int Capacity = 500; - private readonly List _rows = new(); + // Newest-first ring, filled from the bridge thread and drained once per coalescing window (R04 P7). + private readonly LiveTailBuffer _tail = new(Capacity); + private CoalescingRenderScheduler? _render; + // The filtered rows, recomputed on flush / filter change — not on every render. + private IReadOnlyList _visible = Array.Empty(); private bool _connected; private string _levelFilter = ""; private string _scriptFilter = ""; @@ -92,22 +97,19 @@ else ["Trace"] = 0, ["Debug"] = 1, ["Information"] = 2, ["Warning"] = 3, ["Error"] = 4, ["Critical"] = 5, }; - private List VisibleRows + private void RecomputeVisible() { - get + IEnumerable q = _tail.Items; + if (!string.IsNullOrWhiteSpace(_levelFilter) + && LevelRank.TryGetValue(_levelFilter, out var minRank)) { - IEnumerable q = _rows; - if (!string.IsNullOrWhiteSpace(_levelFilter) - && LevelRank.TryGetValue(_levelFilter, out var minRank)) - { - q = q.Where(e => LevelRank.TryGetValue(e.Level, out var r) && r >= minRank); - } - if (!string.IsNullOrWhiteSpace(_scriptFilter)) - { - q = q.Where(e => e.ScriptId.Contains(_scriptFilter, StringComparison.OrdinalIgnoreCase)); - } - return q.ToList(); + q = q.Where(e => LevelRank.TryGetValue(e.Level, out var r) && r >= minRank); } + if (!string.IsNullOrWhiteSpace(_scriptFilter)) + { + q = q.Where(e => e.ScriptId.Contains(_scriptFilter, StringComparison.OrdinalIgnoreCase)); + } + _visible = q.ToList(); } protected override void OnInitialized() @@ -116,6 +118,8 @@ else // 'script-logs' DPS topic). Blazor Server can't self-connect a SignalR HubConnection behind // a reverse proxy — see IInProcessBroadcaster — so we subscribe in-process instead. // Subscribe the state-change event BEFORE reading IsConnected so no transition is missed. + _render = new CoalescingRenderScheduler(CoalescingRenderScheduler.DefaultWindow, + () => InvokeAsync(FlushTail)); ScriptLogs.ConnectionStateChanged += OnConnectionStateChanged; ScriptLogs.Received += OnEntry; _connected = ScriptLogs.IsConnected; @@ -124,19 +128,25 @@ else private void OnConnectionStateChanged(bool connected) => InvokeAsync(() => { _connected = connected; StateHasChanged(); }); - private void OnEntry(ScriptLogEntry entry) => - // Marshal both the mutation and the re-render onto the circuit sync context so this can't - // race ClearAsync (which runs there) over the shared _rows list. - InvokeAsync(() => - { - _rows.Insert(0, entry); - if (_rows.Count > Capacity) _rows.RemoveAt(_rows.Count - 1); - StateHasChanged(); - }); + // Bridge-actor thread: queue only. The first entry of a window arms one render ~200 ms later, so + // a chatty script costs one render per window per tab instead of one per log line. + private void OnEntry(ScriptLogEntry entry) + { + if (_tail.Add(entry)) _render?.Schedule(); + } + + // Render context: drain the queue into the ring and render once. + private void FlushTail() + { + if (!_tail.Flush()) return; + RecomputeVisible(); + StateHasChanged(); + } private async Task ClearAsync() { - _rows.Clear(); + _tail.Clear(); + RecomputeVisible(); await InvokeAsync(StateHasChanged); } @@ -148,9 +158,11 @@ else _ => "chip-idle", }; - public void Dispose() + public async ValueTask DisposeAsync() { ScriptLogs.ConnectionStateChanged -= OnConnectionStateChanged; ScriptLogs.Received -= OnEntry; + // Drains an in-flight window and suppresses later ones, so no flush renders a gone component. + if (_render is not null) await _render.DisposeAsync(); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/CoalescingRenderScheduler.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/CoalescingRenderScheduler.cs new file mode 100644 index 00000000..11ba73be --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/CoalescingRenderScheduler.cs @@ -0,0 +1,79 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.LiveTail; + +/// +/// Coalesces bursts of "please re-render" requests into at most one flush per window. The first +/// after a flush arms a one-shot timer; further calls inside the window are +/// no-ops; when the timer fires the flush callback runs once (the page marshals it onto its render +/// context with InvokeAsync). Disposal drains an in-flight callback and suppresses later ones. +/// +public sealed class CoalescingRenderScheduler : IAsyncDisposable +{ + /// The default coalescing window for live-tail pages. + public static readonly TimeSpan DefaultWindow = TimeSpan.FromMilliseconds(200); + + private readonly TimeSpan _window; + private readonly Func _flush; + private readonly ITimer _timer; + private int _armed; + private volatile bool _disposed; + + /// Creates the scheduler. + /// The coalescing window. + /// The callback run once per window; exceptions are swallowed (the page is the + /// only consumer, and a throw here would surface as an unobserved timer-thread exception). + /// The time source; when null. + public CoalescingRenderScheduler(TimeSpan window, Func flush, TimeProvider? timeProvider = null) + { + _window = window; + _flush = flush; + _timer = (timeProvider ?? TimeProvider.System) + .CreateTimer(OnFire, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); + } + + /// Requests a flush within one window. Safe from any thread. + public void Schedule() + { + if (_disposed) return; + if (Interlocked.CompareExchange(ref _armed, 1, 0) != 0) return; + try + { + _timer.Change(_window, Timeout.InfiniteTimeSpan); + } + catch (ObjectDisposedException) + { + // Raced DisposeAsync — nothing left to render to. + } + } + + private void OnFire(object? state) + { + // Disarm BEFORE flushing: an item arriving during the flush schedules the next window rather + // than being stranded until some later item happens to arrive. + Volatile.Write(ref _armed, 0); + if (_disposed) return; + + // Not async void: a fault is observed on the returned task, never thrown onto the timer thread. + // The circuit going away mid-flush (ObjectDisposedException and friends) is the expected fault, + // and a live-tail render failure is never worth an unhandled exception. + Task task; + try + { + task = _flush(); + } + catch + { + return; + } + + _ = task.ContinueWith(static t => _ = t.Exception, CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + + /// + public async ValueTask DisposeAsync() + { + _disposed = true; + await _timer.DisposeAsync().ConfigureAwait(false); + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/LiveTailBuffer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/LiveTailBuffer.cs new file mode 100644 index 00000000..961dd305 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/LiveTail/LiveTailBuffer.cs @@ -0,0 +1,94 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.LiveTail; + +/// +/// A bounded, newest-first buffer for a live event tail (the /script-log and /alerts +/// pages). Producers call from the bridge actor's thread; it only queues the item. +/// The page drains the queue into a fixed-capacity ring on its render context (), +/// once per coalescing window, so a chatty feed costs one render per window instead of one per event, +/// and no List.Insert(0, …) memmove per event. +/// +/// The event type. +public sealed class LiveTailBuffer +{ + private readonly object _gate = new(); + private readonly T[] _ring; + private readonly List _pending = new(); + private int _head; // index the next item is written to + private int _count; + private IReadOnlyList _snapshot = Array.Empty(); + + /// Creates a buffer that keeps the most recent items. + /// Maximum retained items; must be positive. + public LiveTailBuffer(int capacity) + { + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(capacity); + _ring = new T[capacity]; + } + + /// The maximum number of retained items. + public int Capacity => _ring.Length; + + /// Retained items, newest first, as of the last or . + public IReadOnlyList Items => Volatile.Read(ref _snapshot); + + /// Queues an item. Safe from any thread; does not change until the next flush. + /// The item to queue. + /// when the queue was empty before this call, i.e. the caller should + /// schedule a flush. + public bool Add(T item) + { + lock (_gate) + { + // A burst larger than the ring would be evicted at flush anyway; dropping the oldest pending + // now keeps the queue bounded when no flush runs (e.g. a stalled circuit). + if (_pending.Count >= _ring.Length) _pending.RemoveAt(0); + _pending.Add(item); + return _pending.Count == 1; + } + } + + /// Moves the queued items into the ring (evicting the oldest past capacity) and refreshes . + /// when anything was queued, i.e. a render is worthwhile. + public bool Flush() + { + lock (_gate) + { + if (_pending.Count == 0) return false; + foreach (var item in _pending) + { + _ring[_head] = item; + _head = (_head + 1) % _ring.Length; + if (_count < _ring.Length) _count++; + } + + _pending.Clear(); + Volatile.Write(ref _snapshot, BuildNewestFirst()); + return true; + } + } + + /// Drops every retained and queued item. + public void Clear() + { + lock (_gate) + { + _pending.Clear(); + Array.Clear(_ring); + _head = 0; + _count = 0; + Volatile.Write(ref _snapshot, Array.Empty()); + } + } + + private T[] BuildNewestFirst() + { + var result = new T[_count]; + for (var i = 0; i < _count; i++) + { + var idx = (_head - 1 - i + _ring.Length) % _ring.Length; + result[i] = _ring[idx]; + } + + return result; + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/LiveTail/LiveTailTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/LiveTail/LiveTailTests.cs new file mode 100644 index 00000000..8b075444 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/LiveTail/LiveTailTests.cs @@ -0,0 +1,185 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.LiveTail; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.LiveTail; + +/// +/// R04 P7: the live-tail pages coalesce event bursts into one render per window and keep their rows +/// in a bounded ring. All timing here is driven by a manual , so nothing +/// depends on wall-clock scheduling. +/// +[Trait("Category", "Unit")] +public sealed class LiveTailTests +{ + [Fact] + public void Buffer_items_are_newest_first_and_only_change_on_flush() + { + var buffer = new LiveTailBuffer(capacity: 3); + + buffer.Add(1).ShouldBeTrue(); // first pending ⇒ caller schedules + buffer.Add(2).ShouldBeFalse(); // already pending ⇒ no second schedule + buffer.Items.ShouldBeEmpty(); // not visible until flush + + buffer.Flush().ShouldBeTrue(); + buffer.Items.ShouldBe([2, 1]); + buffer.Flush().ShouldBeFalse(); // nothing queued ⇒ no render worth doing + } + + [Fact] + public void Buffer_evicts_the_oldest_past_capacity() + { + var buffer = new LiveTailBuffer(capacity: 3); + for (var i = 1; i <= 5; i++) buffer.Add(i); + buffer.Flush(); + buffer.Items.ShouldBe([5, 4, 3]); + + buffer.Add(6); + buffer.Flush(); + buffer.Items.ShouldBe([6, 5, 4]); + } + + [Fact] + public void Buffer_Clear_drops_retained_and_pending_items() + { + var buffer = new LiveTailBuffer(capacity: 3); + buffer.Add(1); + buffer.Flush(); + buffer.Add(2); + + buffer.Clear(); + + buffer.Items.ShouldBeEmpty(); + buffer.Flush().ShouldBeFalse(); + // Positive control: the buffer still works after a clear, and the next add re-arms scheduling. + buffer.Add(3).ShouldBeTrue(); + buffer.Flush(); + buffer.Items.ShouldBe([3]); + } + + [Fact] + public async Task Scheduler_runs_one_flush_per_window_for_a_burst() + { + var time = new ManualTimeProvider(); + var flushes = 0; + await using var scheduler = new CoalescingRenderScheduler( + TimeSpan.FromMilliseconds(200), () => { flushes++; return Task.CompletedTask; }, time); + + for (var i = 0; i < 100; i++) scheduler.Schedule(); + flushes.ShouldBe(0); + + time.Advance(TimeSpan.FromMilliseconds(199)); + flushes.ShouldBe(0); + time.Advance(TimeSpan.FromMilliseconds(1)); + flushes.ShouldBe(1); + + // The window re-arms after a fire: a later burst gets exactly one more flush. + scheduler.Schedule(); + scheduler.Schedule(); + time.Advance(TimeSpan.FromMilliseconds(200)); + flushes.ShouldBe(2); + } + + [Fact] + public async Task Scheduler_does_not_flush_after_dispose() + { + var time = new ManualTimeProvider(); + var flushes = 0; + var scheduler = new CoalescingRenderScheduler( + TimeSpan.FromMilliseconds(200), () => { flushes++; return Task.CompletedTask; }, time); + + // Positive control: the scheduler does flush before dispose. + scheduler.Schedule(); + time.Advance(TimeSpan.FromMilliseconds(200)); + flushes.ShouldBe(1); + + scheduler.Schedule(); + await scheduler.DisposeAsync(); + time.Advance(TimeSpan.FromMilliseconds(500)); + flushes.ShouldBe(1); + Should.NotThrow(scheduler.Schedule); + } + + [Fact] + public async Task Scheduler_swallows_a_faulting_flush_and_keeps_working() + { + var time = new ManualTimeProvider(); + var calls = 0; + await using var scheduler = new CoalescingRenderScheduler(TimeSpan.FromMilliseconds(200), () => + { + calls++; + if (calls == 1) throw new ObjectDisposedException("renderer"); + return Task.FromException(new InvalidOperationException("late")); + }, time); + + scheduler.Schedule(); + Should.NotThrow(() => time.Advance(TimeSpan.FromMilliseconds(200))); + scheduler.Schedule(); + Should.NotThrow(() => time.Advance(TimeSpan.FromMilliseconds(200))); + + calls.ShouldBe(2); + } + + [Fact] + public void Pages_use_the_coalescing_tail_instead_of_a_render_per_event() + { + foreach (var page in new[] { "ScriptLog.razor", "Alerts.razor" }) + { + var src = AdminUiSource.Read("Components", "Pages", page); + src.ShouldContain("CoalescingRenderScheduler", customMessage: page); + src.ShouldContain("LiveTailBuffer<", customMessage: page); + src.ShouldNotContain("_rows.Insert(0,", customMessage: page); + } + } + + /// A deterministic : timers fire only when passes their due time. + private sealed class ManualTimeProvider : TimeProvider + { + private readonly List _timers = new(); + private DateTimeOffset _now = new(2026, 10, 22, 0, 0, 0, TimeSpan.Zero); + + public override DateTimeOffset GetUtcNow() => _now; + + public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) + { + var timer = new ManualTimer(this, callback, state); + timer.Change(dueTime, period); + _timers.Add(timer); + return timer; + } + + public void Advance(TimeSpan by) + { + _now += by; + foreach (var t in _timers.ToArray()) t.FireIfDue(_now); + } + + private sealed class ManualTimer(ManualTimeProvider owner, TimerCallback callback, object? state) : ITimer + { + private DateTimeOffset? _due; + private bool _disposed; + + public bool Change(TimeSpan dueTime, TimeSpan period) + { + if (_disposed) throw new ObjectDisposedException(nameof(ManualTimer)); + _due = dueTime == Timeout.InfiniteTimeSpan ? null : owner._now + dueTime; + return true; + } + + public void FireIfDue(DateTimeOffset now) + { + if (_disposed || _due is null || _due > now) return; + _due = null; + callback(state); + } + + public void Dispose() => _disposed = true; + + public ValueTask DisposeAsync() + { + _disposed = true; + return ValueTask.CompletedTask; + } + } + } +} From 10b83d69c5f9d422fad26fa95023dce697fb3949 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:30:50 -0400 Subject: [PATCH 04/20] perf(adminui): /scripts lists a projection and loads source per dialog (R04 P8) The list loaded every script entity including SourceCode to render names and hashes. ScriptListQueries projects {ScriptId, Name, Language, SourceHash} for the list and fetches one script's source when its Source dialog opens, with a generation guard against a superseded open. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Components/Pages/Scripts.razor | 49 +++++++++++---- .../ScriptList/ScriptListQueries.cs | 41 +++++++++++++ .../ScriptList/ScriptListQueriesTests.cs | 59 +++++++++++++++++++ 3 files changed, 138 insertions(+), 11 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptList/ScriptListQueries.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptList/ScriptListQueriesTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor index a946dfec..219b5eaa 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor @@ -3,7 +3,7 @@ @rendermode RenderMode.InteractiveServer @using Microsoft.EntityFrameworkCore @using ZB.MOM.WW.OtOpcUa.Configuration -@using ZB.MOM.WW.OtOpcUa.Configuration.Entities +@using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptList @inject IDbContextFactory DbFactory @@ -89,13 +89,17 @@ else
- @if (src is not null) + @if (src is null || (_sourceLoaded && _sourceCode is null)) { - 
@src.SourceCode
+

This script is no longer loaded.

+ } + else if (!_sourceLoaded) + { +

Loading source…

} else { -

This script is no longer loaded.

+ 
@_sourceCode
}
@@ -108,30 +112,53 @@ else @code { - private List
@e.TimestampUtc.ToString("HH:mm:ss.fff")