nikhiln64 opened a new pull request, #4316:
URL: https://github.com/apache/hertzbeat/pull/4316

   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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to