From 9022e871e3cb3c5384415a2eab768f678dda471a Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 2 Oct 2018 13:57:41 +0200 Subject: [PATCH] avoid sending full notification state to pending/complete --- pkg/models/alert_notifications.go | 8 +++- pkg/services/alerting/notifier.go | 10 ++++- pkg/services/sqlstore/alert_notification.go | 24 ++++++------ .../sqlstore/alert_notification_test.go | 38 +++++++++++-------- 4 files changed, 48 insertions(+), 32 deletions(-) diff --git a/pkg/models/alert_notifications.go b/pkg/models/alert_notifications.go index d908200b36e..2128b469fa4 100644 --- a/pkg/models/alert_notifications.go +++ b/pkg/models/alert_notifications.go @@ -98,12 +98,16 @@ type AlertNotificationState struct { } type SetAlertNotificationStateToPendingCommand struct { + Id int64 AlertRuleStateUpdatedVersion int64 - State *AlertNotificationState + Version int64 + + ResultVersion int64 } type SetAlertNotificationStateToCompleteCommand struct { - State *AlertNotificationState + Id int64 + Version int64 } type GetOrCreateNotificationStateQuery struct { diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index 4f866a58ab7..dae310a8e3f 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -76,7 +76,8 @@ func (n *notificationService) sendAndMarkAsComplete(evalContext *EvalContext, no } cmd := &m.SetAlertNotificationStateToCompleteCommand{ - State: notifierState.state, + Id: not.GetNotifierId(), + Version: notifierState.state.Version, } if err = bus.DispatchCtx(evalContext.Ctx, cmd); err != nil { @@ -94,7 +95,8 @@ func (n *notificationService) sendAndMarkAsComplete(evalContext *EvalContext, no func (n *notificationService) sendNotification(evalContext *EvalContext, notifierState *NotifierState) error { if !evalContext.IsTestRun { setPendingCmd := &m.SetAlertNotificationStateToPendingCommand{ - State: notifierState.state, + Id: notifierState.state.NotifierId, + Version: notifierState.state.Version, AlertRuleStateUpdatedVersion: evalContext.Rule.StateChanges, } @@ -106,6 +108,10 @@ func (n *notificationService) sendNotification(evalContext *EvalContext, notifie if err != nil { return err } + + // We need to update state version to be able to log + // unexpected version conflicts when marking notifications as ok + notifierState.state.Version = setPendingCmd.ResultVersion } return n.sendAndMarkAsComplete(evalContext, notifierState) diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 33d19570a3c..069e6213b03 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -239,12 +239,11 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetAlertNotificationStateToCompleteCommand) error { return withDbSession(ctx, func(sess *DBSession) error { - version := cmd.State.Version + version := cmd.Version var current m.AlertNotificationState - sess.ID(cmd.State.Id).Get(¤t) + sess.ID(cmd.Id).Get(¤t) - cmd.State.State = m.AlertNotificationStateCompleted - cmd.State.Version++ + newVersion := cmd.Version + 1 sql := `UPDATE alert_notification_state SET state = ?, @@ -253,7 +252,7 @@ func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetA WHERE id = ?` - _, err := sess.Exec(sql, cmd.State.State, cmd.State.Version, timeNow().Unix(), cmd.State.Id) + _, err := sess.Exec(sql, m.AlertNotificationStateCompleted, newVersion, timeNow().Unix(), cmd.Id) if err != nil { return err @@ -269,10 +268,7 @@ func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetA func SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *m.SetAlertNotificationStateToPendingCommand) error { return withDbSession(ctx, func(sess *DBSession) error { - currentVersion := cmd.State.Version - cmd.State.State = m.AlertNotificationStatePending - cmd.State.Version++ - + newVersion := cmd.Version + 1 sql := `UPDATE alert_notification_state SET state = ?, version = ?, @@ -283,12 +279,12 @@ func SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *m.SetAl (version = ? OR alert_rule_state_updated_version < ?)` res, err := sess.Exec(sql, - cmd.State.State, - cmd.State.Version, + m.AlertNotificationStatePending, + newVersion, timeNow().Unix(), cmd.AlertRuleStateUpdatedVersion, - cmd.State.Id, - currentVersion, + cmd.Id, + cmd.Version, cmd.AlertRuleStateUpdatedVersion) if err != nil { @@ -300,6 +296,8 @@ func SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *m.SetAl return m.ErrAlertNotificationStateVersionConflict } + cmd.ResultVersion = newVersion + return nil }) } diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 559a1379f91..df360d14efa 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -42,13 +42,16 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Update existing state to pending with correct version should update database", func() { s := *query.Result + cmd := models.SetAlertNotificationStateToPendingCommand{ - State: &s, + Id: s.Id, + Version: s.Version, + AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, } + err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) So(err, ShouldBeNil) - So(cmd.State.Version, ShouldEqual, 1) - So(cmd.State.State, ShouldEqual, models.AlertNotificationStatePending) + So(cmd.ResultVersion, ShouldEqual, 1) query2 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} err = GetOrCreateAlertNotificationState(context.Background(), query2) @@ -58,11 +61,12 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(query2.Result.UpdatedAt, ShouldEqual, now.Unix()) Convey("Update existing state to completed should update database", func() { - s := *cmd.State - cmd := models.SetAlertNotificationStateToCompleteCommand{ - State: &s, + s := *query.Result + setStateCmd := models.SetAlertNotificationStateToCompleteCommand{ + Id: s.Id, + Version: cmd.ResultVersion, } - err := SetAlertNotificationStateToCompleteCommand(context.Background(), &cmd) + err := SetAlertNotificationStateToCompleteCommand(context.Background(), &setStateCmd) So(err, ShouldBeNil) query3 := &models.GetOrCreateNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID} @@ -74,10 +78,10 @@ func TestAlertNotificationSQLAccess(t *testing.T) { }) Convey("Update existing state to completed should update database, but return version mismatch", func() { - cmd.State.Version = 1000 - s := *cmd.State + s := *query.Result cmd := models.SetAlertNotificationStateToCompleteCommand{ - State: &s, + Id: s.Id, + Version: 1000, } err := SetAlertNotificationStateToCompleteCommand(context.Background(), &cmd) So(err, ShouldEqual, models.ErrAlertNotificationStateVersionConflict) @@ -95,7 +99,9 @@ func TestAlertNotificationSQLAccess(t *testing.T) { s := *query.Result s.Version = 1000 cmd := models.SetAlertNotificationStateToPendingCommand{ - State: &s, + Id: s.NotifierId, + Version: s.Version, + AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, } err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) So(err, ShouldEqual, models.ErrAlertNotificationStateVersionConflict) @@ -104,21 +110,23 @@ func TestAlertNotificationSQLAccess(t *testing.T) { Convey("Updating existing state to pending with incorrect version since alert rule state update version is higher", func() { s := *query.Result cmd := models.SetAlertNotificationStateToPendingCommand{ - State: &s, + Id: s.Id, + Version: s.Version, AlertRuleStateUpdatedVersion: 1000, } err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) So(err, ShouldBeNil) - So(cmd.State.Version, ShouldEqual, 1) - So(cmd.State.State, ShouldEqual, models.AlertNotificationStatePending) + So(cmd.ResultVersion, ShouldEqual, 1) }) Convey("different version and same alert state change version should return error", func() { s := *query.Result s.Version = 1000 cmd := models.SetAlertNotificationStateToPendingCommand{ - State: &s, + Id: s.Id, + Version: s.Version, + AlertRuleStateUpdatedVersion: s.AlertRuleStateUpdatedVersion, } err := SetAlertNotificationStateToPendingCommand(context.Background(), &cmd) So(err, ShouldNotBeNil)