perf(comms): alarms-only seed, capped buffers, at-least-once audit pull

2026-08-14 21:10:19 -04:00
parent 1040dc0fcc
commit 2ce0ad7ed1
30 changed files with 1941 additions and 482 deletions
@@ -27,6 +27,8 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
    private readonly ConcurrentDictionary<string, StreamEntry> _activeStreams = new();
    private readonly int _maxConcurrentStreams;
    private readonly TimeSpan _maxStreamLifetime;
+    private readonly int _instanceChannelCapacity;
+    private readonly int _siteAlarmChannelCapacity;
    private volatile bool _ready;
    // Flipped by CancelAllStreams() when the host enters
    // CoordinatedShutdown so SubscribeInstance refuses new streams with
@@ -72,10 +74,17 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
        ISiteStreamSubscriber streamSubscriber,
        ILogger<SiteStreamGrpcServer> logger,
        int maxConcurrentStreams = 100)
-        : this(streamSubscriber, logger, maxConcurrentStreams, TimeSpan.FromHours(4))
+        : this(streamSubscriber, logger, maxConcurrentStreams, TimeSpan.FromHours(4),
+            DefaultInstanceChannelCapacity, DefaultSiteAlarmChannelCapacity)
    {
    }

+    /// <summary>Fallback Debug View send-channel capacity when no options are bound.</summary>
+    internal const int DefaultInstanceChannelCapacity = 1000;
+
+    /// <summary>Fallback site-wide alarm send-channel capacity when no options are bound.</summary>
+    internal const int DefaultSiteAlarmChannelCapacity = 20_000;
+
    /// <summary>
    /// DI constructor — binds <see cref="CommunicationOptions.GrpcMaxConcurrentStreams"/>
    /// and <see cref="CommunicationOptions.GrpcMaxStreamLifetime"/> so the documented
@@ -91,7 +100,9 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
        IOptions<CommunicationOptions> options)
        : this(streamSubscriber, logger,
            options.Value.GrpcMaxConcurrentStreams,
-            options.Value.GrpcMaxStreamLifetime)
+            options.Value.GrpcMaxStreamLifetime,
+            options.Value.GrpcInstanceStreamChannelCapacity,
+            options.Value.GrpcSiteAlarmStreamChannelCapacity)
    {
    }

@@ -99,12 +110,16 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
        ISiteStreamSubscriber streamSubscriber,
        ILogger<SiteStreamGrpcServer> logger,
        int maxConcurrentStreams,
-        TimeSpan maxStreamLifetime)
+        TimeSpan maxStreamLifetime,
+        int instanceChannelCapacity,
+        int siteAlarmChannelCapacity)
    {
        _streamSubscriber = streamSubscriber;
        _logger = logger;
        _maxConcurrentStreams = maxConcurrentStreams;
        _maxStreamLifetime = maxStreamLifetime;
+        _instanceChannelCapacity = Math.Max(1, instanceChannelCapacity);
+        _siteAlarmChannelCapacity = Math.Max(1, siteAlarmChannelCapacity);
    }

    /// <summary>
@@ -209,6 +224,21 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
    /// <summary>Effective per-stream session lifetime. Exposed for tests.</summary>
    internal TimeSpan MaxStreamLifetime => _maxStreamLifetime;

+    /// <summary>Effective Debug View send-channel capacity. Exposed for tests.</summary>
+    internal int InstanceChannelCapacity => _instanceChannelCapacity;
+
+    /// <summary>Effective site-wide alarm send-channel capacity. Exposed for tests.</summary>
+    internal int SiteAlarmChannelCapacity => _siteAlarmChannelCapacity;
+
+    /// <summary>
+    /// Total events evicted from stream send channels on this node since start (both stream
+    /// kinds). Exported as <c>scadabridge.site.stream.events_dropped</c>; exposed here so a
+    /// test — and an operator via diagnostics — can read the raw count.
+    /// </summary>
+    public long DroppedStreamEventCount => Interlocked.Read(ref _droppedStreamEvents);
+
+    private long _droppedStreamEvents;
+
    /// <inheritdoc />
    public override Task SubscribeInstance(
        InstanceStreamRequest request,
@@ -219,7 +249,9 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
            responseStream,
            context,
            relay => _streamSubscriber.Subscribe(request.InstanceUniqueName, relay),
-            request.InstanceUniqueName);
+            request.InstanceUniqueName,
+            _instanceChannelCapacity,
+            streamKind: "instance");

    /// <inheritdoc />
    public override Task SubscribeSite(
@@ -234,7 +266,12 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
            // already drops IsConfiguredPlaceholder rows and maps only the
            // enriched AlarmStateUpdate, so it is reused unchanged.
            _streamSubscriber.SubscribeSiteAlarms,
-            "site-wide alarms");
+            "site-wide alarms",
+            // Its OWN, much larger channel (WP2.3): sharing the Debug View's 1000-slot
+            // DropOldest meant an alarm burst during a WAN stall silently evicted operator-
+            // visible transitions to make room for diagnostics traffic.
+            _siteAlarmChannelCapacity,
+            streamKind: "site-alarms");

    /// <summary>
    /// Shared streaming pipeline behind <see cref="SubscribeInstance"/> and
@@ -250,12 +287,16 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
    /// <param name="context">The server call context (carries the client cancellation token).</param>
    /// <param name="subscribe">Subscribes the relay actor to the hub, returning a subscription id.</param>
    /// <param name="description">Human-readable subscription description for logging.</param>
+    /// <param name="channelCapacity">Send-channel capacity for this stream kind.</param>
+    /// <param name="streamKind">Telemetry tag for this stream kind (<c>instance</c>/<c>site-alarms</c>).</param>
    private async Task RunSubscriptionStreamAsync(
        string correlationId,
        IServerStreamWriter<SiteStreamEvent> responseStream,
        ServerCallContext context,
        Func<IActorRef, string> subscribe,
-        string description)
+        string description,
+        int channelCapacity,
+        string streamKind)
    {
        if (!_ready)
            throw new RpcException(new GrpcStatus(StatusCode.Unavailable, "Server not ready"));
@@ -302,16 +343,20 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase

        long dropped = 0;
        var channel = Channel.CreateBounded<SiteStreamEvent>(
-            new BoundedChannelOptions(1000) { FullMode = BoundedChannelFullMode.DropOldest },
+            new BoundedChannelOptions(channelCapacity) { FullMode = BoundedChannelFullMode.DropOldest },
            _ =>
            {
                // Lossy-under-backpressure is the debug-view spec; make the real
-                // loss visible: first eviction + every 500th thereafter.
+                // loss visible: first eviction + every 500th thereafter, plus a
+                // per-kind counter on the node (WP2.3).
                var n = Interlocked.Increment(ref dropped);
+                Interlocked.Increment(ref _droppedStreamEvents);
+                ScadaBridgeTelemetry.RecordSiteStreamEventDropped(streamKind);
                if (n == 1 || n % 500 == 0)
                    _logger.LogWarning(
-                        "Debug stream {CorrelationId} backpressure: {Dropped} oldest event(s) evicted so far",
-                        correlationId, n);
+                        "Stream {CorrelationId} ({StreamKind}) backpressure: {Dropped} oldest event(s) evicted so far " +
+                        "of a {Capacity}-slot channel",
+                        correlationId, streamKind, n, channelCapacity);
            });

        var actorSeq = Interlocked.Increment(ref _actorCounter);
@@ -348,6 +393,25 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
            "Stream {CorrelationId} started for {Description} (subscription {SubscriptionId})",
            correlationId, description, subscriptionId);

+        // Flush response headers NOW, while the relay is attached and before the first event
+        // (WP2.3). This is the client's "the site accepted my subscription" signal — the
+        // per-site aggregator hangs its once-per-successful-reconnect re-seed off it. Without
+        // an explicit flush, ASP.NET Core defers headers to the first written message, so a
+        // quiet site would never report connected. Best-effort: a client that vanished between
+        // Subscribe and here fails the write, and the read loop below handles the teardown.
+        try
+        {
+            await context.WriteResponseHeadersAsync(Metadata.Empty);
+        }
+        catch (Exception ex)
+        {
+            // Deliberately catch-all: an unsent header must never leak the relay actor and
+            // the _activeStreams entry, which the finally below owns. Any real transport
+            // failure resurfaces on the read/write loop immediately after.
+            _logger.LogDebug(ex,
+                "Could not flush response headers for stream {CorrelationId}; continuing.", correlationId);
+        }
+
        // Telemetry follow-on: the connection is now fully established (Subscribe
        // succeeded, so no leak via the catch above). Count it up here and balance
        // it in the finally below so the scadabridge.site.connection.up gauge is
@@ -519,17 +583,46 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
            ? DateTime.SpecifyKind(request.SinceUtc.ToDateTime(), DateTimeKind.Utc)
            : DateTime.MinValue;

+        // Composite-keyset cursor (WP2.3), mirroring PullSiteCalls: proto3
+        // defaults an unset string to "", which the queue treats as "no cursor"
+        // (legacy inclusive >= behaviour), so an older central is unaffected.
+        var afterId = string.IsNullOrEmpty(request.AfterId) ? null : request.AfterId;
+
+        // AT-LEAST-ONCE (WP2.3). The incoming cursor is central's receipt for
+        // everything at or before it — that is the ONLY proof the site accepts.
+        // Rows are flipped to Reconciled here, at the START of the NEXT pull,
+        // instead of right after the previous response was projected: a fault
+        // between the response leaving the site and central committing it used
+        // to lose the rows outright, because they were already Reconciled and
+        // ReadPendingSinceAsync would never serve them again. The flip runs
+        // before the read so the rows it retires do not consume this batch's
+        // budget. Best-effort — a failure here only costs a re-ship, which
+        // central dedups on EventId.
+        if (since > DateTime.MinValue)
+        {
+            try
+            {
+                await queue.MarkReconciledUpToAsync(since, afterId, context.CancellationToken);
+            }
+            catch (Exception ex)
+            {
+                _logger.LogWarning(ex,
+                    "MarkReconciledUpToAsync failed for cursor since={Since} afterId={AfterId}; rows stay pending for the next pull.",
+                    since, afterId);
+            }
+        }
+
        IReadOnlyList<AuditEvent> events;
        try
        {
            events = await queue.ReadPendingSinceAsync(
-                since, request.BatchSize, context.CancellationToken);
+                since, request.BatchSize, afterId, context.CancellationToken);
        }
        catch (Exception ex)
        {
            _logger.LogError(ex,
-                "ReadPendingSinceAsync failed for since={Since} batch={Batch}; returning empty response.",
-                since, request.BatchSize);
+                "ReadPendingSinceAsync failed for since={Since} batch={Batch} afterId={AfterId}; returning empty response.",
+                since, request.BatchSize, afterId);
            return new PullAuditEventsResponse();
        }

@@ -537,7 +630,8 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
        {
            // batch_size saturated → tell central to issue a follow-up pull
            // with an advanced cursor. The site doesn't compute the cursor —
-            // central walks it forward from the last returned OccurredAtUtc.
+            // central walks it forward from the last returned OccurredAtUtc
+            // (plus, once it sets after_id, that row's EventId).
            MoreAvailable = events.Count >= request.BatchSize,
        };
        foreach (var evt in events)
@@ -545,31 +639,6 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
            response.Events.Add(AuditEventDtoMapper.ToDto(evt));
        }

-        // Flip to Reconciled AFTER projecting the response so a fault below the
-        // try/catch (mid-response, mid-flip) leaves the rows in Pending/Forwarded
-        // and central pulls them again next cycle. The flip itself is
-        // best-effort — its failure is a warning, not a fault, because central
-        // will dedup on EventId on the next pull.
-        var ids = new List<Guid>(events.Count);
-        foreach (var evt in events)
-        {
-            ids.Add(evt.EventId);
-        }
-
-        if (ids.Count > 0)
-        {
-            try
-            {
-                await queue.MarkReconciledAsync(ids, context.CancellationToken);
-            }
-            catch (Exception ex)
-            {
-                _logger.LogWarning(ex,
-                    "MarkReconciledAsync failed after PullAuditEvents response of {Count} rows; rows stay Pending for retry.",
-                    ids.Count);
-            }
-        }
-
        return response;
    }