From da9ef07cfa2dfd26d1e8efdce5598a7ac7806271 Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Mon, 22 Aug 2011 12:01:17 +0100 Subject: * Race condition fixed (fixes #1861). * Reminders get sent at start of during period (fixes #1821) --- lib/mauve/alert.rb | 4 +-- lib/mauve/alert_changed.rb | 4 +-- lib/mauve/alert_group.rb | 33 +++++++++++++++++++++-- lib/mauve/notification.rb | 28 ++++++++++++------- lib/mauve/notifier.rb | 2 +- lib/mauve/person.rb | 67 ++-------------------------------------------- lib/mauve/version.rb | 2 +- 7 files changed, 57 insertions(+), 83 deletions(-) (limited to 'lib') diff --git a/lib/mauve/alert.rb b/lib/mauve/alert.rb index eb5f11b..c294e8c 100644 --- a/lib/mauve/alert.rb +++ b/lib/mauve/alert.rb @@ -392,7 +392,7 @@ module Mauve # # def all_raised - all(:raised_at.not => nil, :cleared_at => nil) - all_acknowledged + all(:raised_at.not => nil, :acknowledged_at => nil, :cleared_at => nil) end def all_acknowledged @@ -400,7 +400,7 @@ module Mauve end def all_cleared - all(:cleared_at.not => nil) - all_acknowledged + all(:cleared_at.not => nil, :acknowledged_at => nil) end # Returns a hash of all the :urgent, :normal and :low alerts. diff --git a/lib/mauve/alert_changed.rb b/lib/mauve/alert_changed.rb index 83332ff..ac864a0 100644 --- a/lib/mauve/alert_changed.rb +++ b/lib/mauve/alert_changed.rb @@ -19,12 +19,10 @@ module Mauve property :remind_at, Time # property :updated_at, Time, :required => true - def inspect + def to_s "#" end - alias to_s inspect - belongs_to :alert def was_relevant=(value) diff --git a/lib/mauve/alert_group.rb b/lib/mauve/alert_group.rb index 114c39b..df76f40 100644 --- a/lib/mauve/alert_group.rb +++ b/lib/mauve/alert_group.rb @@ -95,16 +95,45 @@ module Mauve # # If there are no notifications defined. # - if notifications.nil? + if notifications.nil? logger.warn("No notifications found for #{self.inspect}") return end + # + # This is where we set the reminder -- i.e. on a per-alert-group basis. + # + remind_at = notifications.inject(nil) do |reminder_time, notification| + this_time = notification.remind_at_next(alert) + if reminder_time.nil? or (!this_time.nil? and reminder_time > this_time) + this_time + else + reminder_time + end + end + + # + # OK got the next reminder time. + # + unless remind_at.nil? + this_reminder = AlertChanged.new( + :level => level.to_s, + :alert_id => alert.id, + :person => self.name, + :at => Time.now, + :update_type => alert.update_type, + :remind_at => remind_at, + :was_relevant => true) + + this_reminder.save + end + # # The notifications are specified in the config file. # + sent_to = [] notifications.each do |notification| - notification.notify(alert) + sent_to = notification.notify(alert, sent_to) end end diff --git a/lib/mauve/notification.rb b/lib/mauve/notification.rb index c57d2a9..43de1a7 100644 --- a/lib/mauve/notification.rb +++ b/lib/mauve/notification.rb @@ -145,7 +145,6 @@ module Mauve attr_reader :thread_list def initialize(people, level) - self.level = level self.every = 300 self.people = people @@ -153,7 +152,7 @@ module Mauve def logger ; Log4r::Logger.new self.class.to_s ; end - def notify(alert) + def notify(alert, already_sent_to = []) if people.nil? or people.empty? logger.warn "No people found in for notification #{list}" @@ -161,9 +160,7 @@ module Mauve end # Should we notify at all? - is_relevant = DuringRunner.new(Time.now, alert, &during).now? - - n_sent = 0 + return already_sent_to unless DuringRunner.new(Time.now, alert, &during).now? people.collect do |person| case person @@ -176,16 +173,29 @@ module Mauve [] end end.flatten.uniq.each do |person| - n_sent += 1 if person.send_alert(self.level, alert, is_relevant, remind_at_next(alert)) + # + # A bit of alert de-bouncing. + # + if already_sent_to.include?(person.username) + logger.info("Already sent notification of #{alert} to #{person.username}") + else + Server.notification_push([person, level, alert]) + already_sent_to << person.username + end end - return n_sent + return already_sent_to end def remind_at_next(alert) - return DuringRunner.new(Time.now, alert, &during).find_next(every) if alert.raised? + return nil unless alert.raised? + + if DuringRunner.new(Time.now, alert, &during).now? + return DuringRunner.new(Time.now, alert, &during).find_next(every) + else + return DuringRunner.new(Time.now, alert, &during).find_next(0) + end - return nil end end diff --git a/lib/mauve/notifier.rb b/lib/mauve/notifier.rb index 4737c37..2853485 100644 --- a/lib/mauve/notifier.rb +++ b/lib/mauve/notifier.rb @@ -25,7 +25,7 @@ module Mauve # next if person.nil? - person.do_send_alert(*args) + person.send_alert(*args) end end diff --git a/lib/mauve/person.rb b/lib/mauve/person.rb index 26425da..60ad60a 100644 --- a/lib/mauve/person.rb +++ b/lib/mauve/person.rb @@ -86,72 +86,9 @@ module Mauve end # - # Sends the alert, and updates when the AlertChanged database to set the next reminder. + # Sends the alert # - def send_alert(level, alert, is_relevant=true, remind_at=nil) - # - # First check that we've not just sent an notification to this person for - # this alert - # - last_reminder = AlertChanged.first(:alert => alert, :person => username, :update_type => alert.update_type, :at.gte => (Time.now - 1.minute) ) - - if last_reminder and last_reminder.at >= (Time.now - 1.minute) - # - # - logger.info("Not sending notification to #{username} for #{alert} because one has just been sent.") - return false - end - - # - # For the love of God, do not remind about ack'd or cleared alerts. - # - if alert.acknowledged? or alert.cleared? - remind_at = nil - end - - this_reminder = AlertChanged.new( - :level => level.to_s, - :alert_id => alert.id, - :person => username, - :at => Time.now, - :update_type => alert.update_type, - :remind_at => remind_at, - :was_relevant => is_relevant) - - # - # Check to make sure that we've not got a sooner reminder set. - # - unless remind_at.nil? - next_reminder = AlertChanged.first(:alert => alert, :remind_at.gt => Time.now, :person => username, :update_type => alert.update_type) - - if next_reminder - # - # If the reminder is further in the future than the one we're about - # to put on, then just update it. - # - # Otherwise if it is sooner, we don't need to create a new one. - # - if next_reminder.remind_at > remind_at - next_reminder.remind_at = remind_at - logger.info("Not inserting a new reminder, as there is already one in place sooner") - this_reminder = next_reminder - else - this_reminder = nil - end - end - end - - this_reminder.save unless this_reminder.nil? - - if is_relevant - Server.notification_push([self, level, alert]) - return true - end - - return false - end - - def do_send_alert(level, alert) + def send_alert(level, alert) now = Time.now was_suppressed = self.suppressed? diff --git a/lib/mauve/version.rb b/lib/mauve/version.rb index 1c9b8d7..7e94be0 100644 --- a/lib/mauve/version.rb +++ b/lib/mauve/version.rb @@ -1,5 +1,5 @@ module Mauve - VERSION="3.4.2" + VERSION="3.4.3" end -- cgit v1.2.1