From f1db002b87ea4f433288a575839948d7d273b37a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:13:05 -0400 Subject: [PATCH 1/8] fix(dashboard): bound EventsHub delivery with one drop-oldest pump per session Publish fired a group SendAsync per event and returned, so a slow remote viewer accumulated unbounded pending sends plus cloned MxEvents. Hub delivery now goes through a 256-deep DropOldest channel per hub-watched session with a single drain task and time-bounded sends. The registry counts hub viewers separately and raises HubViewersReleased so the pump is torn down with the last remote viewer; in-process-only sessions start no pump. --- .../Dashboard/Hubs/BoundedHubSend.cs | 52 +++ .../Hubs/DashboardEventBroadcaster.cs | 307 +++++++++++++++--- .../Dashboard/Hubs/EventsHubViewerRegistry.cs | 76 ++++- .../DashboardEventBroadcasterTests.cs | 273 +++++++++++++--- .../Dashboard/EventsHubViewerRegistryTests.cs | 44 +++ 5 files changed, 638 insertions(+), 114 deletions(-) create mode 100644 src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/BoundedHubSend.cs diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/BoundedHubSend.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/BoundedHubSend.cs new file mode 100644 index 0000000..4d14f37 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/BoundedHubSend.cs @@ -0,0 +1,52 @@ +using Microsoft.AspNetCore.SignalR; + +namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; + +/// +/// Time-bounded SignalR send shared by the dashboard hub publishers. A group or +/// broadcast send completes only once every target connection has accepted the +/// frame, so one remote viewer with a full transport buffer would otherwise hold +/// the publisher's single drain loop for as long as that viewer stays slow. +/// +internal static class BoundedHubSend +{ + /// + /// Sends as and waits at most + /// for it. The timeout cancels the send's own token, so + /// the lifetime manager abandons the pending write to the slow connection, and the + /// wait itself is bounded too, so a proxy that ignores the token cannot stall the + /// caller either. + /// + /// Target client proxy (a group or all connections). + /// Client method name. + /// The single message argument. + /// Upper bound on the send. + /// Host shutdown token; its cancellation propagates. + /// + /// when the send completed in time; + /// when it was abandoned at the timeout. + /// + /// was cancelled. + public static async Task SendAsync( + IClientProxy proxy, + string method, + object argument, + TimeSpan timeout, + CancellationToken stoppingToken) + { + using CancellationTokenSource bounded = CancellationTokenSource.CreateLinkedTokenSource(stoppingToken); + bounded.CancelAfter(timeout); + + try + { + await proxy.SendAsync(method, argument, bounded.Token) + .WaitAsync(bounded.Token) + .ConfigureAwait(false); + return true; + } + catch (OperationCanceledException) when (!stoppingToken.IsCancellationRequested) + { + return false; + } + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs index 909e7cc..5789d31 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs @@ -10,10 +10,11 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; /// Broadcasts MxEvents to the two dashboard audiences for a session: remote /// clients subscribed to the session's group, and /// in-process subscribers opened through -/// . Fire-and-forget: we -/// hand the send to the hub context and return immediately so the source gRPC -/// stream is never blocked. Errors are logged once and dropped — keeping the -/// SignalR mirror best-effort preserves the gRPC contract that exists today. +/// . +/// never blocks: both audiences are fed through bounded drop-oldest queues, so the +/// source gRPC stream is never held up by a slow dashboard viewer. Errors are +/// logged and dropped — keeping the SignalR mirror best-effort preserves the gRPC +/// contract. /// /// /// When MxGateway:Dashboard:ShowTagValues is false (the default), tag @@ -26,31 +27,45 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; /// ), while the redaction decides what a permitted /// subscriber sees — so the value-leak seam stays closed whatever the ACL /// admits. +/// +/// Hub delivery goes through one per watched session: a +/// -deep drop-oldest channel drained by a single task +/// that sends one event at a time, each send time-bounded. An earlier version fired +/// a group send per event and returned, so a slow remote viewer accumulated an +/// unbounded number of pending sends — each holding a cloned event — inside the +/// SignalR lifetime manager. The pump is started by the first event published while +/// a hub connection watches the session and torn down when +/// reports the last one gone. +/// /// -/// Hub context used to send to the session's group. -/// -/// Live-subscriber registry consulted before any per-event work is done. Both -/// audiences register here — hub connections by their SignalR connection id, -/// in-process subscriptions by a synthetic one — so the gate stays a single -/// source of truth. -/// -/// Gateway options supplying Dashboard:ShowTagValues. -/// Logger for best-effort mirror failures. -public sealed class DashboardEventBroadcaster( - IHubContext hubContext, - EventsHubViewerRegistry viewerRegistry, - IOptions options, - ILogger logger) : IDashboardEventBroadcaster, IDashboardSessionEventSubscriber +public sealed class DashboardEventBroadcaster : IDashboardEventBroadcaster, IDashboardSessionEventSubscriber, IDisposable { /// /// Queue depth per in-process subscriber. The consumer is a Blazor page /// rendering the newest handful of events, so a burst it cannot keep up with /// is dropped oldest-first rather than allowed to grow — same best-effort - /// contract the SignalR mirror already has. + /// contract the SignalR mirror has. /// private const int InProcessQueueCapacity = 256; - private readonly bool _showTagValues = options.Value.Dashboard.ShowTagValues; + /// + /// Queue depth of a session's hub-delivery pump. Same depth and drop policy as + /// the in-process feed: the remote viewer is a live view, so a burst it cannot + /// keep up with loses its oldest events rather than buffering without bound. + /// + internal const int HubQueueCapacity = 256; + + private static readonly TimeSpan DefaultHubSendTimeout = TimeSpan.FromSeconds(5); + + private readonly IHubContext _hubContext; + + private readonly EventsHubViewerRegistry _viewerRegistry; + + private readonly ILogger _logger; + + private readonly TimeSpan _hubSendTimeout; + + private readonly bool _showTagValues; private readonly object _syncRoot = new(); @@ -63,6 +78,9 @@ public sealed class DashboardEventBroadcaster( private readonly Dictionary _inProcessSubscribers = new(StringComparer.Ordinal); + /// Live hub-delivery pumps per session. Guarded by . + private readonly Dictionary _hubPumps = new(StringComparer.Ordinal); + /// /// Total live in-process subscribers, read without the lock so the common /// case — nobody has a session-details page open — never contends on it. @@ -70,6 +88,61 @@ public sealed class DashboardEventBroadcaster( /// private int _inProcessSubscriberCount; + private bool _disposed; + + /// Initializes a new instance of the class. + /// Hub context used to send to the session's group. + /// + /// Live-subscriber registry consulted before any per-event work is done. Both + /// audiences register here — hub connections by their SignalR connection id, + /// in-process subscriptions by a synthetic one — so the gate stays a single + /// source of truth. + /// + /// Gateway options supplying Dashboard:ShowTagValues. + /// Logger for best-effort mirror failures. + public DashboardEventBroadcaster( + IHubContext hubContext, + EventsHubViewerRegistry viewerRegistry, + IOptions options, + ILogger logger) + : this(hubContext, viewerRegistry, options, logger, DefaultHubSendTimeout) + { + } + + /// Initializes a new instance with a custom hub send timeout (test hook). + /// Hub context used to send to the session's group. + /// Live-subscriber registry. + /// Gateway options supplying Dashboard:ShowTagValues. + /// Logger for best-effort mirror failures. + /// Upper bound on one group send before it is abandoned. + internal DashboardEventBroadcaster( + IHubContext hubContext, + EventsHubViewerRegistry viewerRegistry, + IOptions options, + ILogger logger, + TimeSpan hubSendTimeout) + { + ArgumentNullException.ThrowIfNull(options); + _hubContext = hubContext ?? throw new ArgumentNullException(nameof(hubContext)); + _viewerRegistry = viewerRegistry ?? throw new ArgumentNullException(nameof(viewerRegistry)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _hubSendTimeout = hubSendTimeout; + _showTagValues = options.Value.Dashboard.ShowTagValues; + _viewerRegistry.HubViewersReleased += OnHubViewersReleased; + } + + /// Gets the number of live hub-delivery pumps (one per hub-watched session). + internal int ActiveHubPumpCount + { + get + { + lock (_syncRoot) + { + return _hubPumps.Count; + } + } + } + /// public void Publish(string sessionId, MxEvent mxEvent) { @@ -81,46 +154,126 @@ public sealed class DashboardEventBroadcaster( // Every session's dashboard-mirror subscriber calls Publish for every event, // whether or not a browser is on that session's page. Without this gate the // steady state — no dashboard viewer at all — still paid a deep protobuf - // clone (redaction is on by default) plus a send to an empty SignalR group - // per event. Bail before both. - if (!viewerRegistry.HasViewers(sessionId)) + // clone (redaction is on by default) per event. Bail before it. + if (!_viewerRegistry.HasViewers(sessionId)) { return; } MxEvent outbound = _showTagValues ? mxEvent : RedactValues(mxEvent); - // In-process delivery first: it is synchronous, cannot throw, and must not be - // skipped by the early return the hub send's guard clause takes. DeliverInProcess(sessionId, outbound); - // Wrap the Task acquisition in a try/catch so a hypothetical synchronous throw - // from SendAsync (e.g. an implementation that throws before returning the Task) - // cannot escape Publish. The interface contract is never-throw; fire-and-forget. - Task send; + // In-process-only viewers need no hub send at all; the pump exists only + // while a remote connection is in the session's group. + if (_viewerRegistry.HasHubViewers(sessionId)) + { + EnqueueForHub(sessionId, outbound); + } + } + + /// + /// Stops every hub pump and detaches from the viewer registry. The singleton is + /// disposed with the host; a pump's in-flight send is bounded by its timeout. + /// + public void Dispose() + { + HubPump[] pumps; + lock (_syncRoot) + { + if (_disposed) + { + return; + } + + _disposed = true; + pumps = [.. _hubPumps.Values]; + _hubPumps.Clear(); + } + + _viewerRegistry.HubViewersReleased -= OnHubViewersReleased; + foreach (HubPump pump in pumps) + { + pump.Complete(); + } + } + + /// + /// Queues an event on the session's hub pump, starting the pump on first use. + /// The write happens under the lock so it cannot land on a pump that a + /// concurrent teardown has already completed. + /// + /// Session the event belongs to. + /// The event as the dashboard should see it. + private void EnqueueForHub(string sessionId, MxEvent outbound) + { + lock (_syncRoot) + { + if (_disposed) + { + return; + } + + if (!_hubPumps.TryGetValue(sessionId, out HubPump? pump)) + { + pump = new HubPump(this, sessionId, HubQueueCapacity); + _hubPumps[sessionId] = pump; + } + + pump.TryWrite(outbound); + } + } + + /// + /// Tears down a session's hub pump once its last hub viewer has gone. Re-checks + /// the registry under the lock: a viewer that re-subscribed in between keeps the + /// pump, and one that arrives after the teardown gets a fresh pump on the next + /// event. + /// + /// Session whose last hub viewer left. + private void OnHubViewersReleased(string sessionId) + { + HubPump? pump; + lock (_syncRoot) + { + if (_viewerRegistry.HasHubViewers(sessionId) || !_hubPumps.Remove(sessionId, out pump)) + { + return; + } + } + + pump.Complete(); + } + + /// + /// Sends one event to the session's group, bounded by the hub send timeout. + /// Never throws: a failed or abandoned send is logged and the pump moves on. + /// + /// Session whose group receives the event. + /// The event to send. + /// A task that completes when the send finished or was abandoned. + private async Task SendToHubAsync(string sessionId, MxEvent outbound) + { try { - send = hubContext.Clients - .Group(EventsHub.GroupName(sessionId)) - .SendAsync(EventsHub.EventMessage, outbound); + bool sent = await BoundedHubSend.SendAsync( + _hubContext.Clients.Group(EventsHub.GroupName(sessionId)), + EventsHub.EventMessage, + outbound, + _hubSendTimeout, + CancellationToken.None) + .ConfigureAwait(false); + if (!sent) + { + _logger.LogDebug( + "Dashboard event mirror to session {SessionId} timed out after {Timeout}; continuing.", + sessionId, + _hubSendTimeout); + } } catch (Exception ex) { - logger.LogDebug(ex, "Dashboard event mirror to session {SessionId} threw synchronously.", sessionId); - return; - } - - if (!send.IsCompletedSuccessfully) - { - _ = send.ContinueWith( - t => - { - if (t.Exception is { } ex) - { - logger.LogDebug(ex, "Dashboard event mirror to session {SessionId} failed.", sessionId); - } - }, - TaskScheduler.Default); + _logger.LogDebug(ex, "Dashboard event mirror to session {SessionId} failed.", sessionId); } } @@ -133,7 +286,7 @@ public sealed class DashboardEventBroadcaster( // usable for a subscriber that has no SignalR connection behind it. The // "inproc-" prefix cannot collide with a SignalR connection id and makes the // origin obvious in a debugger. - string connectionId = "inproc-" + Guid.NewGuid().ToString("N"); + string connectionId = EventsHubViewerRegistry.InProcessConnectionIdPrefix + Guid.NewGuid().ToString("N"); InProcessSubscription subscription = new(this, sessionId, connectionId, InProcessQueueCapacity); // Register before the subscriber becomes a delivery target, exactly as @@ -142,7 +295,7 @@ public sealed class DashboardEventBroadcaster( // Publish's gate still reports the session unwatched, silently dropping events // it should receive. The cost of this order is at worst a redaction clone that // reaches nobody for the width of the window. - viewerRegistry.AddViewer(connectionId, sessionId); + _viewerRegistry.AddViewer(connectionId, sessionId); lock (_syncRoot) { @@ -227,12 +380,12 @@ public sealed class DashboardEventBroadcaster( } } - viewerRegistry.RemoveViewer(subscription.ConnectionId, subscription.SessionId); + _viewerRegistry.RemoveViewer(subscription.ConnectionId, subscription.SessionId); // The synthetic connection id is used once and never reconnects, so nothing // else will ever call ReleaseConnection for it; without this the registry // would retain an empty per-connection entry per subscription ever opened. - viewerRegistry.ReleaseConnection(subscription.ConnectionId); + _viewerRegistry.ReleaseConnection(subscription.ConnectionId); } /// @@ -328,4 +481,56 @@ public sealed class DashboardEventBroadcaster( _channel.Writer.TryComplete(); } } + + /// + /// One session's hub-delivery pump: a bounded drop-oldest channel and the single + /// task that drains it into the session's SignalR group, one bounded send at a + /// time. The drain is the only reader, so at most one send per session is ever + /// pending in the lifetime manager. + /// + private sealed class HubPump + { + private readonly Channel _channel; + + /// Initializes a new instance of the class and starts its drain. + /// Broadcaster whose send path the drain uses. + /// Session whose group the pump feeds. + /// Queue depth before the oldest queued event is dropped. + internal HubPump(DashboardEventBroadcaster owner, string sessionId, int capacity) + { + _channel = Channel.CreateBounded(new BoundedChannelOptions(capacity) + { + // DropOldest, not Wait: Publish runs on the gRPC event pipeline and + // must never block, and the newest events are the ones a live view wants. + FullMode = BoundedChannelFullMode.DropOldest, + SingleReader = true, + SingleWriter = false, + }); + + // Task.Run so the drain never starts inline on the publishing thread. + Completion = Task.Run(() => DrainAsync(owner, sessionId)); + } + + /// Gets the drain task; it completes once the channel is completed and emptied. + internal Task Completion { get; } + + /// Queues an event, dropping the oldest queued one when full. Never blocks. + /// The event to queue. + internal void TryWrite(MxEvent mxEvent) => _channel.Writer.TryWrite(mxEvent); + + /// Completes the channel so the drain exits once it is empty. + internal void Complete() => _channel.Writer.TryComplete(); + + private async Task DrainAsync(DashboardEventBroadcaster owner, string sessionId) + { + ChannelReader reader = _channel.Reader; + while (await reader.WaitToReadAsync().ConfigureAwait(false)) + { + while (reader.TryRead(out MxEvent? next)) + { + await owner.SendToHubAsync(sessionId, next).ConfigureAwait(false); + } + } + } + } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHubViewerRegistry.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHubViewerRegistry.cs index ac66b00..965389a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHubViewerRegistry.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHubViewerRegistry.cs @@ -19,11 +19,27 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; /// simply goes away never calls UnsubscribeSession; the hub's /// OnDisconnectedAsync releases everything the connection held. /// +/// +/// Hub viewers are also counted on their own, separately from the in-process +/// subscribers that register under an +/// id: runs one bounded hub-delivery pump +/// per session only while a remote connection is watching it, and tears that pump +/// down on . +/// /// public sealed class EventsHubViewerRegistry { + /// + /// Prefix of the synthetic connection ids in-process subscribers register under. + /// A SignalR connection id never carries it, so it is what tells the two + /// audiences apart for the hub-only count. + /// + public const string InProcessConnectionIdPrefix = "inproc-"; + private readonly ConcurrentDictionary _viewersBySession = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _hubViewersBySession = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary> _sessionsByConnection = new(StringComparer.Ordinal); @@ -53,8 +69,19 @@ public sealed class EventsHubViewerRegistry } _viewersBySession.AddOrUpdate(sessionId, 1, static (_, count) => count + 1); + if (IsHubConnection(connectionId)) + { + _hubViewersBySession.AddOrUpdate(sessionId, 1, static (_, count) => count + 1); + } } + /// + /// Raised with a session id when its last remote hub viewer leaves (unsubscribe + /// or disconnect). In-process subscribers do not raise it. Handlers run on the + /// releasing thread and must not throw or block. + /// + public event Action? HubViewersReleased; + /// /// Records that stopped watching /// . A removal with no matching @@ -75,7 +102,7 @@ public sealed class EventsHubViewerRegistry return; } - ReleaseSession(sessionId); + Release(connectionId, sessionId); } /// @@ -107,7 +134,7 @@ public sealed class EventsHubViewerRegistry // same detached set must not let the session be decremented twice. if (sessions.TryRemove(sessionId, out _)) { - ReleaseSession(sessionId); + Release(connectionId, sessionId); } } } @@ -120,32 +147,63 @@ public sealed class EventsHubViewerRegistry && _viewersBySession.TryGetValue(sessionId, out int count) && count > 0; + /// + /// Gets a value indicating whether any remote connection — + /// as opposed to an in-process subscriber — is watching the session. + /// + /// Session id to test. + /// when at least one hub connection is subscribed. + public bool HasHubViewers(string sessionId) => + !string.IsNullOrEmpty(sessionId) + && _hubViewersBySession.TryGetValue(sessionId, out int count) + && count > 0; + + private static bool IsHubConnection(string connectionId) => + !connectionId.StartsWith(InProcessConnectionIdPrefix, StringComparison.Ordinal); + + /// + /// Releases one subscription from both counts it was recorded in, and raises + /// when it was the session's last hub viewer. + /// + /// Connection that held the subscription. + /// Session id whose counts are released. + private void Release(string connectionId, string sessionId) + { + ReleaseSession(_viewersBySession, sessionId); + if (IsHubConnection(connectionId) && ReleaseSession(_hubViewersBySession, sessionId)) + { + HubViewersReleased?.Invoke(sessionId); + } + } + /// /// Decrements the session's viewer count, dropping the entry entirely at /// zero so the dictionary does not grow one key per session ever viewed. /// The compare-and-swap loop keeps the decrement correct against a /// concurrent on the same session. /// + /// The count map to decrement. /// Session id whose count is released. - private void ReleaseSession(string sessionId) + /// when this release dropped the count to zero. + private static bool ReleaseSession(ConcurrentDictionary counts, string sessionId) { while (true) { - if (!_viewersBySession.TryGetValue(sessionId, out int count)) + if (!counts.TryGetValue(sessionId, out int count)) { - return; + return false; } if (count <= 1) { - if (_viewersBySession.TryRemove(new KeyValuePair(sessionId, count))) + if (counts.TryRemove(new KeyValuePair(sessionId, count))) { - return; + return true; } } - else if (_viewersBySession.TryUpdate(sessionId, count - 1, count)) + else if (counts.TryUpdate(sessionId, count - 1, count)) { - return; + return false; } } } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs index 9e5a0b5..d4b3688 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs @@ -12,7 +12,8 @@ namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Dashboard; /// MxGateway:Dashboard:ShowTagValues: tag values are stripped /// from the mirrored copy when the flag is off, present when it is on, and the /// shared source event is never mutated. Also verifies the viewer gate — the -/// mirror does no work at all for a session nobody is watching. +/// mirror does no work at all for a session nobody is watching, and that hub +/// delivery goes through one bounded drop-oldest pump per hub-watched session. /// public sealed class DashboardEventBroadcasterTests { @@ -22,11 +23,12 @@ public sealed class DashboardEventBroadcasterTests { CapturingHubContext hubContext = new(); EventsHubViewerRegistry viewers = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); MxEvent source = BuildEventWithValue(); broadcaster.Publish("session-1", source); + Assert.Equal(0, broadcaster.ActiveHubPumpCount); Assert.Equal(0, hubContext.SendCount); Assert.Null(hubContext.LastArgument); } @@ -38,40 +40,105 @@ public sealed class DashboardEventBroadcasterTests CapturingHubContext hubContext = new(); EventsHubViewerRegistry viewers = new(); viewers.AddViewer("conn-1", "session-2"); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); broadcaster.Publish("session-1", BuildEventWithValue()); + Assert.Equal(0, broadcaster.ActiveHubPumpCount); Assert.Equal(0, hubContext.SendCount); } - /// Once the last viewer leaves, the mirror stops sending again. + /// Once the last hub viewer leaves, the session's pump is torn down and sending stops. + /// A task that represents the asynchronous operation. [Fact] - public void Publish_AfterLastViewerLeaves_StopsSending() + public async Task Publish_AfterLastViewerLeaves_StopsSending() { CapturingHubContext hubContext = new(); EventsHubViewerRegistry viewers = new(); viewers.AddViewer("conn-1", "session-1"); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); broadcaster.Publish("session-1", BuildEventWithValue()); - Assert.Equal(1, hubContext.SendCount); + await hubContext.WaitForSendCountAsync(1); + Assert.Equal(1, broadcaster.ActiveHubPumpCount); viewers.RemoveViewer("conn-1", "session-1"); + Assert.Equal(0, broadcaster.ActiveHubPumpCount); + broadcaster.Publish("session-1", BuildEventWithValue()); + Assert.Equal(0, broadcaster.ActiveHubPumpCount); Assert.Equal(1, hubContext.SendCount); } + /// + /// A hub viewer that cannot keep up costs a bounded queue, not an unbounded pile of + /// pending sends: while one send is stuck, further events drop oldest-first at the + /// pump's capacity, only one send is ever in flight, and the newest event survives. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Publish_WhenHubSendStalls_BuffersAtMostTheCapacityAndKeepsTheNewest() + { + CapturingHubContext hubContext = new(); + hubContext.Proxy.Gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + EventsHubViewerRegistry viewers = WatchedSession1(); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: true, viewers); + const int published = 2000; + + broadcaster.Publish("session-1", BuildSequencedEvent(0)); + await hubContext.WaitForSendCountAsync(1); + for (int i = 1; i < published; i++) + { + broadcaster.Publish("session-1", BuildSequencedEvent(i)); + } + + // One send stuck in flight, nothing else started behind it. + Assert.Equal(1, hubContext.SendCount); + Assert.Equal(1, hubContext.Proxy.MaxConcurrentSends); + + hubContext.Proxy.Gate.SetResult(); + await hubContext.WaitForLastItemHandleAsync(published - 1); + + // The stuck event plus at most one full queue — the rest were dropped oldest-first. + Assert.InRange(hubContext.SendCount, 2, DashboardEventBroadcaster.HubQueueCapacity + 1); + Assert.Equal(1, hubContext.Proxy.MaxConcurrentSends); + } + + /// + /// A send that never completes is abandoned at the send timeout, so one stuck + /// connection cannot stop the pump delivering later events. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Publish_WhenHubSendNeverCompletes_AbandonsItAtTheTimeoutAndContinues() + { + CapturingHubContext hubContext = new(); + hubContext.Proxy.Gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + hubContext.Proxy.IgnoreCancellation = true; + using DashboardEventBroadcaster broadcaster = Create( + hubContext, + showTagValues: true, + WatchedSession1(), + hubSendTimeout: TimeSpan.FromMilliseconds(50)); + + broadcaster.Publish("session-1", BuildSequencedEvent(1)); + broadcaster.Publish("session-1", BuildSequencedEvent(2)); + + await hubContext.WaitForSendCountAsync(2); + } + /// Values are stripped from the mirror when ShowTagValues is off; metadata survives. [Fact] - public void Publish_WhenShowTagValuesFalse_RedactsValuesButKeepsMetadata() + /// A task that represents the asynchronous operation. + public async Task Publish_WhenShowTagValuesFalse_RedactsValuesButKeepsMetadata() { CapturingHubContext hubContext = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, WatchedSession1()); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, WatchedSession1()); MxEvent source = BuildEventWithValue(); broadcaster.Publish("session-1", source); + await hubContext.WaitForSendCountAsync(1); MxEvent sent = Assert.IsType(hubContext.LastArgument); Assert.Null(sent.Value); @@ -88,13 +155,15 @@ public sealed class DashboardEventBroadcasterTests /// Redaction applies to a clone, so the shared source event keeps its values. [Fact] - public void Publish_WhenShowTagValuesFalse_DoesNotMutateSourceEvent() + /// A task that represents the asynchronous operation. + public async Task Publish_WhenShowTagValuesFalse_DoesNotMutateSourceEvent() { CapturingHubContext hubContext = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, WatchedSession1()); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, WatchedSession1()); MxEvent source = BuildEventWithValue(); broadcaster.Publish("session-1", source); + await hubContext.WaitForSendCountAsync(1); // The redaction must apply to a clone; the shared source keeps its values. Assert.NotNull(source.Value); @@ -106,13 +175,15 @@ public sealed class DashboardEventBroadcasterTests /// Values pass through unredacted when ShowTagValues is on. [Fact] - public void Publish_WhenShowTagValuesTrue_KeepsValues() + /// A task that represents the asynchronous operation. + public async Task Publish_WhenShowTagValuesTrue_KeepsValues() { CapturingHubContext hubContext = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: true, WatchedSession1()); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: true, WatchedSession1()); MxEvent source = BuildEventWithValue(); broadcaster.Publish("session-1", source); + await hubContext.WaitForSendCountAsync(1); MxEvent sent = Assert.IsType(hubContext.LastArgument); Assert.NotNull(sent.Value); @@ -125,16 +196,18 @@ public sealed class DashboardEventBroadcasterTests /// An in-process subscriber gets the same redacted clone the hub group gets, /// and the shared source event is still left untouched. /// + /// A task that represents the asynchronous operation. [Fact] - public void Subscribe_WhenShowTagValuesFalse_DeliversRedactedCloneWithoutMutatingSource() + public async Task Subscribe_WhenShowTagValuesFalse_DeliversRedactedCloneWithoutMutatingSource() { CapturingHubContext hubContext = new(); - EventsHubViewerRegistry viewers = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); + EventsHubViewerRegistry viewers = WatchedSession1(); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); using IDashboardEventSubscription subscription = broadcaster.Subscribe("session-1"); MxEvent source = BuildEventWithValue(); broadcaster.Publish("session-1", source); + await hubContext.WaitForSendCountAsync(1); MxEvent received = ReadOne(subscription); Assert.Null(received.Value); @@ -153,17 +226,16 @@ public sealed class DashboardEventBroadcasterTests Assert.NotNull(source.OnAlarmTransition.LimitValue); } - /// An in-process subscription opens the viewer gate the same way a hub client does. + /// + /// An in-process subscription opens the viewer gate the same way a hub client does, + /// but starts no hub pump: with no remote connection there is nobody to send to. + /// [Fact] public void Subscribe_OpensTheViewerGate() { CapturingHubContext hubContext = new(); EventsHubViewerRegistry viewers = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); - - broadcaster.Publish("session-1", BuildEventWithValue()); - Assert.Equal(0, hubContext.SendCount); - Assert.Null(hubContext.LastArgument); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); using IDashboardEventSubscription subscription = broadcaster.Subscribe("session-1"); @@ -171,8 +243,9 @@ public sealed class DashboardEventBroadcasterTests broadcaster.Publish("session-1", BuildEventWithValue()); - Assert.Equal(1, hubContext.SendCount); - Assert.NotNull(hubContext.LastArgument); + Assert.Equal("session-1", ReadOne(subscription).SessionId); + Assert.Equal(0, broadcaster.ActiveHubPumpCount); + Assert.Equal(0, hubContext.SendCount); } /// Disposing the last in-process subscription restores the no-viewers short-circuit. @@ -181,11 +254,10 @@ public sealed class DashboardEventBroadcasterTests { CapturingHubContext hubContext = new(); EventsHubViewerRegistry viewers = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); IDashboardEventSubscription subscription = broadcaster.Subscribe("session-1"); broadcaster.Publish("session-1", BuildEventWithValue()); - Assert.Equal(1, hubContext.SendCount); Assert.Equal("session-1", ReadOne(subscription).SessionId); subscription.Dispose(); @@ -194,26 +266,28 @@ public sealed class DashboardEventBroadcasterTests broadcaster.Publish("session-1", BuildEventWithValue()); - Assert.Equal(1, hubContext.SendCount); + Assert.Equal(0, hubContext.SendCount); Assert.False(subscription.Reader.TryRead(out _)); } /// Hub viewers and in-process subscribers are audiences of their own session only. + /// A task that represents the asynchronous operation. [Fact] - public void Publish_DeliversOnlyToTheSubscribedSessionsAudience() + public async Task Publish_DeliversOnlyToTheSubscribedSessionsAudience() { CapturingHubContext hubContext = new(); EventsHubViewerRegistry viewers = new(); viewers.AddViewer("conn-1", "session-1"); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); using IDashboardEventSubscription subscription = broadcaster.Subscribe("session-2"); // The hub viewer's session must not spill into the in-process feed. broadcaster.Publish("session-1", BuildEventWithValue()); - Assert.Equal(1, hubContext.SendCount); + await hubContext.WaitForSendCountAsync(1); Assert.False(subscription.Reader.TryRead(out _)); + // The in-process-only session reaches its subscriber and starts no hub pump. broadcaster.Publish("session-2", BuildEventWithValue("session-2")); Assert.Equal("session-2", ReadOne(subscription).SessionId); @@ -221,7 +295,8 @@ public sealed class DashboardEventBroadcasterTests // A session with no audience at all still short-circuits. broadcaster.Publish("session-3", BuildEventWithValue("session-3")); - Assert.Equal(2, hubContext.SendCount); + Assert.Equal(1, broadcaster.ActiveHubPumpCount); + Assert.Equal(1, hubContext.SendCount); } /// Disposing twice is a no-op and cannot release a sibling subscription's registration. @@ -230,7 +305,7 @@ public sealed class DashboardEventBroadcasterTests { CapturingHubContext hubContext = new(); EventsHubViewerRegistry viewers = new(); - DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); + using DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers); IDashboardEventSubscription first = broadcaster.Subscribe("session-1"); using IDashboardEventSubscription second = broadcaster.Subscribe("session-1"); @@ -241,7 +316,6 @@ public sealed class DashboardEventBroadcasterTests broadcaster.Publish("session-1", BuildEventWithValue()); - Assert.Equal(1, hubContext.SendCount); Assert.False(first.Reader.TryRead(out _)); Assert.Equal("session-1", ReadOne(second).SessionId); } @@ -258,7 +332,8 @@ public sealed class DashboardEventBroadcasterTests private static DashboardEventBroadcaster Create( CapturingHubContext hubContext, bool showTagValues, - EventsHubViewerRegistry viewers) + EventsHubViewerRegistry viewers, + TimeSpan? hubSendTimeout = null) { GatewayOptions gatewayOptions = new() { @@ -269,7 +344,21 @@ public sealed class DashboardEventBroadcasterTests hubContext, viewers, Options.Create(gatewayOptions), - NullLogger.Instance); + NullLogger.Instance, + hubSendTimeout ?? TimeSpan.FromSeconds(30)); + } + + /// Builds a value-less event whose item handle carries its publish order. + /// Publish order stamped into . + /// The event. + private static MxEvent BuildSequencedEvent(int sequence) + { + return new MxEvent + { + Family = MxEventFamily.OnDataChange, + SessionId = "session-1", + ItemHandle = sequence, + }; } /// A registry with one hub connection watching session-1. @@ -313,81 +402,157 @@ public sealed class DashboardEventBroadcasterTests /// Gets the group manager. public IGroupManager Groups { get; } = new NoopGroupManager(); + /// Gets the capturing proxy every client lookup resolves to. + public CapturingClientProxy Proxy => _clients.Proxy; + /// Gets the first argument of the most recent send call. - public object? LastArgument => _clients.GroupProxy.LastArgument; + public object? LastArgument => _clients.Proxy.LastArgument; /// Gets the number of send calls this fake has observed. - public int SendCount => _clients.GroupProxy.SendCount; + public int SendCount => _clients.Proxy.SendCount; + + /// Waits until at least sends have started. + /// The send count to wait for. + /// A task that completes once the count is reached. + public Task WaitForSendCountAsync(int count) => + WaitUntilAsync(() => SendCount >= count, $"{count} sends (saw {SendCount})"); + + /// Waits until a send carrying the given item handle has started. + /// Item handle of the event to wait for. + /// A task that completes once the event was sent. + public Task WaitForLastItemHandleAsync(int itemHandle) => + WaitUntilAsync( + () => LastArgument is MxEvent sent && sent.ItemHandle == itemHandle, + $"a send of item handle {itemHandle}"); + + private static async Task WaitUntilAsync(Func condition, string description) + { + DateTime deadline = DateTime.UtcNow.AddSeconds(10); + while (!condition()) + { + if (DateTime.UtcNow > deadline) + { + Assert.Fail($"Timed out waiting for {description}."); + } + + await Task.Delay(5); + } + } } private sealed class CapturingHubClients : IHubClients { /// Gets the capturing client proxy shared by this fake. - public CapturingClientProxy GroupProxy { get; } = new(); + public CapturingClientProxy Proxy { get; } = new(); /// Gets the shared capturing proxy for all connections. - public IClientProxy All => GroupProxy; + public IClientProxy All => Proxy; /// Returns the shared capturing proxy, ignoring exclusions. /// Ignored. /// The shared capturing proxy. - public IClientProxy AllExcept(IReadOnlyList excludedConnectionIds) => GroupProxy; + public IClientProxy AllExcept(IReadOnlyList excludedConnectionIds) => Proxy; /// Returns the shared capturing proxy for any connection. /// Ignored. /// The shared capturing proxy. - public IClientProxy Client(string connectionId) => GroupProxy; + public IClientProxy Client(string connectionId) => Proxy; /// Returns the shared capturing proxy for any connection set. /// Ignored. /// The shared capturing proxy. - public IClientProxy Clients(IReadOnlyList connectionIds) => GroupProxy; + public IClientProxy Clients(IReadOnlyList connectionIds) => Proxy; /// Returns the shared capturing proxy for any group. /// Ignored. /// The shared capturing proxy. - public IClientProxy Group(string groupName) => GroupProxy; + public IClientProxy Group(string groupName) => Proxy; /// Returns the shared capturing proxy for any group, ignoring exclusions. /// Ignored. /// Ignored. /// The shared capturing proxy. - public IClientProxy GroupExcept(string groupName, IReadOnlyList excludedConnectionIds) => GroupProxy; + public IClientProxy GroupExcept(string groupName, IReadOnlyList excludedConnectionIds) => Proxy; /// Returns the shared capturing proxy for any group set. /// Ignored. /// The shared capturing proxy. - public IClientProxy Groups(IReadOnlyList groupNames) => GroupProxy; + public IClientProxy Groups(IReadOnlyList groupNames) => Proxy; /// Returns the shared capturing proxy for any user. /// Ignored. /// The shared capturing proxy. - public IClientProxy User(string userId) => GroupProxy; + public IClientProxy User(string userId) => Proxy; /// Returns the shared capturing proxy for any user set. /// Ignored. /// The shared capturing proxy. - public IClientProxy Users(IReadOnlyList userIds) => GroupProxy; + public IClientProxy Users(IReadOnlyList userIds) => Proxy; } private sealed class CapturingClientProxy : IClientProxy { + private int _sendCount; + private int _inFlight; + private int _maxConcurrentSends; + private object? _lastArgument; + + /// + /// Gets or sets a gate every send waits on before completing; null completes + /// sends synchronously. Set it before the first publish. + /// + public TaskCompletionSource? Gate { get; set; } + + /// Gets or sets a value indicating whether a gated send ignores its cancellation token. + public bool IgnoreCancellation { get; set; } + /// Gets the first argument of the most recent send call. - public object? LastArgument { get; private set; } + public object? LastArgument => Volatile.Read(ref _lastArgument); /// Gets the number of send calls made through this proxy. - public int SendCount { get; private set; } + public int SendCount => Volatile.Read(ref _sendCount); - /// Records the send call arguments and completes synchronously. + /// Gets the highest number of sends that were in flight at once. + public int MaxConcurrentSends => Volatile.Read(ref _maxConcurrentSends); + + /// Records the send call arguments and completes once the gate (if any) opens. /// The SignalR method name. /// The method arguments. /// Token to observe for cancellation. - /// A completed task. + /// A task that completes when the send is released. public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default) { - SendCount++; - LastArgument = args.Length > 0 ? args[0] : null; - return Task.CompletedTask; + Volatile.Write(ref _lastArgument, args.Length > 0 ? args[0] : null); + Interlocked.Increment(ref _sendCount); + + TaskCompletionSource? gate = Gate; + if (gate is null || gate.Task.IsCompleted) + { + return Task.CompletedTask; + } + + return TrackAsync(gate.Task, IgnoreCancellation ? CancellationToken.None : cancellationToken); + } + + private async Task TrackAsync(Task gate, CancellationToken cancellationToken) + { + int inFlight = Interlocked.Increment(ref _inFlight); + int observed; + do + { + observed = Volatile.Read(ref _maxConcurrentSends); + } + while (inFlight > observed + && Interlocked.CompareExchange(ref _maxConcurrentSends, inFlight, observed) != observed); + + try + { + await gate.WaitAsync(cancellationToken); + } + finally + { + Interlocked.Decrement(ref _inFlight); + } } } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/EventsHubViewerRegistryTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/EventsHubViewerRegistryTests.cs index acd5712..17384d8 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/EventsHubViewerRegistryTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/EventsHubViewerRegistryTests.cs @@ -164,4 +164,48 @@ public sealed class EventsHubViewerRegistryTests Assert.False(registry.HasViewers("session-1")); } + + /// In-process subscribers count as viewers but not as hub viewers. + [Fact] + public void HasHubViewers_CountsOnlyRemoteConnections() + { + EventsHubViewerRegistry registry = new(); + + registry.AddViewer(EventsHubViewerRegistry.InProcessConnectionIdPrefix + "a", "session-1"); + + Assert.True(registry.HasViewers("session-1")); + Assert.False(registry.HasHubViewers("session-1")); + + registry.AddViewer("conn-1", "session-1"); + + Assert.True(registry.HasHubViewers("session-1")); + } + + /// + /// The released event fires once, when the last hub viewer leaves — whether it + /// unsubscribes or disconnects — and never for in-process subscribers. + /// + [Fact] + public void HubViewersReleased_FiresWhenTheLastHubViewerLeaves() + { + EventsHubViewerRegistry registry = new(); + List released = []; + registry.HubViewersReleased += released.Add; + + registry.AddViewer("conn-1", "session-1"); + registry.AddViewer("conn-2", "session-1"); + registry.AddViewer(EventsHubViewerRegistry.InProcessConnectionIdPrefix + "a", "session-1"); + + registry.RemoveViewer("conn-1", "session-1"); + Assert.Empty(released); + + registry.ReleaseConnection("conn-2"); + Assert.Equal(["session-1"], released); + Assert.False(registry.HasHubViewers("session-1")); + + // The in-process subscriber still keeps the session watched, and leaving raises nothing. + Assert.True(registry.HasViewers("session-1")); + registry.RemoveViewer(EventsHubViewerRegistry.InProcessConnectionIdPrefix + "a", "session-1"); + Assert.Equal(["session-1"], released); + } } From 3688fa19e6d7f502e43885178b9a01859ff86779 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:14:53 -0400 Subject: [PATCH 2/8] fix(dashboard): time-bound alarm hub sends and mark each resubscribe AlarmsHubPublisher awaited every group send unbounded inside the StreamAsync loop, so one slow client stalled the drain until the monitor dropped the subscriber and the resubscribe replayed the full snapshot to everyone. Each broadcast is now bounded (2s) and abandoned on timeout, and every (re)subscription is preceded by an AlarmFeedReset frame so clients discard state before the replayed snapshot. --- .../Dashboard/Hubs/AlarmFeedReset.cs | 14 ++ .../Dashboard/Hubs/AlarmsHub.cs | 8 + .../Dashboard/Hubs/AlarmsHubPublisher.cs | 132 ++++++++++++--- .../Dashboard/AlarmsHubPublisherTests.cs | 159 +++++++++++++++--- 4 files changed, 270 insertions(+), 43 deletions(-) create mode 100644 src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmFeedReset.cs diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmFeedReset.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmFeedReset.cs new file mode 100644 index 0000000..e1742f4 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmFeedReset.cs @@ -0,0 +1,14 @@ +namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; + +/// +/// Payload of the frame: the publisher has +/// (re)subscribed to the central alarm monitor, which primes the subscription with +/// a full active-alarm snapshot. A client discards its accumulated alarm state on +/// receipt and rebuilds it from the active_alarm rows up to the next +/// snapshot_complete. +/// +/// +/// Monotonic subscription counter for this gateway process, starting at 1. Lets a +/// client tell a resubscribe from a duplicate delivery. +/// +public sealed record AlarmFeedReset(long Generation); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHub.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHub.cs index d48f997..d0c6287 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHub.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHub.cs @@ -23,11 +23,19 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; [Authorize(Policy = DashboardAuthenticationDefaults.AlarmsPolicy)] public sealed class AlarmsHub : Hub { + /// Group every admitted connection joins on connect. public const string AllAlarmsGroup = "__alarms__"; /// Method name used to push AlarmFeedMessage values to clients. public const string AlarmMessage = "AlarmFeed"; + /// + /// Method name of the marker sent ahead of every + /// (re)subscription to the central monitor. A client discards its accumulated + /// alarm state on receipt and rebuilds it from the snapshot that follows. + /// + public const string ResetMessage = "AlarmFeedReset"; + /// public override async Task OnConnectedAsync() { diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHubPublisher.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHubPublisher.cs index c280d73..adce9ca 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHubPublisher.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHubPublisher.cs @@ -31,18 +31,69 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; /// narrowing this loop could apply even in principle. Adding a filter here would imply a /// per-caller scope the feed does not carry. /// +/// +/// Each broadcast is time-bounded (). A group send +/// completes only once every member accepted the frame, so awaiting it unbounded +/// let one slow browser stall this loop; the stalled loop stopped draining its +/// monitor subscription, the monitor dropped it as a slow subscriber, and the +/// resubscribe replayed the whole active-alarm snapshot to every client. A send that +/// misses the bound is abandoned and the loop moves on. +/// +/// +/// Every subscription — the first and each resubscribe — is preceded by an +/// frame. The monitor primes every new +/// subscription with a full snapshot, so a client that keeps accumulating +/// active_alarm rows across a resubscribe would double-count or keep alarms +/// that cleared while the feed was down; the marker tells it to discard its state +/// and rebuild from the snapshot that follows. +/// /// -/// The gateway's central alarm feed. -/// Hub context used to broadcast to the alarms group. -/// Gateway options supplying Dashboard:ShowTagValues. -/// Logger for best-effort broadcast failures. -public sealed class AlarmsHubPublisher( - IGatewayAlarmService alarmService, - IHubContext hubContext, - IOptions options, - ILogger logger) : BackgroundService +public sealed class AlarmsHubPublisher : BackgroundService { - private readonly bool _showTagValues = options.Value.Dashboard.ShowTagValues; + /// Upper bound on one alarm broadcast before it is abandoned. + internal static readonly TimeSpan DefaultSendTimeout = TimeSpan.FromSeconds(2); + + private readonly IGatewayAlarmService _alarmService; + private readonly IHubContext _hubContext; + private readonly ILogger _logger; + private readonly TimeSpan _sendTimeout; + private readonly bool _showTagValues; + private long _subscriptionGeneration; + + /// Initializes a new instance of the class. + /// The gateway's central alarm feed. + /// Hub context used to broadcast to the alarms group. + /// Gateway options supplying Dashboard:ShowTagValues. + /// Logger for best-effort broadcast failures. + public AlarmsHubPublisher( + IGatewayAlarmService alarmService, + IHubContext hubContext, + IOptions options, + ILogger logger) + : this(alarmService, hubContext, options, logger, DefaultSendTimeout) + { + } + + /// Initializes a new instance with a custom send timeout (test hook). + /// The gateway's central alarm feed. + /// Hub context used to broadcast to the alarms group. + /// Gateway options supplying Dashboard:ShowTagValues. + /// Logger for best-effort broadcast failures. + /// Upper bound on one broadcast. + internal AlarmsHubPublisher( + IGatewayAlarmService alarmService, + IHubContext hubContext, + IOptions options, + ILogger logger, + TimeSpan sendTimeout) + { + ArgumentNullException.ThrowIfNull(options); + _alarmService = alarmService ?? throw new ArgumentNullException(nameof(alarmService)); + _hubContext = hubContext ?? throw new ArgumentNullException(nameof(hubContext)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _sendTimeout = sendTimeout; + _showTagValues = options.Value.Dashboard.ShowTagValues; + } /// protected override async Task ExecuteAsync(CancellationToken stoppingToken) @@ -54,7 +105,16 @@ public sealed class AlarmsHubPublisher( { try { - await foreach (AlarmFeedMessage message in alarmService + // Sent before the subscription opens, so it reaches clients ahead of + // the snapshot the monitor primes the new subscription with. + long generation = Interlocked.Increment(ref _subscriptionGeneration); + await BroadcastAsync( + AlarmsHub.ResetMessage, + new AlarmFeedReset(generation), + stoppingToken) + .ConfigureAwait(false); + + await foreach (AlarmFeedMessage message in _alarmService .StreamAsync(alarmFilterPrefix: null, stoppingToken) .ConfigureAwait(false)) { @@ -63,18 +123,8 @@ public sealed class AlarmsHubPublisher( break; } - try - { - AlarmFeedMessage outbound = _showTagValues ? message : RedactValues(message); - await hubContext.Clients - .Group(AlarmsHub.AllAlarmsGroup) - .SendAsync(AlarmsHub.AlarmMessage, outbound, stoppingToken) - .ConfigureAwait(false); - } - catch (Exception ex) when (ex is not OperationCanceledException) - { - logger.LogWarning(ex, "Alarm broadcast failed; continuing."); - } + AlarmFeedMessage outbound = _showTagValues ? message : RedactValues(message); + await BroadcastAsync(AlarmsHub.AlarmMessage, outbound, stoppingToken).ConfigureAwait(false); } } catch (OperationCanceledException) @@ -83,7 +133,7 @@ public sealed class AlarmsHubPublisher( } catch (Exception ex) { - logger.LogWarning(ex, "Alarm subscription faulted; reconnecting in 5s."); + _logger.LogWarning(ex, "Alarm subscription faulted; reconnecting in 5s."); try { await Task.Delay(TimeSpan.FromSeconds(5), stoppingToken).ConfigureAwait(false); @@ -96,6 +146,40 @@ public sealed class AlarmsHubPublisher( } } + /// + /// Sends one frame to the alarms group within the send timeout. A slow or failed + /// send is logged and dropped so the feed keeps draining; only host shutdown + /// propagates. + /// + /// Client method name. + /// The frame payload. + /// Host shutdown token. + /// A task that completes when the send finished or was abandoned. + private async Task BroadcastAsync(string method, object argument, CancellationToken stoppingToken) + { + try + { + bool sent = await BoundedHubSend.SendAsync( + _hubContext.Clients.Group(AlarmsHub.AllAlarmsGroup), + method, + argument, + _sendTimeout, + stoppingToken) + .ConfigureAwait(false); + if (!sent) + { + _logger.LogDebug( + "Alarm broadcast {Method} exceeded {Timeout} and was abandoned; continuing.", + method, + _sendTimeout); + } + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Alarm broadcast failed; continuing."); + } + } + /// /// Produces a copy of with the alarm value fields /// cleared, leaving every other field — reference, severity, state, category, diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/AlarmsHubPublisherTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/AlarmsHubPublisherTests.cs index ea9e9fb..21711f8 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/AlarmsHubPublisherTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/AlarmsHubPublisherTests.cs @@ -15,7 +15,9 @@ namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Dashboard; /// both value-bearing payload arms are stripped from the copy broadcast to /// browser clients when the flag is off, present when it is on, and the source /// — shared with the gRPC StreamAlarms -/// subscribers and the alarms page — is never mutated. +/// subscribers and the alarms page — is never mutated. Also verifies that a +/// slow client cannot stall the feed and that every (re)subscription is announced +/// with a reset marker ahead of its snapshot. /// public sealed class AlarmsHubPublisherTests { @@ -136,6 +138,81 @@ public sealed class AlarmsHubPublisherTests Assert.True(hubContext.Sent[2].SnapshotStatus.Truncated); } + /// + /// Every subscription — the first and each resubscribe after the monitor ends a + /// stream — is preceded by a reset marker, so a client knows to discard its + /// state before the replayed snapshot arrives instead of double-counting it. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ExecuteAsync_OnResubscribe_SendsResetMarkerBeforeTheReplayedSnapshot() + { + AlarmFeedMessage first = BuildActiveAlarm(); + AlarmFeedMessage replayed = BuildActiveAlarm(); + ScriptedAlarmService alarmService = new([first], [replayed]); + CapturingHubContext hubContext = new(); + AlarmsHubPublisher publisher = CreatePublisher(alarmService, hubContext, AlarmsHubPublisher.DefaultSendTimeout); + + using CancellationTokenSource cts = new(); + await publisher.StartAsync(cts.Token).WaitAsync(TestTimeout); + await WaitUntilAsync(() => hubContext.Sent.Count >= 2); + await cts.CancelAsync(); + await publisher.StopAsync(CancellationToken.None); + + IReadOnlyList<(string Method, object? Argument)> frames = hubContext.Frames; + Assert.Equal( + [AlarmsHub.ResetMessage, AlarmsHub.AlarmMessage, AlarmsHub.ResetMessage, AlarmsHub.AlarmMessage], + frames.Select(frame => frame.Method).Take(4)); + Assert.Equal(1, Assert.IsType(frames[0].Argument).Generation); + Assert.Equal(2, Assert.IsType(frames[2].Argument).Generation); + Assert.Same(first, frames[1].Argument); + Assert.Same(replayed, frames[3].Argument); + } + + /// + /// A broadcast that never completes — one client whose transport is wedged — is + /// abandoned at the send timeout, so the loop keeps draining the monitor + /// subscription instead of stalling until the monitor drops it. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ExecuteAsync_WhenABroadcastNeverCompletes_AbandonsItAndKeepsDraining() + { + AlarmFeedMessage stuck = BuildTransition(); + AlarmFeedMessage next = BuildActiveAlarm(); + ScriptedAlarmService alarmService = new([stuck, next]); + CapturingHubContext hubContext = new(); + hubContext.Proxy.StallFirstAlarm = true; + AlarmsHubPublisher publisher = CreatePublisher(alarmService, hubContext, TimeSpan.FromMilliseconds(50)); + + using CancellationTokenSource cts = new(); + await publisher.StartAsync(cts.Token).WaitAsync(TestTimeout); + await WaitUntilAsync(() => hubContext.Sent.Count >= 2); + await cts.CancelAsync(); + await publisher.StopAsync(CancellationToken.None); + + Assert.Equal("Tank02.Level.Lo", hubContext.Sent[1].ActiveAlarm.AlarmFullReference); + } + + private static AlarmsHubPublisher CreatePublisher( + IGatewayAlarmService alarmService, + CapturingHubContext hubContext, + TimeSpan sendTimeout, + bool showTagValues = true) + { + GatewayOptions gatewayOptions = new() + { + Dashboard = new DashboardOptions { ShowTagValues = showTagValues }, + }; + + return new AlarmsHubPublisher( + alarmService, + hubContext, + Options.Create(gatewayOptions), + NullLogger.Instance, + sendTimeout); + } + /// /// Starts the publisher over a scripted feed, waits until every scripted /// message has been broadcast, and stops it. @@ -149,16 +226,11 @@ public sealed class AlarmsHubPublisherTests { ScriptedAlarmService alarmService = new(messages); CapturingHubContext hubContext = new(); - GatewayOptions gatewayOptions = new() - { - Dashboard = new DashboardOptions { ShowTagValues = showTagValues }, - }; - - AlarmsHubPublisher publisher = new( + AlarmsHubPublisher publisher = CreatePublisher( alarmService, hubContext, - Options.Create(gatewayOptions), - NullLogger.Instance); + AlarmsHubPublisher.DefaultSendTimeout, + showTagValues); using CancellationTokenSource cts = new(); await publisher.StartAsync(cts.Token).WaitAsync(TestTimeout); @@ -221,12 +293,15 @@ public sealed class AlarmsHubPublisherTests } /// - /// Yields a scripted message list once and then stays open until cancelled, - /// so the publisher's reconnect loop never re-subscribes mid-test. + /// Yields one scripted batch per subscription. Every batch but the last ends its + /// stream — the monitor completing a subscription — so the publisher resubscribes; + /// the last batch stays open until cancelled, so the reconnect loop then settles. /// - /// The messages to yield, in order. - private sealed class ScriptedAlarmService(IReadOnlyList messages) : IGatewayAlarmService + /// The messages each successive subscription yields, in order. + private sealed class ScriptedAlarmService(params IReadOnlyList[] batches) : IGatewayAlarmService { + private int _subscriptions; + /// public GatewayAlarmMonitorState State => GatewayAlarmMonitorState.Monitoring; @@ -247,11 +322,24 @@ public sealed class AlarmsHubPublisherTests string? alarmFilterPrefix, [EnumeratorCancellation] CancellationToken cancellationToken) { + int index = Interlocked.Increment(ref _subscriptions) - 1; + IReadOnlyList messages = batches[Math.Min(index, batches.Length - 1)]; + if (index >= batches.Length) + { + // Past the script: stay open and quiet rather than replaying the last batch. + messages = []; + } + foreach (AlarmFeedMessage message in messages) { yield return message; } + if (index < batches.Length - 1) + { + yield break; + } + try { await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken).ConfigureAwait(false); @@ -277,8 +365,14 @@ public sealed class AlarmsHubPublisherTests /// Gets the group manager. public IGroupManager Groups { get; } = new NoopGroupManager(); - /// Gets every message the publisher broadcast, in order. + /// Gets the capturing proxy every client lookup resolves to. + public CapturingClientProxy Proxy => _clients.GroupProxy; + + /// Gets every alarm message the publisher broadcast, in order. public IReadOnlyList Sent => _clients.GroupProxy.Sent; + + /// Gets every frame the publisher broadcast — alarm messages and reset markers — in order. + public IReadOnlyList<(string Method, object? Argument)> Frames => _clients.GroupProxy.Frames; } private sealed class CapturingHubClients : IHubClients @@ -334,6 +428,26 @@ public sealed class AlarmsHubPublisherTests private sealed class CapturingClientProxy : IClientProxy { private readonly List _sent = []; + private readonly List<(string Method, object? Argument)> _frames = []; + private bool _stalled; + + /// + /// Gets or sets a value indicating whether the first alarm send never completes + /// and ignores its cancellation token — a wedged client transport. + /// + public bool StallFirstAlarm { get; set; } + + /// Gets every frame sent through this proxy, in order. + public IReadOnlyList<(string Method, object? Argument)> Frames + { + get + { + lock (_sent) + { + return [.. _frames]; + } + } + } /// Gets every alarm message sent through this proxy, in order. public IReadOnlyList Sent @@ -347,22 +461,29 @@ public sealed class AlarmsHubPublisherTests } } - /// Records the broadcast message and completes synchronously. + /// Records the broadcast frame and completes synchronously unless stalled. /// The SignalR method name. /// The method arguments. /// Token to observe for cancellation. - /// A completed task. + /// A completed task, or a never-completing one for the stalled send. public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default) { - if (args.Length > 0 && args[0] is AlarmFeedMessage message) + bool stall = false; + lock (_sent) { - lock (_sent) + _frames.Add((method, args.Length > 0 ? args[0] : null)); + if (args.Length > 0 && args[0] is AlarmFeedMessage message) { _sent.Add(message); + if (StallFirstAlarm && !_stalled) + { + _stalled = true; + stall = true; + } } } - return Task.CompletedTask; + return stall ? new TaskCompletionSource().Task : Task.CompletedTask; } } From 6733519ae5f599e5d8c7c08ad505c0f118e48c06 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:18:02 -0400 Subject: [PATCH 3/8] fix(dashboard): derive gateway status from health checks; send config once per hub connection SEC-27: DashboardSnapshot.GatewayStatus was hardcoded Healthy. It is now the aggregate HealthCheckService status, evaluated once per snapshot tick (2s bound, zero-wait gate) and cached for GetSnapshot; Unknown before the first evaluation, on failure, or without a health service. The startup-static EffectiveGatewayConfiguration is JsonIgnore'd on the snapshot, so /hubs/snapshot ticks carry only volatile state. The hub sends it once per connection (ConfigurationUpdated) and serves GetConfiguration(). Snapshot broadcasts are time-bounded (1s) so one slow client cannot delay ticks. StatusBadge maps Unhealthy to Bad. --- .../Components/Shared/StatusBadge.razor | 2 +- .../Dashboard/DashboardSnapshot.cs | 28 ++++- .../Dashboard/DashboardSnapshotService.cs | 74 +++++++++++- .../Dashboard/Hubs/DashboardSnapshotHub.cs | 25 +++- .../Hubs/DashboardSnapshotPublisher.cs | 30 ++++- .../DashboardSnapshotPublisherTests.cs | 42 ++++++- .../DashboardSnapshotServiceTests.cs | 109 +++++++++++++++++- 7 files changed, 291 insertions(+), 19 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor index e7e96a2..a160fad 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor @@ -10,7 +10,7 @@ "Ready" or "Healthy" or "Active" => StatusState.Ok, "Creating" or "StartingWorker" or "WaitingForPipe" or "InitializingWorker" or "Closing" or "Stale" or "Degraded" or "Expiring" => StatusState.Warn, - "Faulted" or "Unavailable" or "Expired" => StatusState.Bad, + "Faulted" or "Unavailable" or "Expired" or "Unhealthy" => StatusState.Bad, _ => StatusState.Idle, }; } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshot.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshot.cs index 4ed16d0..4a2f68d 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshot.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshot.cs @@ -1,7 +1,33 @@ +using System.Text.Json.Serialization; using ZB.MOM.WW.MxGateway.Server.Configuration; namespace ZB.MOM.WW.MxGateway.Server.Dashboard; +/// +/// One dashboard refresh: gateway status, sessions, workers, metrics, faults, API +/// keys, the effective configuration, and the Galaxy summary. +/// +/// +/// is excluded from JSON serialization. It is +/// startup-static, yet it is the largest part of the record, and the only wire this +/// record crosses is the /hubs/snapshot tick sent every snapshot interval. +/// Remote clients receive it once per connection instead, as +/// (or on demand via +/// ). In-process pages read the +/// property directly and are unaffected. +/// +/// When the snapshot was built. +/// When the gateway process started. +/// Elapsed time since start. +/// Aggregate health-check status, or Unknown. +/// Server assembly version. +/// Most recent sessions, newest first. +/// Workers that are not closed. +/// Gateway counters and gauges. +/// Recent session and worker faults. +/// API key summaries (no secrets). +/// Effective configuration; not serialized on the hub tick. +/// Galaxy hierarchy cache summary. public sealed record DashboardSnapshot( DateTimeOffset GeneratedAt, DateTimeOffset GatewayStartedAt, @@ -13,5 +39,5 @@ public sealed record DashboardSnapshot( IReadOnlyList Metrics, IReadOnlyList Faults, IReadOnlyList ApiKeys, - EffectiveGatewayConfiguration Configuration, + [property: JsonIgnore] EffectiveGatewayConfiguration Configuration, DashboardGalaxySummary Galaxy); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs index 25b2e42..f6b9750 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs @@ -1,4 +1,5 @@ using System.Runtime.CompilerServices; +using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; @@ -14,7 +15,16 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard; public sealed class DashboardSnapshotService : IDashboardSnapshotService { - private const string HealthyStatus = "Healthy"; + /// + /// Status shown before the first health evaluation completes, when the health + /// evaluation fails, and when no is registered. + /// Deliberately not "Healthy": claiming health that was never checked is how the + /// dashboard used to show a green gateway status throughout an outage. + /// + internal const string UnknownStatus = "Unknown"; + + /// Upper bound on one health evaluation, so a hung check cannot stall the snapshot tick. + private static readonly TimeSpan GatewayStatusRefreshTimeout = TimeSpan.FromSeconds(2); /// /// Minimum spacing between API key list reads. The list is a SQLite query whose @@ -39,7 +49,10 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService private readonly int _recentFaultLimit; private readonly int _recentSessionLimit; private readonly ILogger _logger; + private readonly HealthCheckService? _healthCheckService; private readonly SemaphoreSlim _apiKeySummaryRefreshGate = new(1, 1); + private readonly SemaphoreSlim _gatewayStatusRefreshGate = new(1, 1); + private string _gatewayStatus = UnknownStatus; private IReadOnlyList _apiKeySummaries = Array.Empty(); private long _apiKeySummariesRefreshedAtTicks = NeverRefreshedTicks; // The effective configuration is built from IOptions and is startup-static: @@ -65,6 +78,11 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService /// Gateway configuration options. /// Provider for current time; defaults to system time. /// Optional logger for the dashboard snapshot service. + /// + /// The gateway's registered health checks. + /// is the aggregate status of one evaluation per snapshot tick; without it the + /// status stays . + /// public DashboardSnapshotService( ISessionRegistry sessionRegistry, GatewayMetrics metrics, @@ -73,7 +91,8 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService IApiKeyAdminStore apiKeyAdminStore, IOptions options, TimeProvider? timeProvider = null, - ILogger? logger = null) + ILogger? logger = null, + HealthCheckService? healthCheckService = null) { _sessionRegistry = sessionRegistry ?? throw new ArgumentNullException(nameof(sessionRegistry)); _metrics = metrics ?? throw new ArgumentNullException(nameof(metrics)); @@ -88,6 +107,7 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService _recentFaultLimit = options.Value.Dashboard.RecentFaultLimit; _recentSessionLimit = options.Value.Dashboard.RecentSessionLimit; _logger = logger ?? NullLogger.Instance; + _healthCheckService = healthCheckService; } /// @@ -111,7 +131,7 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService GeneratedAt: generatedAt, GatewayStartedAt: _gatewayStartedAt, GatewayUptime: generatedAt - _gatewayStartedAt, - GatewayStatus: HealthyStatus, + GatewayStatus: Volatile.Read(ref _gatewayStatus), GatewayVersion: typeof(DashboardSnapshotService).Assembly.GetName().Version?.ToString() ?? "unknown", Sessions: sessionSummaries, Workers: workerSummaries, @@ -171,6 +191,7 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService } await RefreshApiKeySummariesAsync(cancellationToken).ConfigureAwait(false); + await RefreshGatewayStatusAsync(cancellationToken).ConfigureAwait(false); yield return GetSnapshot(); using PeriodicTimer timer = new(_snapshotInterval, _timeProvider); @@ -192,10 +213,57 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService } await RefreshApiKeySummariesAsync(cancellationToken).ConfigureAwait(false); + await RefreshGatewayStatusAsync(cancellationToken).ConfigureAwait(false); yield return GetSnapshot(); } } + /// + /// Evaluates the registered health checks once and caches the aggregate status + /// for , which is synchronous and also serves page seeds + /// and hub connects between ticks. One evaluation per tick, not per + /// call: the checks include an auth-store query, and the + /// tick is already the cadence at which the dashboard refreshes. + /// + /// Token to cancel the evaluation. + /// A task that represents the asynchronous operation. + private async Task RefreshGatewayStatusAsync(CancellationToken cancellationToken) + { + if (_healthCheckService is null) + { + return; + } + + // Zero-timeout gate: a concurrent enumeration (the hub publisher and the page + // feed each run one) skips rather than queueing a second evaluation. + if (!await _gatewayStatusRefreshGate.WaitAsync(0, cancellationToken).ConfigureAwait(false)) + { + return; + } + + try + { + using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeout.CancelAfter(GatewayStatusRefreshTimeout); + HealthReport report = await _healthCheckService.CheckHealthAsync(timeout.Token).ConfigureAwait(false); + Volatile.Write(ref _gatewayStatus, report.Status.ToString()); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } + catch (Exception exception) + { + // A health evaluation that cannot complete is not evidence of health. + Volatile.Write(ref _gatewayStatus, UnknownStatus); + _logger.LogDebug(exception, "Dashboard gateway health evaluation failed."); + } + finally + { + _gatewayStatusRefreshGate.Release(); + } + } + private static DashboardSessionSummary CreateSessionSummary( GatewaySession session, GatewayMetricsSnapshot metricsSnapshot) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotHub.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotHub.cs index 4f340be..aa70d7e 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotHub.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotHub.cs @@ -1,13 +1,15 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.SignalR; +using ZB.MOM.WW.MxGateway.Server.Configuration; namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; /// /// SignalR hub that pushes a fresh on every -/// snapshot refresh. New connections receive the current snapshot -/// immediately via ; subsequent refreshes are -/// broadcast by . +/// snapshot refresh. New connections receive the effective configuration and the +/// current snapshot immediately via ; subsequent +/// refreshes are broadcast by without the +/// configuration, which is startup-static and serialized only once per connection. /// /// /// Connections are counted into @@ -22,13 +24,28 @@ public sealed class DashboardSnapshotHub( /// Method name used to push snapshot updates to clients. public const string SnapshotMessage = "SnapshotUpdated"; + /// + /// Method name of the one-per-connection effective-configuration push. Snapshot + /// ticks omit the configuration, so a client keeps the copy it got here. + /// + public const string ConfigurationMessage = "ConfigurationUpdated"; + + /// + /// Returns the effective gateway configuration — the same object + /// carries — for a client that needs it again. + /// + /// The effective configuration. + public EffectiveGatewayConfiguration GetConfiguration() => snapshotService.GetSnapshot().Configuration; + /// public override async Task OnConnectedAsync() { // Count the viewer before seeding it, so the publisher resumes its tick // no later than the first snapshot this connection renders. connectionCounter.Increment(); - await Clients.Caller.SendAsync(SnapshotMessage, snapshotService.GetSnapshot()).ConfigureAwait(false); + DashboardSnapshot snapshot = snapshotService.GetSnapshot(); + await Clients.Caller.SendAsync(ConfigurationMessage, snapshot.Configuration).ConfigureAwait(false); + await Clients.Caller.SendAsync(SnapshotMessage, snapshot).ConfigureAwait(false); await base.OnConnectedAsync().ConfigureAwait(false); } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotPublisher.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotPublisher.cs index 85701b2..3ad8c5f 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotPublisher.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotPublisher.cs @@ -26,11 +26,20 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; /// connected. While the count is zero the publisher does not advance the snapshot /// enumerator at all, so the producing iterator stays suspended and builds nothing. /// +/// +/// Each broadcast is bounded by a send timeout (one second by default, the default +/// snapshot interval). A broadcast completes only once every connection accepted the +/// frame, so awaiting it unbounded let one slow remote client delay every tick for +/// everyone; a send that misses the bound is abandoned and the next tick goes out on +/// time. The tick omits the effective configuration (see +/// ), so each broadcast is the volatile state only. +/// /// public sealed class DashboardSnapshotPublisher : BackgroundService { private static readonly TimeSpan DefaultReconnectDelay = TimeSpan.FromSeconds(5); private static readonly TimeSpan DefaultIdlePollInterval = TimeSpan.FromSeconds(1); + private static readonly TimeSpan DefaultSendTimeout = TimeSpan.FromSeconds(1); private readonly IDashboardSnapshotService _snapshotService; private readonly IHubContext _hubContext; @@ -38,6 +47,7 @@ public sealed class DashboardSnapshotPublisher : BackgroundService private readonly ILogger _logger; private readonly TimeSpan _reconnectDelay; private readonly TimeSpan _idlePollInterval; + private readonly TimeSpan _sendTimeout; /// Initializes a new instance of the DashboardSnapshotPublisher class. /// The snapshot service to subscribe to. @@ -64,13 +74,15 @@ public sealed class DashboardSnapshotPublisher : BackgroundService /// The logger instance. /// The delay before reconnecting after a subscription failure. /// How often the idle publisher re-checks for a connected viewer. + /// Upper bound on one broadcast; defaults to one second. internal DashboardSnapshotPublisher( IDashboardSnapshotService snapshotService, IHubContext hubContext, DashboardSnapshotHubConnectionCounter connectionCounter, ILogger logger, TimeSpan reconnectDelay, - TimeSpan idlePollInterval) + TimeSpan idlePollInterval, + TimeSpan? sendTimeout = null) { _snapshotService = snapshotService; _hubContext = hubContext; @@ -78,6 +90,7 @@ public sealed class DashboardSnapshotPublisher : BackgroundService _logger = logger; _reconnectDelay = reconnectDelay; _idlePollInterval = idlePollInterval; + _sendTimeout = sendTimeout ?? DefaultSendTimeout; } /// @@ -117,10 +130,19 @@ public sealed class DashboardSnapshotPublisher : BackgroundService try { - await _hubContext.Clients - .All - .SendAsync(DashboardSnapshotHub.SnapshotMessage, snapshot, stoppingToken) + bool sent = await BoundedHubSend.SendAsync( + _hubContext.Clients.All, + DashboardSnapshotHub.SnapshotMessage, + snapshot, + _sendTimeout, + stoppingToken) .ConfigureAwait(false); + if (!sent) + { + _logger.LogDebug( + "Snapshot broadcast exceeded {Timeout} and was abandoned; the next tick proceeds.", + _sendTimeout); + } } catch (Exception ex) when (ex is not OperationCanceledException) { diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotPublisherTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotPublisherTests.cs index 02607fc..b983377 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotPublisherTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotPublisherTests.cs @@ -138,6 +138,35 @@ public sealed class DashboardSnapshotPublisherTests Assert.True(snapshotService.PullCount >= 1); } + /// + /// A broadcast that never completes — one remote client whose transport is wedged — + /// is abandoned at the send timeout, so later ticks still go out instead of every + /// viewer waiting on the slowest one. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ExecuteAsync_WhenABroadcastNeverCompletes_LaterTicksStillGoOut() + { + CountingSnapshotService snapshotService = new(); + RecordingHubContext hubContext = new(); + hubContext.Proxy.StallFirstSend = true; + DashboardSnapshotPublisher publisher = new( + snapshotService, + hubContext, + ConnectedCounter(), + NullLogger.Instance, + TimeSpan.FromMilliseconds(50), + IdlePollInterval, + sendTimeout: TimeSpan.FromMilliseconds(50)); + + using CancellationTokenSource cts = new(); + await publisher.StartAsync(cts.Token).WaitAsync(TestTimeout); + await WaitUntilAsync(() => hubContext.SendCount >= 3); + + await cts.CancelAsync(); + await publisher.StopAsync(CancellationToken.None); + } + /// Creates a connection counter that already has one live viewer. /// A counter reporting a single connection. private static DashboardSnapshotHubConnectionCounter ConnectedCounter() @@ -271,6 +300,9 @@ public sealed class DashboardSnapshotPublisherTests /// Gets the group manager. public IGroupManager Groups { get; } = new NoopGroupManager(); + /// Gets the recording proxy every client lookup resolves to. + public RecordingClientProxy Proxy => _clients.AllProxy; + /// Gets the number of send calls recorded. public int SendCount => _clients.AllProxy.SendCount; } @@ -329,6 +361,12 @@ public sealed class DashboardSnapshotPublisherTests { private int _sendCount; + /// + /// Gets or sets a value indicating whether the first send never completes and + /// ignores its cancellation token — a wedged client transport. + /// + public bool StallFirstSend { get; set; } + /// Gets the number of send calls recorded. public int SendCount => Volatile.Read(ref _sendCount); @@ -339,8 +377,8 @@ public sealed class DashboardSnapshotPublisherTests /// A task that represents the asynchronous operation. public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default) { - Interlocked.Increment(ref _sendCount); - return Task.CompletedTask; + int count = Interlocked.Increment(ref _sendCount); + return StallFirstSend && count == 1 ? new TaskCompletionSource().Task : Task.CompletedTask; } } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotServiceTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotServiceTests.cs index 09efbf6..81b3357 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotServiceTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotServiceTests.cs @@ -1,4 +1,6 @@ using System.Globalization; +using System.Text.Json; +using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Options; using Microsoft.Extensions.Time.Testing; using ZB.MOM.WW.Auth.Abstractions.ApiKeys; @@ -18,7 +20,8 @@ namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Dashboard; public sealed class DashboardSnapshotServiceTests { /// - /// Verifies snapshot returns empty collections and healthy status when registry is empty. + /// Verifies snapshot returns empty collections when registry is empty, and an + /// Unknown status when no health check service is available to evaluate. /// [Fact] public void GetSnapshot_WhenRegistryEmpty_ReturnsEmptyOperationalState() @@ -32,10 +35,76 @@ public sealed class DashboardSnapshotServiceTests Assert.Empty(snapshot.Workers); Assert.Empty(snapshot.Faults); Assert.Contains(snapshot.Metrics, metric => metric.Name == "mxgateway.sessions.open" && metric.Value == 0); - Assert.Equal("Healthy", snapshot.GatewayStatus); + Assert.Equal(DashboardSnapshotService.UnknownStatus, snapshot.GatewayStatus); Assert.NotNull(snapshot.Configuration); } + /// + /// SEC-27: the gateway status is the aggregate of the registered health checks, + /// evaluated on the snapshot tick, not a hardcoded "Healthy". + /// + /// The aggregate status the health checks report. + /// A task that represents the asynchronous operation. + [Theory] + [InlineData(HealthStatus.Healthy)] + [InlineData(HealthStatus.Degraded)] + [InlineData(HealthStatus.Unhealthy)] + public async Task WatchSnapshotsAsync_ReportsTheHealthCheckStatus(HealthStatus status) + { + using GatewayMetrics metrics = new(); + StubHealthCheckService health = new(status); + DashboardSnapshotService service = CreateService(new SessionRegistry(), metrics, healthCheckService: health); + using CancellationTokenSource cancellation = new(); + await using IAsyncEnumerator enumerator = service + .WatchSnapshotsAsync(cancellation.Token) + .GetAsyncEnumerator(cancellation.Token); + + Assert.True(await enumerator.MoveNextAsync().AsTask().WaitAsync(TimeSpan.FromSeconds(5))); + + Assert.Equal(status.ToString(), enumerator.Current.GatewayStatus); + + // Cached for the synchronous seed path too, without a second evaluation. + Assert.Equal(status.ToString(), service.GetSnapshot().GatewayStatus); + Assert.Equal(1, health.CheckCount); + } + + /// A health evaluation that throws is reported as Unknown, never as Healthy. + /// A task that represents the asynchronous operation. + [Fact] + public async Task WatchSnapshotsAsync_WhenHealthEvaluationThrows_ReportsUnknown() + { + using GatewayMetrics metrics = new(); + StubHealthCheckService health = new(HealthStatus.Healthy) { Throw = true }; + DashboardSnapshotService service = CreateService(new SessionRegistry(), metrics, healthCheckService: health); + using CancellationTokenSource cancellation = new(); + await using IAsyncEnumerator enumerator = service + .WatchSnapshotsAsync(cancellation.Token) + .GetAsyncEnumerator(cancellation.Token); + + Assert.True(await enumerator.MoveNextAsync().AsTask().WaitAsync(TimeSpan.FromSeconds(5))); + + Assert.Equal(DashboardSnapshotService.UnknownStatus, enumerator.Current.GatewayStatus); + } + + /// + /// The startup-static configuration is not part of the serialized snapshot, so the + /// hub tick carries only volatile state; the hub sends it once per connection. + /// + [Fact] + public void GetSnapshot_WhenSerialized_OmitsTheConfiguration() + { + using GatewayMetrics metrics = new(); + DashboardSnapshotService service = CreateService(new SessionRegistry(), metrics); + + DashboardSnapshot snapshot = service.GetSnapshot(); + using JsonDocument json = JsonDocument.Parse(JsonSerializer.Serialize(snapshot, JsonSerializerOptions.Web)); + + Assert.NotNull(snapshot.Configuration); + Assert.False(json.RootElement.TryGetProperty("configuration", out _)); + Assert.True(json.RootElement.TryGetProperty("gatewayStatus", out _)); + Assert.True(json.RootElement.TryGetProperty("sessions", out _)); + } + /// /// Verifies snapshot projects active, faulted, and closed session states with worker and metrics data. /// @@ -615,7 +684,8 @@ public sealed class DashboardSnapshotServiceTests GatewayOptions? options = null, IGalaxyHierarchyCache? galaxyHierarchyCache = null, IApiKeyAdminStore? apiKeyAdminStore = null, - TimeProvider? timeProvider = null) + TimeProvider? timeProvider = null, + HealthCheckService? healthCheckService = null) { GatewayOptions resolvedOptions = options ?? new GatewayOptions { @@ -633,7 +703,38 @@ public sealed class DashboardSnapshotServiceTests galaxyHierarchyCache ?? new StubGalaxyHierarchyCache(GalaxyHierarchyCacheEntry.Empty), apiKeyAdminStore ?? new FakeApiKeyAdminStore(), Options.Create(resolvedOptions), - timeProvider); + timeProvider, + healthCheckService: healthCheckService); + } + + private sealed class StubHealthCheckService(HealthStatus status) : HealthCheckService + { + private int _checkCount; + + /// Gets or sets a value indicating whether the evaluation throws. + public bool Throw { get; set; } + + /// Gets the number of evaluations requested. + public int CheckCount => Volatile.Read(ref _checkCount); + + /// + public override Task CheckHealthAsync( + Func? predicate, + CancellationToken cancellationToken = default) + { + Interlocked.Increment(ref _checkCount); + if (Throw) + { + throw new InvalidOperationException("health evaluation failed"); + } + + return Task.FromResult(new HealthReport( + new Dictionary + { + ["stub"] = new(status, null, TimeSpan.Zero, null, null), + }, + TimeSpan.Zero)); + } } private sealed class StubGalaxyHierarchyCache(GalaxyHierarchyCacheEntry current) : IGalaxyHierarchyCache From 2a7d6facceef1b80c704d93d2d31a507da38d03d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:19:34 -0400 Subject: [PATCH 4/8] perf(dashboard): memoize AlarmsPage filtered rows and area list FilteredAlarms() ran the full filter+sort twice per render (Showing card and table) and Areas() once more, including every search keystroke render. Both now go through DashboardAlarmView, recomputed only when the alarm generation (bumped per poll) or the filter criteria change. --- .../Components/Pages/AlarmsPage.razor | 48 ++----- .../Dashboard/DashboardAlarmView.cs | 125 ++++++++++++++++++ .../Dashboard/DashboardAlarmViewTests.cs | 83 ++++++++++++ 3 files changed, 221 insertions(+), 35 deletions(-) create mode 100644 src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAlarmView.cs create mode 100644 src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardAlarmViewTests.cs diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor index b85b34e..690a6be 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor @@ -58,6 +58,8 @@ + @* FilteredAlarms() is memoized on (alarm generation, filter inputs), so this card and the + table below share one computation per change instead of filtering twice per render. *@ @@ -189,6 +191,10 @@ private int _maxSeverity = 1000; private string _search = string.Empty; + // Bumped whenever _alarms is replaced, so the memoized view recomputes exactly then. + private readonly DashboardAlarmView _view = new(); + private long _alarmGeneration; + // Upper bound on waiting for either background loop while disposing, mirroring // DashboardPageBase's snapshot-watch drain: both loops marshal renders through the // renderer's dispatcher and disposal can run on that same dispatcher, so the wait is @@ -295,42 +301,13 @@ return _workerPid is int pid ? $"{refreshed} · worker pid {pid}" : refreshed; } - private IReadOnlyList Areas() - { - return _alarms - .Select(alarm => alarm.Area) - .Where(area => !string.IsNullOrWhiteSpace(area)) - .Distinct(StringComparer.OrdinalIgnoreCase) - .OrderBy(area => area, StringComparer.OrdinalIgnoreCase) - .ToArray(); - } + private IReadOnlyList Areas() => _view.Areas(_alarmGeneration, _alarms); - private IReadOnlyList FilteredAlarms() - { - string query = _search.Trim(); - return _alarms - .Where(MatchesState) - .Where(alarm => _areaFilter.Length == 0 - || string.Equals(alarm.Area, _areaFilter, StringComparison.OrdinalIgnoreCase)) - .Where(alarm => alarm.Severity >= _minSeverity && alarm.Severity <= _maxSeverity) - .Where(alarm => query.Length == 0 - || alarm.Reference.Contains(query, StringComparison.OrdinalIgnoreCase) - || alarm.Source.Contains(query, StringComparison.OrdinalIgnoreCase) - || alarm.Description.Contains(query, StringComparison.OrdinalIgnoreCase)) - .OrderByDescending(alarm => alarm.Severity) - .ThenByDescending(alarm => alarm.LastTransition ?? DateTimeOffset.MinValue) - .ToArray(); - } - - private bool MatchesState(DashboardActiveAlarm alarm) - { - return alarm.State switch - { - AlarmConditionState.Active => _showActive, - AlarmConditionState.ActiveAcked => _showAcked, - _ => true, - }; - } + private IReadOnlyList FilteredAlarms() => + _view.Filtered( + _alarmGeneration, + new DashboardAlarmFilterCriteria(_showActive, _showAcked, _areaFilter, _minSeverity, _maxSeverity, _search), + _alarms); private static string StateText(AlarmConditionState state) { @@ -439,6 +416,7 @@ _lastRefresh = DateTimeOffset.UtcNow; _alarms.Clear(); _alarms.AddRange(result.Alarms); + _alarmGeneration++; _unackedCount = _alarms.Count(alarm => alarm.State == AlarmConditionState.Active); _ackedCount = _alarms.Count(alarm => alarm.State == AlarmConditionState.ActiveAcked); StateHasChanged(); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAlarmView.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAlarmView.cs new file mode 100644 index 0000000..afa4aa6 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAlarmView.cs @@ -0,0 +1,125 @@ +using ZB.MOM.WW.MxGateway.Contracts.Proto; + +namespace ZB.MOM.WW.MxGateway.Server.Dashboard; + +/// The Alarms page's filter inputs, compared by value to decide whether a filtered view is stale. +/// Include unacknowledged Active alarms. +/// Include ActiveAcked alarms. +/// Area to match (case-insensitive), or empty for all. +/// Lowest severity shown. +/// Highest severity shown. +/// Reference/source/description text search; trimmed before matching. +public sealed record DashboardAlarmFilterCriteria( + bool ShowActive, + bool ShowAcked, + string Area, + int MinSeverity, + int MaxSeverity, + string Search); + +/// +/// Memoized projections of the Alarms page's alarm list: the filtered, sorted rows and +/// the distinct area list. A render used to run the full filter-and-sort twice (the +/// "Showing" card and the table) and the area projection once more, on every render — +/// including the renders that typing in the search box triggers per keystroke. Each +/// projection is now recomputed only when the alarm list (its generation) or, for the +/// rows, the filter criteria change. +/// +/// +/// Owned by one page instance and touched only from its render dispatcher, so it is not +/// synchronized. +/// +public sealed class DashboardAlarmView +{ + private long _filteredGeneration = -1; + private DashboardAlarmFilterCriteria? _filteredCriteria; + private IReadOnlyList _filtered = []; + private long _areasGeneration = -1; + private IReadOnlyList _areas = []; + + /// Gets how many times the filtered rows were actually computed (diagnostic, for tests). + internal int FilterComputations { get; private set; } + + /// Gets how many times the area list was actually computed (diagnostic, for tests). + internal int AreaComputations { get; private set; } + + /// Returns the alarms matching , highest severity then newest first. + /// + /// Version of ; the caller bumps it whenever it replaces the list's contents. + /// + /// The current filter inputs. + /// The full active-alarm list. + /// The filtered rows; the same instance while generation and criteria are unchanged. + public IReadOnlyList Filtered( + long generation, + DashboardAlarmFilterCriteria criteria, + IReadOnlyList alarms) + { + ArgumentNullException.ThrowIfNull(criteria); + ArgumentNullException.ThrowIfNull(alarms); + + if (generation == _filteredGeneration && criteria == _filteredCriteria) + { + return _filtered; + } + + _filtered = Apply(criteria, alarms); + _filteredGeneration = generation; + _filteredCriteria = criteria; + FilterComputations++; + return _filtered; + } + + /// Returns the distinct non-blank areas of , sorted case-insensitively. + /// Version of . + /// The full active-alarm list. + /// The area list; the same instance while the generation is unchanged. + public IReadOnlyList Areas(long generation, IReadOnlyList alarms) + { + ArgumentNullException.ThrowIfNull(alarms); + + if (generation == _areasGeneration) + { + return _areas; + } + + _areas = alarms + .Select(alarm => alarm.Area) + .Where(area => !string.IsNullOrWhiteSpace(area)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(area => area, StringComparer.OrdinalIgnoreCase) + .ToArray(); + _areasGeneration = generation; + AreaComputations++; + return _areas; + } + + private static DashboardActiveAlarm[] Apply( + DashboardAlarmFilterCriteria criteria, + IReadOnlyList alarms) + { + string query = criteria.Search.Trim(); + return alarms + .Where(alarm => MatchesState(criteria, alarm)) + .Where(alarm => criteria.Area.Length == 0 + || string.Equals(alarm.Area, criteria.Area, StringComparison.OrdinalIgnoreCase)) + .Where(alarm => alarm.Severity >= criteria.MinSeverity && alarm.Severity <= criteria.MaxSeverity) + .Where(alarm => query.Length == 0 + || alarm.Reference.Contains(query, StringComparison.OrdinalIgnoreCase) + || alarm.Source.Contains(query, StringComparison.OrdinalIgnoreCase) + || alarm.Description.Contains(query, StringComparison.OrdinalIgnoreCase)) + .OrderByDescending(alarm => alarm.Severity) + .ThenByDescending(alarm => alarm.LastTransition ?? DateTimeOffset.MinValue) + .ToArray(); + } + + private static bool MatchesState(DashboardAlarmFilterCriteria criteria, DashboardActiveAlarm alarm) + { + return alarm.State switch + { + AlarmConditionState.Active => criteria.ShowActive, + AlarmConditionState.ActiveAcked => criteria.ShowAcked, + _ => true, + }; + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardAlarmViewTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardAlarmViewTests.cs new file mode 100644 index 0000000..d9a5645 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardAlarmViewTests.cs @@ -0,0 +1,83 @@ +using ZB.MOM.WW.MxGateway.Contracts.Proto; +using ZB.MOM.WW.MxGateway.Server.Dashboard; + +namespace ZB.MOM.WW.MxGateway.Tests.Dashboard; + +/// +/// Verifies the Alarms page's memoized projections: one computation per change of +/// the alarm generation or filter inputs, however many times a render asks. +/// +public sealed class DashboardAlarmViewTests +{ + private static readonly DashboardAlarmFilterCriteria DefaultCriteria = + new(ShowActive: true, ShowAcked: false, Area: string.Empty, MinSeverity: 0, MaxSeverity: 1000, Search: string.Empty); + + /// Repeated reads at the same generation and criteria reuse one computation. + [Fact] + public void Filtered_WithUnchangedInputs_ComputesOnce() + { + DashboardAlarmView view = new(); + DashboardActiveAlarm[] alarms = [Alarm("A", 500), Alarm("B", 800)]; + + IReadOnlyList first = view.Filtered(1, DefaultCriteria, alarms); + IReadOnlyList second = view.Filtered(1, DefaultCriteria with { }, alarms); + + Assert.Same(first, second); + Assert.Equal(1, view.FilterComputations); + Assert.Equal(["B", "A"], first.Select(alarm => alarm.Reference)); + } + + /// A changed filter input or a new alarm generation recomputes. + [Fact] + public void Filtered_WhenCriteriaOrGenerationChange_Recomputes() + { + DashboardAlarmView view = new(); + DashboardActiveAlarm[] alarms = [Alarm("Tank01.Hi", 500), Alarm("Pump02.Lo", 800)]; + + view.Filtered(1, DefaultCriteria, alarms); + IReadOnlyList searched = view.Filtered(1, DefaultCriteria with { Search = "tank" }, alarms); + + Assert.Equal(2, view.FilterComputations); + Assert.Equal(["Tank01.Hi"], searched.Select(alarm => alarm.Reference)); + + DashboardActiveAlarm[] refreshed = [Alarm("Tank03.Hi", 900)]; + IReadOnlyList next = view.Filtered(2, DefaultCriteria with { Search = "tank" }, refreshed); + + Assert.Equal(3, view.FilterComputations); + Assert.Equal(["Tank03.Hi"], next.Select(alarm => alarm.Reference)); + } + + /// The area list is computed once per alarm generation, independent of filter inputs. + [Fact] + public void Areas_ComputesOncePerGeneration() + { + DashboardAlarmView view = new(); + DashboardActiveAlarm[] alarms = [Alarm("A", 1, "north"), Alarm("B", 1, "North"), Alarm("C", 1, "east"), Alarm("D", 1, " ")]; + + IReadOnlyList first = view.Areas(1, alarms); + IReadOnlyList second = view.Areas(1, alarms); + + Assert.Same(first, second); + Assert.Equal(1, view.AreaComputations); + Assert.Equal(["east", "north"], first); + + view.Areas(2, alarms); + Assert.Equal(2, view.AreaComputations); + } + + private static DashboardActiveAlarm Alarm(string reference, int severity, string area = "") + { + return new DashboardActiveAlarm( + Reference: reference, + Provider: string.Empty, + Area: area, + Source: string.Empty, + AlarmType: string.Empty, + Severity: severity, + State: AlarmConditionState.Active, + LastTransition: null, + OperatorUser: string.Empty, + OperatorComment: string.Empty, + Description: string.Empty); + } +} From 5d4aefc99c1ef9160b1e950262c5efb3972b2702 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:22:17 -0400 Subject: [PATCH 5/8] fix(dashboard): per-viewer share of the live-data advise set The 256-tag LRU was global, so two Browse circuits on different areas evicted each other's whole sets every poll. Tags now record the viewer that read them last; each active viewer (read within 30s) owns MaxSubscribedTags/active (floor 32). Eviction takes other viewers' excess over their share first (inactive viewers' share is zero), then the reader's own untouched tags, and never strips a neighbour within its share (soft cap). Single-viewer behaviour is unchanged. IDashboardLiveDataService gains a viewerId ReadAsync overload. --- .../Dashboard/DashboardLiveDataService.cs | 222 +++++++++++++++--- .../Dashboard/IDashboardLiveDataService.cs | 18 ++ .../DashboardLiveDataServiceTests.cs | 70 +++++- 3 files changed, 279 insertions(+), 31 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs index 2239c65..a1aa716 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs @@ -34,20 +34,38 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync // // The bound is per-read, not absolute: a read may never evict a tag it is itself // about to return, so a single read of more distinct tags than the cap leaves the - // set that large. The invariant EvictForAsync actually maintains is + // set that large. With a single viewer the invariant EvictForAsync maintains is // // |advise set| after a read <= max(MaxSubscribedTags, distinct tags in that read) // - // and any overshoot is squeezed back out by the next read that subscribes a tag + // (with several active viewers the per-viewer share below can hold it higher, by + // at most what the other viewers hold within their shares) and any overshoot is squeezed back out by the next read that subscribes a tag // (see EvictForAsync). A browse page requests far fewer tags than the cap, so in // practice the set settles at MaxSubscribedTags. private const int MaxSubscribedTags = 256; + // Per-viewer soft cap: MaxSubscribedTags is shared by every active viewer (Browse + // circuit), so each owns at most MaxSubscribedTags / activeViewers tags before its + // own least-recently-read tags become the first eviction candidates — never fewer + // than this floor. Without the split, two circuits browsing different areas + // thrashed the single global LRU: each read evicted the other circuit's whole set, + // which then re-advised it two seconds later. + private const int MinTagsPerViewer = 32; + + // A viewer that has not read within this window no longer counts as active: its + // share is released and its tags become the first eviction candidates. Generous + // against the Browse page's 2 s poll, so a briefly paused circuit keeps its share. + private static readonly TimeSpan ActiveViewerWindow = TimeSpan.FromSeconds(30); + + // Viewer id for callers that do not identify themselves. + private const string SharedViewerId = ""; + private static readonly TimeSpan ReadTimeout = TimeSpan.FromSeconds(5); private readonly ISessionManager _sessionManager; private readonly IGatewayAlarmService _alarmService; private readonly ILogger _logger; + private readonly TimeProvider _timeProvider; private readonly SemaphoreSlim _gate = new(1, 1); /// @@ -70,6 +88,10 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync private readonly LinkedList _recency = new(); + // Per-viewer ownership and activity. A tag is owned by the viewer that read it + // last; only touched under _gate. + private readonly Dictionary _viewers = new(StringComparer.Ordinal); + private GatewaySession? _session; private int _serverHandle; private bool _disposed; @@ -79,22 +101,31 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync /// Gateway central alarm service. /// Gateway options supplying Dashboard:ShowTagValues. /// Diagnostic logger. + /// Clock for viewer activity; defaults to system time. public DashboardLiveDataService( ISessionManager sessionManager, IGatewayAlarmService alarmService, IOptions options, - ILogger logger) + ILogger logger, + TimeProvider? timeProvider = null) { ArgumentNullException.ThrowIfNull(options); _sessionManager = sessionManager ?? throw new ArgumentNullException(nameof(sessionManager)); _alarmService = alarmService ?? throw new ArgumentNullException(nameof(alarmService)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _showTagValues = options.Value.Dashboard.ShowTagValues; + _timeProvider = timeProvider ?? TimeProvider.System; } + /// + public Task ReadAsync( + IReadOnlyCollection tagAddresses, + CancellationToken cancellationToken) => ReadAsync(tagAddresses, viewerId: null, cancellationToken); + /// public async Task ReadAsync( IReadOnlyCollection tagAddresses, + string? viewerId, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(tagAddresses); @@ -109,15 +140,19 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync (GatewaySession session, int serverHandle) = await EnsureReadyAsync(cancellationToken) .ConfigureAwait(false); - string[] toSubscribe = TouchAndCollectNewTags(tagAddresses, out int justReadCount); + string viewer = viewerId ?? SharedViewerId; + DateTimeOffset now = _timeProvider.GetUtcNow(); + TouchViewer(viewer, now); + + string[] toSubscribe = TouchAndCollectNewTags(tagAddresses, viewer, out HashSet justRead); if (toSubscribe.Length > 0) { - await EvictForAsync(session, serverHandle, toSubscribe.Length, justReadCount, cancellationToken) + await EvictForAsync(session, serverHandle, toSubscribe.Length, justRead, viewer, now, cancellationToken) .ConfigureAwait(false); IReadOnlyList subscribeResults = await session .SubscribeBulkAsync(serverHandle, toSubscribe, cancellationToken) .ConfigureAwait(false); - TrackSubscribed(toSubscribe, subscribeResults); + TrackSubscribed(toSubscribe, subscribeResults, viewer); } IReadOnlyList results = await session @@ -175,18 +210,20 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync } // Promotes every already-advised tag in this read to the front of the recency - // list and returns the tags that still need subscribing (distinct, in request - // order). `justReadCount` is how many distinct tags of this read were already - // advised — they now occupy the front of the list and must never be evicted to - // make room for the same read's new tags. Callers must hold _gate. + // list, hands its ownership to the reading viewer, and returns the tags that + // still need subscribing (distinct, in request order). `justRead` is every + // distinct tag of this read — none of them may be evicted to make room for the + // same read's new tags. Callers must hold _gate. // // Every tag of one read is equally recently read; the recency list needs a total // order anyway, so the whole service uses one tie-break: later in the request wins. // Promoting in request order gives that here, and TrackSubscribed inserts new tags // the same way. - private string[] TouchAndCollectNewTags(IReadOnlyCollection tagAddresses, out int justReadCount) + private string[] TouchAndCollectNewTags( + IReadOnlyCollection tagAddresses, + string viewer, + out HashSet justRead) { - int touched = 0; List toSubscribe = []; HashSet seen = new(StringComparer.OrdinalIgnoreCase); @@ -200,10 +237,14 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync _recency.AddFirst(node); } - if (seen.Add(tag)) + if (!string.Equals(node.Value.Owner, viewer, StringComparison.Ordinal)) { - touched++; + AdjustOwned(node.Value.Owner, -1); + AdjustOwned(viewer, +1); + node.Value = node.Value with { Owner = viewer }; } + + seen.Add(tag); } else if (seen.Add(tag)) { @@ -211,7 +252,7 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync } } - justReadCount = touched; + justRead = seen; return [.. toSubscribe]; } @@ -221,7 +262,17 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync // from tracking regardless, and the session-invalidation path already handles // gateway/worker drift. Callers must hold _gate. // - // Eviction stops at the tags this read just touched (`justReadCount`), so a read + // Victims are chosen in two passes, each least-recently-read first: + // 1. tags of OTHER viewers that are over their share (PerViewerCap; an + // inactive viewer's share is zero); + // 2. the reading viewer's own tags that this read did not touch. + // A viewer within its share is never evicted by someone else's read, so the + // cap is soft: when every other viewer sits within its share and the reader's + // own older tags run out, the set ends over MaxSubscribedTags rather than + // stripping a neighbour. With one viewer, pass 1 is empty and pass 2 is the + // plain global LRU. + // + // Eviction never touches the tags this read just touched (`justRead`), so a read // whose own distinct tags outnumber the cap ends over it — see MaxSubscribedTags // for the exact invariant. That overshoot is not sticky: the next read that // subscribes anything computes `overflow` against the oversized set and evicts the @@ -236,26 +287,56 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync GatewaySession session, int serverHandle, int incomingCount, - int justReadCount, + HashSet justRead, + string reader, + DateTimeOffset now, CancellationToken cancellationToken) { int overflow = _subscribed.Count + incomingCount - MaxSubscribedTags; - int evictable = _subscribed.Count - justReadCount; - int evictCount = Math.Min(overflow, evictable); - if (evictCount <= 0) + if (overflow <= 0) { return; } - List evictedHandles = new(evictCount); - for (int i = 0; i < evictCount && _recency.Last is { } oldest; i++) + int share = PerViewerCap(now); + List evictedHandles = []; + int evictCount = 0; + + // Pass 1: other viewers over their share. + for (LinkedListNode? node = _recency.Last; node is not null && overflow > 0;) { - _recency.RemoveLast(); - _subscribed.Remove(oldest.Value.TagAddress); - if (oldest.Value.ItemHandle != 0) + LinkedListNode? previous = node.Previous; + string owner = node.Value.Owner; + if (!string.Equals(owner, reader, StringComparison.Ordinal) + && !justRead.Contains(node.Value.TagAddress) + && OwnedCount(owner) > (IsActive(owner, now) ? share : 0)) { - evictedHandles.Add(oldest.Value.ItemHandle); + Evict(node, evictedHandles); + overflow--; + evictCount++; } + + node = previous; + } + + // Pass 2: the reader's own tags this read did not touch. + for (LinkedListNode? node = _recency.Last; node is not null && overflow > 0;) + { + LinkedListNode? previous = node.Previous; + if (string.Equals(node.Value.Owner, reader, StringComparison.Ordinal) + && !justRead.Contains(node.Value.TagAddress)) + { + Evict(node, evictedHandles); + overflow--; + evictCount++; + } + + node = previous; + } + + if (evictCount == 0) + { + return; } _logger.LogDebug( @@ -282,12 +363,81 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync } } + // Removes one tag from tracking, releasing its owner's count and collecting its + // item handle for the batch unadvise. Callers must hold _gate. + private void Evict(LinkedListNode node, List evictedHandles) + { + _recency.Remove(node); + _subscribed.Remove(node.Value.TagAddress); + AdjustOwned(node.Value.Owner, -1); + if (node.Value.ItemHandle != 0) + { + evictedHandles.Add(node.Value.ItemHandle); + } + } + + // Each active viewer's share of MaxSubscribedTags, floored at MinTagsPerViewer. + // Callers must hold _gate. + private int PerViewerCap(DateTimeOffset now) + { + int active = _viewers.Values.Count(state => now - state.LastReadAt <= ActiveViewerWindow); + return Math.Max(MinTagsPerViewer, MaxSubscribedTags / Math.Max(1, active)); + } + + private bool IsActive(string viewer, DateTimeOffset now) => + _viewers.TryGetValue(viewer, out ViewerState? state) && now - state.LastReadAt <= ActiveViewerWindow; + + private int OwnedCount(string viewer) => + _viewers.TryGetValue(viewer, out ViewerState? state) ? state.OwnedCount : 0; + + // Records a read by `viewer` and forgets viewers that are both inactive and own + // nothing, so the map does not grow one entry per Browse page ever opened. + // Callers must hold _gate. + private void TouchViewer(string viewer, DateTimeOffset now) + { + if (!_viewers.TryGetValue(viewer, out ViewerState? state)) + { + state = new ViewerState(); + _viewers[viewer] = state; + } + + state.LastReadAt = now; + + foreach (KeyValuePair entry in _viewers.ToArray()) + { + if (entry.Value.OwnedCount <= 0 && now - entry.Value.LastReadAt > ActiveViewerWindow) + { + _viewers.Remove(entry.Key); + } + } + } + + // Callers must hold _gate. + private void AdjustOwned(string viewer, int delta) + { + if (!_viewers.TryGetValue(viewer, out ViewerState? state)) + { + if (delta <= 0) + { + return; + } + + state = new ViewerState { LastReadAt = DateTimeOffset.MinValue }; + _viewers[viewer] = state; + } + + state.OwnedCount = Math.Max(0, state.OwnedCount + delta); + } + // Records the freshly advised tags as the most recently read, keeping each // tag's item handle so eviction can unadvise it. Tags the worker failed to // advise are still tracked (matching the pre-cap behaviour of not retrying // them on every read) but carry no handle, so eviction just forgets them. // Callers must hold _gate. - private void TrackSubscribed(IReadOnlyList tagAddresses, IReadOnlyList results) + private void TrackSubscribed( + IReadOnlyList tagAddresses, + IReadOnlyList results, + string viewer) { Dictionary handles = new(results.Count, StringComparer.OrdinalIgnoreCase); foreach (SubscribeResult result in results) @@ -303,7 +453,8 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync foreach (string tag in tagAddresses) { handles.TryGetValue(tag, out int itemHandle); - _subscribed[tag] = _recency.AddFirst(new SubscribedTag(tag, itemHandle)); + _subscribed[tag] = _recency.AddFirst(new SubscribedTag(tag, itemHandle, viewer)); + AdjustOwned(viewer, +1); } } @@ -314,6 +465,10 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync { _subscribed.Clear(); _recency.Clear(); + foreach (ViewerState state in _viewers.Values) + { + state.OwnedCount = 0; + } } // Returns a Ready session + its Register server handle, opening a fresh @@ -424,5 +579,14 @@ public sealed class DashboardLiveDataService : IDashboardLiveDataService, IAsync // One entry of the advise set. ItemHandle is the handle the worker bound for // the tag, or 0 when the subscribe failed and there is nothing to unadvise. - private readonly record struct SubscribedTag(string TagAddress, int ItemHandle); + // Owner is the viewer that read the tag most recently. + private readonly record struct SubscribedTag(string TagAddress, int ItemHandle, string Owner); + + // One viewer's slice of the advise set. + private sealed class ViewerState + { + public int OwnedCount { get; set; } + + public DateTimeOffset LastReadAt { get; set; } + } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardLiveDataService.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardLiveDataService.cs index 698cc5b..e37306a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardLiveDataService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardLiveDataService.cs @@ -20,6 +20,24 @@ public interface IDashboardLiveDataService IReadOnlyCollection tagAddresses, CancellationToken cancellationToken); + /// + /// Reads the supplied tags on behalf of one viewer (a Browse page circuit). + /// The shared advise set is budgeted per active viewer, so the viewer id is + /// what stops one circuit's reads from evicting another circuit's whole set. + /// Never throws, as . + /// + /// Fully-qualified tag references to read. + /// + /// Stable id of the reading viewer for the life of its page; + /// reads under one shared anonymous viewer. + /// + /// Token to cancel the read. + /// The read result, or an error-bearing result on failure. + Task ReadAsync( + IReadOnlyCollection tagAddresses, + string? viewerId, + CancellationToken cancellationToken) => ReadAsync(tagAddresses, cancellationToken); + /// /// Queries the currently-active alarm set for the dashboard session. /// Never throws — failures are surfaced in diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardLiveDataServiceTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardLiveDataServiceTests.cs index 820e29b..af52910 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardLiveDataServiceTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardLiveDataServiceTests.cs @@ -2,6 +2,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Time.Testing; using ZB.MOM.WW.MxGateway.Contracts.Proto; using ZB.MOM.WW.MxGateway.Server.Configuration; using ZB.MOM.WW.MxGateway.Server.Dashboard; @@ -135,6 +136,69 @@ public sealed class DashboardLiveDataServiceTests Assert.Equal(5, worker.UnsubscribedHandles.Count); } + /// + /// Verifies the per-viewer share: with two active circuits each owns half the cap, + /// so a second circuit's large read evicts only the first circuit's excess above + /// its share — never its whole set — and keeps every tag it read itself. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ReadAsync_WithTwoViewers_EvictsOnlyTheOtherViewersExcessOverItsShare() + { + RecordingWorkerClient worker = new(); + await using FakeSessionManager sessionManager = new(worker); + await using DashboardLiveDataService service = CreateService(sessionManager); + + string[] first = CreateTagAddresses(200, "First"); + string[] second = CreateTagAddresses(200, "Second"); + await service.ReadAsync(first, "circuit-a", CancellationToken.None); + DashboardLiveReadResult result = await service.ReadAsync(second, "circuit-b", CancellationToken.None); + + Assert.Null(result.Error); + + // Two active viewers -> share of 128 each. circuit-a drops its 72 least-recently-read + // tags and keeps 128; under the old global LRU it lost 144 (400 - 256). + int share = MaxSubscribedTags / 2; + Assert.Equal(first[..(first.Length - share)].Select(worker.HandleFor), worker.UnsubscribedHandles); + Assert.DoesNotContain(worker.UnsubscribedHandles, handle => second.Select(worker.HandleFor).Contains(handle)); + + // circuit-a re-reading its retained tags subscribes nothing new and evicts nothing. + worker.UnsubscribedHandles.Clear(); + int subscribedBefore = worker.SubscribedTags.Count; + await service.ReadAsync(first[^share..], "circuit-a", CancellationToken.None); + Assert.Equal(subscribedBefore, worker.SubscribedTags.Count); + Assert.Empty(worker.UnsubscribedHandles); + } + + /// + /// Verifies an inactive viewer (a closed circuit that stopped polling) loses its + /// share: its tags are the first eviction candidates once the activity window passes. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ReadAsync_WhenAViewerGoesInactive_ItsTagsAreEvictedFirst() + { + RecordingWorkerClient worker = new(); + await using FakeSessionManager sessionManager = new(worker); + FakeTimeProvider clock = new(); + await using DashboardLiveDataService service = CreateService(sessionManager, timeProvider: clock); + + string[] stale = CreateTagAddresses(100, "Stale"); + string[] live = CreateTagAddresses(156, "Live"); + await service.ReadAsync(live, "open-circuit", CancellationToken.None); + await service.ReadAsync(stale, "closed-circuit", CancellationToken.None); + Assert.Empty(worker.UnsubscribedHandles); + + clock.Advance(TimeSpan.FromMinutes(1)); + + // The stale tags were read after the live ones, so a global LRU would evict live[10] + // (the oldest tag this read does not touch). The stale owner is inactive, though, so + // its share is zero and its oldest tag goes first. + await service.ReadAsync([.. live[..10], "Live.New.PV"], "open-circuit", CancellationToken.None); + + Assert.Equal([worker.HandleFor(stale[0])], worker.UnsubscribedHandles); + } + /// /// Verifies a tag the worker failed to advise still occupies a slot but is evicted /// without any unsubscribe command — there is no item handle to unadvise. @@ -280,7 +344,8 @@ public sealed class DashboardLiveDataServiceTests private static DashboardLiveDataService CreateService( ISessionManager sessionManager, - bool showTagValues = false) + bool showTagValues = false, + TimeProvider? timeProvider = null) { GatewayOptions gatewayOptions = new() { @@ -291,7 +356,8 @@ public sealed class DashboardLiveDataServiceTests sessionManager, new FakeGatewayAlarmService(), Options.Create(gatewayOptions), - NullLogger.Instance); + NullLogger.Instance, + timeProvider); } private static string[] CreateTagAddresses(int count, string prefix = "Tank") From db7833609dddc8b54f5b2c71c7bbd935257798cd Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:25:46 -0400 Subject: [PATCH 6/8] fix(dashboard): debounced off-dispatcher Browse search, bounded disposal, circuit-aware polling - Search: the oninput setter scanned the whole Galaxy synchronously per keystroke. It now debounces 150ms, runs on the thread pool with cancellation (newer keystroke supersedes), and reads a shared singleton DashboardBrowseSearchIndex of pre-lowercased references keyed on the Galaxy cache Sequence. - Disposal: DisposeAsync awaited both loops unbounded and the loops caught only OCE. Loops now handle faults per iteration (poll faults surface in _readError, deploy-notifier faults in a retrying warning), mutate the tree on the dispatcher, drain concurrently with a 5s bound, and the CTSs are disposed in finally. - Circuits: DisconnectedCircuitRetentionPeriod is 30s (was the 3 min default) and a scoped DashboardCircuitConnectionTracker (CircuitHandler) lets the 2s ReadBulk poll skip ticks while the browser is disconnected. - Reads pass a per-page viewer id to the live-data per-viewer share. --- .../Components/Pages/BrowsePage.razor | 324 ++++++++++++++---- .../Dashboard/DashboardBrowseSearchIndex.cs | 117 +++++++ .../DashboardCircuitConnectionTracker.cs | 42 +++ .../DashboardServiceCollectionExtensions.cs | 22 +- .../DashboardBrowseSearchIndexTests.cs | 95 +++++ .../DashboardHubsRegistrationTests.cs | 41 +++ 6 files changed, 573 insertions(+), 68 deletions(-) create mode 100644 src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardBrowseSearchIndex.cs create mode 100644 src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardCircuitConnectionTracker.cs create mode 100644 src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardBrowseSearchIndexTests.cs diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/BrowsePage.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/BrowsePage.razor index 6ff5265..33abb90 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/BrowsePage.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/BrowsePage.razor @@ -4,6 +4,8 @@ @inject IDashboardLiveDataService LiveData @inject IDashboardBrowseService BrowseService @inject IGalaxyDeployNotifier DeployNotifier +@inject DashboardBrowseSearchIndex SearchIndex +@inject DashboardCircuitConnectionTracker Circuit @using ZB.MOM.WW.GalaxyRepository @using ZB.MOM.WW.GalaxyRepository.Grpc @@ -36,7 +38,15 @@ } else if (!string.IsNullOrWhiteSpace(Search)) { - @if (_searchMatches.Count == 0) + @if (!string.IsNullOrWhiteSpace(_searchError)) + { +
Search failed: @_searchError
+ } + else if (_searchPending && _searchMatches.Count == 0) + { +
Searching…
+ } + else if (_searchMatches.Count == 0) {
No attributes match “@Search”.
} @@ -115,6 +125,13 @@
Live read failed: @_readError
} + @if (!string.IsNullOrWhiteSpace(_deployError)) + { +
+ Galaxy redeploy notifications failed (retrying): @_deployError +
+ } + @if (_subscribed.Count == 0) {
@@ -194,13 +211,35 @@ @code { private const int SearchResultLimit = 300; + // Keystrokes inside this window coalesce into one search: typing a tag name no + // longer rescans the Galaxy once per character. + private static readonly TimeSpan SearchDebounce = TimeSpan.FromMilliseconds(150); + + // Delay before resubscribing to deploy notifications after the stream ends or faults. + private static readonly TimeSpan DeployResubscribeDelay = TimeSpan.FromSeconds(5); + + // Upper bound on waiting for the page's background loops while disposing, the same + // bound DashboardPageBase and AlarmsPage use: the loops marshal renders through the + // renderer's dispatcher and disposal can run on that same dispatcher, so the wait is + // bounded rather than unconditional. + private static readonly TimeSpan LoopDrainTimeout = TimeSpan.FromSeconds(5); + + // Identifies this page to the shared live-data service, which budgets the advise set + // per viewer so another circuit's reads cannot evict this page's whole panel. + private readonly string _viewerId = "browse-" + Guid.NewGuid().ToString("N"); + private List _roots = []; private ulong _cacheSequence; private string? _staleBanner; - private CancellationTokenSource _deployCts = new(); + private readonly CancellationTokenSource _deployCts = new(); private Task? _deployTask; + private string? _deployError; private string _search = string.Empty; private IReadOnlyList _searchMatches = []; + private bool _searchPending; + private string? _searchError; + private CancellationTokenSource? _searchCts; + private Task? _searchTask; private readonly List _subscribed = []; private readonly Dictionary _values = new(StringComparer.Ordinal); // Per-tag bookkeeping for the Updated column: the signature of the value @@ -225,8 +264,8 @@ BrowseLevelResult roots = BrowseService.GetRoots(new BrowseFilterArgs()); _roots = [.. roots.Nodes]; _cacheSequence = roots.CacheSequence; - _pollTask = PollLoopAsync(); - _deployTask = SubscribeToDeployEventsAsync(); + _pollTask = PollLoopAsync(_cts.Token); + _deployTask = SubscribeToDeployEventsAsync(_deployCts.Token); } private async Task LoadChildrenAsync(DashboardBrowseNode node) @@ -249,30 +288,55 @@ await InvokeAsync(StateHasChanged); } - private async Task SubscribeToDeployEventsAsync() + // Faults are handled inside the loop: a notifier stream that faults or ends is + // surfaced on the page and resubscribed after a delay, so the tree does not silently + // stop tracking redeploys. Only cancellation ends the loop, so DrainAsync never + // observes a faulted task. + private async Task SubscribeToDeployEventsAsync(CancellationToken cancellationToken) { - try + while (!cancellationToken.IsCancellationRequested) { - await foreach (GalaxyDeployEventInfo info in DeployNotifier - .SubscribeAsync(_deployCts.Token) - .ConfigureAwait(false)) + try { - // First Latest replay echoes the sequence we already projected - // from — skip those to avoid a spurious "redeployed" banner. - if (info.Sequence == 0 || (ulong)info.Sequence == _cacheSequence) + await foreach (GalaxyDeployEventInfo info in DeployNotifier + .SubscribeAsync(cancellationToken) + .ConfigureAwait(false)) { - continue; - } + // First Latest replay echoes the sequence we already projected + // from — skip those to avoid a spurious "redeployed" banner. + if (info.Sequence == 0 || (ulong)info.Sequence == _cacheSequence) + { + continue; + } - BrowseLevelResult roots = BrowseService.GetRoots(new BrowseFilterArgs()); - _roots = [.. roots.Nodes]; - _cacheSequence = roots.CacheSequence; - _staleBanner = "Galaxy redeployed — tree refreshed."; - await InvokeAsync(StateHasChanged); + await InvokeAsync(() => + { + BrowseLevelResult roots = BrowseService.GetRoots(new BrowseFilterArgs()); + _roots = [.. roots.Nodes]; + _cacheSequence = roots.CacheSequence; + _staleBanner = "Galaxy redeployed — tree refreshed."; + _deployError = null; + StateHasChanged(); + }).ConfigureAwait(false); + } + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + return; + } + catch (Exception ex) + { + await ReportAsync(() => _deployError = ex.Message).ConfigureAwait(false); + } + + try + { + await Task.Delay(DeployResubscribeDelay, cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; } - } - catch (OperationCanceledException) - { } } @@ -288,42 +352,101 @@ + $"{entry.AlarmAttributeCount:N0} alarm attributes"; } + // Bound to the filter box with @bind:event="oninput", so the setter runs once per + // keystroke on the render dispatcher. It only schedules the search; the scan itself + // runs after the debounce, off the dispatcher, and a newer keystroke cancels it. private string Search { get => _search; set { _search = value ?? string.Empty; - _searchMatches = ComputeSearch(_search); + ScheduleSearch(_search); } } - private IReadOnlyList ComputeSearch(string rawQuery) + private void ScheduleSearch(string query) { - string query = rawQuery.Trim(); - if (query.Length == 0) + CancelQuietly(_searchCts); + _searchCts = null; + _searchError = null; + + if (query.Trim().Length == 0) { - return []; + _searchMatches = []; + _searchPending = false; + return; } - List matches = []; - foreach (GalaxyObject galaxyObject in GalaxyCache.Current.Objects) + // Linked to the page token, so disposal cancels an in-flight search too. The + // search task owns the source and disposes it when it finishes. + CancellationTokenSource searchCts = CancellationTokenSource.CreateLinkedTokenSource(_cts.Token); + _searchCts = searchCts; + _searchPending = true; + _searchTask = RunSearchAsync(query, searchCts); + } + + private async Task RunSearchAsync(string query, CancellationTokenSource searchCts) + { + CancellationToken cancellationToken = searchCts.Token; + try { - foreach (GalaxyAttribute attr in galaxyObject.Attributes) + await Task.Delay(SearchDebounce, cancellationToken).ConfigureAwait(false); + GalaxyHierarchyCacheEntry entry = GalaxyCache.Current; + IReadOnlyList matches = await Task.Run( + () => SearchIndex.Search(entry, query, SearchResultLimit, cancellationToken), + cancellationToken) + .ConfigureAwait(false); + + await InvokeAsync(() => { - if (attr.FullTagReference.Contains(query, StringComparison.OrdinalIgnoreCase) - || attr.AttributeName.Contains(query, StringComparison.OrdinalIgnoreCase)) + // Only the latest query may publish: a slower, superseded scan that + // finishes late must not overwrite newer results. + if (ReferenceEquals(_searchCts, searchCts) && !cancellationToken.IsCancellationRequested) { - matches.Add(attr); - if (matches.Count >= SearchResultLimit) - { - return matches; - } + _searchMatches = matches; + _searchPending = false; + StateHasChanged(); } - } + }).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Superseded by a newer keystroke, or the page is going away. + } + catch (Exception ex) + { + await ReportAsync(() => + { + if (ReferenceEquals(_searchCts, searchCts)) + { + _searchPending = false; + _searchError = ex.Message; + } + }).ConfigureAwait(false); + } + finally + { + searchCts.Dispose(); + } + } + + // The search task disposes its own source, so a cancel can race that disposal. + private static void CancelQuietly(CancellationTokenSource? source) + { + if (source is null) + { + return; } - return matches; + try + { + source.Cancel(); + } + catch (ObjectDisposedException) + { + // The search already finished; there is nothing left to cancel. + } } private static string FormatType(GalaxyAttribute attr) @@ -422,17 +545,66 @@ : "When the dashboard first observed this value — MXAccess did not supply a source timestamp for this tag."; } - private async Task PollLoopAsync() + // Fault handling sits inside the loop, matching AlarmsPage: a fault on one tick is + // surfaced in the "Live read failed" banner and retried on the next tick rather than + // ending polling for the life of the page. Cancellation is the only exit, so the loop + // task never faults — which is what DrainAsync in DisposeAsync relies on. + // + // A tick is skipped while the circuit's browser is disconnected: a closed tab keeps + // its circuit for the retention period, and polling ReadBulk on the shared dashboard + // worker for a page nobody can see is pure cost. The first tick after a reconnect + // refreshes as normal. + private async Task PollLoopAsync(CancellationToken cancellationToken) { - try + using PeriodicTimer timer = new(TimeSpan.FromSeconds(2)); + while (true) { - using PeriodicTimer timer = new(TimeSpan.FromSeconds(2)); - while (await timer.WaitForNextTickAsync(_cts.Token).ConfigureAwait(false)) + try + { + if (!await timer.WaitForNextTickAsync(cancellationToken).ConfigureAwait(false)) + { + return; + } + } + catch (OperationCanceledException) + { + return; + } + + if (!Circuit.IsConnected) + { + continue; + } + + try { await InvokeAsync(RefreshValuesAsync).ConfigureAwait(false); } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + return; + } + catch (Exception ex) + { + await ReportAsync(() => _readError = ex.Message).ConfigureAwait(false); + } } - catch (OperationCanceledException) + } + + // Best-effort: the fault being reported may itself be an InvokeAsync against a + // disposed renderer, in which case the dispatch fails the same way and there is no + // page left to show it on. + private async Task ReportAsync(Action update) + { + try + { + await InvokeAsync(() => + { + update(); + StateHasChanged(); + }).ConfigureAwait(false); + } + catch { } } @@ -445,7 +617,7 @@ } string[] tags = [.. _subscribed]; - DashboardLiveReadResult result = await LiveData.ReadAsync(tags, _cts.Token); + DashboardLiveReadResult result = await LiveData.ReadAsync(tags, _viewerId, _cts.Token); _readError = result.Error; _workerPid = result.WorkerProcessId; DateTimeOffset now = DateTimeOffset.UtcNow; @@ -454,7 +626,7 @@ // Stamp the observed-change time only when the value/quality // signature actually changes, so the Updated column does not // tick on every poll for a static tag. - string signature = $"{value.ValueText}{value.Quality}{value.Ok}"; + string signature = $"{value.ValueText}{value.Quality}{value.Ok}"; if (!_valueSignature.TryGetValue(value.TagAddress, out string? previous) || previous != signature) { @@ -471,31 +643,49 @@ /// public async ValueTask DisposeAsync() { - await _cts.CancelAsync(); - await _deployCts.CancelAsync(); - if (_pollTask is not null) + try { - try - { - await _pollTask; - } - catch (OperationCanceledException) - { - } + await _cts.CancelAsync().ConfigureAwait(false); + await _deployCts.CancelAsync().ConfigureAwait(false); + + // Drained together, not in sequence: a wedged dispatcher blocks every loop at + // once, so sequential drains would multiply the bound. DrainAsync tolerates null. + await Task.WhenAll(DrainAsync(_pollTask), DrainAsync(_deployTask), DrainAsync(_searchTask)) + .ConfigureAwait(false); } - if (_deployTask is not null) + finally { - try - { - await _deployTask; - } - catch (OperationCanceledException) - { - } + // Always released, even if cancellation or a drain throws. A search still in + // flight disposes its own linked source. + _cts.Dispose(); + _deployCts.Dispose(); + GC.SuppressFinalize(this); + } + } + + // The accepted cost of the bound is an abandoned loop that keeps its timer or + // notifier subscription until it does unwind; waiting forever on a dispatcher that + // is not draining would wedge the circuit teardown itself. + private static async Task DrainAsync(Task? loop) + { + if (loop is null) + { + return; } - _cts.Dispose(); - _deployCts.Dispose(); - GC.SuppressFinalize(this); + try + { + await loop.WaitAsync(LoopDrainTimeout).ConfigureAwait(false); + } + catch (TimeoutException) + { + } + catch (OperationCanceledException) + { + } + catch + { + // Other disposal-time errors are best-effort. + } } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardBrowseSearchIndex.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardBrowseSearchIndex.cs new file mode 100644 index 0000000..55bba77 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardBrowseSearchIndex.cs @@ -0,0 +1,117 @@ +using ZB.MOM.WW.GalaxyRepository; +using ZB.MOM.WW.GalaxyRepository.Grpc; + +namespace ZB.MOM.WW.MxGateway.Server.Dashboard; + +/// +/// Flat, pre-lowercased attribute index behind the Browse page's filter box, shared by +/// every circuit and rebuilt only when the Galaxy cache sequence changes. +/// +/// +/// The filter used to walk every object and attribute of the cached Galaxy, doing two +/// case-insensitive Contains per attribute, synchronously in the property setter +/// bound to oninput — so a large Galaxy was rescanned on the render dispatcher +/// once per keystroke, per circuit. The page now debounces and runs the search off the +/// dispatcher; this index makes each search a single pass of ordinal substring checks +/// over strings lowercased once per Galaxy deploy rather than once per comparison. +/// Lowercasing is invariant-culture, which matches the previous +/// behaviour for the ASCII tag names +/// Galaxy uses. +/// +public sealed class DashboardBrowseSearchIndex +{ + // How often a scan checks its cancellation token. A superseded keystroke stops + // within this many attributes rather than finishing the whole Galaxy. + private const int CancellationCheckInterval = 1024; + + private IndexSnapshot? _snapshot; + private int _buildCount; + + /// Gets how many times the index has been (re)built (diagnostic, for tests). + internal int BuildCount => Volatile.Read(ref _buildCount); + + /// + /// Returns up to attributes whose full tag reference or + /// attribute name contains , case-insensitively, in + /// Galaxy object order. + /// + /// The current Galaxy cache entry; its sequence keys the index. + /// The filter text as typed; it is trimmed. + /// Maximum number of matches. + /// Cancelled when a newer query supersedes this one. + /// The matches; empty for a blank query. + /// was cancelled. + public IReadOnlyList Search( + GalaxyHierarchyCacheEntry entry, + string rawQuery, + int limit, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(entry); + + string query = (rawQuery ?? string.Empty).Trim().ToLowerInvariant(); + if (query.Length == 0 || limit <= 0) + { + return []; + } + + IndexedAttribute[] attributes = GetOrBuild(entry, cancellationToken); + List matches = []; + for (int i = 0; i < attributes.Length; i++) + { + if (i % CancellationCheckInterval == 0) + { + cancellationToken.ThrowIfCancellationRequested(); + } + + IndexedAttribute candidate = attributes[i]; + if (candidate.FullTagReference.Contains(query, StringComparison.Ordinal) + || candidate.AttributeName.Contains(query, StringComparison.Ordinal)) + { + matches.Add(candidate.Attribute); + if (matches.Count >= limit) + { + break; + } + } + } + + return matches; + } + + private IndexedAttribute[] GetOrBuild(GalaxyHierarchyCacheEntry entry, CancellationToken cancellationToken) + { + IndexSnapshot? current = Volatile.Read(ref _snapshot); + if (current is not null && current.Sequence == entry.Sequence) + { + return current.Attributes; + } + + // A racing build for the same sequence is harmless: the projection is pure, so + // either winner stores equivalent content. + List attributes = []; + foreach (GalaxyObject galaxyObject in entry.Objects) + { + cancellationToken.ThrowIfCancellationRequested(); + foreach (GalaxyAttribute attribute in galaxyObject.Attributes) + { + attributes.Add(new IndexedAttribute( + attribute.FullTagReference.ToLowerInvariant(), + attribute.AttributeName.ToLowerInvariant(), + attribute)); + } + } + + IndexSnapshot built = new(entry.Sequence, [.. attributes]); + Volatile.Write(ref _snapshot, built); + Interlocked.Increment(ref _buildCount); + return built.Attributes; + } + + private sealed record IndexSnapshot(long Sequence, IndexedAttribute[] Attributes); + + private readonly record struct IndexedAttribute( + string FullTagReference, + string AttributeName, + GalaxyAttribute Attribute); +} diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardCircuitConnectionTracker.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardCircuitConnectionTracker.cs new file mode 100644 index 0000000..1b1ca9a --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardCircuitConnectionTracker.cs @@ -0,0 +1,42 @@ +using Microsoft.AspNetCore.Components.Server.Circuits; + +namespace ZB.MOM.WW.MxGateway.Server.Dashboard; + +/// +/// Per-circuit view of whether the browser is still attached. Registered scoped, so a +/// page injecting it sees its own circuit, and also registered as that circuit's +/// so Blazor drives it. +/// +/// +/// A closed or backgrounded browser tab leaves its circuit alive, disconnected, for +/// +/// before Blazor disposes it, and every timer on the page keeps running in that +/// window. The Browse page's two-second ReadBulk poll is the costly one — it +/// drives the shared dashboard worker — so it skips its tick while +/// is false and resumes on reconnect. +/// +public sealed class DashboardCircuitConnectionTracker : CircuitHandler +{ + private volatile bool _connected = true; + + /// + /// Gets a value indicating whether the circuit's browser connection is up. True + /// until Blazor reports the connection down, and also outside a circuit (prerender), + /// where no handler callbacks ever arrive. + /// + public bool IsConnected => _connected; + + /// + public override Task OnConnectionUpAsync(Circuit circuit, CancellationToken cancellationToken) + { + _connected = true; + return Task.CompletedTask; + } + + /// + public override Task OnConnectionDownAsync(Circuit circuit, CancellationToken cancellationToken) + { + _connected = false; + return Task.CompletedTask; + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs index e8632aa..3390a83 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Components.Server.Circuits; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -16,6 +17,16 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard; ///
public static class DashboardServiceCollectionExtensions { + /// + /// How long a disconnected dashboard circuit is kept for reconnect before Blazor + /// disposes it. The framework default is three minutes, during which a closed Browse + /// tab's page — and its two-second live-read poll against the shared dashboard + /// worker — keeps running. Thirty seconds still rides out a network blip or a + /// laptop lid; a longer outage reloads the page, which costs nothing here because + /// every page re-seeds from in-process state. + /// + public static readonly TimeSpan DisconnectedCircuitRetention = TimeSpan.FromSeconds(30); + /// /// Registers all dashboard services, authentication, and Razor components. /// @@ -54,6 +65,14 @@ public static class DashboardServiceCollectionExtensions // nothing in this process builds a hub connection or needs a token for one. services.AddSingleton(); services.AddScoped(); + // Singleton: the lowercased attribute index is keyed on the Galaxy cache sequence + // and shared by every circuit's Browse filter, so it is built once per deploy. + services.AddSingleton(); + // Scoped (one per circuit) and registered as that circuit's CircuitHandler, so the + // Browse page can pause its live-read poll while its browser is disconnected. + services.AddScoped(); + services.AddScoped( + static provider => provider.GetRequiredService()); // Singleton: EventsHub instances are transient (one per hub invocation), so the // subscriber bookkeeping they share with the broadcaster must outlive them. services.AddSingleton(); @@ -76,7 +95,8 @@ public static class DashboardServiceCollectionExtensions services.AddAntiforgery(); services.AddCascadingAuthenticationState(); services.AddRazorComponents() - .AddInteractiveServerComponents(); + .AddInteractiveServerComponents(circuit => + circuit.DisconnectedCircuitRetentionPeriod = DisconnectedCircuitRetention); services.AddSignalR(); // DEV/TEST ONLY. Read directly from configuration here because authentication scheme diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardBrowseSearchIndexTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardBrowseSearchIndexTests.cs new file mode 100644 index 0000000..98cbb6e --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardBrowseSearchIndexTests.cs @@ -0,0 +1,95 @@ +using ZB.MOM.WW.GalaxyRepository; +using ZB.MOM.WW.GalaxyRepository.Grpc; +using ZB.MOM.WW.MxGateway.Server.Dashboard; + +namespace ZB.MOM.WW.MxGateway.Tests.Dashboard; + +/// +/// Verifies the Browse filter's shared index: case-insensitive matching on reference +/// and attribute name, the result limit, one build per Galaxy cache sequence, and +/// cancellation of a superseded scan. +/// +public sealed class DashboardBrowseSearchIndexTests +{ + /// Matches either the full reference or the attribute name, ignoring case. + [Fact] + public void Search_MatchesReferenceOrAttributeNameCaseInsensitively() + { + DashboardBrowseSearchIndex index = new(); + GalaxyHierarchyCacheEntry entry = CreateEntry(sequence: 1); + + IReadOnlyList byReference = index.Search(entry, " PUMP_001.p ", 10, CancellationToken.None); + IReadOnlyList byName = index.Search(entry, "levels", 10, CancellationToken.None); + + Assert.Equal(["Pump_001.PV"], byReference.Select(attr => attr.FullTagReference)); + Assert.Equal(["Tank_001.Levels[]"], byName.Select(attr => attr.FullTagReference)); + Assert.Empty(index.Search(entry, " ", 10, CancellationToken.None)); + } + + /// Stops at the limit, in Galaxy object order. + [Fact] + public void Search_StopsAtTheLimit() + { + DashboardBrowseSearchIndex index = new(); + + IReadOnlyList matches = index.Search(CreateEntry(sequence: 1), "_001", 2, CancellationToken.None); + + Assert.Equal(["Pump_001.PV", "Pump_001.Alarm"], matches.Select(attr => attr.FullTagReference)); + } + + /// The index is built once per cache sequence and rebuilt when the sequence moves. + [Fact] + public void Search_RebuildsOnlyWhenTheCacheSequenceChanges() + { + DashboardBrowseSearchIndex index = new(); + GalaxyHierarchyCacheEntry first = CreateEntry(sequence: 1); + + index.Search(first, "pump", 10, CancellationToken.None); + index.Search(first, "tank", 10, CancellationToken.None); + Assert.Equal(1, index.BuildCount); + + GalaxyHierarchyCacheEntry redeployed = CreateEntry(sequence: 2, extraTag: "Valve_001.Open"); + IReadOnlyList matches = index.Search(redeployed, "valve", 10, CancellationToken.None); + + Assert.Equal(2, index.BuildCount); + Assert.Equal(["Valve_001.Open"], matches.Select(attr => attr.FullTagReference)); + } + + /// A cancelled (superseded) search throws instead of finishing the scan. + [Fact] + public void Search_WhenCancelled_Throws() + { + DashboardBrowseSearchIndex index = new(); + using CancellationTokenSource cancelled = new(); + cancelled.Cancel(); + + Assert.ThrowsAny( + () => index.Search(CreateEntry(sequence: 1), "pump", 10, cancelled.Token)); + } + + private static GalaxyHierarchyCacheEntry CreateEntry(long sequence, string? extraTag = null) + { + GalaxyObject pump = new() { GobjectId = 1, TagName = "Pump_001" }; + pump.Attributes.Add(new GalaxyAttribute { AttributeName = "PV", FullTagReference = "Pump_001.PV" }); + pump.Attributes.Add(new GalaxyAttribute { AttributeName = "Alarm", FullTagReference = "Pump_001.Alarm" }); + + GalaxyObject tank = new() { GobjectId = 2, TagName = "Tank_001" }; + tank.Attributes.Add(new GalaxyAttribute { AttributeName = "Levels", FullTagReference = "Tank_001.Levels[]" }); + + List objects = [pump, tank]; + if (extraTag is not null) + { + GalaxyObject extra = new() { GobjectId = 3, TagName = extraTag.Split('.')[0] }; + extra.Attributes.Add(new GalaxyAttribute { AttributeName = extraTag.Split('.')[1], FullTagReference = extraTag }); + objects.Add(extra); + } + + return GalaxyHierarchyCacheEntry.Empty with + { + Status = GalaxyCacheStatus.Healthy, + Sequence = sequence, + Objects = objects, + Index = GalaxyHierarchyIndex.Build(objects), + }; + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardHubsRegistrationTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardHubsRegistrationTests.cs index a2f9cb2..f7e04ab 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardHubsRegistrationTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardHubsRegistrationTests.cs @@ -1,6 +1,9 @@ using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Components.Server; +using Microsoft.AspNetCore.Components.Server.Circuits; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using ZB.MOM.WW.MxGateway.Server; using ZB.MOM.WW.MxGateway.Server.Dashboard; using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; @@ -61,4 +64,42 @@ public sealed class DashboardHubsRegistrationTests Assert.Same(broadcaster, subscriber); Assert.Same(app.Services.GetRequiredService(), broadcaster); } + + /// + /// A closed tab's circuit (and its Browse poll) is retained for 30 seconds, not the + /// framework's three minutes, and each circuit gets a connection tracker that is also + /// its , so the Browse poll can pause while disconnected. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Build_WhenDashboardEnabled_ShortensCircuitRetentionAndTracksConnection() + { + await using WebApplication app = GatewayApplication.Build([]); + + CircuitOptions circuitOptions = app.Services.GetRequiredService>().Value; + Assert.Equal(TimeSpan.FromSeconds(30), circuitOptions.DisconnectedCircuitRetentionPeriod); + Assert.Equal(DashboardServiceCollectionExtensions.DisconnectedCircuitRetention, circuitOptions.DisconnectedCircuitRetentionPeriod); + + using IServiceScope scope = app.Services.CreateScope(); + DashboardCircuitConnectionTracker tracker = scope.ServiceProvider.GetRequiredService(); + Assert.Contains(scope.ServiceProvider.GetServices(), handler => ReferenceEquals(handler, tracker)); + + Assert.True(tracker.IsConnected); + await tracker.OnConnectionDownAsync(null!, CancellationToken.None); + Assert.False(tracker.IsConnected); + await tracker.OnConnectionUpAsync(null!, CancellationToken.None); + Assert.True(tracker.IsConnected); + } + + /// The Browse filter index is one shared instance, built once per Galaxy deploy. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Build_WhenDashboardEnabled_SharesOneBrowseSearchIndex() + { + await using WebApplication app = GatewayApplication.Build([]); + + Assert.Same( + app.Services.GetRequiredService(), + app.Services.GetRequiredService()); + } } From 2c963b723013b7148cd9ad69df9f92e2eaea8676 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:27:14 -0400 Subject: [PATCH 7/8] docs(dashboard): describe the pages as shipped and the bounded hub/feed behaviour Rewrite the Pages 'Show:' lists to what each page actually renders (no per-session subscriber/pending-command/queue counters, no worker memory/CPU, no per-family command counters, counters not rates). Document the per-session EventsHub pump, alarm-hub send bound and AlarmFeedReset marker, configuration sent once per snapshot-hub connection, health-derived gateway status, Browse search debounce/index, bounded disposal, 30s circuit retention with connection-gated polling, per-viewer advise-set share, and AlarmsPage memo. --- docs/GatewayDashboardDesign.md | 224 ++++++++++++++++++++++----------- 1 file changed, 151 insertions(+), 73 deletions(-) diff --git a/docs/GatewayDashboardDesign.md b/docs/GatewayDashboardDesign.md index be2544f..1c01074 100644 --- a/docs/GatewayDashboardDesign.md +++ b/docs/GatewayDashboardDesign.md @@ -273,9 +273,14 @@ bearer). Each hub class is `[Authorize(Policy = HubClientsPolicy)]`. | Hub | Path | Producer | Payload | Routing | |---|---|---|---|---| -| `DashboardSnapshotHub` | `/hubs/snapshot` | `DashboardSnapshotPublisher` (BackgroundService consuming `IDashboardSnapshotService.WatchSnapshotsAsync`) | `DashboardSnapshot` | Sent to all connected clients on every snapshot tick, but only while at least one client is connected (see "Idle gating" below); new connections receive the current snapshot synchronously in `OnConnectedAsync`. | -| `AlarmsHub` | `/hubs/alarms` | `AlarmsHubPublisher` (BackgroundService consuming `IGatewayAlarmService.StreamAsync(filter: null)`) | `AlarmFeedMessage` (`active_alarm` / `snapshot_complete` / `transition` / `provider_status` / `snapshot_status`) | Guarded by `MxGateway.Dashboard.Alarms`, not `HubClients`: a real authenticated dashboard role is required, so the `Authentication:Mode=Disabled` and anonymous-localhost bypasses are excluded (SEC-40). Connected clients auto-join `__alarms__`; all admitted clients receive every message — the feed is galaxy-wide and carries no session provenance, so no per-caller scoping applies (see "Alarm-feed visibility"). Publisher auto-reconnects every 5s on stream faults. The alarm value fields (`current_value` / `limit_value`) are stripped from a deep-cloned copy of the message when `Dashboard:ShowTagValues` is false (the default) — on both value-bearing payload arms, so neither a live transition nor a snapshot record leaks a process value; every other field still renders. The source message is never mutated: it fans out from the same feed to gRPC `StreamAlarms` subscribers, which this dashboard-display flag does not govern. Arms carrying no value are forwarded as-is, uncloned. | -| `EventsHub` | `/hubs/events` | `DashboardEventBroadcaster` invoked by each session's internal dashboard-mirror subscriber on its `SessionEventDistributor` (registered when the session becomes Ready) | `MxEvent` | Clients call `SubscribeSession(sessionId)` to join `session:{id}`, which also registers them in `EventsHubViewerRegistry` — the mirror is gated on that registry, which counts hub and in-process viewers alike (see "Mirror gating" below). The dashboard is a first-class distributor subscriber, so it receives the session's events whether or not a gRPC client is streaming. It sees RAW session events — not the per-gRPC-subscriber `AfterWorkerSequence` filtering that `EventStreamService` applies at its own boundary — because the dashboard is a separate LDAP-authenticated monitoring view meant to show the session's full event activity. Tag values are stripped from the mirrored `MxEvent` copy by `DashboardEventBroadcaster` when `Dashboard:ShowTagValues` is false (the default) — event metadata (tag reference, quality, status, timestamps) still renders, but the value fields are blanked, so no value leaks through this seam. `SubscribeSession` is gated by `IDashboardSessionAcl` (SEC-25 / TST-15): a denied caller gets a `HubException`, is not joined to the group, and is not registered as a viewer, so the mirror stays off for a session nobody is legitimately watching. The same ACL gates the in-process seam the session-details page uses, so neither path is the weaker one. Value redaction remains an independent layer — it bounds what a *permitted* subscriber sees. | +| `DashboardSnapshotHub` | `/hubs/snapshot` | `DashboardSnapshotPublisher` (BackgroundService consuming `IDashboardSnapshotService.WatchSnapshotsAsync`) | `SnapshotUpdated`: `DashboardSnapshot` **without** `configuration`; `ConfigurationUpdated`: `EffectiveGatewayConfiguration` | Ticks go to all connected clients on every snapshot interval, but only while at least one client is connected (see "Idle gating" below). The effective configuration is startup-static and the largest part of the record, so it is `[JsonIgnore]`d on `DashboardSnapshot` and sent once per connection instead: `OnConnectedAsync` pushes `ConfigurationUpdated` and then the current `SnapshotUpdated`, and the hub method `GetConfiguration()` returns it on demand. Each tick broadcast is bounded to 1 s; a send that misses the bound is abandoned, so one slow client cannot delay the next tick for everyone. | +| `AlarmsHub` | `/hubs/alarms` | `AlarmsHubPublisher` (BackgroundService consuming `IGatewayAlarmService.StreamAsync(filter: null)`) | `AlarmFeedMessage` (`active_alarm` / `snapshot_complete` / `transition` / `provider_status` / `snapshot_status`) | Guarded by `MxGateway.Dashboard.Alarms`, not `HubClients`: a real authenticated dashboard role is required, so the `Authentication:Mode=Disabled` and anonymous-localhost bypasses are excluded (SEC-40). Connected clients auto-join `__alarms__`; all admitted clients receive every message — the feed is galaxy-wide and carries no session provenance, so no per-caller scoping applies (see "Alarm-feed visibility"). Publisher auto-reconnects every 5s on stream faults. Every subscription to the monitor — the first and each resubscribe — is preceded by an `AlarmFeedReset` frame (`{ generation }`, monotonic per process): the monitor primes each new subscription with a full `active_alarm` snapshot ending in `snapshot_complete`, so a client must discard its accumulated alarm state on the marker and rebuild from that snapshot rather than double-counting it. Each broadcast is bounded to 2 s and abandoned on timeout: awaiting it unbounded let one slow client stall the drain until the monitor dropped the publisher as a slow subscriber, and the resubscribe then replayed the whole snapshot to every client. The alarm value fields (`current_value` / `limit_value`) are stripped from a deep-cloned copy of the message when `Dashboard:ShowTagValues` is false (the default) — on both value-bearing payload arms, so neither a live transition nor a snapshot record leaks a process value; every other field still renders. The source message is never mutated: it fans out from the same feed to gRPC `StreamAlarms` subscribers, which this dashboard-display flag does not govern. Arms carrying no value are forwarded as-is, uncloned. | +| `EventsHub` | `/hubs/events` | `DashboardEventBroadcaster` invoked by each session's internal dashboard-mirror subscriber on its `SessionEventDistributor` (registered when the session becomes Ready) | `MxEvent` | Clients call `SubscribeSession(sessionId)` to join `session:{id}`, which also registers them in `EventsHubViewerRegistry`. Delivery to the group goes through one bounded per-session pump (see "Mirror gating"), so a slow viewer loses its oldest events instead of accumulating pending sends — the mirror is gated on that registry, which counts hub and in-process viewers alike (see "Mirror gating" below). The dashboard is a first-class distributor subscriber, so it receives the session's events whether or not a gRPC client is streaming. It sees RAW session events — not the per-gRPC-subscriber `AfterWorkerSequence` filtering that `EventStreamService` applies at its own boundary — because the dashboard is a separate LDAP-authenticated monitoring view meant to show the session's full event activity. Tag values are stripped from the mirrored `MxEvent` copy by `DashboardEventBroadcaster` when `Dashboard:ShowTagValues` is false (the default) — event metadata (tag reference, quality, status, timestamps) still renders, but the value fields are blanked, so no value leaks through this seam. `SubscribeSession` is gated by `IDashboardSessionAcl` (SEC-25 / TST-15): a denied caller gets a `HubException`, is not joined to the group, and is not registered as a viewer, so the mirror stays off for a session nobody is legitimately watching. The same ACL gates the in-process seam the session-details page uses, so neither path is the weaker one. Value redaction remains an independent layer — it bounds what a *permitted* subscriber sees. | + +No hub client ships in this repository: the server-rendered pages read the +in-process seams above, and the hubs serve external consumers. Those consumers +must honour `AlarmFeedReset` and keep the configuration from +`ConfigurationUpdated`, since ticks no longer carry it. ### Default cadences @@ -343,12 +348,17 @@ subscribers. Two races motivate the extra state: gate open, and cancelling on their behalf would stop a live pump that other viewers depend on. -Two per-tick costs inside the snapshot itself are bounded independently of either gate: +Three per-tick costs inside the snapshot itself are bounded independently of either gate: - the effective configuration (`EffectiveGatewayConfiguration`) is built once and cached. It is a projection of `IOptions`, which the gateway binds at startup and never reloads, so rebuilding the whole option tree every tick - produced an identical object; + produced an identical object. For the same reason it is not serialized on the + `/hubs/snapshot` tick (see the hub table above); +- the gateway status is one `HealthCheckService.CheckHealthAsync` evaluation per + tick, bounded to 2 s and behind a zero-wait gate so concurrent enumerations (the + hub publisher and the page feed) do not both evaluate; `GetSnapshot()` reads the + cached result; - the API key summaries are refreshed at most once every 15 seconds (`ApiKeySummaryRefreshInterval`) instead of on every tick. The list is a SQLite read whose content changes only when an operator creates, rotates, or revokes a @@ -359,7 +369,7 @@ Two per-tick costs inside the snapshot itself are bounded independently of eithe Avoid pushing every MXAccess data-change event into a wider broadcast group. Events are routed strictly per session (`session:{id}` groups on the hub, per-session subscriber lists in process); the snapshot seams continue to carry -aggregate event counters and rates. +aggregate event counters. ### Mirror gating @@ -394,10 +404,21 @@ group. With `ShowTagValues` true there is no clone at all — the original `MxEv instance is handed to both audiences — so the "one clone per event" cost holds only in the redacting configuration, and in the value-showing one both audiences share a reference to the session pipeline's own event object. In-process delivery runs first -and synchronously — it cannot -throw, and it must not be skipped by the guard clause around the hub send — into -per-subscriber bounded drop-oldest channels, so a page that falls behind loses its -oldest queued events rather than blocking the session's event pipeline. +and synchronously — it cannot throw — into per-subscriber bounded drop-oldest +channels, so a page that falls behind loses its oldest queued events rather than +blocking the session's event pipeline. + +Hub delivery has the same shape. `EventsHubViewerRegistry` counts remote hub +connections separately from `inproc-` subscribers (`HasHubViewers`), and while a +session has at least one, `Publish` writes the event into that session's +`HubPump`: a 256-deep drop-oldest channel drained by a single task that sends one +event at a time to `session:{id}`, each send bounded to 5 s. A session watched only +in process starts no pump. The pump starts on the first event published while a +hub viewer is present and is torn down when the registry raises +`HubViewersReleased` for the session's last hub viewer (unsubscribe or disconnect). +Before this, `Publish` fired a group `SendAsync` per event and returned; a slow +remote viewer made each send stay pending in the SignalR lifetime manager, and the +pending sends — each holding a cloned event — grew without bound. The mirror subscriber itself is still registered on the `SessionEventDistributor` for the session's whole lifetime; only the per-event work is gated. Starting and @@ -411,55 +432,67 @@ clone and send this gate already removes. ### Dashboard home -Show top-level status: +The Overview page (`/`) shows, from the snapshot: -- gateway status, -- gateway version, -- uptime, -- open sessions, -- workers running, -- sessions faulted, -- command rate, -- command failure count, -- event rate, -- event queue depth, -- worker restart/kill count. +- gateway status badge — the aggregate status of the registered ASP.NET Core + health checks (`Healthy` / `Degraded` / `Unhealthy`), evaluated once per snapshot + tick with a 2 s bound and cached for the synchronous `GetSnapshot()` seed path. It + reads `Unknown` before the first evaluation, when an evaluation throws or times + out, and when no `HealthCheckService` is registered — never a hardcoded `Healthy` + (SEC-27); +- the snapshot's generation time; +- metric cards: uptime (with the gateway version as detail), open sessions, workers + running, worker event queue depth, failed commands, events received, faults, and + queue overflows — all cumulative counters or point-in-time gauges, not rates; +- a Galaxy Repository section: cache status, last deploy time, object count (with + area count), attribute, historized-attribute and alarm-attribute counts, and the + last refresh time; +- the recent faults list (`Dashboard:RecentFaultLimit`). -Use Bootstrap cards for individual metric summaries. Keep the layout compact -and operational. +There is no per-family command or event rate, no faulted-session counter, and no +worker restart/kill card on this page; `mxgateway.workers.killed` and +`mxgateway.workers.exited` are in the snapshot's metric list but not rendered. +Metric summaries use Bootstrap cards; the layout stays compact and operational. ### Sessions page -Show active and recent sessions in a table: +Shows the most recent sessions (`Dashboard:RecentSessionLimit`, newest first) in a +table with these columns: -- session id, -- client identity or API key display name, -- state, +- session id (links to the session details page), +- session state, +- client identity (redacted), - backend, -- worker process id, +- worker process id and worker state, +- events received for the session (the `mxgateway.events.received` per-session + counter), - open time, - last client activity, - last worker heartbeat, -- active event subscribers, -- pending commands, -- event queue depth, -- last fault summary. +- last fault summary (abbreviated, full text on hover), +- Close / Kill actions for Admin principals. -Rows should link to session details. +Event subscriber counts, pending command counts, and per-session queue depth are +not tracked per session and are not shown. ### Session details page -Show: +Shows: -- session metadata, -- worker metadata, -- command counters by method, -- event counters by family, -- active server handles and item counts if gateway shadow state has them, -- latest faults, -- last heartbeat payload, +- a session section: state badge, backend, client identity, client session name, + client correlation id (all redacted), open time, last activity, lease expiry, + events received, and last fault; +- a worker section: process id, worker state, and last heartbeat time; +- a "Recent events" table — the newest 50 events from the in-process mirror, with + worker sequence, family, server handle, item handle, and status (no values), plus + a live/offline pill; gated by the session ACL, so a caller who may not observe the + session sees "Not authorized" instead; - admin Close session / Kill worker controls (Admin role only). +Per-method command counters, per-family event counters for the session, server +handle / item counts, and the heartbeat payload are not shown — the gateway keeps +no per-session shadow state for them. + The Sessions list, the Workers list, and this details page all render the same admin controls when the signed-in principal carries the `Admin` role; viewers and the localhost-anonymous bypass see no action affordances and the server @@ -484,33 +517,31 @@ management audit path. ### Workers page -Show: +Shows one row per worker that is not closed, with: - worker process id, -- session id, -- executable path/version, -- state, -- startup duration, -- memory and CPU if available, -- last heartbeat, -- current command correlation id, -- pending command count, -- event queue depth, -- restart/kill reason if terminal. +- worker state, +- owning session id (links to the session details page), +- last heartbeat time, +- last fault summary (the owning session's final fault), +- a Kill worker action for Admin principals. + +Executable path and version, startup duration, memory/CPU, in-flight command +correlation, pending command count, and per-worker queue depth are not collected +and are not shown; the worker executable path is on the Settings page. ### Events page -Show aggregate event diagnostics: +Shows aggregate event diagnostics from the snapshot: -- event rate by session, -- event rate by event family, -- total events since start, +- events received since start, +- worker event queue depth and gRPC stream queue depth (gauges), - queue overflow count, -- stream disconnect count, -- recent terminal faults. +- gRPC stream disconnect count, +- an "Event Families" table: cumulative events received per event family. -Do not display full tag values by default. If value display is later added, make -it opt-in and redacted. +The figures are counters since start, not rates; per-session event counts are on +the Sessions page. No tag values are shown here. ### Browse page @@ -537,6 +568,33 @@ at the service, not in the page: one decision point, and the value never enters the render tree. Set the flag true to make the panel the explicit opt-in tag-value surface it was originally designed as. +The filter box is debounced: keystrokes within 150 ms coalesce into one search, +which runs on the thread pool with a cancellation token that the next keystroke +cancels, and only the latest query may publish its results. The search reads +`DashboardBrowseSearchIndex`, a singleton holding every attribute's full reference +and name lowercased once, keyed on the Galaxy cache `Sequence` and rebuilt only when +a deploy moves it. The previous filter rescanned the whole cached Galaxy with +case-insensitive comparisons, synchronously on the render dispatcher, once per +keystroke per circuit. + +The page's two background loops — the 2 s live-read poll and the Galaxy +deploy-notification subscription — handle faults inside the loop and retry; only +cancellation ends them. A poll fault surfaces in the "Live read failed" banner; a +notifier fault surfaces as a "redeploy notifications failed (retrying)" warning and +resubscribes after 5 s; the tree rebuild after a redeploy runs on the renderer +dispatcher. `DisposeAsync` cancels the loops and any in-flight search, drains them +concurrently with the same 5 s bound `DashboardPageBase` uses, and disposes its +cancellation sources in a `finally`. + +A closed or backgrounded tab leaves its circuit alive, disconnected, for +`DisconnectedCircuitRetentionPeriod`, which the dashboard sets to **30 seconds** +(`DashboardServiceCollectionExtensions.DisconnectedCircuitRetention`) instead of +the framework's three minutes. Within that window the poll does not read: a scoped +`DashboardCircuitConnectionTracker`, registered as the circuit's `CircuitHandler`, +reports the browser connection down and the poll skips its ticks until it comes +back. Every tick it does run is a `ReadBulk` against the shared dashboard worker, +so a closed tab previously kept the worker busy for three minutes. + ### Alarms page `/dashboard/alarms` lists the alarms the gateway's central alarm monitor @@ -552,7 +610,10 @@ client, no loopback socket, and no hub token — and the same subscription carri `snapshot_status` frame behind the truncated-snapshot banner ("Alarm snapshot may be incomplete"), so a capped provider fetch is caveated the moment the monitor decides it. The alarm rows themselves still come from the three-second poll, which also -re-asserts the truncation verdict as the reconcile baseline. If `MxGateway:Alarms:Enabled` is +re-asserts the truncation verdict as the reconcile baseline. The filtered rows and +the area list are memoized in `DashboardAlarmView` on the poll's alarm generation and +the filter inputs, so the "Showing" card and the table share one filter-and-sort per +change rather than recomputing it several times per render. If `MxGateway:Alarms:Enabled` is false the central monitor never starts, and the page says so instead of showing an empty list with no explanation. @@ -599,6 +660,19 @@ overflow against the oversized set and evicts the whole excess in one pass (a subscribes nothing new evicts nothing, but neither can it grow the set. A browse page requests far fewer tags than the cap, so in practice the set settles at 256. +The 256 is shared between viewers rather than one global LRU. Each Browse page +reads under its own viewer id, every advised tag records the viewer that read it +last, and each active viewer (one that read within the last 30 s) owns a share of +`256 / active viewers`, floored at 32. When a read needs room, eviction first takes +the least-recently-read tags of *other* viewers that are over their share — an +inactive viewer's share is zero, so a closed page's tags go first — and then the +reader's own tags that this read did not touch. A viewer within its share is never +evicted by someone else's read, so with several viewers the cap is soft: the set +can sit above 256 rather than strip a neighbour. Under the global LRU, two circuits +browsing different areas evicted each other's whole panels on every poll and +re-advised them two seconds later. With a single viewer the behaviour is exactly +the global LRU described above. + The Alarms page does **not** use the dashboard session: alarm data comes from the gateway's always-on central monitor. `QueryAlarmsAsync` reads `IGatewayAlarmService.CurrentAlarms` — the monitor's in-process cache — so the @@ -665,20 +739,24 @@ Secrets and pepper values are never logged. ### Settings page -Show read-only effective configuration: +Shows the read-only effective configuration: -- worker executable path, -- configured timeouts, -- queue capacities, -- auth mode, -- SQLite auth database path with sensitive parts redacted if needed, -- dashboard enabled state, +- authentication: mode, SQLite auth database path, pepper secret name, and whether + migrations run on startup; +- LDAP: enabled, server and port, fallback servers, transport, search base, + service account DN, service password (redacted), and the username and group + attributes; - the LDAP group mappings that decide what a signed-in user may see — `Dashboard:GroupToRole` (group → `Administrator` / `Viewer`) and - `Dashboard:GroupToTag` (group → the session tags it grants), -- `Dashboard:UntaggedSessionVisibility`, which decides who sees a session whose - owning API key carries no dashboard tags, -- protocol version. + `Dashboard:GroupToTag` (group → the session tags it grants); +- worker: executable path, required architecture, and the startup, shutdown, and + heartbeat-grace timeouts; +- sessions: default command timeout and max sessions; +- events: queue capacity and backpressure policy; +- dashboard: enabled, anonymous localhost, snapshot interval, recent fault and + session limits, `ShowTagValues`, and `Dashboard:UntaggedSessionVisibility` + (who sees a session whose owning API key carries no dashboard tags); +- the worker protocol version. Do not show API key secrets or pepper values. From bd7abf4e3580b53527649dbf4ac6ca4f508fff78 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:32:17 -0400 Subject: [PATCH 8/8] fix(dashboard): re-check hub viewers under lock before starting a pump; document alarm send-timeout tradeoff --- docs/GatewayDashboardDesign.md | 2 +- .../Dashboard/Hubs/DashboardEventBroadcaster.cs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/GatewayDashboardDesign.md b/docs/GatewayDashboardDesign.md index 1c01074..e6feaa7 100644 --- a/docs/GatewayDashboardDesign.md +++ b/docs/GatewayDashboardDesign.md @@ -274,7 +274,7 @@ bearer). Each hub class is `[Authorize(Policy = HubClientsPolicy)]`. | Hub | Path | Producer | Payload | Routing | |---|---|---|---|---| | `DashboardSnapshotHub` | `/hubs/snapshot` | `DashboardSnapshotPublisher` (BackgroundService consuming `IDashboardSnapshotService.WatchSnapshotsAsync`) | `SnapshotUpdated`: `DashboardSnapshot` **without** `configuration`; `ConfigurationUpdated`: `EffectiveGatewayConfiguration` | Ticks go to all connected clients on every snapshot interval, but only while at least one client is connected (see "Idle gating" below). The effective configuration is startup-static and the largest part of the record, so it is `[JsonIgnore]`d on `DashboardSnapshot` and sent once per connection instead: `OnConnectedAsync` pushes `ConfigurationUpdated` and then the current `SnapshotUpdated`, and the hub method `GetConfiguration()` returns it on demand. Each tick broadcast is bounded to 1 s; a send that misses the bound is abandoned, so one slow client cannot delay the next tick for everyone. | -| `AlarmsHub` | `/hubs/alarms` | `AlarmsHubPublisher` (BackgroundService consuming `IGatewayAlarmService.StreamAsync(filter: null)`) | `AlarmFeedMessage` (`active_alarm` / `snapshot_complete` / `transition` / `provider_status` / `snapshot_status`) | Guarded by `MxGateway.Dashboard.Alarms`, not `HubClients`: a real authenticated dashboard role is required, so the `Authentication:Mode=Disabled` and anonymous-localhost bypasses are excluded (SEC-40). Connected clients auto-join `__alarms__`; all admitted clients receive every message — the feed is galaxy-wide and carries no session provenance, so no per-caller scoping applies (see "Alarm-feed visibility"). Publisher auto-reconnects every 5s on stream faults. Every subscription to the monitor — the first and each resubscribe — is preceded by an `AlarmFeedReset` frame (`{ generation }`, monotonic per process): the monitor primes each new subscription with a full `active_alarm` snapshot ending in `snapshot_complete`, so a client must discard its accumulated alarm state on the marker and rebuild from that snapshot rather than double-counting it. Each broadcast is bounded to 2 s and abandoned on timeout: awaiting it unbounded let one slow client stall the drain until the monitor dropped the publisher as a slow subscriber, and the resubscribe then replayed the whole snapshot to every client. The alarm value fields (`current_value` / `limit_value`) are stripped from a deep-cloned copy of the message when `Dashboard:ShowTagValues` is false (the default) — on both value-bearing payload arms, so neither a live transition nor a snapshot record leaks a process value; every other field still renders. The source message is never mutated: it fans out from the same feed to gRPC `StreamAlarms` subscribers, which this dashboard-display flag does not govern. Arms carrying no value are forwarded as-is, uncloned. | +| `AlarmsHub` | `/hubs/alarms` | `AlarmsHubPublisher` (BackgroundService consuming `IGatewayAlarmService.StreamAsync(filter: null)`) | `AlarmFeedMessage` (`active_alarm` / `snapshot_complete` / `transition` / `provider_status` / `snapshot_status`) | Guarded by `MxGateway.Dashboard.Alarms`, not `HubClients`: a real authenticated dashboard role is required, so the `Authentication:Mode=Disabled` and anonymous-localhost bypasses are excluded (SEC-40). Connected clients auto-join `__alarms__`; all admitted clients receive every message — the feed is galaxy-wide and carries no session provenance, so no per-caller scoping applies (see "Alarm-feed visibility"). Publisher auto-reconnects every 5s on stream faults. Every subscription to the monitor — the first and each resubscribe — is preceded by an `AlarmFeedReset` frame (`{ generation }`, monotonic per process): the monitor primes each new subscription with a full `active_alarm` snapshot ending in `snapshot_complete`, so a client must discard its accumulated alarm state on the marker and rebuild from that snapshot rather than double-counting it. Each broadcast is bounded to 2 s and abandoned on timeout: awaiting it unbounded let one slow client stall the drain until the monitor dropped the publisher as a slow subscriber, and the resubscribe then replayed the whole snapshot to every client. The tradeoff: a client whose send is abandoned misses that one frame and receives no reset for it, so its alarm view can be stale until the next resubscribe (or until SignalR drops the stalled connection); there is no in-family consumer of `/hubs/alarms` today. The alarm value fields (`current_value` / `limit_value`) are stripped from a deep-cloned copy of the message when `Dashboard:ShowTagValues` is false (the default) — on both value-bearing payload arms, so neither a live transition nor a snapshot record leaks a process value; every other field still renders. The source message is never mutated: it fans out from the same feed to gRPC `StreamAlarms` subscribers, which this dashboard-display flag does not govern. Arms carrying no value are forwarded as-is, uncloned. | | `EventsHub` | `/hubs/events` | `DashboardEventBroadcaster` invoked by each session's internal dashboard-mirror subscriber on its `SessionEventDistributor` (registered when the session becomes Ready) | `MxEvent` | Clients call `SubscribeSession(sessionId)` to join `session:{id}`, which also registers them in `EventsHubViewerRegistry`. Delivery to the group goes through one bounded per-session pump (see "Mirror gating"), so a slow viewer loses its oldest events instead of accumulating pending sends — the mirror is gated on that registry, which counts hub and in-process viewers alike (see "Mirror gating" below). The dashboard is a first-class distributor subscriber, so it receives the session's events whether or not a gRPC client is streaming. It sees RAW session events — not the per-gRPC-subscriber `AfterWorkerSequence` filtering that `EventStreamService` applies at its own boundary — because the dashboard is a separate LDAP-authenticated monitoring view meant to show the session's full event activity. Tag values are stripped from the mirrored `MxEvent` copy by `DashboardEventBroadcaster` when `Dashboard:ShowTagValues` is false (the default) — event metadata (tag reference, quality, status, timestamps) still renders, but the value fields are blanked, so no value leaks through this seam. `SubscribeSession` is gated by `IDashboardSessionAcl` (SEC-25 / TST-15): a denied caller gets a `HubException`, is not joined to the group, and is not registered as a viewer, so the mirror stays off for a session nobody is legitimately watching. The same ACL gates the in-process seam the session-details page uses, so neither path is the weaker one. Value redaction remains an independent layer — it bounds what a *permitted* subscriber sees. | No hub client ships in this repository: the server-rendered pages read the diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs index 5789d31..1287aaf 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs @@ -209,7 +209,10 @@ public sealed class DashboardEventBroadcaster : IDashboardEventBroadcaster, IDas { lock (_syncRoot) { - if (_disposed) + // Re-check under the lock: the last viewer may have left between the + // caller's unlocked check and here, and its release handler (which takes + // this lock) would then never see a pump created after it ran. + if (_disposed || !_viewerRegistry.HasHubViewers(sessionId)) { return; }