From 0642137a1135b519fdc9f755ab75d6fa11936643 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Mon, 27 Oct 2014 23:50:12 +0100 Subject: [PATCH] Fix alert expiry crashbug. No, the array representation of a binary heap is of course *not* sorted. Change-Id: Ib18c9b7e1bee24391f98d73135ac19c77026b168 --- manager/manager.go | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/manager/manager.go b/manager/manager.go index b3be96a7..63f67759 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -15,7 +15,6 @@ package manager import ( "container/heap" - "sort" "strings" "sync" "time" @@ -98,6 +97,15 @@ func (aggs aggregatesByNextNotification) Less(i, j int) bool { return aggs.AlertAggregates[i].NextNotification.Before(aggs.AlertAggregates[j].NextNotification) } +// rebuildFrom rebuilds the aggregatesByNextNotification index from a provided +// authoritative AlertAggregates slice. +func (aggs *aggregatesByNextNotification) rebuildFrom(aa AlertAggregates) { + aggs.AlertAggregates = aggs.AlertAggregates[:0] + for _, a := range aa { + aggs.Push(a) + } +} + func (aggs AlertAggregates) Swap(i, j int) { aggs[i], aggs[j] = aggs[j], aggs[i] } @@ -264,36 +272,23 @@ func (s *memoryAlertManager) removeExpiredAggregates() { // aggregates remain in the heap. for { if len(s.aggregatesByLastRefreshed.AlertAggregates) == 0 { - return + break } agg := heap.Pop(&s.aggregatesByLastRefreshed).(*AlertAggregate) if time.Since(agg.LastRefreshed) > s.minRefreshInterval { delete(s.aggregates, agg.Alert.Fingerprint()) - - // Also remove the aggregate from the last-notification-time index. - n := len(s.aggregatesByNextNotification.AlertAggregates) - i := sort.Search(n, func(i int) bool { - return !agg.NextNotification.After(s.aggregatesByNextNotification.AlertAggregates[i].NextNotification) - }) - if i == n { - panic("Missing alert aggregate in aggregatesByNextNotification index") - } else { - for j := i; j < n; j++ { - if s.aggregatesByNextNotification.AlertAggregates[j] == agg { - heap.Remove(&s.aggregatesByNextNotification, j) - break - } - } - } - s.needsNotificationRefresh = true } else { heap.Push(&s.aggregatesByLastRefreshed, agg) - return + break } } + + if s.needsNotificationRefresh { + s.aggregatesByNextNotification.rebuildFrom(s.aggregatesByLastRefreshed.AlertAggregates) + } } // Check whether one of the filtered (uninhibited, unsilenced) alerts should