Merge fix/c3-session-lifecycle: GWC-32 WorkerClient dispose is terminal, GWC-34 attach-after-dispose throws SessionNotReady, GWC-37 subtag watch-list coverage graded on provider status + re-resolved at failover edge

This commit is contained in:
Joseph Doherty
2026-08-20 09:39:53 -04:00
21 changed files with 864 additions and 50 deletions
@@ -15,4 +15,32 @@ public static class AlarmProviderReasons
/// distinguish a forced subtag mode from an unexpected degraded failover.
/// </summary>
public const string ForcedSubtag = "Forced subtag mode (configuration)";
/// <summary>
/// Reason prefix set when the effective mode can need the subtag watch-list
/// (<c>Auto</c> or <c>ForceSubtag</c>) but the list resolved to zero targets, so the
/// subtag provider covers no alarms (GWC-37). In <c>Auto</c> this is a warning about the
/// standby, not about the currently-serving alarm manager, which is why the dashboard
/// renders it against the <c>ALARMMGR</c> mode as a coverage badge rather than a failover.
/// </summary>
public const string SubtagWatchListEmpty =
"Subtag watch-list is empty: the subtag provider covers no alarms";
/// <summary>
/// Reason prefix set when Galaxy Repository alarm discovery was enabled but failed, so
/// the watch-list carries only what configuration contributed and may be incomplete
/// (GWC-37). Distinct from <see cref="SubtagWatchListEmpty"/>: discovery failing with a
/// usable config-only list is a completeness warning, not a total loss of coverage.
/// </summary>
public const string SubtagDiscoveryFailed =
"Galaxy Repository alarm discovery failed: the subtag watch-list may be incomplete";
/// <summary>
/// Reason prefix set when a re-resolution at the failover edge produced a different
/// watch-list than the one sent at <c>SubscribeAlarms</c> (GWC-37). The worker keeps the
/// list it was given for the life of the subscription, so the difference is reported
/// rather than applied; a monitor restart re-resolves and re-sends.
/// </summary>
public const string SubtagWatchListStale =
"Subtag watch-list differs from the one the worker is using";
}
@@ -0,0 +1,29 @@
using ZB.MOM.WW.MxGateway.Contracts.Proto;
namespace ZB.MOM.WW.MxGateway.Server.Alarms;
/// <summary>
/// The outcome of one subtag watch-list resolution: the composed targets plus enough
/// provenance for the caller to tell an empty list that means "nothing was configured"
/// from one that means "discovery was configured and silently produced nothing"
/// (GWC-37). <see cref="IAlarmWatchListResolver.ResolveAsync"/> deliberately never throws
/// on a discovery failure, so without this the two are indistinguishable at the call site
/// and a Galaxy Repository outage at startup can only be found in a log line.
/// </summary>
/// <param name="Targets">The resolved watch-list, in order; possibly empty.</param>
/// <param name="DiscoveryAttempted">
/// Galaxy Repository discovery was enabled for this resolution
/// (<c>Fallback:Discovery:UseGalaxyRepository</c>).
/// </param>
/// <param name="DiscoveryFailed">
/// Discovery was attempted and failed, so <paramref name="Targets"/> holds only what
/// configuration contributed.
/// </param>
/// <param name="DiscoveryFailureMessage">
/// The discovery failure's message, or <c>null</c> when discovery did not fail.
/// </param>
public sealed record AlarmWatchListResolution(
IReadOnlyList<AlarmSubtagTarget> Targets,
bool DiscoveryAttempted,
bool DiscoveryFailed,
string? DiscoveryFailureMessage);
@@ -31,7 +31,7 @@ public sealed class AlarmWatchListResolver : IAlarmWatchListResolver
}
/// <inheritdoc />
public async Task<IReadOnlyList<AlarmSubtagTarget>> ResolveAsync(
public async Task<AlarmWatchListResolution> ResolveAsync(
AlarmsOptions options,
CancellationToken cancellationToken = default)
{
@@ -39,6 +39,11 @@ public sealed class AlarmWatchListResolver : IAlarmWatchListResolver
AlarmDiscoveryOptions discovery = options.Fallback.Discovery;
// GWC-37: the caller cannot grade an empty watch-list without knowing whether discovery
// was asked to contribute and whether it failed while doing so, because the failure is
// deliberately swallowed below.
string? discoveryFailureMessage = null;
// Config fallback area used only for explicit IncludeAttributes entries (which
// carry no discovered area): discovery area, else the default area (may be empty).
string configFallbackArea = string.IsNullOrEmpty(discovery.Area) ? options.DefaultArea : discovery.Area;
@@ -68,6 +73,7 @@ public sealed class AlarmWatchListResolver : IAlarmWatchListResolver
_logger.LogWarning(
ex,
"Galaxy Repository alarm-attribute discovery failed; continuing with configuration-only watch-list.");
discoveryFailureMessage = ex.Message;
rows = [];
}
@@ -148,7 +154,11 @@ public sealed class AlarmWatchListResolver : IAlarmWatchListResolver
_logger.LogDebug("Resolved alarm subtag watch-list with {TargetCount} target(s).", targets.Count);
}
return targets;
return new AlarmWatchListResolution(
targets,
DiscoveryAttempted: discovery.UseGalaxyRepository,
DiscoveryFailed: discoveryFailureMessage is not null,
discoveryFailureMessage);
}
/// <summary>
@@ -57,6 +57,20 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic
private string _providerReason = string.Empty;
private DateTimeOffset _providerSince = DateTimeOffset.UtcNow;
// GWC-37 subtag-coverage verdict, guarded by _sync: whether the watch-list the worker is
// using can actually cover anything, and why not. Independent of _providerDegraded (which
// means "the subtag provider is the one serving"): in Auto mode the alarm manager can be
// serving perfectly while the standby behind it covers zero alarms, and that is invisible
// until the failover that needs it. Folded into BuildProviderStatus so it reaches the feed,
// the dashboard badge and every late-joining subscriber rather than only a startup log line.
private bool _subtagCoverageMissing;
private string _subtagCoverageReason = string.Empty;
// The watch-list references actually sent to the worker in SubscribeAlarms, guarded by _sync.
// The worker holds that list for the life of the subscription, so a later re-resolution is
// compared against this to report drift rather than silently diverging.
private IReadOnlyCollection<string> _sentWatchListReferences = [];
// Whether the worker's most recent reconcile fetch was capped, guarded by _sync.
// Written only by ApplyReconcile (and cleared with the cache), so it describes the last full
// reconcile — not necessarily the current _alarms contents, which live transitions keep moving
@@ -329,12 +343,25 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic
private async Task SubscribeAlarmsAsync(string sessionId, string subscription, CancellationToken cancellationToken)
{
IReadOnlyList<AlarmSubtagTarget> watchList = await _watchListResolver
AlarmWatchListResolution resolution = await _watchListResolver
.ResolveAsync(_options, cancellationToken)
.ConfigureAwait(false);
IReadOnlyList<AlarmSubtagTarget> watchList = resolution.Targets;
AlarmProviderMode forcedMode = MapForcedMode(_options.Fallback.Mode);
// GWC-37: grade the coverage before the command goes out, so a Galaxy Repository outage at
// startup is a standing status on the feed instead of one log line an operator reads weeks
// later — when the failover that needed the watch-list finds it empty. Broadcast because
// feed subscribers outlive the monitor's worker session and may already be attached.
lock (_sync)
{
_sentWatchListReferences = watchList
.Select(target => target.AlarmFullReference)
.ToHashSet(StringComparer.OrdinalIgnoreCase);
SetSubtagCoverage(GradeSubtagCoverage(forcedMode, resolution, staleAgainstSent: false), broadcast: true);
}
_logger.LogInformation(
"Alarm subscribe: forcedMode={ForcedMode} configMode={ConfigMode} watchList={WatchListCount}.",
forcedMode, _options.Fallback.Mode, watchList.Count);
@@ -518,6 +545,20 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic
AlarmProviderMode toMode = change.Mode;
string reason = change.Reason ?? string.Empty;
// GWC-37: this is the moment the subtag watch-list stops being theoretical — the provider
// that is about to serve is the one built from it. Re-resolve BEFORE the status is
// published so the frame the operator sees on the failover carries the coverage verdict
// rather than a second frame later, and so a watch-list that has drifted since
// SubscribeAlarms is reported instead of silently diverging. The re-resolution is NOT
// re-sent: the worker rejects a second SubscribeAlarms while subscribed, and the only way
// to replace the list — Unsubscribe then Subscribe — rebuilds the FailoverAlarmConsumer,
// which restarts on the primary that just failed and discards the warm standby state, so
// it would flap the feed rather than repair it. A monitor restart re-resolves and re-sends.
if (toMode == AlarmProviderMode.Subtag)
{
await RefreshSubtagCoverageAsync(cancellationToken).ConfigureAwait(false);
}
AlarmProviderStatus status;
int fromModeInt;
lock (_sync)
@@ -565,14 +606,141 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic
}
}
// GWC-37: re-resolves the subtag watch-list and re-grades the coverage verdict without
// touching the worker's subscription. Best-effort: a resolver that throws (only cancellation
// and programming errors do — discovery failure is reported in the resolution) leaves the
// verdict from the last subscribe in place, because a stale verdict is better than none.
// Does not broadcast: the callers publish a status frame of their own immediately after.
private async Task RefreshSubtagCoverageAsync(CancellationToken cancellationToken)
{
AlarmProviderMode forcedMode = MapForcedMode(_options.Fallback.Mode);
AlarmWatchListResolution resolution;
try
{
resolution = await _watchListResolver.ResolveAsync(_options, cancellationToken).ConfigureAwait(false);
}
catch (OperationCanceledException)
{
throw;
}
catch (Exception exception)
{
_logger.LogWarning(
exception,
"Re-resolving the alarm subtag watch-list failed; keeping the coverage verdict from the last subscribe.");
return;
}
lock (_sync)
{
bool stale = !resolution.Targets
.Select(target => target.AlarmFullReference)
.ToHashSet(StringComparer.OrdinalIgnoreCase)
.SetEquals(_sentWatchListReferences);
SetSubtagCoverage(GradeSubtagCoverage(forcedMode, resolution, stale), broadcast: false);
}
}
// Grades one resolution against the effective mode. Returns the reason the subtag coverage is
// unsound, or an empty string when it is sound (or irrelevant, as under ForceAlarmManager).
// Empty targets are the total-loss case; a failed discovery with a usable config-only list is
// a completeness warning; drift is reported separately because the worker cannot be updated.
private string GradeSubtagCoverage(
AlarmProviderMode forcedMode,
AlarmWatchListResolution resolution,
bool staleAgainstSent)
{
// ForceAlarmManager never builds a subtag consumer, so its watch-list is genuinely unused.
if (forcedMode == AlarmProviderMode.Alarmmgr)
{
return string.Empty;
}
// Nothing was asked of discovery and nothing was configured: subtag fallback is simply not
// in use in this deployment (Auto with an empty list subscribes alarmmgr-only), so there is
// no coverage to warn about.
if (resolution.Targets.Count == 0
&& !resolution.DiscoveryAttempted
&& _options.Fallback.Discovery.IncludeAttributes.Length == 0
&& forcedMode != AlarmProviderMode.Subtag)
{
return string.Empty;
}
List<string> reasons = [];
if (resolution.Targets.Count == 0)
{
reasons.Add(AlarmProviderReasons.SubtagWatchListEmpty);
}
if (resolution.DiscoveryFailed)
{
reasons.Add(resolution.DiscoveryFailureMessage is { Length: > 0 } message
? $"{AlarmProviderReasons.SubtagDiscoveryFailed} ({message})"
: AlarmProviderReasons.SubtagDiscoveryFailed);
}
if (staleAgainstSent)
{
reasons.Add(
$"{AlarmProviderReasons.SubtagWatchListStale} ({resolution.Targets.Count} target(s) now "
+ $"resolve against {_sentWatchListReferences.Count} sent at subscribe); restart the alarm "
+ "monitor to re-send it");
}
return string.Join("; ", reasons);
}
// Caller holds _sync. Applies a coverage verdict, logging and (optionally) broadcasting only
// when it actually changed so a stable degraded state does not repeat a frame every pass.
private void SetSubtagCoverage(string reason, bool broadcast)
{
bool missing = reason.Length > 0;
if (missing == _subtagCoverageMissing && string.Equals(reason, _subtagCoverageReason, StringComparison.Ordinal))
{
return;
}
_subtagCoverageMissing = missing;
_subtagCoverageReason = reason;
if (missing)
{
_logger.LogWarning("Alarm subtag coverage is unsound: {Reason}", reason);
}
else
{
_logger.LogInformation("Alarm subtag coverage is sound.");
}
if (broadcast)
{
BroadcastToAll(new AlarmFeedMessage { ProviderStatus = BuildProviderStatus() });
}
}
// Caller holds _sync. Builds an AlarmProviderStatus snapshot of the current state.
//
// GWC-37: degraded is the OR of "the subtag provider is the one serving" and "the subtag
// watch-list cannot cover anything". The second is what makes an empty watch-list visible in
// Auto mode, where the alarm manager is serving happily and the standby behind it is empty.
// The reasons are concatenated when both apply, which is the worst case worth naming loudly:
// failed over to a provider that covers nothing.
private AlarmProviderStatus BuildProviderStatus()
{
bool degraded = _providerDegraded || _subtagCoverageMissing;
string reason = (_providerReason, _subtagCoverageReason) switch
{
("", var coverage) => coverage,
(var mode, "") => mode,
(var mode, var coverage) => $"{mode} — {coverage}",
};
return new AlarmProviderStatus
{
Mode = _providerMode,
Degraded = _providerDegraded,
Reason = _providerReason,
Degraded = degraded,
Reason = reason,
Since = Timestamp.FromDateTimeOffset(_providerSince),
};
}
@@ -18,13 +18,15 @@ public interface IAlarmWatchListResolver
/// <param name="options">Alarm configuration carrying discovery and subtag-name settings.</param>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>
/// The resolved <see cref="AlarmSubtagTarget"/> watch-list, possibly empty.
/// The resolved <see cref="AlarmSubtagTarget"/> watch-list, possibly empty, together
/// with the discovery provenance the caller needs to grade it.
/// Discovery being unavailable never throws — it yields an empty (or
/// config-only) list and the caller decides what to do with it. Cancellation
/// config-only) list with <see cref="AlarmWatchListResolution.DiscoveryFailed"/> set,
/// and the caller decides what to do with it. Cancellation
/// is the one exception: a triggered <paramref name="cancellationToken"/>
/// still propagates an <see cref="OperationCanceledException"/>.
/// </returns>
Task<IReadOnlyList<AlarmSubtagTarget>> ResolveAsync(
Task<AlarmWatchListResolution> ResolveAsync(
AlarmsOptions options,
CancellationToken cancellationToken = default);
}
@@ -30,6 +30,14 @@ public sealed record DashboardAlarmProviderStatus(
/// </summary>
public const string ForcedSubtagLabel = "Subtag monitoring (forced)";
/// <summary>
/// Badge label shown when the alarm manager is serving normally but the subtag standby
/// behind it covers nothing — an empty or undiscoverable watch-list (GWC-37). The feed is
/// healthy today; what is degraded is the fallback that would take over tomorrow, so the
/// badge names the standby rather than claiming the feed has failed over.
/// </summary>
public const string NoSubtagStandbyLabel = "Alarm Manager (no subtag standby)";
private const string HealthyBadge = "bg-success";
private const string DegradedBadge = "bg-warning text-dark";
@@ -69,8 +77,18 @@ public sealed record DashboardAlarmProviderStatus(
&& status.Mode == AlarmProviderMode.Subtag
&& string.Equals(reason, AlarmProviderReasons.ForcedSubtag, StringComparison.Ordinal);
string label = !degraded ? AlarmManagerLabel : forced ? ForcedSubtagLabel : DegradedLabel;
string badge = !degraded ? HealthyBadge : forced ? ForcedSubtagBadge : DegradedBadge;
// GWC-37: degraded WITHOUT subtag mode means the alarm manager is still serving and the
// standby is the unsound half. "Subtag monitoring (degraded)" would misreport which
// provider is live, so this state gets its own label; the amber badge is shared because
// the operator action — fix discovery — is equally overdue in both.
bool standbyOnlyWarning = degraded && status.Mode != AlarmProviderMode.Subtag;
string label = !degraded
? AlarmManagerLabel
: standbyOnlyWarning ? NoSubtagStandbyLabel : forced ? ForcedSubtagLabel : DegradedLabel;
string badge = !degraded
? HealthyBadge
: standbyOnlyWarning ? DegradedBadge : forced ? ForcedSubtagBadge : DegradedBadge;
return new DashboardAlarmProviderStatus(
Mode: status.Mode,
@@ -39,6 +39,14 @@ public sealed class GatewaySession
private bool _everHadEventSubscriber;
private SessionEventDistributor? _eventDistributor;
private bool _eventDistributorStarted;
// GWC-34: set under _syncRoot when DisposeAsync nulls _eventDistributor, making the
// "one distributor per session, never reset" lifecycle symmetric at both ends. Without it
// EnsureDistributorCreated reads the null as "not created yet" and builds a REPLACEMENT
// distributor after teardown — one whose pump can never start (_eventDistributorStarted is a
// one-way latch) and whose channel is never completed, so an attach that stalled across the
// close+dispose registers on it and blocks in WaitToReadAsync until the client's own deadline,
// silently and invisibly to metrics.
private bool _eventDistributorDisposed;
private bool _dashboardMirrorStarted;
private IEventSubscriberLease? _dashboardMirrorLease;
private Task? _dashboardMirrorTask;
@@ -532,10 +540,23 @@ public sealed class GatewaySession
// start). Both the construction and the started-flag flip happen under _syncRoot so two
// concurrent callers (e.g. MarkReady's dashboard mirror and a racing first
// AttachEventSubscriber) agree on a single distributor and a single start.
//
// GWC-34: after DisposeAsync the null distributor means "gone", not "not built yet", so this
// throws instead of constructing a replacement nothing will ever pump. The attach paths already
// roll back and surface the exception, which is the same SessionNotReady the readiness gates
// raise — a caller that loses the race to teardown now fails immediately instead of hanging.
private SessionEventDistributor EnsureDistributorCreated(out bool startNow)
{
lock (_syncRoot)
{
if (_eventDistributorDisposed)
{
throw new SessionManagerException(
SessionManagerErrorCode.SessionNotReady,
$"Session {SessionId} is not ready for event streaming. Its event distributor was "
+ "disposed with the session.");
}
if (_eventDistributor is null)
{
EventOptions eventOptions = _eventStreaming.EventOptions;
@@ -672,7 +693,37 @@ public sealed class GatewaySession
// dashboard mirror only disconnects itself and never faults the session.
// These three calls are OUTSIDE _syncRoot to avoid holding it across
// EnsureDistributorCreated's own lock and StartAsync's Task.Run.
SessionEventDistributor distributor = EnsureDistributorCreated(out bool startNow);
//
// GWC-34: the state check above can be overtaken by a close+dispose before we get here, and
// EnsureDistributorCreated now says so instead of building an orphan distributor. The
// dashboard mirror is a best-effort observability attachment on a session that is already
// gone, so it is abandoned here rather than propagated out of MarkReady — the mirror-started
// latch stays set (nothing may start one on a disposed session) and the loop's token source
// is released so it is not orphaned.
SessionEventDistributor distributor;
bool startNow;
try
{
distributor = EnsureDistributorCreated(out startNow);
}
catch (SessionManagerException exception)
{
CancellationTokenSource? abandonedCts;
lock (_syncRoot)
{
abandonedCts = _dashboardMirrorCts;
_dashboardMirrorCts = null;
}
abandonedCts?.Dispose();
_eventStreaming.DistributorLogger.LogDebug(
exception,
"Dashboard event mirror for session {SessionId} was abandoned: the session was disposed "
+ "before the mirror could attach.",
SessionId);
return;
}
IEventSubscriberLease lease = distributor.Register(isInternal: true);
StartPumpIfRequested(distributor, startNow);
@@ -1867,6 +1918,9 @@ public sealed class GatewaySession
{
distributor = _eventDistributor;
_eventDistributor = null;
// GWC-34: latch the null-out so EnsureDistributorCreated cannot read it as
// "not created yet" and hand a racing attach a distributor that will never pump.
_eventDistributorDisposed = true;
}
if (distributor is not null)
@@ -420,12 +420,32 @@ public sealed class WorkerClient : IWorkerClient
/// <returns>A task that represents the asynchronous operation.</returns>
public async ValueTask DisposeAsync()
{
if (_disposed)
// GWC-32: disposal is the third channel-completing path, and it must enter a terminal state
// like the other two (MarkClosed, SetFaulted) BEFORE the channels are completed. Otherwise a
// client disposed while still Ready keeps reporting IsTerminalState() == false, and a read
// loop already inside DispatchEnvelope (the _stopCts.Cancel below only interrupts the NEXT
// ReadAsync) takes StageWorkerEvent's overflow arm on the completed staging channel: a
// spurious ProtocolViolation fault, a worker-event-staging overflow metric and a redundant
// kill, all during an intentional teardown. MarkClosed is deliberately not reused here: it
// would overwrite Faulted with Closed and it re-runs the whole teardown sequence this method
// performs itself. The transition is skipped when the state is already terminal, so a
// dispose-after-fault keeps Faulted and a double dispose changes nothing.
bool becameTerminalHere;
lock (_syncRoot)
{
return;
if (_disposed)
{
return;
}
_disposed = true;
becameTerminalHere = _state is not (WorkerClientState.Closed or WorkerClientState.Faulted);
if (becameTerminalHere)
{
_state = WorkerClientState.Closed;
}
}
_disposed = true;
// Drop out of the worker queue-depth gauge before teardown: whatever this client still holds
// is about to be discarded, and the sum must not keep counting a client that is going away.
_eventQueueDepthRegistration?.Dispose();
@@ -434,6 +454,16 @@ public sealed class WorkerClient : IWorkerClient
_outboundEnvelopes.Writer.TryComplete();
_eventStaging.Writer.TryComplete();
_events.Writer.TryComplete();
if (becameTerminalHere)
{
// A worker that reached Ready and is now being disposed without a prior Close or fault
// (SessionManager.OpenSessionAsync's failure arm does exactly this) stopped, so the
// running-worker gauge must come down. RecordWorkerStoppedOnce is latched on
// _workerStartRecorded, so a client that already recorded its stop through MarkClosed or
// SetFaulted cannot record a second one here.
RecordWorkerStoppedOnce("dispose");
}
CompletePendingCommands(
new WorkerClientException(
WorkerClientErrorCode.GatewayShutdown,
@@ -647,8 +677,10 @@ public sealed class WorkerClient : IWorkerClient
/// Hands a received worker event to the dedicated event writer without blocking the read loop.
/// <c>TryWrite</c> is non-blocking, so the read loop keeps dispatching replies, heartbeats and
/// faults regardless of how backed up the event path is. It returns <c>false</c> in two cases:
/// the staging channel has been completed during shutdown (the event is safely dropped because
/// the client is already terminal), or the channel is full at its
/// the staging channel has been completed by one of the three completing paths —
/// <see cref="MarkClosed"/>, <see cref="SetFaulted"/> or <see cref="DisposeAsync"/>, each of
/// which enters a terminal state before completing the channels (GWC-32), so the event is
/// safely dropped because the client is already terminal — or the channel is full at its
/// 2 × <see cref="WorkerClientOptions.EventChannelCapacity"/> bound. The latter means the
/// consumer has been draining slower than the worker produces for the whole time it took the
/// worker to emit that many further events, so the client is faulted immediately (GWC-24) —