fix(alarms): dedup reconcile/live duplicate broadcasts on the alarm feed (NEXT-03)

A periodic reconcile can synthesize a repair transition whose matching live transition is still buffered in the alarm lease; both then broadcast as indistinguishable duplicates on StreamAlarms and the dashboard hub. Nothing serializes the two paths, and a correct serialization needs a worker-side high-water mark on QueryActiveAlarms (proto + worker change + a stall path), so this closes the common case with a local best-effort dedup instead: both paths already carry the same worker-derived identity — the worker stamps record.TransitionTimestampUtc into both OnAlarmTransitionEvent's transition_timestamp and ActiveAlarmSnapshot.last_transition_timestamp — so ApplyTransition suppresses a live transition whose (timestamp, resulting state) the cache already carries from a repair. The Clear leg has no cache entry left to compare, so ApplyReconcile tombstones each synthesized Clear by the instance's original_raise_timestamp for one reconcile generation; a matching live Clear consumes the tombstone, while a new raise/clear cycle carries a newer raise timestamp and passes. Suppression fires only on a positive marker match — unset timestamps keep today's behavior — so the documented at-least-once consumer contract stands (gateway.md, Sessions.md updated in the same change). Tests: two new regressions drive the exact race through the GWC-26 harness (repair-then-buffered-live for Raise and for Clear, each with a genuine follow-up transition proving no over-suppression); GatewayAlarmMonitor suites 18/18.
2026-08-10 06:06:12 -04:00
parent 8624e21372
commit 2c03e0a684
4 changed files with 232 additions and 11 deletions
@@ -34,6 +34,14 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic
    private readonly Dictionary<string, ActiveAlarmSnapshot> _alarms = new(StringComparer.Ordinal);
    private readonly List<Subscriber> _subscribers = [];

+    // NEXT-03 dedup tombstones, guarded by _sync: alarm instances whose Clear was synthesized by
+    // the most recent reconcile pass, keyed by reference with the instance's original raise
+    // timestamp as the identity marker. A buffered live Clear for the same instance is a duplicate
+    // of the repair and is suppressed. One generation deep: each reconcile pass replaces the map,
+    // so a tombstone lives at least one reconcile interval — far longer than the lease buffer the
+    // duplicate would be sitting in — and the map stays bounded by the feed's churn per interval.
+    private readonly Dictionary<string, Timestamp> _clearedByReconcile = new(StringComparer.Ordinal);
+
    // Current provider status (mode + degraded + reason + since), guarded by _sync.
    // Initialized to the alarm-manager, not-degraded baseline so a late joiner sees
    // a sensible status even before any OnAlarmProviderModeChanged event arrives.
@@ -413,17 +421,60 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic
        {
            if (transition.TransitionKind == AlarmTransitionKind.Clear)
            {
-                _alarms.Remove(reference);
+                bool wasKnown = _alarms.Remove(reference);
+                if (!wasKnown && IsDuplicateOfReconcileClear(reference, transition))
+                {
+                    return;
+                }
            }
            else
            {
-                _alarms[reference] = SnapshotFromTransition(transition);
+                ActiveAlarmSnapshot snapshot = SnapshotFromTransition(transition);
+                bool duplicate = _alarms.TryGetValue(reference, out ActiveAlarmSnapshot? existing)
+                    && IsDuplicateOfCachedState(existing, snapshot);
+                _alarms[reference] = snapshot;
+                if (duplicate)
+                {
+                    return;
+                }
            }

            Broadcast(new AlarmFeedMessage { Transition = transition }, reference);
        }
    }

+    // NEXT-03: best-effort dedup of the reconcile/live race. A reconcile that already synthesized
+    // this transition as a feed repair left the cache carrying the worker's transition timestamp
+    // and resulting state — both derived from the same worker-side value the live transition
+    // carries — so an exact (timestamp, state) match means this live transition's outcome has
+    // already been broadcast. Suppress only on a positive match: an unset timestamp on either
+    // side keeps today's at-least-once behavior.
+    private static bool IsDuplicateOfCachedState(ActiveAlarmSnapshot existing, ActiveAlarmSnapshot incoming)
+    {
+        return existing.LastTransitionTimestamp is not null
+            && incoming.LastTransitionTimestamp is not null
+            && existing.LastTransitionTimestamp.Equals(incoming.LastTransitionTimestamp)
+            && existing.CurrentState == incoming.CurrentState;
+    }
+
+    // NEXT-03, the Clear leg. A reconcile Clear repair removes the cache entry before the buffered
+    // live Clear drains, so there is no cached state to compare against; the tombstone recorded by
+    // ApplyReconcile identifies the cleared instance by its original raise timestamp instead. The
+    // match consumes the tombstone, so a genuinely new raise/clear cycle (which carries a newer
+    // original raise timestamp) is never swallowed. Caller holds _sync.
+    private bool IsDuplicateOfReconcileClear(string reference, OnAlarmTransitionEvent transition)
+    {
+        if (transition.OriginalRaiseTimestamp is not null
+            && _clearedByReconcile.TryGetValue(reference, out Timestamp? clearedInstance)
+            && clearedInstance.Equals(transition.OriginalRaiseTimestamp))
+        {
+            _clearedByReconcile.Remove(reference);
+            return true;
+        }
+
+        return false;
+    }
+
    // Handles the worker's provider-mode-change event: updates the stored provider
    // status, broadcasts it to every subscriber (provider status is global, not
    // alarm-scoped), records the switch metric, and forces a cache reconcile so the
@@ -533,11 +584,14 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic
    //
    // Delivery semantics: feed repair transitions are AT-LEAST-ONCE, not exactly-once. A reconcile
    // reads the worker's current state while the corresponding live transition may still be
-    // buffered in the alarm lease's channel; both then broadcast, and the two are indistinguishable
-    // on the feed. This is inherent to the reconcile design and pre-dates the acked-state delta
-    // (the Raise/Clear repair has always had it), since nothing serializes a reconcile against the
-    // in-flight live stream. Consumers must therefore treat alarm state idempotently — apply a
-    // transition as "set the alarm to this state", never as an increment or a toggle.
+    // buffered in the alarm lease's channel; both would then broadcast, and the two are
+    // indistinguishable on the feed, since nothing serializes a reconcile against the in-flight
+    // live stream. ApplyTransition narrows that window with a best-effort dedup (NEXT-03): a live
+    // transition whose worker timestamp and resulting state the cache already carries — or whose
+    // Clear matches a tombstone recorded below — was already broadcast as a repair and is
+    // suppressed. The dedup fires only on a positive marker match, so the contract stays
+    // at-least-once: consumers must still treat alarm state idempotently — apply a transition as
+    // "set the alarm to this state", never as an increment or a toggle.
    private void ApplyReconcile(IEnumerable<ActiveAlarmSnapshot> snapshots)
    {
        Dictionary<string, ActiveAlarmSnapshot> next = new(StringComparer.Ordinal);
@@ -551,10 +605,19 @@ public sealed class GatewayAlarmMonitor : BackgroundService, IGatewayAlarmServic

        lock (_sync)
        {
+            // Previous-generation tombstones have outlived the buffered live transitions they
+            // guard against (one full reconcile interval); start this pass's generation fresh.
+            _clearedByReconcile.Clear();
+
            foreach (KeyValuePair<string, ActiveAlarmSnapshot> existing in _alarms)
            {
                if (!next.ContainsKey(existing.Key))
                {
+                    if (existing.Value.OriginalRaiseTimestamp is not null)
+                    {
+                        _clearedByReconcile[existing.Key] = existing.Value.OriginalRaiseTimestamp;
+                    }
+
                    Broadcast(
                        new AlarmFeedMessage { Transition = TransitionFromSnapshot(existing.Value, AlarmTransitionKind.Clear) },
                        existing.Key);