Skip to content

Commit

Permalink
notifier: fix a bug that prevented graceful shutdown in certain cases
Browse files Browse the repository at this point in the history
- The notifier was never checking if it should shutdown if there were always at least one notification to send
- After a failure, the notifier is supposed to back-off, but when Clair is stopping, the backoff was interrupted immediately and did not retry. Instead it selected a new notification to send (most likely: the same one) and looped quickly/weirdly.
  • Loading branch information
Quentin-M committed Dec 17, 2015
1 parent 480589a commit 2ea86c5
Showing 1 changed file with 12 additions and 7 deletions.
19 changes: 12 additions & 7 deletions notifier/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,20 +104,25 @@ func Run(config *config.NotifierConfig, stopper *utils.Stopper) {
// Register healthchecker.
health.RegisterHealthchecker("notifier", Healthcheck)

for {
for running := true; running; {
// Find task.
// TODO(Quentin-M): Combine node and notification.
node, notification := findTask(whoAmI, stopper)
if node == "" && notification == nil {
// Interrupted while finding a task, Clair is stopping.
break
}

// Handle task.
done := make(chan bool, 1)
go func() {
if handleTask(notification, stopper, config.Attempts) {
success, interrupted := handleTask(notification, stopper, config.Attempts)
if success {
database.MarkNotificationAsSent(node)
}
if interrupted {
running = false
}
database.Unlock(node, whoAmI)
done <- true
}()
Expand Down Expand Up @@ -161,13 +166,13 @@ func findTask(whoAmI string, stopper *utils.Stopper) (string, database.Notificat
}
}

func handleTask(notification database.Notification, st *utils.Stopper, maxAttempts int) bool {
func handleTask(notification database.Notification, st *utils.Stopper, maxAttempts int) (bool, bool) {
// Get notification content.
// TODO(Quentin-M): Split big notifications.
notificationContent, err := notification.GetContent()
if err != nil {
log.Warningf("could not get content of notification '%s': %s", notification.GetName(), err)
return false
return false, false
}

// Create notification.
Expand All @@ -185,14 +190,14 @@ func handleTask(notification database.Notification, st *utils.Stopper, maxAttemp
// Max attempts exceeded.
if attempts >= maxAttempts {
log.Infof("giving up on sending notification '%s' to notifier '%s': max attempts exceeded (%d)\n", notification.GetName(), notifierName, maxAttempts)
return false
return false, false
}

// Backoff.
if backOff > 0 {
log.Infof("waiting %v before retrying to send notification '%s' to notifier '%s' (Attempt %d / %d)\n", backOff, notification.GetName(), notifierName, attempts+1, maxAttempts)
if !st.Sleep(backOff) {
return false
return false, true
}
}

Expand All @@ -210,7 +215,7 @@ func handleTask(notification database.Notification, st *utils.Stopper, maxAttemp
}

log.Infof("successfully sent notification '%s'\n", notification.GetName())
return true
return true, false
}

// Healthcheck returns the health of the notifier service.
Expand Down

0 comments on commit 2ea86c5

Please sign in to comment.