Skip to content

[bugfix] Alarm group convergence must not resolve while members are still firing - #4316

Open
nikhiln64 wants to merge 1 commit into
apache:masterfrom
nikhiln64:fix/alarm-group-converge-4160
Open

[bugfix] Alarm group convergence must not resolve while members are still firing#4316
nikhiln64 wants to merge 1 commit into
apache:masterfrom
nikhiln64:fix/alarm-group-converge-4160

Conversation

@nikhiln64

Copy link
Copy Markdown

Fixes #4160.

The alarm group convergence path in AlarmGroupReduce decides a group's status from the wrong set of alerts, and because of that it does two bad things. It flips a group to resolved while other members are still firing, and it silently throws away recovery events so the database record stays firing forever. Both come from the same place. The per group cache, cache.getAlertFingerprints(), is fully cleared after every send, so by the time the code asks whether the group is firing or resolved it is only looking at the alerts that happened to arrive during the current send window rather than at every member that is still active.

The first symptom is a premature resolve. After a send that carried both CPU firing and Memory firing, the cache is emptied. When CPU recovers a lone CPU resolved lands in that emptied cache, shouldSendGroupImmediately sees a cache where everything is resolved, and the whole group goes out as resolved even though Memory never stopped firing. You can see it in the payloads where the resolved group push is immediately followed by another Memory firing push, which would be impossible if Memory had actually recovered.

The second symptom is a lost recovery. After a firing send arms the repeat interval, the cache is cleared again. CPU keeps firing and Memory recovers, so the cache now holds a firing CPU next to a resolved Memory. determineGroupStatus returns firing because CPU is firing, the repeat interval throttle in sendGroupAlert returns early, and then runCheckAndSendGroups clears the cache anyway. The Memory resolved transition is gone. It is never emitted, the plugin never sees it, and the alert stays firing in the database with no endAt and no auto resolve.

The fix is to stop treating the cache as a per window scratch buffer and let it hold the live set of members. This change keeps firing alerts across sends instead of clearing everything, removes only the resolved members after they have actually been emitted, and makes sure the firing repeat interval throttle suppresses repeated firing notifications only and never a pending resolved transition. With that in place the group stays firing while any member is still active, a member recovery rides out inside the still firing group, and the group only resolves once every member has genuinely cleared.

I added two regression tests in AlarmGroupReduceTest. One asserts that no emitted group ever carries resolved status while a member is still firing, the other asserts that a member recovery inside the repeat interval window is still emitted. Both fail on the current code and pass with this change, and the affected module builds green.

One honest note. This is timing sensitive and not perfectly reproducible from the outside, but the code paths are deterministic and the fix follows directly from them. I deliberately left two pre existing behaviours out of scope to keep the change focused, that lastRepeatTime is not reset when a group fully drains, and that empty group caches are never removed from groupCacheMap. Happy to look at either separately if you would like.

…ring (apache#4160)

The group cache was flushed after every send, so group status was computed
only over the alerts seen within the current send window instead of all
active members. A lone resolved alert whose firing siblings had been flushed
made the whole group resolve, and a resolved transition arriving while the
group was firing inside the repeat-interval window was cleared away before
ever being emitted.

Retain active alerts across sends, drop only the resolved members after they
have actually been emitted, and never let the firing repeat-interval throttle
swallow a pending recovery.
@nikhiln64
nikhiln64 force-pushed the fix/alarm-group-converge-4160 branch from 32c8edf to 216674c Compare August 14, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Alarm group convergence: group wrongly flips to resolved and recovery events get silently dropped

1 participant