[bugfix] Alarm group convergence must not resolve while members are still firing - #4316
Open
nikhiln64 wants to merge 1 commit into
Open
[bugfix] Alarm group convergence must not resolve while members are still firing#4316nikhiln64 wants to merge 1 commit into
nikhiln64 wants to merge 1 commit into
Conversation
…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
force-pushed
the
fix/alarm-group-converge-4160
branch
from
August 14, 2026 22:41
32c8edf to
216674c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.