From 23c70c4368b6adc1854d983817f0ba3936512092 Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 24 Oct 2023 14:10:18 +0300 Subject: [PATCH 01/17] add get trigger checks function and merge --- cmd/api/config.go | 6 ++ cmd/api/config_test.go | 5 + cmd/api/main.go | 3 +- cmd/checker/main.go | 2 +- cmd/cli/main.go | 2 +- cmd/config.go | 20 ++++ cmd/filter/main.go | 2 +- cmd/notifier/config.go | 6 ++ cmd/notifier/main.go | 3 +- database/redis/config.go | 11 ++ .../contact_notification_history_test.go | 1 + database/redis/database.go | 19 +++- database/redis/last_check.go | 22 ++++ database/redis/last_check_test.go | 101 ++++++++++++++++++ database/redis/reply/check.go | 21 ++++ database/redis/reply/notification.go | 3 + datatypes.go | 43 ++++++-- datatypes_test.go | 26 +++++ go.mod | 2 +- helpers.go | 37 +++++++ helpers_test.go | 77 ++++++++++++- local/api.yml | 4 + local/notifier.yml | 4 + notifier/events/event_test.go | 2 + notifier/notifier.go | 2 +- notifier/scheduler.go | 2 + notifier/scheduler_test.go | 1 + 27 files changed, 406 insertions(+), 21 deletions(-) diff --git a/cmd/api/config.go b/cmd/api/config.go index 50ec41623..7078632d0 100644 --- a/cmd/api/config.go +++ b/cmd/api/config.go @@ -21,6 +21,7 @@ type config struct { Remote cmd.RemoteConfig `yaml:"remote"` Prometheus cmd.PrometheusConfig `yaml:"prometheus"` NotificationHistory cmd.NotificationHistoryConfig `yaml:"notification_history"` + Notification cmd.NotificationConfig `yaml:"notification"` } type apiConfig struct { @@ -114,6 +115,11 @@ func getDefault() config { NotificationHistoryTTL: "48h", NotificationHistoryQueryLimit: int(notifier.NotificationsLimitUnlimited), }, + Notification: cmd.NotificationConfig{ + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + }, Logger: cmd.LoggerConfig{ LogFile: "stdout", LogLevel: "info", diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index 75cd644ab..42fffa171 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -102,6 +102,11 @@ func Test_webConfig_getDefault(t *testing.T) { NotificationHistoryTTL: "48h", NotificationHistoryQueryLimit: -1, }, + Notification: cmd.NotificationConfig{ + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + }, } result := getDefault() diff --git a/cmd/api/main.go b/cmd/api/main.go index 879a99d55..df5ff2568 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -83,7 +83,8 @@ func main() { databaseSettings := applicationConfig.Redis.GetSettings() notificationHistorySettings := applicationConfig.NotificationHistory.GetSettings() - database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, redis.API) + notificationSettings := applicationConfig.Notification.GetSettings() + database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, notificationSettings, redis.API) // Start Index right before HTTP listener. Fail if index cannot start searchIndex := index.NewSearchIndex(logger, database, telemetry.Metrics) diff --git a/cmd/checker/main.go b/cmd/checker/main.go index 5de14c9ea..29b85735c 100644 --- a/cmd/checker/main.go +++ b/cmd/checker/main.go @@ -83,7 +83,7 @@ func main() { logger.Info().Msg("Debug: checker started") databaseSettings := config.Redis.GetSettings() - database := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.Checker) + database := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Checker) remoteConfig := config.Remote.GetRemoteSourceSettings() prometheusConfig := config.Prometheus.GetPrometheusSourceSettings() diff --git a/cmd/cli/main.go b/cmd/cli/main.go index 5fda18983..13c35b45d 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -345,7 +345,7 @@ func initApp() (cleanupConfig, moira.Logger, moira.Database) { } databaseSettings := config.Redis.GetSettings() - dataBase := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.Cli) + dataBase := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Cli) return config.Cleanup, logger, dataBase } diff --git a/cmd/config.go b/cmd/config.go index 267db3edc..8af184fe5 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -79,6 +79,26 @@ func (notificationHistoryConfig *NotificationHistoryConfig) GetSettings() redis. } } +// NotificationConfig is a config that stores the necessary configuration of the notifier +type NotificationConfig struct { + // Need to determine if notification is delayed - the difference between creation time and sending time + // is greater than DelayedTime + DelayedTime string `yaml:"delayed_time"` + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionTimeout string `yaml:"transaction_timeout"` + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionMaxRetries int `yaml:"transaction_max_retries"` +} + +// GetSettings returns notification storage configuration +func (notificationConfig *NotificationConfig) GetSettings() redis.NotificationConfig { + return redis.NotificationConfig{ + DelayedTime: to.Duration(notificationConfig.DelayedTime), + TransactionTimeout: to.Duration(notificationConfig.TransactionTimeout), + TransactionMaxRetries: notificationConfig.TransactionMaxRetries, + } +} + // GraphiteConfig is graphite metrics config structure that initialises at the start of moira type GraphiteConfig struct { // If true, graphite sender will be enabled. diff --git a/cmd/filter/main.go b/cmd/filter/main.go index ced22bb6e..2b4d64f12 100644 --- a/cmd/filter/main.go +++ b/cmd/filter/main.go @@ -87,7 +87,7 @@ func main() { } filterMetrics := metrics.ConfigureFilterMetrics(telemetry.Metrics) - database := redis.NewDatabase(logger, config.Redis.GetSettings(), redis.NotificationHistoryConfig{}, redis.Filter) + database := redis.NewDatabase(logger, config.Redis.GetSettings(), redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Filter) retentionConfigFile, err := os.Open(config.Filter.RetentionConfig) if err != nil { diff --git a/cmd/notifier/config.go b/cmd/notifier/config.go index e3679ade9..3c4d4528d 100644 --- a/cmd/notifier/config.go +++ b/cmd/notifier/config.go @@ -21,6 +21,7 @@ type config struct { Prometheus cmd.PrometheusConfig `yaml:"prometheus"` ImageStores cmd.ImageStoreConfig `yaml:"image_store"` NotificationHistory cmd.NotificationHistoryConfig `yaml:"notification_history"` + Notification cmd.NotificationConfig `yaml:"notification"` } type entityLogConfig struct { @@ -93,6 +94,11 @@ func getDefault() config { NotificationHistoryTTL: "48h", NotificationHistoryQueryLimit: int(notifier.NotificationsLimitUnlimited), }, + Notification: cmd.NotificationConfig{ + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + }, Notifier: notifierConfig{ SenderTimeout: "10s", ResendingTimeout: "1:00", diff --git a/cmd/notifier/main.go b/cmd/notifier/main.go index 8a75bb410..a90fb2159 100644 --- a/cmd/notifier/main.go +++ b/cmd/notifier/main.go @@ -81,7 +81,8 @@ func main() { databaseSettings := config.Redis.GetSettings() notificationHistorySettings := config.NotificationHistory.GetSettings() - database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, redis.Notifier) + notificationSettings := config.Notification.GetSettings() + database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, notificationSettings, redis.Notifier) remoteConfig := config.Remote.GetRemoteSourceSettings() prometheusConfig := config.Prometheus.GetPrometheusSourceSettings() diff --git a/database/redis/config.go b/database/redis/config.go index 045ed7b21..18b357657 100644 --- a/database/redis/config.go +++ b/database/redis/config.go @@ -21,3 +21,14 @@ type NotificationHistoryConfig struct { NotificationHistoryTTL time.Duration NotificationHistoryQueryLimit int } + +// Notifier configuration in redis +type NotificationConfig struct { + // Need to determine if notification is delayed - the difference between creation time and sending time + // is greater than DelayedTime + DelayedTime time.Duration + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionTimeout time.Duration + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionMaxRetries int +} diff --git a/database/redis/contact_notification_history_test.go b/database/redis/contact_notification_history_test.go index cb1961828..b57e60c17 100644 --- a/database/redis/contact_notification_history_test.go +++ b/database/redis/contact_notification_history_test.go @@ -46,6 +46,7 @@ var inputScheduledNotification = moira.ScheduledNotification{ Throttled: false, SendFail: 1, Timestamp: time.Now().Unix(), + CreatedAt: time.Now().Unix(), } var eventsShouldBeInDb = []*moira.NotificationEventHistoryItem{ diff --git a/database/redis/database.go b/database/redis/database.go index f760049b2..5824a02da 100644 --- a/database/redis/database.go +++ b/database/redis/database.go @@ -46,9 +46,11 @@ type DbConnector struct { source DBSource clock moira.Clock notificationHistory NotificationHistoryConfig + // Notifier configuration in redis + notification NotificationConfig } -func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHistoryConfig, source DBSource) *DbConnector { +func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHistoryConfig, n NotificationConfig, source DBSource) *DbConnector { client := redis.NewUniversalClient(&redis.UniversalOptions{ MasterName: config.MasterName, Addrs: config.Addrs, @@ -78,7 +80,9 @@ func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHist source: source, clock: clock.NewSystemClock(), notificationHistory: nh, + notification: n, } + return &connector } @@ -90,7 +94,13 @@ func NewTestDatabase(logger moira.Logger) *DbConnector { NotificationHistoryConfig{ NotificationHistoryTTL: time.Hour * 48, NotificationHistoryQueryLimit: 1000, - }, testSource) + }, + NotificationConfig{ + DelayedTime: time.Minute, + TransactionTimeout: 200 * time.Millisecond, + TransactionMaxRetries: 10, + }, + testSource) } // NewTestDatabaseWithIncorrectConfig use it only for tests @@ -101,6 +111,11 @@ func NewTestDatabaseWithIncorrectConfig(logger moira.Logger) *DbConnector { NotificationHistoryTTL: time.Hour * 48, NotificationHistoryQueryLimit: 1000, }, + NotificationConfig{ + DelayedTime: time.Minute, + TransactionTimeout: 200 * time.Millisecond, + TransactionMaxRetries: 10, + }, testSource) } diff --git a/database/redis/last_check.go b/database/redis/last_check.go index 21ac48aeb..408375cfd 100644 --- a/database/redis/last_check.go +++ b/database/redis/last_check.go @@ -198,6 +198,28 @@ func (connector *DbConnector) checkDataScoreChanged(triggerID string, checkData return oldScore != float64(checkData.Score) } +// getTriggersLastCheck returns an array of trigger checks by the passed ids, if the trigger does not exist, it is nil +func (connector *DbConnector) getTriggersLastCheck(triggerIDs []string) ([]*moira.CheckData, error) { + ctx := connector.context + pipe := (*connector.client).TxPipeline() + + results := make([]*redis.StringCmd, len(triggerIDs)) + for i, id := range triggerIDs { + var result *redis.StringCmd + if id != "" { + result = pipe.Get(ctx, metricLastCheckKey(id)) + } + results[i] = result + } + + _, err := pipe.Exec(ctx) + if err != nil && err != redis.Nil { + return nil, err + } + + return reply.Checks(results) +} + var badStateTriggersKey = "moira-bad-state-triggers" var triggersChecksKey = "moira-triggers-checks" diff --git a/database/redis/last_check_test.go b/database/redis/last_check_test.go index 5d7cebceb..f4fc17e05 100644 --- a/database/redis/last_check_test.go +++ b/database/redis/last_check_test.go @@ -469,6 +469,107 @@ func TestLastCheckErrorConnection(t *testing.T) { }) } +func TestGetTriggersLastCheck(t *testing.T) { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 1, + }, moira.TriggerSourceNotSet) + + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Timestamp: 2, + }, moira.TriggerSourceNotSet) + + _ = dataBase.SetTriggerLastCheck("test3", &moira.CheckData{ + Timestamp: 3, + }, moira.TriggerSourceNotSet) + + Convey("getTriggersLastCheck manipulations", t, func() { + Convey("Test with nil id array", func() { + actual, err := dataBase.getTriggersLastCheck(nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{}) + }) + + Convey("Test with correct id array", func() { + actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test3"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 2, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 3, + MetricsToTargetRelation: map[string]string{}, + }, + }) + }) + + Convey("Test with deleted trigger", func() { + dataBase.RemoveTriggerLastCheck("test2") //nolint + defer func() { + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Timestamp: 2, + }, moira.TriggerSourceNotSet) + }() + + actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test3"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + nil, + { + Timestamp: 3, + MetricsToTargetRelation: map[string]string{}, + }, + }) + }) + + Convey("Test with a nonexistent trigger id", func() { + actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test4"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 2, + MetricsToTargetRelation: map[string]string{}, + }, + nil, + }) + }) + + Convey("Test with an empty trigger id", func() { + actual, err := dataBase.getTriggersLastCheck([]string{"", "test2", "test3"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + nil, + { + Timestamp: 2, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 3, + MetricsToTargetRelation: map[string]string{}, + }, + }) + }) + }) +} + func TestMaintenanceUserSave(t *testing.T) { logger, _ := logging.GetLogger("dataBase") dataBase := NewTestDatabase(logger) diff --git a/database/redis/reply/check.go b/database/redis/reply/check.go index 72f0c6167..d4e188057 100644 --- a/database/redis/reply/check.go +++ b/database/redis/reply/check.go @@ -110,6 +110,27 @@ func Check(rep *redis.StringCmd) (moira.CheckData, error) { return checkSE.toCheckData(), nil } +// Checks converts an array of redis DB reply to moira.CheckData objects, if reply is nil, then checkdata is nil +func Checks(replies []*redis.StringCmd) ([]*moira.CheckData, error) { + checks := make([]*moira.CheckData, len(replies)) + + for i, value := range replies { + if value != nil { + check, err := Check(value) + if err != nil { + if err != database.ErrNil { + return nil, err + } + continue + } + + checks[i] = &check + } + } + + return checks, nil +} + // GetCheckBytes is a function that takes moira.CheckData and turns it to bytes that will be saved in redis. func GetCheckBytes(check moira.CheckData) ([]byte, error) { checkSE := toCheckDataStorageElement(check) diff --git a/database/redis/reply/notification.go b/database/redis/reply/notification.go index 399be8f0e..e23dfb347 100644 --- a/database/redis/reply/notification.go +++ b/database/redis/reply/notification.go @@ -18,6 +18,7 @@ type scheduledNotificationStorageElement struct { Throttled bool `json:"throttled"` SendFail int `json:"send_fail"` Timestamp int64 `json:"timestamp"` + CreatedAt int64 `json:"created_at"` } func toScheduledNotificationStorageElement(notification moira.ScheduledNotification) scheduledNotificationStorageElement { @@ -29,6 +30,7 @@ func toScheduledNotificationStorageElement(notification moira.ScheduledNotificat Throttled: notification.Throttled, SendFail: notification.SendFail, Timestamp: notification.Timestamp, + CreatedAt: notification.CreatedAt, } } @@ -41,6 +43,7 @@ func (n scheduledNotificationStorageElement) toScheduledNotification() moira.Sch Throttled: n.Throttled, SendFail: n.SendFail, Timestamp: n.Timestamp, + CreatedAt: n.CreatedAt, } } diff --git a/datatypes.go b/datatypes.go index d30a20db8..36ccca5ce 100644 --- a/datatypes.go +++ b/datatypes.go @@ -242,6 +242,25 @@ type ScheduledNotification struct { Throttled bool `json:"throttled" example:"false"` SendFail int `json:"send_fail" example:"0"` Timestamp int64 `json:"timestamp" example:"1594471927" format:"int64"` + CreatedAt int64 `json:"created_at" example:"1594471900" format:"int64"` +} + +type ScheduledNotificationState int + +const ( + IgnoredNotification ScheduledNotificationState = iota + ValidNotification + RemovedNotification +) + +// Less is needed for the ScheduledNotification to match the Comparable interface +func (notification *ScheduledNotification) Less(other Comparable) (bool, error) { + otherNotification, ok := other.(*ScheduledNotification) + if !ok { + return false, fmt.Errorf("cannot to compare ScheduledNotification with different type") + } + + return notification.Timestamp < otherNotification.Timestamp, nil } // MatchedMetric represents parsed and matched metric data @@ -365,17 +384,19 @@ type CheckData struct { // MetricsToTargetRelation is a map that holds relation between metric names that was alone during last // check and targets that fetched this metric // {"t1": "metric.name.1", "t2": "metric.name.2"} - MetricsToTargetRelation map[string]string `json:"metrics_to_target_relation" example:"t1:metric.name.1,t2:metric.name.2"` - Score int64 `json:"score" example:"100" format:"int64"` - State State `json:"state" example:"OK"` - Maintenance int64 `json:"maintenance,omitempty" example:"0" format:"int64"` - MaintenanceInfo MaintenanceInfo `json:"maintenance_info"` - Timestamp int64 `json:"timestamp,omitempty" example:"1590741916" format:"int64"` - EventTimestamp int64 `json:"event_timestamp,omitempty" example:"1590741878" format:"int64"` - LastSuccessfulCheckTimestamp int64 `json:"last_successful_check_timestamp" example:"1590741916" format:"int64"` - Suppressed bool `json:"suppressed,omitempty" example:"true"` - SuppressedState State `json:"suppressed_state,omitempty"` - Message string `json:"msg,omitempty"` + MetricsToTargetRelation map[string]string `json:"metrics_to_target_relation" example:"t1:metric.name.1,t2:metric.name.2"` + Score int64 `json:"score" example:"100" format:"int64"` + State State `json:"state" example:"OK"` + Maintenance int64 `json:"maintenance,omitempty" example:"0" format:"int64"` + MaintenanceInfo MaintenanceInfo `json:"maintenance_info"` + // Timestamp - time, which means when the checker last checked this trigger, updated every checkInterval seconds + Timestamp int64 `json:"timestamp,omitempty" example:"1590741916" format:"int64"` + EventTimestamp int64 `json:"event_timestamp,omitempty" example:"1590741878" format:"int64"` + // LastSuccessfulCheckTimestamp - time of the last check of the trigger, during which there were no errors + LastSuccessfulCheckTimestamp int64 `json:"last_successful_check_timestamp" example:"1590741916" format:"int64"` + Suppressed bool `json:"suppressed,omitempty" example:"true"` + SuppressedState State `json:"suppressed_state,omitempty"` + Message string `json:"msg,omitempty"` } // RemoveMetricState is a function that removes MetricState from map of states. diff --git a/datatypes_test.go b/datatypes_test.go index 0294e92ea..f55c1d21b 100644 --- a/datatypes_test.go +++ b/datatypes_test.go @@ -724,3 +724,29 @@ func testMaintenance(conveyMessage string, actualInfo MaintenanceInfo, maintenan So(lastCheckTest.Maintenance, ShouldEqual, maintenance) }) } + +func TestScheduledNotificationLess(t *testing.T) { + Convey("Test Scheduled notification Less function", t, func() { + notification := &ScheduledNotification{ + Timestamp: 5, + } + + Convey("Test Less with nil", func() { + actual, err := notification.Less(nil) + So(err, ShouldResemble, fmt.Errorf("cannot to compare ScheduledNotification with different type")) + So(actual, ShouldBeFalse) + }) + + Convey("Test Less with less notification :)", func() { + actual, err := notification.Less(&ScheduledNotification{Timestamp: 1}) + So(err, ShouldBeNil) + So(actual, ShouldBeFalse) + }) + + Convey("Test Less with greater notification", func() { + actual, err := notification.Less(&ScheduledNotification{Timestamp: 10}) + So(err, ShouldBeNil) + So(actual, ShouldBeTrue) + }) + }) +} diff --git a/go.mod b/go.mod index 7ceaad8d9..bad535f89 100644 --- a/go.mod +++ b/go.mod @@ -144,7 +144,7 @@ require ( github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.16.0 // indirect github.com/stretchr/objx v0.5.0 // indirect - github.com/stretchr/testify v1.8.4 // indirect + github.com/stretchr/testify v1.8.4 github.com/subosito/gotenv v1.4.2 // indirect github.com/tinylib/msgp v1.1.8 // indirect github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect diff --git a/helpers.go b/helpers.go index 1c4a367a5..6f38b6c54 100644 --- a/helpers.go +++ b/helpers.go @@ -213,3 +213,40 @@ func ReplaceSubstring(str, begin, end, replaced string) string { } return result } + +type Comparable interface { + Less(other Comparable) (bool, error) +} + +// Merge is a generic function that performs a merge of two sorted arrays into one sorted array +func MergeToSorted[T Comparable](arr1, arr2 []T) ([]T, error) { + merged := make([]T, 0, len(arr1)+len(arr2)) + i, j := 0, 0 + + for i < len(arr1) && j < len(arr2) { + less, err := arr1[i].Less(arr2[j]) + if err != nil { + return nil, err + } + + if less { + merged = append(merged, arr1[i]) + i++ + } else { + merged = append(merged, arr2[j]) + j++ + } + } + + for i < len(arr1) { + merged = append(merged, arr1[i]) + i++ + } + + for j < len(arr2) { + merged = append(merged, arr2[j]) + j++ + } + + return merged, nil +} diff --git a/helpers_test.go b/helpers_test.go index 803376070..d9c3ca4ee 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -242,7 +242,7 @@ func testRounding(baseTimestamp, retention int64) { } func TestReplaceSubstring(t *testing.T) { - Convey("test replace substring", t, func() { + Convey("Test replace substring", t, func() { Convey("replacement string in the middle", func() { So(ReplaceSubstring("telebot: Post https://api.telegram.org/botXXX/getMe", "/bot", "/", "[DELETED]"), ShouldResemble, "telebot: Post https://api.telegram.org/bot[DELETED]/getMe") }) @@ -264,3 +264,78 @@ func TestReplaceSubstring(t *testing.T) { }) }) } + +type myInt int + +func (m myInt) Less(other Comparable) (bool, error) { + otherInt := other.(myInt) + return m < otherInt, nil +} + +type myTest struct { + value int +} + +func (test myTest) Less(other Comparable) (bool, error) { + otherTest := other.(myTest) + return test.value < otherTest.value, nil +} + +func TestMergeToSorted(t *testing.T) { + Convey("Test MergeToSorted function", t, func() { + Convey("Test with two nil arrays", func() { + merged, err := MergeToSorted[myInt](nil, nil) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{}) + }) + + Convey("Test with one nil array", func() { + merged, err := MergeToSorted[myInt](nil, []myInt{1, 2, 3}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{1, 2, 3}) + }) + + Convey("Test with two arrays", func() { + merged, err := MergeToSorted[myInt]([]myInt{4, 5}, []myInt{1, 2, 3}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{1, 2, 3, 4, 5}) + }) + + Convey("Test with empty array", func() { + merged, err := MergeToSorted[myInt]([]myInt{-4, 5}, []myInt{}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{-4, 5}) + }) + + Convey("Test with sorted values but mixed up", func() { + merged, err := MergeToSorted[myInt]([]myInt{1, 9, 10}, []myInt{4, 8, 12}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{1, 4, 8, 9, 10, 12}) + }) + + Convey("Test with structure type", func() { + arr1 := []myTest{ + { + value: 1, + }, + { + value: 2, + }, + } + + arr2 := []myTest{ + { + value: -2, + }, + { + value: -1, + }, + } + + expected := append(arr2, arr1...) + merged, err := MergeToSorted[myTest](arr1, arr2) + So(err, ShouldBeNil) + So(merged, ShouldResemble, expected) + }) + }) +} diff --git a/local/api.yml b/local/api.yml index c0b4939db..b662ef88d 100644 --- a/local/api.yml +++ b/local/api.yml @@ -53,6 +53,10 @@ web: notification_history: ttl: 48h query_limit: 10000 +notification: + delayed_time: 1m + transaction_timeout: 200ms + transaction_max_retries: 10 log: log_file: stdout log_level: debug diff --git a/local/notifier.yml b/local/notifier.yml index a4a19f8fc..103b0849f 100644 --- a/local/notifier.yml +++ b/local/notifier.yml @@ -42,6 +42,10 @@ notifier: notification_history: ttl: 48h query_limit: 10000 +notification: + delayed_time: 1m + transaction_timeout: 200ms + transaction_max_retries: 10 log: log_file: stdout log_level: info diff --git a/notifier/events/event_test.go b/notifier/events/event_test.go index 17e9ff892..a88b50849 100644 --- a/notifier/events/event_test.go +++ b/notifier/events/event_test.go @@ -50,6 +50,7 @@ func TestEvent(t *testing.T) { }, SendFail: 0, Timestamp: time.Now().Unix(), + CreatedAt: time.Now().Unix(), Throttled: false, Contact: contact, } @@ -88,6 +89,7 @@ func TestEvent(t *testing.T) { }, SendFail: 0, Timestamp: now.Unix(), + CreatedAt: now.Unix(), Throttled: false, Contact: contact, } diff --git a/notifier/notifier.go b/notifier/notifier.go index ea945412d..3e7cb04c6 100644 --- a/notifier/notifier.go +++ b/notifier/notifier.go @@ -148,7 +148,7 @@ func (notifier *StandardNotifier) resend(pkg *NotificationPackage, reason string logger := getLogWithPackageContext(¬ifier.logger, pkg, ¬ifier.config) logger.Warning(). - Int("number_of_retires", pkg.FailCount). + Int("number_of_retries", pkg.FailCount). String("reason", reason). Msg("Can't send message. Retry again in 1 min") diff --git a/notifier/scheduler.go b/notifier/scheduler.go index 6456f6e69..81f69eb4b 100644 --- a/notifier/scheduler.go +++ b/notifier/scheduler.go @@ -59,12 +59,14 @@ func (scheduler *StandardScheduler) ScheduleNotification(now time.Time, event mo Throttled: throttled, SendFail: sendFail, Timestamp: next.Unix(), + CreatedAt: now.Unix(), Plotting: plotting, } logger.Debug(). String("notification_timestamp", next.Format("2006/01/02 15:04:05")). Int64("notification_timestamp_unix", next.Unix()). + Int64("notification_created_at_unix", now.Unix()). Msg("Scheduled notification") return notification } diff --git a/notifier/scheduler_test.go b/notifier/scheduler_test.go index 5d0e98c39..aedcc85e3 100644 --- a/notifier/scheduler_test.go +++ b/notifier/scheduler_test.go @@ -61,6 +61,7 @@ func TestThrottling(t *testing.T) { Throttled: false, Timestamp: now.Unix(), SendFail: 0, + CreatedAt: now.Unix(), } Convey("Test sendFail more than 0, and no throttling, should send message in one minute", t, func() { From a097c39bf70a3989707469834bc4df1a7cc91cbd Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 24 Oct 2023 20:47:32 +0300 Subject: [PATCH 02/17] merge with base and fix tests --- cmd/api/config.go | 6 + cmd/api/config_test.go | 5 + cmd/api/main.go | 3 +- cmd/checker/main.go | 2 +- cmd/cli/main.go | 2 +- cmd/config.go | 20 + cmd/filter/main.go | 2 +- cmd/notifier/config.go | 6 + cmd/notifier/main.go | 3 +- database/redis/config.go | 11 + .../contact_notification_history_test.go | 1 + database/redis/database.go | 19 +- database/redis/last_check.go | 22 + database/redis/last_check_test.go | 101 ++ database/redis/notification.go | 297 ++++-- database/redis/notification_test.go | 908 ++++++++++++++++-- database/redis/reply/check.go | 21 + database/redis/reply/notification.go | 3 + datatypes.go | 85 +- datatypes_test.go | 185 ++++ go.mod | 2 +- helpers.go | 37 + helpers_test.go | 77 +- local/api.yml | 4 + local/notifier.yml | 4 + notifier/events/event_test.go | 2 + notifier/notifier.go | 2 +- notifier/scheduler.go | 2 + notifier/scheduler_test.go | 1 + 29 files changed, 1658 insertions(+), 175 deletions(-) diff --git a/cmd/api/config.go b/cmd/api/config.go index 50ec41623..7078632d0 100644 --- a/cmd/api/config.go +++ b/cmd/api/config.go @@ -21,6 +21,7 @@ type config struct { Remote cmd.RemoteConfig `yaml:"remote"` Prometheus cmd.PrometheusConfig `yaml:"prometheus"` NotificationHistory cmd.NotificationHistoryConfig `yaml:"notification_history"` + Notification cmd.NotificationConfig `yaml:"notification"` } type apiConfig struct { @@ -114,6 +115,11 @@ func getDefault() config { NotificationHistoryTTL: "48h", NotificationHistoryQueryLimit: int(notifier.NotificationsLimitUnlimited), }, + Notification: cmd.NotificationConfig{ + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + }, Logger: cmd.LoggerConfig{ LogFile: "stdout", LogLevel: "info", diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index 75cd644ab..42fffa171 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -102,6 +102,11 @@ func Test_webConfig_getDefault(t *testing.T) { NotificationHistoryTTL: "48h", NotificationHistoryQueryLimit: -1, }, + Notification: cmd.NotificationConfig{ + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + }, } result := getDefault() diff --git a/cmd/api/main.go b/cmd/api/main.go index 879a99d55..df5ff2568 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -83,7 +83,8 @@ func main() { databaseSettings := applicationConfig.Redis.GetSettings() notificationHistorySettings := applicationConfig.NotificationHistory.GetSettings() - database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, redis.API) + notificationSettings := applicationConfig.Notification.GetSettings() + database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, notificationSettings, redis.API) // Start Index right before HTTP listener. Fail if index cannot start searchIndex := index.NewSearchIndex(logger, database, telemetry.Metrics) diff --git a/cmd/checker/main.go b/cmd/checker/main.go index 5de14c9ea..29b85735c 100644 --- a/cmd/checker/main.go +++ b/cmd/checker/main.go @@ -83,7 +83,7 @@ func main() { logger.Info().Msg("Debug: checker started") databaseSettings := config.Redis.GetSettings() - database := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.Checker) + database := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Checker) remoteConfig := config.Remote.GetRemoteSourceSettings() prometheusConfig := config.Prometheus.GetPrometheusSourceSettings() diff --git a/cmd/cli/main.go b/cmd/cli/main.go index 5fda18983..13c35b45d 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -345,7 +345,7 @@ func initApp() (cleanupConfig, moira.Logger, moira.Database) { } databaseSettings := config.Redis.GetSettings() - dataBase := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.Cli) + dataBase := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Cli) return config.Cleanup, logger, dataBase } diff --git a/cmd/config.go b/cmd/config.go index 267db3edc..8af184fe5 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -79,6 +79,26 @@ func (notificationHistoryConfig *NotificationHistoryConfig) GetSettings() redis. } } +// NotificationConfig is a config that stores the necessary configuration of the notifier +type NotificationConfig struct { + // Need to determine if notification is delayed - the difference between creation time and sending time + // is greater than DelayedTime + DelayedTime string `yaml:"delayed_time"` + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionTimeout string `yaml:"transaction_timeout"` + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionMaxRetries int `yaml:"transaction_max_retries"` +} + +// GetSettings returns notification storage configuration +func (notificationConfig *NotificationConfig) GetSettings() redis.NotificationConfig { + return redis.NotificationConfig{ + DelayedTime: to.Duration(notificationConfig.DelayedTime), + TransactionTimeout: to.Duration(notificationConfig.TransactionTimeout), + TransactionMaxRetries: notificationConfig.TransactionMaxRetries, + } +} + // GraphiteConfig is graphite metrics config structure that initialises at the start of moira type GraphiteConfig struct { // If true, graphite sender will be enabled. diff --git a/cmd/filter/main.go b/cmd/filter/main.go index ced22bb6e..2b4d64f12 100644 --- a/cmd/filter/main.go +++ b/cmd/filter/main.go @@ -87,7 +87,7 @@ func main() { } filterMetrics := metrics.ConfigureFilterMetrics(telemetry.Metrics) - database := redis.NewDatabase(logger, config.Redis.GetSettings(), redis.NotificationHistoryConfig{}, redis.Filter) + database := redis.NewDatabase(logger, config.Redis.GetSettings(), redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Filter) retentionConfigFile, err := os.Open(config.Filter.RetentionConfig) if err != nil { diff --git a/cmd/notifier/config.go b/cmd/notifier/config.go index e3679ade9..3c4d4528d 100644 --- a/cmd/notifier/config.go +++ b/cmd/notifier/config.go @@ -21,6 +21,7 @@ type config struct { Prometheus cmd.PrometheusConfig `yaml:"prometheus"` ImageStores cmd.ImageStoreConfig `yaml:"image_store"` NotificationHistory cmd.NotificationHistoryConfig `yaml:"notification_history"` + Notification cmd.NotificationConfig `yaml:"notification"` } type entityLogConfig struct { @@ -93,6 +94,11 @@ func getDefault() config { NotificationHistoryTTL: "48h", NotificationHistoryQueryLimit: int(notifier.NotificationsLimitUnlimited), }, + Notification: cmd.NotificationConfig{ + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + }, Notifier: notifierConfig{ SenderTimeout: "10s", ResendingTimeout: "1:00", diff --git a/cmd/notifier/main.go b/cmd/notifier/main.go index 8a75bb410..a90fb2159 100644 --- a/cmd/notifier/main.go +++ b/cmd/notifier/main.go @@ -81,7 +81,8 @@ func main() { databaseSettings := config.Redis.GetSettings() notificationHistorySettings := config.NotificationHistory.GetSettings() - database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, redis.Notifier) + notificationSettings := config.Notification.GetSettings() + database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, notificationSettings, redis.Notifier) remoteConfig := config.Remote.GetRemoteSourceSettings() prometheusConfig := config.Prometheus.GetPrometheusSourceSettings() diff --git a/database/redis/config.go b/database/redis/config.go index 045ed7b21..18b357657 100644 --- a/database/redis/config.go +++ b/database/redis/config.go @@ -21,3 +21,14 @@ type NotificationHistoryConfig struct { NotificationHistoryTTL time.Duration NotificationHistoryQueryLimit int } + +// Notifier configuration in redis +type NotificationConfig struct { + // Need to determine if notification is delayed - the difference between creation time and sending time + // is greater than DelayedTime + DelayedTime time.Duration + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionTimeout time.Duration + // TransactionTimeout defines the timeout between fetch notifications transactions + TransactionMaxRetries int +} diff --git a/database/redis/contact_notification_history_test.go b/database/redis/contact_notification_history_test.go index cb1961828..b57e60c17 100644 --- a/database/redis/contact_notification_history_test.go +++ b/database/redis/contact_notification_history_test.go @@ -46,6 +46,7 @@ var inputScheduledNotification = moira.ScheduledNotification{ Throttled: false, SendFail: 1, Timestamp: time.Now().Unix(), + CreatedAt: time.Now().Unix(), } var eventsShouldBeInDb = []*moira.NotificationEventHistoryItem{ diff --git a/database/redis/database.go b/database/redis/database.go index f760049b2..5824a02da 100644 --- a/database/redis/database.go +++ b/database/redis/database.go @@ -46,9 +46,11 @@ type DbConnector struct { source DBSource clock moira.Clock notificationHistory NotificationHistoryConfig + // Notifier configuration in redis + notification NotificationConfig } -func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHistoryConfig, source DBSource) *DbConnector { +func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHistoryConfig, n NotificationConfig, source DBSource) *DbConnector { client := redis.NewUniversalClient(&redis.UniversalOptions{ MasterName: config.MasterName, Addrs: config.Addrs, @@ -78,7 +80,9 @@ func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHist source: source, clock: clock.NewSystemClock(), notificationHistory: nh, + notification: n, } + return &connector } @@ -90,7 +94,13 @@ func NewTestDatabase(logger moira.Logger) *DbConnector { NotificationHistoryConfig{ NotificationHistoryTTL: time.Hour * 48, NotificationHistoryQueryLimit: 1000, - }, testSource) + }, + NotificationConfig{ + DelayedTime: time.Minute, + TransactionTimeout: 200 * time.Millisecond, + TransactionMaxRetries: 10, + }, + testSource) } // NewTestDatabaseWithIncorrectConfig use it only for tests @@ -101,6 +111,11 @@ func NewTestDatabaseWithIncorrectConfig(logger moira.Logger) *DbConnector { NotificationHistoryTTL: time.Hour * 48, NotificationHistoryQueryLimit: 1000, }, + NotificationConfig{ + DelayedTime: time.Minute, + TransactionTimeout: 200 * time.Millisecond, + TransactionMaxRetries: 10, + }, testSource) } diff --git a/database/redis/last_check.go b/database/redis/last_check.go index 21ac48aeb..408375cfd 100644 --- a/database/redis/last_check.go +++ b/database/redis/last_check.go @@ -198,6 +198,28 @@ func (connector *DbConnector) checkDataScoreChanged(triggerID string, checkData return oldScore != float64(checkData.Score) } +// getTriggersLastCheck returns an array of trigger checks by the passed ids, if the trigger does not exist, it is nil +func (connector *DbConnector) getTriggersLastCheck(triggerIDs []string) ([]*moira.CheckData, error) { + ctx := connector.context + pipe := (*connector.client).TxPipeline() + + results := make([]*redis.StringCmd, len(triggerIDs)) + for i, id := range triggerIDs { + var result *redis.StringCmd + if id != "" { + result = pipe.Get(ctx, metricLastCheckKey(id)) + } + results[i] = result + } + + _, err := pipe.Exec(ctx) + if err != nil && err != redis.Nil { + return nil, err + } + + return reply.Checks(results) +} + var badStateTriggersKey = "moira-bad-state-triggers" var triggersChecksKey = "moira-triggers-checks" diff --git a/database/redis/last_check_test.go b/database/redis/last_check_test.go index 5d7cebceb..f4fc17e05 100644 --- a/database/redis/last_check_test.go +++ b/database/redis/last_check_test.go @@ -469,6 +469,107 @@ func TestLastCheckErrorConnection(t *testing.T) { }) } +func TestGetTriggersLastCheck(t *testing.T) { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 1, + }, moira.TriggerSourceNotSet) + + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Timestamp: 2, + }, moira.TriggerSourceNotSet) + + _ = dataBase.SetTriggerLastCheck("test3", &moira.CheckData{ + Timestamp: 3, + }, moira.TriggerSourceNotSet) + + Convey("getTriggersLastCheck manipulations", t, func() { + Convey("Test with nil id array", func() { + actual, err := dataBase.getTriggersLastCheck(nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{}) + }) + + Convey("Test with correct id array", func() { + actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test3"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 2, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 3, + MetricsToTargetRelation: map[string]string{}, + }, + }) + }) + + Convey("Test with deleted trigger", func() { + dataBase.RemoveTriggerLastCheck("test2") //nolint + defer func() { + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Timestamp: 2, + }, moira.TriggerSourceNotSet) + }() + + actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test3"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + nil, + { + Timestamp: 3, + MetricsToTargetRelation: map[string]string{}, + }, + }) + }) + + Convey("Test with a nonexistent trigger id", func() { + actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test4"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 2, + MetricsToTargetRelation: map[string]string{}, + }, + nil, + }) + }) + + Convey("Test with an empty trigger id", func() { + actual, err := dataBase.getTriggersLastCheck([]string{"", "test2", "test3"}) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.CheckData{ + nil, + { + Timestamp: 2, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 3, + MetricsToTargetRelation: map[string]string{}, + }, + }) + }) + }) +} + func TestMaintenanceUserSave(t *testing.T) { logger, _ := logging.GetLogger("dataBase") dataBase := NewTestDatabase(logger) diff --git a/database/redis/notification.go b/database/redis/notification.go index f22871a95..928212a30 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -1,6 +1,7 @@ package redis import ( + "context" "errors" "fmt" "strconv" @@ -16,7 +17,6 @@ import ( ) const ( - transactionTriesLimit = 10 transactionHeuristicLimit = 10000 ) @@ -106,16 +106,15 @@ func (connector *DbConnector) RemoveNotification(notificationKey string) (int64, foundNotifications = append(foundNotifications, notification) } } - return connector.removeNotifications(foundNotifications) + + return connector.removeNotifications(connector.context, (*connector.client).TxPipeline(), foundNotifications) } -func (connector *DbConnector) removeNotifications(notifications []*moira.ScheduledNotification) (int64, error) { +func (connector *DbConnector) removeNotifications(ctx context.Context, pipe redis.Pipeliner, notifications []*moira.ScheduledNotification) (int64, error) { if len(notifications) == 0 { return 0, nil } - ctx := connector.context - pipe := (*connector.client).TxPipeline() for _, notification := range notifications { notificationString, err := reply.GetNotificationBytes(*notification) if err != nil { @@ -126,7 +125,7 @@ func (connector *DbConnector) removeNotifications(notifications []*moira.Schedul response, err := pipe.Exec(ctx) if err != nil { - return 0, fmt.Errorf("failed to remove notifier-notification: %s", err.Error()) + return 0, fmt.Errorf("failed to remove notifications: %w", err) } total := int64(0) @@ -138,6 +137,115 @@ func (connector *DbConnector) removeNotifications(notifications []*moira.Schedul return total, nil } +// GetDelayedTimeInSeconds returns the time, if the difference between notification +// creation and sending time is greater than this time, the notification will be considered delayed +func (connector *DbConnector) GetDelayedTimeInSeconds() int64 { + return int64(connector.notification.DelayedTime.Seconds()) +} + +// filterNotificationsByDelay filters notifications into delayed and not delayed notifications +func filterNotificationsByDelay(notifications []*moira.ScheduledNotification, delayedTime int64) ([]*moira.ScheduledNotification, []*moira.ScheduledNotification) { + delayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) + notDelayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) + + for _, notification := range notifications { + if notification.IsDelayed(delayedTime) { + delayedNotifications = append(delayedNotifications, notification) + continue + } + notDelayedNotifications = append(notDelayedNotifications, notification) + } + + return delayedNotifications, notDelayedNotifications +} + +// getNotificationsTriggerChecks returns notifications trigger checks, optimized for the case when there are many notifications at one trigger +func (connector *DbConnector) getNotificationsTriggerChecks(notifications []*moira.ScheduledNotification) ([]*moira.CheckData, error) { + checkDataMap := make(map[string]*moira.CheckData, len(notifications)) + for _, notification := range notifications { + if notification != nil { + checkDataMap[notification.Trigger.ID] = nil + } + } + + triggerIDs := make([]string, 0, len(checkDataMap)) + for triggerID := range checkDataMap { + triggerIDs = append(triggerIDs, triggerID) + } + + triggersLastCheck, err := connector.getTriggersLastCheck(triggerIDs) + if err != nil { + return nil, err + } + + for i, triggerID := range triggerIDs { + checkDataMap[triggerID] = triggersLastCheck[i] + } + + result := make([]*moira.CheckData, len(notifications)) + for i, notification := range notifications { + result[i] = checkDataMap[notification.Trigger.ID] + } + + return result, nil +} + +// filterNotificationsByState filters notifications based on their state to the corresponding arrays +func (connector *DbConnector) filterNotificationsByState(notifications []*moira.ScheduledNotification) ([]*moira.ScheduledNotification, []*moira.ScheduledNotification, error) { + validNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) + toRemoveNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) + + triggerChecks, err := connector.getNotificationsTriggerChecks(notifications) + if err != nil { + return nil, nil, fmt.Errorf("failed to get notifications trigger checks: %w", err) + } + + for i := range notifications { + switch notifications[i].GetState(triggerChecks[i]) { + case moira.ValidNotification: + validNotifications = append(validNotifications, notifications[i]) + + case moira.RemovedNotification: + toRemoveNotifications = append(toRemoveNotifications, notifications[i]) + } + } + + return validNotifications, toRemoveNotifications, nil +} + +/* +handleNotifications filters notifications into delayed and not delayed, +then filters delayed notifications by notification state, then merges the 2 arrays +of not delayed and valid delayed notifications into a single sorted array + +Returns valid notifications in sorted order by timestamp and notifications to remove +*/ +func (connector *DbConnector) handleNotifications(notifications []*moira.ScheduledNotification) ([]*moira.ScheduledNotification, []*moira.ScheduledNotification, error) { + if len(notifications) == 0 { + return notifications, nil, nil + } + + delayedNotifications, notDelayedNotifications := filterNotificationsByDelay(notifications, connector.GetDelayedTimeInSeconds()) + + if len(delayedNotifications) == 0 { + return notDelayedNotifications, notDelayedNotifications, nil + } + + validNotifications, toRemoveNotifications, err := connector.filterNotificationsByState(delayedNotifications) + if err != nil { + return nil, nil, fmt.Errorf("failed to filter delayed notifications by state: %w", err) + } + + validNotifications, err = moira.MergeToSorted[*moira.ScheduledNotification](validNotifications, notDelayedNotifications) + if err != nil { + return nil, nil, fmt.Errorf("failed to merge valid and not delayed notifications into sorted array: %w", err) + } + + toRemoveNotifications = append(toRemoveNotifications, validNotifications...) + + return validNotifications, toRemoveNotifications, nil +} + // FetchNotifications fetch notifications by given timestamp and delete it func (connector *DbConnector) FetchNotifications(to int64, limit int64) ([]*moira.ScheduledNotification, error) { if limit == 0 { @@ -146,7 +254,7 @@ func (connector *DbConnector) FetchNotifications(to int64, limit int64) ([]*moir // No limit if limit == notifier.NotificationsLimitUnlimited { - return connector.fetchNotificationsNoLimit(to) + return connector.fetchNotifications(to, nil) } count, err := connector.notificationsCount(to) @@ -156,10 +264,10 @@ func (connector *DbConnector) FetchNotifications(to int64, limit int64) ([]*moir // Hope count will be not greater then limit when we call fetchNotificationsNoLimit if limit > transactionHeuristicLimit && count < limit/2 { - return connector.fetchNotificationsNoLimit(to) + return connector.fetchNotifications(to, nil) } - return connector.fetchNotificationsWithLimit(to, limit) + return connector.fetchNotifications(to, &limit) } func (connector *DbConnector) notificationsCount(to int64) (int64, error) { @@ -176,102 +284,151 @@ func (connector *DbConnector) notificationsCount(to int64) (int64, error) { } // fetchNotificationsWithLimit reads and drops notifications from DB with limit -func (connector *DbConnector) fetchNotificationsWithLimit(to int64, limit int64) ([]*moira.ScheduledNotification, error) { - // fetchNotifecationsWithLimitDo uses WATCH, so transaction may fail and will retry it +func (connector *DbConnector) fetchNotifications(to int64, limit *int64) ([]*moira.ScheduledNotification, error) { + // fetchNotificationsDo uses WATCH, so transaction may fail and will retry it // see https://redis.io/topics/transactions - for i := 0; i < transactionTriesLimit; i++ { - res, err := connector.fetchNotificationsWithLimitDo(to, limit) + for i := 0; i < connector.notification.TransactionMaxRetries; i++ { + res, err := connector.fetchNotificationsDo(to, limit) if err == nil { return res, nil } - if !errors.As(err, &transactionError{}) { + if !errors.Is(err, &transactionError{}) { return nil, err } - time.Sleep(200 * time.Millisecond) //nolint + connector.logger.Info(). + Int("transaction_retries", i+1). + Msg("Transaction error. Retry") + + time.Sleep(connector.notification.TransactionTimeout) } return nil, fmt.Errorf("transaction tries limit exceeded") } -// same as fetchNotificationsWithLimit, but only once -func (connector *DbConnector) fetchNotificationsWithLimitDo(to int64, limit int64) ([]*moira.ScheduledNotification, error) { - // see https://redis.io/topics/transactions - - ctx := connector.context - c := *connector.client - - // start optimistic transaction and get notifications with LIMIT - var response *redis.StringSliceCmd - - err := c.Watch(ctx, func(tx *redis.Tx) error { - rng := &redis.ZRangeBy{Min: "-inf", Max: strconv.FormatInt(to, 10), Offset: 0, Count: limit} - response = tx.ZRangeByScore(ctx, notifierNotificationsKey, rng) - - return response.Err() - }, notifierNotificationsKey) - - if err != nil { - return nil, fmt.Errorf("failed to ZRANGEBYSCORE: %s", err) +// getNotificationsInTxWithLimit receives notifications from the database by a certain time +// sorted by timestamp in one transaction with or without limit, depending on whether limit is nil +func getNotificationsInTxWithLimit(ctx context.Context, tx *redis.Tx, to int64, limit *int64) ([]*moira.ScheduledNotification, error) { + var rng *redis.ZRangeBy + if limit != nil { + rng = &redis.ZRangeBy{Min: "-inf", Max: strconv.FormatInt(to, 10), Offset: 0, Count: *limit} + } else { + rng = &redis.ZRangeBy{Min: "-inf", Max: strconv.FormatInt(to, 10)} } - notifications, err := reply.Notifications(response) - if err != nil { - return nil, fmt.Errorf("failed to EXEC: %s", err) + response := tx.ZRangeByScore(ctx, notifierNotificationsKey, rng) + if response.Err() != nil { + return nil, fmt.Errorf("failed to ZRANGEBYSCORE: %w", response.Err()) } + return reply.Notifications(response) +} + +/* +getLimitedNotifications restricts the list of notifications by last timestamp. There are two possible cases +with arrays of notifications with timestamps: + + - [1, 2, 3, 3], after limitNotifications we get the array [1, 2], + further, since the array size is not equal to the passed one, we return [1, 2] + + - [1, 1, 1], after limitNotifications we will get array [1, 1, 1], its size is equal to the initial one, + so we will get all notifications from the database with the last timestamp <= 1, i.e., + if the database at this moment has [1, 1, 1, 1, 1], then the output will be [1, 1, 1, 1, 1] + +This is to ensure that notifications with the same timestamp are always clumped into a single stack +*/ +func getLimitedNotifications( + ctx context.Context, + tx *redis.Tx, + limit *int64, + notifications []*moira.ScheduledNotification, +) ([]*moira.ScheduledNotification, error) { if len(notifications) == 0 { return notifications, nil } - // ZRANGEBYSCORE with LIMIT may return not all notification with last timestamp - // (e.g. if we have notifications with timestamps [1, 2, 3, 3, 3] and limit == 3) - // but ZREMRANGEBYSCORE does not have LIMIT, so will delete all notifications with last timestamp - // (ts 3 in our example) and then run ZRANGEBYSCORE with our new last timestamp (2 in our example). + limitedNotifications := notifications - notificationsLimited := limitNotifications(notifications) - lastTs := notificationsLimited[len(notificationsLimited)-1].Timestamp + if limit != nil { + limitedNotifications = limitNotifications(notifications) + lastTs := limitedNotifications[len(limitedNotifications)-1].Timestamp - if len(notifications) == len(notificationsLimited) { - // this means that all notifications have same timestamp, - // we hope that all notifications with same timestamp should fit our memory - return connector.fetchNotificationsNoLimit(lastTs) + if len(notifications) == len(limitedNotifications) { + // this means that all notifications have same timestamp, + // we hope that all notifications with same timestamp should fit our memory + var err error + limitedNotifications, err = getNotificationsInTxWithLimit(ctx, tx, lastTs, nil) + if err != nil { + return nil, fmt.Errorf("failed to get notification without limit in transaction: %w", err) + } + } } - pipe := c.TxPipeline() - pipe.ZRemRangeByScore(ctx, notifierNotificationsKey, "-inf", strconv.FormatInt(lastTs, 10)) - rangeResponse, errDelete := pipe.Exec(ctx) + return limitedNotifications, nil +} - if errDelete != nil { - return nil, fmt.Errorf("failed to EXEC: %w", errDelete) - } +// fetchNotificationsDo performs fetching of notifications within a single transaction +func (connector *DbConnector) fetchNotificationsDo(to int64, limit *int64) ([]*moira.ScheduledNotification, error) { + // See https://redis.io/topics/transactions - // someone has changed notifierNotificationsKey while we do our job - // and transaction fail (no notifications were deleted) :( - deleteCount, errConvert := rangeResponse[0].(*redis.IntCmd).Result() - if errConvert != nil || deleteCount == 0 { - return nil, &transactionError{} - } + ctx := connector.context + c := *connector.client - return notificationsLimited, nil -} + result := make([]*moira.ScheduledNotification, 0) -// FetchNotifications fetch notifications by given timestamp and delete it -func (connector *DbConnector) fetchNotificationsNoLimit(to int64) ([]*moira.ScheduledNotification, error) { - ctx := connector.context - pipe := (*connector.client).TxPipeline() - pipe.ZRangeByScore(ctx, notifierNotificationsKey, &redis.ZRangeBy{Min: "-inf", Max: strconv.FormatInt(to, 10)}) - pipe.ZRemRangeByScore(ctx, notifierNotificationsKey, "-inf", strconv.FormatInt(to, 10)) - response, err := pipe.Exec(ctx) + // it is necessary to do these operations in one transaction to avoid data race + err := c.Watch(ctx, func(tx *redis.Tx) error { + notifications, err := getNotificationsInTxWithLimit(ctx, tx, to, limit) + if err != nil { + if errors.Is(err, redis.TxFailedErr) { + return &transactionError{} + } + return fmt.Errorf("failed to get notifications with limit in transaction: %w", err) + } + + if len(notifications) == 0 { + return nil + } + + // ZRANGEBYSCORE with LIMIT may return not all notifications with last timestamp + // (e.g. we have notifications with timestamps [1, 2, 3, 3, 3] and limit = 3, + // we will get [1, 2, 3]) other notifications with timestamp 3 remain in the database, so then + // limit notifications by last timestamp and return and delete valid notifications with our new timestamp [1, 2] + limitedNotifications, err := getLimitedNotifications(ctx, tx, limit, notifications) + if err != nil { + return fmt.Errorf("failed to get limited notifications: %w", err) + } + + validNotifications, toRemoveNotifications, err := connector.handleNotifications(limitedNotifications) + if err != nil { + return fmt.Errorf("failed to validate notifications: %w", err) + } + result = validNotifications + + _, err = tx.TxPipelined(ctx, func(pipe redis.Pipeliner) error { + // someone has changed notifierNotificationsKey while we do our job + // and transaction fail (no notifications were deleted) :( + if _, err = connector.removeNotifications(ctx, pipe, toRemoveNotifications); err != nil { + if errors.Is(err, redis.TxFailedErr) { + return &transactionError{} + } + return fmt.Errorf("failed to remove notifications in transaction: %w", err) + } + + return nil + }) + + return err + }, notifierNotificationsKey) if err != nil { - return nil, fmt.Errorf("failed to EXEC: %s", err) + return nil, err } - return reply.Notifications(response[0].(*redis.StringSliceCmd)) + return result, nil } // AddNotification store notification at given timestamp diff --git a/database/redis/notification_test.go b/database/redis/notification_test.go index 7b262ea5d..52a0502b9 100644 --- a/database/redis/notification_test.go +++ b/database/redis/notification_test.go @@ -6,9 +6,11 @@ import ( "testing" "time" + "github.com/go-redis/redis/v8" "github.com/moira-alert/moira" logging "github.com/moira-alert/moira/logging/zerolog_adapter" "github.com/moira-alert/moira/notifier" + "github.com/stretchr/testify/assert" . "github.com/smartystreets/goconvey/convey" ) @@ -20,18 +22,21 @@ func TestScheduledNotification(t *testing.T) { defer dataBase.Flush() Convey("ScheduledNotification manipulation", t, func() { - now := time.Now().Unix() + now = time.Now().Unix() notificationNew := moira.ScheduledNotification{ SendFail: 1, - Timestamp: now + 3600, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, } notification := moira.ScheduledNotification{ SendFail: 2, Timestamp: now, + CreatedAt: now, } notificationOld := moira.ScheduledNotification{ SendFail: 3, - Timestamp: now - 3600, + Timestamp: now - dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, } Convey("Test add and get by pages", func() { @@ -53,7 +58,7 @@ func TestScheduledNotification(t *testing.T) { }) Convey("Test fetch notifications", func() { - actual, err := dataBase.FetchNotifications(now-3600, notifier.NotificationsLimitUnlimited) //nolint + actual, err := dataBase.FetchNotifications(now-dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld}) @@ -62,7 +67,7 @@ func TestScheduledNotification(t *testing.T) { So(total, ShouldEqual, 2) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ification, ¬ificationNew}) - actual, err = dataBase.FetchNotifications(now+3600, notifier.NotificationsLimitUnlimited) //nolint + actual, err = dataBase.FetchNotifications(now+dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ification, ¬ificationNew}) @@ -73,7 +78,7 @@ func TestScheduledNotification(t *testing.T) { }) Convey("Test fetch notifications limit 0", func() { - actual, err := dataBase.FetchNotifications(now-3600, 0) //nolint + actual, err := dataBase.FetchNotifications(now-dataBase.GetDelayedTimeInSeconds(), 0) //nolint So(err, ShouldBeError) So(actual, ShouldBeNil) //nolint }) @@ -96,7 +101,7 @@ func TestScheduledNotification(t *testing.T) { Contact: moira.ContactData{ID: id1}, Event: moira.NotificationEvent{SubscriptionID: &id1}, SendFail: 3, - Timestamp: now + 3600, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), } addNotifications(dataBase, []moira.ScheduledNotification{notification1, notification2, notification3}) actual, total, err := dataBase.GetNotifications(0, -1) @@ -113,7 +118,7 @@ func TestScheduledNotification(t *testing.T) { So(total, ShouldEqual, 1) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ification3}) - total, err = dataBase.RemoveNotification(strings.Join([]string{fmt.Sprintf("%v", now+3600), id1, id1}, "")) //nolint + total, err = dataBase.RemoveNotification(strings.Join([]string{fmt.Sprintf("%v", now+dataBase.GetDelayedTimeInSeconds()), id1, id1}, "")) //nolint So(err, ShouldBeNil) So(total, ShouldEqual, 1) @@ -122,7 +127,7 @@ func TestScheduledNotification(t *testing.T) { So(total, ShouldEqual, 0) So(actual, ShouldResemble, []*moira.ScheduledNotification{}) - actual, err = dataBase.FetchNotifications(now+3600, notifier.NotificationsLimitUnlimited) //nolint + actual, err = dataBase.FetchNotifications(now+dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{}) }) @@ -145,7 +150,7 @@ func TestScheduledNotification(t *testing.T) { Contact: moira.ContactData{ID: id1}, Event: moira.NotificationEvent{SubscriptionID: &id1}, SendFail: 3, - Timestamp: now + 3600, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), } addNotifications(dataBase, []moira.ScheduledNotification{notification1, notification2, notification3}) actual, total, err := dataBase.GetNotifications(0, -1) @@ -161,7 +166,7 @@ func TestScheduledNotification(t *testing.T) { So(total, ShouldEqual, 0) So(actual, ShouldResemble, []*moira.ScheduledNotification{}) - actual, err = dataBase.FetchNotifications(now+3600, notifier.NotificationsLimitUnlimited) //nolint + actual, err = dataBase.FetchNotifications(now+dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{}) }) @@ -214,23 +219,26 @@ func TestFetchNotifications(t *testing.T) { defer dataBase.Flush() Convey("FetchNotifications manipulation", t, func() { - now := time.Now().Unix() + now = time.Now().Unix() notificationNew := moira.ScheduledNotification{ SendFail: 1, - Timestamp: now + 3600, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, } notification := moira.ScheduledNotification{ SendFail: 2, Timestamp: now, + CreatedAt: now, } notificationOld := moira.ScheduledNotification{ SendFail: 3, - Timestamp: now - 3600, + Timestamp: now - dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, } Convey("Test fetch notifications with limit if all notifications has diff timestamp", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.FetchNotifications(now+6000, 1) //nolint + actual, err := dataBase.FetchNotifications(now+dataBase.GetDelayedTimeInSeconds(), 1) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld}) @@ -245,7 +253,7 @@ func TestFetchNotifications(t *testing.T) { Convey("Test fetch notifications with limit little bit greater than count if all notifications has diff timestamp", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.FetchNotifications(now+6000, 4) //nolint + actual, err := dataBase.FetchNotifications(now+dataBase.GetDelayedTimeInSeconds(), 4) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification}) @@ -261,7 +269,7 @@ func TestFetchNotifications(t *testing.T) { Convey("Test fetch notifications with limit greater than count if all notifications has diff timestamp", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.FetchNotifications(now+6000, 200000) //nolint + actual, err := dataBase.FetchNotifications(now+dataBase.GetDelayedTimeInSeconds(), 200000) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) @@ -276,7 +284,7 @@ func TestFetchNotifications(t *testing.T) { Convey("Test fetch notifications without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.FetchNotifications(now+6000, notifier.NotificationsLimitUnlimited) //nolint + actual, err := dataBase.FetchNotifications(now+dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) @@ -291,62 +299,193 @@ func TestFetchNotifications(t *testing.T) { }) } -func TestNotificationsCount(t *testing.T) { +func TestGetNotificationsInTxWithLimit(t *testing.T) { logger, _ := logging.GetLogger("dataBase") dataBase := NewTestDatabase(logger) dataBase.Flush() defer dataBase.Flush() - Convey("notificationsCount in db", t, func() { - now := time.Now().Unix() + client := *dataBase.client + ctx := dataBase.context + + Convey("Test getNotificationsInTxWithLimit", t, func() { + var limit int64 = 0 + now = time.Now().Unix() notificationNew := moira.ScheduledNotification{ SendFail: 1, - Timestamp: now + 3600, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, } notification := moira.ScheduledNotification{ SendFail: 2, Timestamp: now, + CreatedAt: now, } notificationOld := moira.ScheduledNotification{ SendFail: 3, - Timestamp: now - 3600, + Timestamp: now - dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, } - Convey("Test all notification with different ts in db", func() { + Convey("Test with zero notifications without limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{}) + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{}) + return nil + }, notifierNotificationsKey) + So(err, ShouldBeNil) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test all notifications without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.notificationsCount(now + 6000) + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) + return nil + }, notifierNotificationsKey) So(err, ShouldBeNil) - So(actual, ShouldResemble, int64(3)) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) - Convey("Test get 0 notification with ts in db", func() { + Convey("Test all notifications with limit != count", func() { + limit = 1 addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.notificationsCount(now - 7000) + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, &limit) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld}) + return nil + }, notifierNotificationsKey) So(err, ShouldBeNil) - So(actual, ShouldResemble, int64(0)) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) - Convey("Test part notification in db with ts", func() { + Convey("Test all notifications with limit = count", func() { + limit = 3 addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.notificationsCount(now) + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, &limit) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) + return nil + }, notifierNotificationsKey) So(err, ShouldBeNil) - So(actual, ShouldResemble, int64(2)) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) + }) +} - Convey("Test 0 notification in db", func() { - addNotifications(dataBase, []moira.ScheduledNotification{}) - actual, err := dataBase.notificationsCount(now) +func TestGetLimitedNotifications(t *testing.T) { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + client := *dataBase.client + ctx := dataBase.context + + Convey("Test getLimitedNotifications", t, func() { + var limit int64 = 0 + now = time.Now().Unix() + notificationNew := moira.ScheduledNotification{ + SendFail: 1, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, + } + notification := moira.ScheduledNotification{ + SendFail: 2, + Timestamp: now, + CreatedAt: now, + } + notificationOld := moira.ScheduledNotification{ + SendFail: 3, + Timestamp: now - dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, + } + + Convey("Test all notifications with different timestamps without limit", func() { + notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getLimitedNotifications(ctx, tx, nil, notifications) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) + return nil + }, notifierNotificationsKey) + So(err, ShouldBeNil) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test all notifications with different timestamps and limit", func() { + notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getLimitedNotifications(ctx, tx, &limit, notifications) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification}) + return nil + }, notifierNotificationsKey) + So(err, ShouldBeNil) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test all notifications with same timestamp and limit", func() { + notification.Timestamp = now + notificationNew.Timestamp = now + notificationOld.Timestamp = now + defer func() { + notificationNew.Timestamp = now + dataBase.GetDelayedTimeInSeconds() + notificationOld.Timestamp = now - dataBase.GetDelayedTimeInSeconds() + }() + + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notification, notificationNew}) + notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} + expected := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getLimitedNotifications(ctx, tx, &limit, notifications) + So(err, ShouldBeNil) + assert.ElementsMatch(t, actual, expected) + return nil + }, notifierNotificationsKey) + So(err, ShouldBeNil) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test not all notifications with same timestamp and limit", func() { + notification.Timestamp = now + notificationNew.Timestamp = now + notificationOld.Timestamp = now + defer func() { + notificationNew.Timestamp = now + dataBase.GetDelayedTimeInSeconds() + notificationOld.Timestamp = now - dataBase.GetDelayedTimeInSeconds() + }() + + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notification, notificationNew}) + notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification} + expected := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} + err := client.Watch(ctx, func(tx *redis.Tx) error { + actual, err := getLimitedNotifications(ctx, tx, &limit, notifications) + So(err, ShouldBeNil) + assert.ElementsMatch(t, actual, expected) + return nil + }, notifierNotificationsKey) So(err, ShouldBeNil) - So(actual, ShouldResemble, int64(0)) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) @@ -354,54 +493,451 @@ func TestNotificationsCount(t *testing.T) { }) } -func TestFetchNotificationsWithLimitDo(t *testing.T) { +func TestFilterNotificationsByState(t *testing.T) { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + notificationOld := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, + Event: moira.NotificationEvent{ + Metric: "test", + }, + SendFail: 1, + Timestamp: now - dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now - dataBase.GetDelayedTimeInSeconds(), + } + notification := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, + Event: moira.NotificationEvent{ + Metric: "test1", + }, + SendFail: 2, + Timestamp: now, + CreatedAt: now, + } + notificationNew := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test1", + }, + Event: moira.NotificationEvent{ + Metric: "test1", + }, + SendFail: 3, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, + } + + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 100, + }, moira.TriggerSourceNotSet) + + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Metrics: map[string]moira.MetricState{ + "test": {}, + }, + Timestamp: 100, + }, moira.TriggerSourceNotSet) + + Convey("Test filter notifications by state", t, func() { + Convey("With empty notifications", func() { + validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + }) + + Convey("With zero removed notifications", func() { + validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + }) + + Convey("With removed check data", func() { + dataBase.RemoveTriggerLastCheck("test1") //nolint + defer func() { + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 100, + }, moira.TriggerSourceNotSet) + }() + + validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notification}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationNew}) + }) + + Convey("With metric on maintenance", func() { + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 130}, nil, "test", 100) //nolint + defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint + + validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notification, notificationNew}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + }) + + Convey("With trigger on maintenance", func() { + var triggerMaintenance int64 = 130 + dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint + defer func() { + triggerMaintenance = 0 + dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint + }() + + validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notification}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + }) + }) +} + +func TestHandleNotifications(t *testing.T) { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + notificationOld := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, + SendFail: 1, + Timestamp: now - dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now - dataBase.GetDelayedTimeInSeconds(), + } + notification := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test1", + }, + SendFail: 2, + Timestamp: now, + CreatedAt: now, + } + notificationNew := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test1", + }, + SendFail: 3, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, + } + + // create delayed notifications + notificationOld2 := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, + Event: moira.NotificationEvent{ + Metric: "test", + }, + SendFail: 4, + Timestamp: now - dataBase.GetDelayedTimeInSeconds() + 1, + CreatedAt: now - 2*dataBase.GetDelayedTimeInSeconds(), + } + notificationNew2 := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test1", + }, + Event: moira.NotificationEvent{ + Metric: "test", + }, + SendFail: 5, + Timestamp: now + dataBase.GetDelayedTimeInSeconds() + 1, + CreatedAt: now, + } + notificationNew3 := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, + Event: moira.NotificationEvent{ + Metric: "test2", + }, + SendFail: 6, + Timestamp: now + dataBase.GetDelayedTimeInSeconds() + 2, + CreatedAt: now, + } + + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 100, + }, moira.TriggerSourceNotSet) + + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Metrics: map[string]moira.MetricState{ + "test": {}, + }, + Timestamp: 100, + }, moira.TriggerSourceNotSet) + + Convey("Test handle notifications", t, func() { + Convey("Without delayed notifications", func() { + validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notification, notificationNew}) + }) + + Convey("With both delayed and not delayed valid notifications", func() { + validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + }) + + Convey("With both delayed and not delayed notifications and removed check data", func() { + dataBase.RemoveTriggerLastCheck("test1") //nolint + defer func() { + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 100, + }, moira.TriggerSourceNotSet) + }() + + validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew3}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationNew2, notificationOld, notificationOld2, notification, notificationNew, notificationNew3}) + }) + + Convey("With both delayed and not delayed valid notifications and metric on maintenance", func() { + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 130}, nil, "test", 100) //nolint + defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint + + validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notification, notificationNew, notificationNew2, notificationNew3}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notification, notificationNew, notificationNew2, notificationNew3}) + }) + + Convey("With both delayed and not delayed valid notifications and trigger on maintenance", func() { + var triggerMaintenance int64 = 130 + dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint + defer func() { + triggerMaintenance = 0 + dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint + }() + + validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + So(err, ShouldBeNil) + So(validNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew3}) + So(removedNotifications, ShouldResemble, []*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew3}) + }) + }) +} + +func TestNotificationsCount(t *testing.T) { logger, _ := logging.GetLogger("dataBase") dataBase := NewTestDatabase(logger) dataBase.Flush() defer dataBase.Flush() Convey("notificationsCount in db", t, func() { - now := time.Now().Unix() + now = time.Now().Unix() notificationNew := moira.ScheduledNotification{ SendFail: 1, - Timestamp: now + 3600, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, } notification := moira.ScheduledNotification{ SendFail: 2, Timestamp: now, + CreatedAt: now, } notificationOld := moira.ScheduledNotification{ SendFail: 3, - Timestamp: now - 3600, - } - notification4 := moira.ScheduledNotification{ - SendFail: 4, - Timestamp: now - 3600, + Timestamp: now - dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now - dataBase.GetDelayedTimeInSeconds(), } - Convey("Test all notification with ts and limit in db", func() { + Convey("Test all notification with different ts in db", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.fetchNotificationsWithLimitDo(now+6000, 1) //nolint + actual, err := dataBase.notificationsCount(now + dataBase.GetDelayedTimeInSeconds()*2) So(err, ShouldBeNil) - So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld}) + So(actual, ShouldResemble, int64(3)) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test get 0 notification with ts in db", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) + actual, err := dataBase.notificationsCount(now - dataBase.GetDelayedTimeInSeconds()*2) + So(err, ShouldBeNil) + So(actual, ShouldResemble, int64(0)) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) - Convey("Test 0 notification with ts and limit in empty db", func() { + Convey("Test part notification in db with ts", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) + actual, err := dataBase.notificationsCount(now) + So(err, ShouldBeNil) + So(actual, ShouldResemble, int64(2)) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test 0 notification in db", func() { addNotifications(dataBase, []moira.ScheduledNotification{}) - actual, err := dataBase.fetchNotificationsWithLimitDo(now+6000, 10) //nolint + actual, err := dataBase.notificationsCount(now) So(err, ShouldBeNil) - So(actual, ShouldResemble, []*moira.ScheduledNotification{}) + So(actual, ShouldResemble, int64(0)) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) + }) +} - Convey("Test all notification with ts and big limit in db", func() { +func TestFetchNotificationsDo(t *testing.T) { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + var limit int64 + + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Metrics: map[string]moira.MetricState{ + "test1": {}, + }, + Timestamp: 110, + }, moira.TriggerSourceNotSet) + + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Metrics: map[string]moira.MetricState{ + "test1": {}, + "test2": {}, + }, + Timestamp: 110, + }, moira.TriggerSourceNotSet) + + now := time.Now().Unix() + notificationOld := moira.ScheduledNotification{ + SendFail: 1, + Timestamp: now - dataBase.GetDelayedTimeInSeconds() + 1, + CreatedAt: now - dataBase.GetDelayedTimeInSeconds() + 1, + } + notification4 := moira.ScheduledNotification{ + SendFail: 2, + Timestamp: now - dataBase.GetDelayedTimeInSeconds() + 2, + CreatedAt: now - dataBase.GetDelayedTimeInSeconds() + 2, + } + notificationNew := moira.ScheduledNotification{ + SendFail: 3, + Timestamp: now + dataBase.GetDelayedTimeInSeconds(), + CreatedAt: now, + } + notification := moira.ScheduledNotification{ + SendFail: 4, + Timestamp: now, + CreatedAt: now, + } + + // create delayed notifications + notificationOld2 := moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, + Event: moira.NotificationEvent{ + Metric: "test1", + }, + SendFail: 5, + Timestamp: now - dataBase.GetDelayedTimeInSeconds() + 3, + CreatedAt: now - 2*dataBase.GetDelayedTimeInSeconds(), + } + notificationNew2 := moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test1", + }, + Event: moira.NotificationEvent{ + Metric: "test1", + }, + SendFail: 6, + Timestamp: now + dataBase.GetDelayedTimeInSeconds() + 1, + CreatedAt: now, + } + notificationNew3 := moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, + Event: moira.NotificationEvent{ + Metric: "test2", + }, + SendFail: 7, + Timestamp: now + dataBase.GetDelayedTimeInSeconds() + 2, + CreatedAt: now, + } + + Convey("Test fetchNotificationsDo", t, func() { + Convey("Test all notifications with diff ts in db", func() { + Convey("With limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) + limit = 1 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld}) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Without limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + }) + + Convey("Test zero notifications with ts in empty db", func() { + Convey("With limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{}) + limit = 10 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{}) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Without limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{}) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{}) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + }) + + Convey("Test all notification with ts and without limit in db", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld, notification4}) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification4, ¬ification, ¬ificationNew}) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test all notifications with ts and big limit in db", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.fetchNotificationsWithLimitDo(now+6000, 100) //nolint + limit = 100 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification}) @@ -409,9 +945,10 @@ func TestFetchNotificationsWithLimitDo(t *testing.T) { So(err, ShouldBeNil) }) - Convey("Test notification with ts and small limit in db", func() { + Convey("Test notifications with ts and small limit in db", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld, notification4}) - actual, err := dataBase.fetchNotificationsWithLimitDo(now+6000, 3) //nolint + limit = 3 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification4}) @@ -419,15 +956,135 @@ func TestFetchNotificationsWithLimitDo(t *testing.T) { So(err, ShouldBeNil) }) - Convey("Test notification with ts and limit = count", func() { + Convey("Test notifications with ts and limit = count", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.fetchNotificationsWithLimitDo(now+6000, 3) //nolint + limit = 3 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification}) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) + + Convey("Test delayed notifications with ts and deleted trigger", func() { + dataBase.RemoveTriggerLastCheck("test1") //nolint + defer func() { + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Metrics: map[string]moira.MetricState{ + "test1": {}, + }, + Timestamp: 110, + }, moira.TriggerSourceNotSet) + }() + + Convey("With big limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + limit = 100 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, &limit) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew}) + + allNotifications, count, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{¬ificationNew3}) + So(count, ShouldEqual, 1) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Without limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew3}) + + allNotifications, count, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + So(count, ShouldEqual, 0) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + }) + + Convey("Test notifications with ts and metric on maintenance", func() { + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": 130}, nil, "test", 100) //nolint + defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": 0}, nil, "test", 100) //nolint + + Convey("With limit = count", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + limit = 6 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, &limit) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew2}) + + allNotifications, count, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{¬ificationNew3}) + So(count, ShouldEqual, 1) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Without limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew2}) + + allNotifications, count, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{¬ificationNew3}) + So(count, ShouldEqual, 1) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + }) + + Convey("Test delayed notifications with ts and trigger on maintenance", func() { + var triggerMaintenance int64 = 130 + dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint + defer func() { + triggerMaintenance = 0 + dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint + }() + + Convey("With small limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + limit = 3 + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, &limit) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2}) + + allNotifications, count, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{¬ification, ¬ificationNew, ¬ificationNew2, ¬ificationNew3}) + So(count, ShouldEqual, 4) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("without limit", func() { + addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, nil) + So(err, ShouldBeNil) + So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew3}) + + allNotifications, count, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{¬ificationNew2}) + So(count, ShouldEqual, 1) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + }) }) } @@ -487,56 +1144,139 @@ func TestLimitNotifications(t *testing.T) { //nolint }) } -func TestFetchNotificationsNoLimit(t *testing.T) { +func TestFilterNotificationsByDelay(t *testing.T) { + Convey("filterNotificationsByDelay manipulations", t, func() { + notification1 := &moira.ScheduledNotification{ + Timestamp: 105, + CreatedAt: 100, + } + notification2 := &moira.ScheduledNotification{ + Timestamp: 110, + CreatedAt: 100, + } + notification3 := &moira.ScheduledNotification{ + Timestamp: 120, + CreatedAt: 100, + } + + Convey("Test with zero notifications", func() { + notifications := []*moira.ScheduledNotification{} + delayed, notDelayed := filterNotificationsByDelay(notifications, 1) + So(delayed, ShouldResemble, []*moira.ScheduledNotification{}) + So(notDelayed, ShouldResemble, []*moira.ScheduledNotification{}) + }) + + Convey("Test with zero delayed notifications", func() { + notifications := []*moira.ScheduledNotification{notification1, notification2, notification3} + delayed, notDelayed := filterNotificationsByDelay(notifications, 50) + So(delayed, ShouldResemble, []*moira.ScheduledNotification{}) + So(notDelayed, ShouldResemble, notifications) + }) + + Convey("Test with zero not delayed notifications", func() { + notifications := []*moira.ScheduledNotification{notification1, notification2, notification3} + delayed, notDelayed := filterNotificationsByDelay(notifications, 2) + So(delayed, ShouldResemble, notifications) + So(notDelayed, ShouldResemble, []*moira.ScheduledNotification{}) + }) + + Convey("Test with one delayed and two not delayed notifications", func() { + notifications := []*moira.ScheduledNotification{notification1, notification2, notification3} + delayed, notDelayed := filterNotificationsByDelay(notifications, 15) + So(delayed, ShouldResemble, []*moira.ScheduledNotification{notification3}) + So(notDelayed, ShouldResemble, []*moira.ScheduledNotification{notification1, notification2}) + }) + }) +} + +func TestGetNotificationsTriggerChecks(t *testing.T) { logger, _ := logging.GetLogger("dataBase") dataBase := NewTestDatabase(logger) dataBase.Flush() defer dataBase.Flush() - Convey("fetchNotificationsNoLimit manipulation", t, func() { - now := time.Now().Unix() - notificationNew := moira.ScheduledNotification{ - SendFail: 1, - Timestamp: now + 3600, - } - notification := moira.ScheduledNotification{ - SendFail: 2, - Timestamp: now, + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 1, + }, moira.TriggerSourceNotSet) + _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ + Timestamp: 2, + }, moira.TriggerSourceNotSet) + + Convey("getNotificationsTriggerChecks manipulations", t, func() { + notification1 := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test1", + }, } - notificationOld := moira.ScheduledNotification{ - SendFail: 3, - Timestamp: now - 3600, + notification2 := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test1", + }, } - notification4 := moira.ScheduledNotification{ - SendFail: 4, - Timestamp: now - 3600, + notification3 := &moira.ScheduledNotification{ + Trigger: moira.TriggerData{ + ID: "test2", + }, } - Convey("Test all notifications with diff ts in db", func() { - addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.fetchNotificationsNoLimit(now + 6000) + Convey("Test with zero notifications", func() { + notifications := []*moira.ScheduledNotification{} + triggerChecks, err := dataBase.getNotificationsTriggerChecks(notifications) So(err, ShouldBeNil) - So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) + So(triggerChecks, ShouldResemble, []*moira.CheckData{}) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) - Convey("Test zero notifications in db", func() { - addNotifications(dataBase, []moira.ScheduledNotification{}) - actual, err := dataBase.fetchNotificationsNoLimit(now + 6000) + Convey("Test with correct notifications", func() { + notifications := []*moira.ScheduledNotification{notification1, notification2, notification3} + triggerChecks, err := dataBase.getNotificationsTriggerChecks(notifications) So(err, ShouldBeNil) - So(actual, ShouldResemble, []*moira.ScheduledNotification{}) + So(triggerChecks, ShouldResemble, []*moira.CheckData{ + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 1, + MetricsToTargetRelation: map[string]string{}, + }, + { + Timestamp: 2, + MetricsToTargetRelation: map[string]string{}, + }, + }) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) }) - Convey("Test all notifications with various ts in db", func() { - addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld, notification4}) - actual, err := dataBase.fetchNotificationsNoLimit(now + 6000) + Convey("Test notifications with removed test1 trigger check", func() { + dataBase.RemoveTriggerLastCheck("test1") //nolint + defer func() { + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ + Timestamp: 1, + }, moira.TriggerSourceNotSet) + }() + + notifications := []*moira.ScheduledNotification{notification1, notification2, notification3} + triggerChecks, err := dataBase.getNotificationsTriggerChecks(notifications) So(err, ShouldBeNil) - So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification4, ¬ification, ¬ificationNew}) + So(triggerChecks, ShouldResemble, []*moira.CheckData{nil, nil, {Timestamp: 2, MetricsToTargetRelation: map[string]string{}}}) + + err = dataBase.RemoveAllNotifications() + So(err, ShouldBeNil) + }) + + Convey("Test notifications with removed all trigger checks", func() { + dataBase.RemoveTriggerLastCheck("test1") //nolint + dataBase.RemoveTriggerLastCheck("test2") //nolint + + notifications := []*moira.ScheduledNotification{notification1, notification2, notification3} + triggerChecks, err := dataBase.getNotificationsTriggerChecks(notifications) + So(err, ShouldBeNil) + So(triggerChecks, ShouldResemble, []*moira.CheckData{nil, nil, nil}) err = dataBase.RemoveAllNotifications() So(err, ShouldBeNil) diff --git a/database/redis/reply/check.go b/database/redis/reply/check.go index 72f0c6167..d4e188057 100644 --- a/database/redis/reply/check.go +++ b/database/redis/reply/check.go @@ -110,6 +110,27 @@ func Check(rep *redis.StringCmd) (moira.CheckData, error) { return checkSE.toCheckData(), nil } +// Checks converts an array of redis DB reply to moira.CheckData objects, if reply is nil, then checkdata is nil +func Checks(replies []*redis.StringCmd) ([]*moira.CheckData, error) { + checks := make([]*moira.CheckData, len(replies)) + + for i, value := range replies { + if value != nil { + check, err := Check(value) + if err != nil { + if err != database.ErrNil { + return nil, err + } + continue + } + + checks[i] = &check + } + } + + return checks, nil +} + // GetCheckBytes is a function that takes moira.CheckData and turns it to bytes that will be saved in redis. func GetCheckBytes(check moira.CheckData) ([]byte, error) { checkSE := toCheckDataStorageElement(check) diff --git a/database/redis/reply/notification.go b/database/redis/reply/notification.go index 399be8f0e..e23dfb347 100644 --- a/database/redis/reply/notification.go +++ b/database/redis/reply/notification.go @@ -18,6 +18,7 @@ type scheduledNotificationStorageElement struct { Throttled bool `json:"throttled"` SendFail int `json:"send_fail"` Timestamp int64 `json:"timestamp"` + CreatedAt int64 `json:"created_at"` } func toScheduledNotificationStorageElement(notification moira.ScheduledNotification) scheduledNotificationStorageElement { @@ -29,6 +30,7 @@ func toScheduledNotificationStorageElement(notification moira.ScheduledNotificat Throttled: notification.Throttled, SendFail: notification.SendFail, Timestamp: notification.Timestamp, + CreatedAt: notification.CreatedAt, } } @@ -41,6 +43,7 @@ func (n scheduledNotificationStorageElement) toScheduledNotification() moira.Sch Throttled: n.Throttled, SendFail: n.SendFail, Timestamp: n.Timestamp, + CreatedAt: n.CreatedAt, } } diff --git a/datatypes.go b/datatypes.go index d30a20db8..2be3f4ef0 100644 --- a/datatypes.go +++ b/datatypes.go @@ -242,6 +242,48 @@ type ScheduledNotification struct { Throttled bool `json:"throttled" example:"false"` SendFail int `json:"send_fail" example:"0"` Timestamp int64 `json:"timestamp" example:"1594471927" format:"int64"` + CreatedAt int64 `json:"created_at" example:"1594471900" format:"int64"` +} + +type ScheduledNotificationState int + +const ( + IgnoredNotification ScheduledNotificationState = iota + ValidNotification + RemovedNotification +) + +// Less is needed for the ScheduledNotification to match the Comparable interface +func (notification *ScheduledNotification) Less(other Comparable) (bool, error) { + otherNotification, ok := other.(*ScheduledNotification) + if !ok { + return false, fmt.Errorf("cannot to compare ScheduledNotification with different type") + } + + return notification.Timestamp < otherNotification.Timestamp, nil +} + +// IsDelayed checks if the notification is delayed, the difference between the send time and the create time +// is greater than the delayedTime +func (notification *ScheduledNotification) IsDelayed(delayedTime int64) bool { + return notification.CreatedAt != 0 && notification.Timestamp-notification.CreatedAt > delayedTime +} + +/* +GetState checks: + - If the trigger for which the notification was generated has been deleted, returns Removed state + - If the metric is on Maintenance, returns Ignored state + - If the trigger is on Maintenance, returns Ignored state + +Otherwise returns Valid state +*/ +func (notification *ScheduledNotification) GetState(triggerCheck *CheckData) ScheduledNotificationState { + if triggerCheck == nil { + return RemovedNotification + } else if !triggerCheck.IsMetricOnMaintenance(notification.Event.Metric) && !triggerCheck.IsTriggerOnMaintenance() { + return ValidNotification + } + return IgnoredNotification } // MatchedMetric represents parsed and matched metric data @@ -365,17 +407,19 @@ type CheckData struct { // MetricsToTargetRelation is a map that holds relation between metric names that was alone during last // check and targets that fetched this metric // {"t1": "metric.name.1", "t2": "metric.name.2"} - MetricsToTargetRelation map[string]string `json:"metrics_to_target_relation" example:"t1:metric.name.1,t2:metric.name.2"` - Score int64 `json:"score" example:"100" format:"int64"` - State State `json:"state" example:"OK"` - Maintenance int64 `json:"maintenance,omitempty" example:"0" format:"int64"` - MaintenanceInfo MaintenanceInfo `json:"maintenance_info"` - Timestamp int64 `json:"timestamp,omitempty" example:"1590741916" format:"int64"` - EventTimestamp int64 `json:"event_timestamp,omitempty" example:"1590741878" format:"int64"` - LastSuccessfulCheckTimestamp int64 `json:"last_successful_check_timestamp" example:"1590741916" format:"int64"` - Suppressed bool `json:"suppressed,omitempty" example:"true"` - SuppressedState State `json:"suppressed_state,omitempty"` - Message string `json:"msg,omitempty"` + MetricsToTargetRelation map[string]string `json:"metrics_to_target_relation" example:"t1:metric.name.1,t2:metric.name.2"` + Score int64 `json:"score" example:"100" format:"int64"` + State State `json:"state" example:"OK"` + Maintenance int64 `json:"maintenance,omitempty" example:"0" format:"int64"` + MaintenanceInfo MaintenanceInfo `json:"maintenance_info"` + // Timestamp - time, which means when the checker last checked this trigger, updated every checkInterval seconds + Timestamp int64 `json:"timestamp,omitempty" example:"1590741916" format:"int64"` + EventTimestamp int64 `json:"event_timestamp,omitempty" example:"1590741878" format:"int64"` + // LastSuccessfulCheckTimestamp - time of the last check of the trigger, during which there were no errors + LastSuccessfulCheckTimestamp int64 `json:"last_successful_check_timestamp" example:"1590741916" format:"int64"` + Suppressed bool `json:"suppressed,omitempty" example:"true"` + SuppressedState State `json:"suppressed_state,omitempty"` + Message string `json:"msg,omitempty"` } // RemoveMetricState is a function that removes MetricState from map of states. @@ -388,6 +432,25 @@ func (checkData *CheckData) RemoveMetricsToTargetRelation() { checkData.MetricsToTargetRelation = make(map[string]string) } +// IsTriggerOnMaintenance checks if the trigger is on Maintenance +func (checkData *CheckData) IsTriggerOnMaintenance() bool { + return checkData.Timestamp <= checkData.Maintenance +} + +// IsMetricOnMaintenance checks if the metric of the given trigger is on Maintenance +func (checkData *CheckData) IsMetricOnMaintenance(metric string) bool { + if checkData.Metrics == nil { + return false + } + + metricState, ok := checkData.Metrics[metric] + if !ok { + return false + } + + return checkData.Timestamp <= metricState.Maintenance +} + // MetricState represents metric state data for given timestamp type MetricState struct { EventTimestamp int64 `json:"event_timestamp" example:"1590741878" format:"int64"` diff --git a/datatypes_test.go b/datatypes_test.go index 0294e92ea..724bf3285 100644 --- a/datatypes_test.go +++ b/datatypes_test.go @@ -296,6 +296,58 @@ func TestScheduledNotification_GetKey(t *testing.T) { }) } +func TestScheduledNotification_GetState(t *testing.T) { + Convey("Test get state of scheduled notifications", t, func() { + notification := ScheduledNotification{ + Event: NotificationEvent{ + Metric: "test", + }, + } + + Convey("Get Removed state with nil check data", func() { + state := notification.GetState(nil) + So(state, ShouldEqual, RemovedNotification) + }) + + Convey("Get Ignored state with metric on maintenance", func() { + state := notification.GetState(&CheckData{ + Metrics: map[string]MetricState{ + "test": { + Maintenance: 100, + }, + }, + Timestamp: 80, + }) + So(state, ShouldEqual, IgnoredNotification) + }) + + Convey("Get Ignored state with trigger on maintenance", func() { + state := notification.GetState(&CheckData{ + Maintenance: 100, + Timestamp: 80, + }) + So(state, ShouldEqual, IgnoredNotification) + }) + + Convey("Get Valid state with trigger without metrics", func() { + state := notification.GetState(&CheckData{ + Timestamp: 80, + }) + So(state, ShouldEqual, ValidNotification) + }) + + Convey("Get Valid state with trigger with test metric", func() { + state := notification.GetState(&CheckData{ + Metrics: map[string]MetricState{ + "test": {}, + }, + Timestamp: 80, + }) + So(state, ShouldEqual, ValidNotification) + }) + }) +} + func TestCheckData_GetOrCreateMetricState(t *testing.T) { Convey("Test no metric", t, func() { checkData := CheckData{ @@ -373,6 +425,84 @@ func TestTrigger_IsSimple(t *testing.T) { }) } +func TestCheckData_IsTriggerOnMaintenance(t *testing.T) { + Convey("IsTriggerOnMaintenance manipulations", t, func() { + checkData := &CheckData{ + Maintenance: 120, + Timestamp: 100, + } + + Convey("Test with trigger check Maintenance more than last check timestamp", func() { + actual := checkData.IsTriggerOnMaintenance() + So(actual, ShouldBeTrue) + }) + + Convey("Test with trigger check Maintenance less than last check timestamp", func() { + checkData.Timestamp = 150 + defer func() { + checkData.Timestamp = 100 + }() + + actual := checkData.IsTriggerOnMaintenance() + So(actual, ShouldBeFalse) + }) + }) +} + +func TestCheckData_IsMetricOnMaintenance(t *testing.T) { + Convey("isMetricOnMaintenance manipulations", t, func() { + checkData := &CheckData{ + Metrics: map[string]MetricState{ + "test1": { + Maintenance: 110, + }, + "test2": {}, + }, + Timestamp: 100, + } + + Convey("Test with a metric that is not in the trigger", func() { + actual := checkData.IsMetricOnMaintenance("") + So(actual, ShouldBeFalse) + }) + + Convey("Test with metrics that are in the trigger but not on maintenance", func() { + actual := checkData.IsMetricOnMaintenance("test2") + So(actual, ShouldBeFalse) + }) + + Convey("Test with metrics that are in the trigger and on maintenance", func() { + actual := checkData.IsMetricOnMaintenance("test1") + So(actual, ShouldBeTrue) + }) + + Convey("Test with the metric that is in the trigger, but the last successful check of the trigger is more than Maintenance", func() { + checkData.Timestamp = 120 + defer func() { + checkData.Timestamp = 100 + }() + + actual := checkData.IsMetricOnMaintenance("test1") + So(actual, ShouldBeFalse) + }) + + Convey("Test with trigger without metrics", func() { + checkData.Metrics = make(map[string]MetricState) + defer func() { + checkData.Metrics = map[string]MetricState{ + "test1": { + Maintenance: 110, + }, + "test2": {}, + } + }() + + actual := checkData.IsMetricOnMaintenance("test1") + So(actual, ShouldBeFalse) + }) + }) +} + func TestCheckData_GetEventTimestamp(t *testing.T) { Convey("Get event timestamp", t, func() { checkData := CheckData{Timestamp: 800, EventTimestamp: 0} @@ -724,3 +854,58 @@ func testMaintenance(conveyMessage string, actualInfo MaintenanceInfo, maintenan So(lastCheckTest.Maintenance, ShouldEqual, maintenance) }) } + +func TestScheduledNotificationLess(t *testing.T) { + Convey("Test Scheduled notification Less function", t, func() { + notification := &ScheduledNotification{ + Timestamp: 5, + } + + Convey("Test Less with nil", func() { + actual, err := notification.Less(nil) + So(err, ShouldResemble, fmt.Errorf("cannot to compare ScheduledNotification with different type")) + So(actual, ShouldBeFalse) + }) + + Convey("Test Less with less notification :)", func() { + actual, err := notification.Less(&ScheduledNotification{Timestamp: 1}) + So(err, ShouldBeNil) + So(actual, ShouldBeFalse) + }) + + Convey("Test Less with greater notification", func() { + actual, err := notification.Less(&ScheduledNotification{Timestamp: 10}) + So(err, ShouldBeNil) + So(actual, ShouldBeTrue) + }) + }) +} + +func TestScheduledNotificationIsDelayed(t *testing.T) { + Convey("Test Scheduled notification IsDelayed function", t, func() { + notification := &ScheduledNotification{ + Timestamp: 5, + } + + var delayedTime int64 = 2 + + Convey("Test notification with empty created at field", func() { + actual := notification.IsDelayed(delayedTime) + So(actual, ShouldBeFalse) + }) + + notification.CreatedAt = 1 + + Convey("Test notification which is to be defined as delayed", func() { + actual := notification.IsDelayed(delayedTime) + So(actual, ShouldBeTrue) + }) + + notification.CreatedAt = 4 + + Convey("Test notification which is to be defined as not delayed", func() { + actual := notification.IsDelayed(delayedTime) + So(actual, ShouldBeFalse) + }) + }) +} diff --git a/go.mod b/go.mod index 7ceaad8d9..bad535f89 100644 --- a/go.mod +++ b/go.mod @@ -144,7 +144,7 @@ require ( github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.16.0 // indirect github.com/stretchr/objx v0.5.0 // indirect - github.com/stretchr/testify v1.8.4 // indirect + github.com/stretchr/testify v1.8.4 github.com/subosito/gotenv v1.4.2 // indirect github.com/tinylib/msgp v1.1.8 // indirect github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect diff --git a/helpers.go b/helpers.go index 1c4a367a5..6f38b6c54 100644 --- a/helpers.go +++ b/helpers.go @@ -213,3 +213,40 @@ func ReplaceSubstring(str, begin, end, replaced string) string { } return result } + +type Comparable interface { + Less(other Comparable) (bool, error) +} + +// Merge is a generic function that performs a merge of two sorted arrays into one sorted array +func MergeToSorted[T Comparable](arr1, arr2 []T) ([]T, error) { + merged := make([]T, 0, len(arr1)+len(arr2)) + i, j := 0, 0 + + for i < len(arr1) && j < len(arr2) { + less, err := arr1[i].Less(arr2[j]) + if err != nil { + return nil, err + } + + if less { + merged = append(merged, arr1[i]) + i++ + } else { + merged = append(merged, arr2[j]) + j++ + } + } + + for i < len(arr1) { + merged = append(merged, arr1[i]) + i++ + } + + for j < len(arr2) { + merged = append(merged, arr2[j]) + j++ + } + + return merged, nil +} diff --git a/helpers_test.go b/helpers_test.go index 803376070..d9c3ca4ee 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -242,7 +242,7 @@ func testRounding(baseTimestamp, retention int64) { } func TestReplaceSubstring(t *testing.T) { - Convey("test replace substring", t, func() { + Convey("Test replace substring", t, func() { Convey("replacement string in the middle", func() { So(ReplaceSubstring("telebot: Post https://api.telegram.org/botXXX/getMe", "/bot", "/", "[DELETED]"), ShouldResemble, "telebot: Post https://api.telegram.org/bot[DELETED]/getMe") }) @@ -264,3 +264,78 @@ func TestReplaceSubstring(t *testing.T) { }) }) } + +type myInt int + +func (m myInt) Less(other Comparable) (bool, error) { + otherInt := other.(myInt) + return m < otherInt, nil +} + +type myTest struct { + value int +} + +func (test myTest) Less(other Comparable) (bool, error) { + otherTest := other.(myTest) + return test.value < otherTest.value, nil +} + +func TestMergeToSorted(t *testing.T) { + Convey("Test MergeToSorted function", t, func() { + Convey("Test with two nil arrays", func() { + merged, err := MergeToSorted[myInt](nil, nil) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{}) + }) + + Convey("Test with one nil array", func() { + merged, err := MergeToSorted[myInt](nil, []myInt{1, 2, 3}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{1, 2, 3}) + }) + + Convey("Test with two arrays", func() { + merged, err := MergeToSorted[myInt]([]myInt{4, 5}, []myInt{1, 2, 3}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{1, 2, 3, 4, 5}) + }) + + Convey("Test with empty array", func() { + merged, err := MergeToSorted[myInt]([]myInt{-4, 5}, []myInt{}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{-4, 5}) + }) + + Convey("Test with sorted values but mixed up", func() { + merged, err := MergeToSorted[myInt]([]myInt{1, 9, 10}, []myInt{4, 8, 12}) + So(err, ShouldBeNil) + So(merged, ShouldResemble, []myInt{1, 4, 8, 9, 10, 12}) + }) + + Convey("Test with structure type", func() { + arr1 := []myTest{ + { + value: 1, + }, + { + value: 2, + }, + } + + arr2 := []myTest{ + { + value: -2, + }, + { + value: -1, + }, + } + + expected := append(arr2, arr1...) + merged, err := MergeToSorted[myTest](arr1, arr2) + So(err, ShouldBeNil) + So(merged, ShouldResemble, expected) + }) + }) +} diff --git a/local/api.yml b/local/api.yml index c0b4939db..b662ef88d 100644 --- a/local/api.yml +++ b/local/api.yml @@ -53,6 +53,10 @@ web: notification_history: ttl: 48h query_limit: 10000 +notification: + delayed_time: 1m + transaction_timeout: 200ms + transaction_max_retries: 10 log: log_file: stdout log_level: debug diff --git a/local/notifier.yml b/local/notifier.yml index a4a19f8fc..103b0849f 100644 --- a/local/notifier.yml +++ b/local/notifier.yml @@ -42,6 +42,10 @@ notifier: notification_history: ttl: 48h query_limit: 10000 +notification: + delayed_time: 1m + transaction_timeout: 200ms + transaction_max_retries: 10 log: log_file: stdout log_level: info diff --git a/notifier/events/event_test.go b/notifier/events/event_test.go index 17e9ff892..a88b50849 100644 --- a/notifier/events/event_test.go +++ b/notifier/events/event_test.go @@ -50,6 +50,7 @@ func TestEvent(t *testing.T) { }, SendFail: 0, Timestamp: time.Now().Unix(), + CreatedAt: time.Now().Unix(), Throttled: false, Contact: contact, } @@ -88,6 +89,7 @@ func TestEvent(t *testing.T) { }, SendFail: 0, Timestamp: now.Unix(), + CreatedAt: now.Unix(), Throttled: false, Contact: contact, } diff --git a/notifier/notifier.go b/notifier/notifier.go index ea945412d..3e7cb04c6 100644 --- a/notifier/notifier.go +++ b/notifier/notifier.go @@ -148,7 +148,7 @@ func (notifier *StandardNotifier) resend(pkg *NotificationPackage, reason string logger := getLogWithPackageContext(¬ifier.logger, pkg, ¬ifier.config) logger.Warning(). - Int("number_of_retires", pkg.FailCount). + Int("number_of_retries", pkg.FailCount). String("reason", reason). Msg("Can't send message. Retry again in 1 min") diff --git a/notifier/scheduler.go b/notifier/scheduler.go index 6456f6e69..81f69eb4b 100644 --- a/notifier/scheduler.go +++ b/notifier/scheduler.go @@ -59,12 +59,14 @@ func (scheduler *StandardScheduler) ScheduleNotification(now time.Time, event mo Throttled: throttled, SendFail: sendFail, Timestamp: next.Unix(), + CreatedAt: now.Unix(), Plotting: plotting, } logger.Debug(). String("notification_timestamp", next.Format("2006/01/02 15:04:05")). Int64("notification_timestamp_unix", next.Unix()). + Int64("notification_created_at_unix", now.Unix()). Msg("Scheduled notification") return notification } diff --git a/notifier/scheduler_test.go b/notifier/scheduler_test.go index 5d0e98c39..aedcc85e3 100644 --- a/notifier/scheduler_test.go +++ b/notifier/scheduler_test.go @@ -61,6 +61,7 @@ func TestThrottling(t *testing.T) { Throttled: false, Timestamp: now.Unix(), SendFail: 0, + CreatedAt: now.Unix(), } Convey("Test sendFail more than 0, and no throttling, should send message in one minute", t, func() { From 520396aa6de26661ec95b4fd214c08a07e2d61bb Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 24 Oct 2023 20:52:23 +0300 Subject: [PATCH 03/17] merge with feat/add-getting-checkdatas-and-merge --- cmd/api/config.go | 7 ++++--- cmd/api/config_test.go | 7 ++++--- cmd/config.go | 12 ++++++++---- cmd/notifier/config.go | 7 ++++--- database/redis/config.go | 5 ++++- database/redis/database.go | 14 ++++++++------ database/redis/notification.go | 6 +----- local/api.yml | 1 + local/notifier.yml | 1 + 9 files changed, 35 insertions(+), 25 deletions(-) diff --git a/cmd/api/config.go b/cmd/api/config.go index 7078632d0..0837f4bde 100644 --- a/cmd/api/config.go +++ b/cmd/api/config.go @@ -116,9 +116,10 @@ func getDefault() config { NotificationHistoryQueryLimit: int(notifier.NotificationsLimitUnlimited), }, Notification: cmd.NotificationConfig{ - DelayedTime: "1m", - TransactionTimeout: "200ms", - TransactionMaxRetries: 10, + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + TransactionHeuristicLimit: 10000, }, Logger: cmd.LoggerConfig{ LogFile: "stdout", diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index 42fffa171..12c72a869 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -103,9 +103,10 @@ func Test_webConfig_getDefault(t *testing.T) { NotificationHistoryQueryLimit: -1, }, Notification: cmd.NotificationConfig{ - DelayedTime: "1m", - TransactionTimeout: "200ms", - TransactionMaxRetries: 10, + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + TransactionHeuristicLimit: 10000, }, } diff --git a/cmd/config.go b/cmd/config.go index 8af184fe5..412e14a28 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -86,16 +86,20 @@ type NotificationConfig struct { DelayedTime string `yaml:"delayed_time"` // TransactionTimeout defines the timeout between fetch notifications transactions TransactionTimeout string `yaml:"transaction_timeout"` - // TransactionTimeout defines the timeout between fetch notifications transactions + // TransactionMaxRetries defines the maximum number of attempts to make a transaction TransactionMaxRetries int `yaml:"transaction_max_retries"` + // TransactionHeuristicLimit maximum allowable limit, after this limit all notifications + // without limit will be taken + TransactionHeuristicLimit int64 `yaml:"transaction_heuristic_limit"` } // GetSettings returns notification storage configuration func (notificationConfig *NotificationConfig) GetSettings() redis.NotificationConfig { return redis.NotificationConfig{ - DelayedTime: to.Duration(notificationConfig.DelayedTime), - TransactionTimeout: to.Duration(notificationConfig.TransactionTimeout), - TransactionMaxRetries: notificationConfig.TransactionMaxRetries, + DelayedTime: to.Duration(notificationConfig.DelayedTime), + TransactionTimeout: to.Duration(notificationConfig.TransactionTimeout), + TransactionMaxRetries: notificationConfig.TransactionMaxRetries, + TransactionHeuristicLimit: notificationConfig.TransactionHeuristicLimit, } } diff --git a/cmd/notifier/config.go b/cmd/notifier/config.go index 3c4d4528d..5cfddf9b0 100644 --- a/cmd/notifier/config.go +++ b/cmd/notifier/config.go @@ -95,9 +95,10 @@ func getDefault() config { NotificationHistoryQueryLimit: int(notifier.NotificationsLimitUnlimited), }, Notification: cmd.NotificationConfig{ - DelayedTime: "1m", - TransactionTimeout: "200ms", - TransactionMaxRetries: 10, + DelayedTime: "1m", + TransactionTimeout: "200ms", + TransactionMaxRetries: 10, + TransactionHeuristicLimit: 10000, }, Notifier: notifierConfig{ SenderTimeout: "10s", diff --git a/database/redis/config.go b/database/redis/config.go index 18b357657..676a48b4a 100644 --- a/database/redis/config.go +++ b/database/redis/config.go @@ -29,6 +29,9 @@ type NotificationConfig struct { DelayedTime time.Duration // TransactionTimeout defines the timeout between fetch notifications transactions TransactionTimeout time.Duration - // TransactionTimeout defines the timeout between fetch notifications transactions + // TransactionMaxRetries defines the maximum number of attempts to make a transaction TransactionMaxRetries int + // TransactionHeuristicLimit maximum allowable limit, after this limit all notifications + // without limit will be taken + TransactionHeuristicLimit int64 } diff --git a/database/redis/database.go b/database/redis/database.go index 5824a02da..07b0c72c6 100644 --- a/database/redis/database.go +++ b/database/redis/database.go @@ -96,9 +96,10 @@ func NewTestDatabase(logger moira.Logger) *DbConnector { NotificationHistoryQueryLimit: 1000, }, NotificationConfig{ - DelayedTime: time.Minute, - TransactionTimeout: 200 * time.Millisecond, - TransactionMaxRetries: 10, + DelayedTime: time.Minute, + TransactionTimeout: 200 * time.Millisecond, + TransactionMaxRetries: 10, + TransactionHeuristicLimit: 10000, }, testSource) } @@ -112,9 +113,10 @@ func NewTestDatabaseWithIncorrectConfig(logger moira.Logger) *DbConnector { NotificationHistoryQueryLimit: 1000, }, NotificationConfig{ - DelayedTime: time.Minute, - TransactionTimeout: 200 * time.Millisecond, - TransactionMaxRetries: 10, + DelayedTime: time.Minute, + TransactionTimeout: 200 * time.Millisecond, + TransactionMaxRetries: 10, + TransactionHeuristicLimit: 10000, }, testSource) } diff --git a/database/redis/notification.go b/database/redis/notification.go index 928212a30..ebb74f616 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -16,10 +16,6 @@ import ( "github.com/moira-alert/moira/database/redis/reply" ) -const ( - transactionHeuristicLimit = 10000 -) - // Custom error for transaction error type transactionError struct{} @@ -263,7 +259,7 @@ func (connector *DbConnector) FetchNotifications(to int64, limit int64) ([]*moir } // Hope count will be not greater then limit when we call fetchNotificationsNoLimit - if limit > transactionHeuristicLimit && count < limit/2 { + if limit > connector.notification.TransactionHeuristicLimit && count < limit/2 { return connector.fetchNotifications(to, nil) } diff --git a/local/api.yml b/local/api.yml index b662ef88d..34dad26bb 100644 --- a/local/api.yml +++ b/local/api.yml @@ -57,6 +57,7 @@ notification: delayed_time: 1m transaction_timeout: 200ms transaction_max_retries: 10 + transaction_heuristic_limit: 10000 log: log_file: stdout log_level: debug diff --git a/local/notifier.yml b/local/notifier.yml index 103b0849f..ff0e2b8de 100644 --- a/local/notifier.yml +++ b/local/notifier.yml @@ -46,6 +46,7 @@ notification: delayed_time: 1m transaction_timeout: 200ms transaction_max_retries: 10 + transaction_heuristic_limit: 10000 log: log_file: stdout log_level: info From 5ce2646def5d8590b7e2355177dc6d2ded54b441 Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 24 Oct 2023 20:57:43 +0300 Subject: [PATCH 04/17] fix merge conflicts --- database/redis/notification.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index ebb74f616..ac598a0cc 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -139,7 +139,7 @@ func (connector *DbConnector) GetDelayedTimeInSeconds() int64 { return int64(connector.notification.DelayedTime.Seconds()) } -// filterNotificationsByDelay filters notifications into delayed and not delayed notifications +// filterNotificationsByDelay filters notifications into delayed and not delayed notifications func filterNotificationsByDelay(notifications []*moira.ScheduledNotification, delayedTime int64) ([]*moira.ScheduledNotification, []*moira.ScheduledNotification) { delayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) notDelayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) From 7ea2e81e6e8064797a156defe2aa15bb239d4037 Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 24 Oct 2023 20:58:23 +0300 Subject: [PATCH 05/17] remove unused space --- database/redis/notification.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index ac598a0cc..ebb74f616 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -139,7 +139,7 @@ func (connector *DbConnector) GetDelayedTimeInSeconds() int64 { return int64(connector.notification.DelayedTime.Seconds()) } -// filterNotificationsByDelay filters notifications into delayed and not delayed notifications +// filterNotificationsByDelay filters notifications into delayed and not delayed notifications func filterNotificationsByDelay(notifications []*moira.ScheduledNotification, delayedTime int64) ([]*moira.ScheduledNotification, []*moira.ScheduledNotification) { delayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) notDelayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) From 38ba699c5a62abb8fff028b763348da21f689824 Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 24 Oct 2023 21:13:02 +0300 Subject: [PATCH 06/17] make private enum --- datatypes.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/datatypes.go b/datatypes.go index 2be3f4ef0..f367c1618 100644 --- a/datatypes.go +++ b/datatypes.go @@ -245,10 +245,10 @@ type ScheduledNotification struct { CreatedAt int64 `json:"created_at" example:"1594471900" format:"int64"` } -type ScheduledNotificationState int +type scheduledNotificationState int const ( - IgnoredNotification ScheduledNotificationState = iota + IgnoredNotification scheduledNotificationState = iota ValidNotification RemovedNotification ) @@ -277,7 +277,7 @@ GetState checks: Otherwise returns Valid state */ -func (notification *ScheduledNotification) GetState(triggerCheck *CheckData) ScheduledNotificationState { +func (notification *ScheduledNotification) GetState(triggerCheck *CheckData) scheduledNotificationState { if triggerCheck == nil { return RemovedNotification } else if !triggerCheck.IsMetricOnMaintenance(notification.Event.Metric) && !triggerCheck.IsTriggerOnMaintenance() { From 9a7f364ae099bb44616a90eb8927b7d83243d43d Mon Sep 17 00:00:00 2001 From: almostinf Date: Wed, 25 Oct 2023 10:32:10 +0300 Subject: [PATCH 07/17] change transaction error handling --- database/redis/notification.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index ebb74f616..f7a343f72 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -317,6 +317,9 @@ func getNotificationsInTxWithLimit(ctx context.Context, tx *redis.Tx, to int64, response := tx.ZRangeByScore(ctx, notifierNotificationsKey, rng) if response.Err() != nil { + if errors.Is(response.Err(), redis.TxFailedErr) { + return nil, &transactionError{} + } return nil, fmt.Errorf("failed to ZRANGEBYSCORE: %w", response.Err()) } @@ -379,9 +382,6 @@ func (connector *DbConnector) fetchNotificationsDo(to int64, limit *int64) ([]*m err := c.Watch(ctx, func(tx *redis.Tx) error { notifications, err := getNotificationsInTxWithLimit(ctx, tx, to, limit) if err != nil { - if errors.Is(err, redis.TxFailedErr) { - return &transactionError{} - } return fmt.Errorf("failed to get notifications with limit in transaction: %w", err) } From f0d74014492510bf7aee441c6fad7fe71a2faba0 Mon Sep 17 00:00:00 2001 From: almostinf Date: Wed, 25 Oct 2023 10:52:59 +0300 Subject: [PATCH 08/17] fix else after return --- datatypes.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datatypes.go b/datatypes.go index f367c1618..98d62e514 100644 --- a/datatypes.go +++ b/datatypes.go @@ -280,7 +280,8 @@ Otherwise returns Valid state func (notification *ScheduledNotification) GetState(triggerCheck *CheckData) scheduledNotificationState { if triggerCheck == nil { return RemovedNotification - } else if !triggerCheck.IsMetricOnMaintenance(notification.Event.Metric) && !triggerCheck.IsTriggerOnMaintenance() { + } + if !triggerCheck.IsMetricOnMaintenance(notification.Event.Metric) && !triggerCheck.IsTriggerOnMaintenance() { return ValidNotification } return IgnoredNotification From b57d5a49515f612465023c4c70d7868095fbeed5 Mon Sep 17 00:00:00 2001 From: almostinf Date: Fri, 27 Oct 2023 22:52:55 +0300 Subject: [PATCH 09/17] fix maintenance and doc --- database/redis/notification_test.go | 33 ++++++++----------------- datatypes.go | 6 ++--- datatypes_test.go | 37 +++++++++++++---------------- 3 files changed, 30 insertions(+), 46 deletions(-) diff --git a/database/redis/notification_test.go b/database/redis/notification_test.go index 52a0502b9..105a54851 100644 --- a/database/redis/notification_test.go +++ b/database/redis/notification_test.go @@ -533,15 +533,12 @@ func TestFilterNotificationsByState(t *testing.T) { CreatedAt: now, } - _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ - Timestamp: 100, - }, moira.TriggerSourceNotSet) + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{}, moira.TriggerSourceNotSet) _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ Metrics: map[string]moira.MetricState{ "test": {}, }, - Timestamp: 100, }, moira.TriggerSourceNotSet) Convey("Test filter notifications by state", t, func() { @@ -562,9 +559,7 @@ func TestFilterNotificationsByState(t *testing.T) { Convey("With removed check data", func() { dataBase.RemoveTriggerLastCheck("test1") //nolint defer func() { - _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ - Timestamp: 100, - }, moira.TriggerSourceNotSet) + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{}, moira.TriggerSourceNotSet) }() validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) @@ -574,7 +569,7 @@ func TestFilterNotificationsByState(t *testing.T) { }) Convey("With metric on maintenance", func() { - dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 130}, nil, "test", 100) //nolint + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) @@ -584,7 +579,7 @@ func TestFilterNotificationsByState(t *testing.T) { }) Convey("With trigger on maintenance", func() { - var triggerMaintenance int64 = 130 + var triggerMaintenance int64 = time.Now().Add(time.Hour).Unix() dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint defer func() { triggerMaintenance = 0 @@ -665,15 +660,12 @@ func TestHandleNotifications(t *testing.T) { CreatedAt: now, } - _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ - Timestamp: 100, - }, moira.TriggerSourceNotSet) + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{}, moira.TriggerSourceNotSet) _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ Metrics: map[string]moira.MetricState{ "test": {}, }, - Timestamp: 100, }, moira.TriggerSourceNotSet) Convey("Test handle notifications", t, func() { @@ -694,9 +686,7 @@ func TestHandleNotifications(t *testing.T) { Convey("With both delayed and not delayed notifications and removed check data", func() { dataBase.RemoveTriggerLastCheck("test1") //nolint defer func() { - _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{ - Timestamp: 100, - }, moira.TriggerSourceNotSet) + _ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{}, moira.TriggerSourceNotSet) }() validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) @@ -706,7 +696,7 @@ func TestHandleNotifications(t *testing.T) { }) Convey("With both delayed and not delayed valid notifications and metric on maintenance", func() { - dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 130}, nil, "test", 100) //nolint + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) @@ -716,7 +706,7 @@ func TestHandleNotifications(t *testing.T) { }) Convey("With both delayed and not delayed valid notifications and trigger on maintenance", func() { - var triggerMaintenance int64 = 130 + var triggerMaintenance int64 = time.Now().Add(time.Hour).Unix() dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint defer func() { triggerMaintenance = 0 @@ -809,7 +799,6 @@ func TestFetchNotificationsDo(t *testing.T) { Metrics: map[string]moira.MetricState{ "test1": {}, }, - Timestamp: 110, }, moira.TriggerSourceNotSet) _ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{ @@ -817,7 +806,6 @@ func TestFetchNotificationsDo(t *testing.T) { "test1": {}, "test2": {}, }, - Timestamp: 110, }, moira.TriggerSourceNotSet) now := time.Now().Unix() @@ -974,7 +962,6 @@ func TestFetchNotificationsDo(t *testing.T) { Metrics: map[string]moira.MetricState{ "test1": {}, }, - Timestamp: 110, }, moira.TriggerSourceNotSet) }() @@ -1011,7 +998,7 @@ func TestFetchNotificationsDo(t *testing.T) { }) Convey("Test notifications with ts and metric on maintenance", func() { - dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": 130}, nil, "test", 100) //nolint + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": 0}, nil, "test", 100) //nolint Convey("With limit = count", func() { @@ -1047,7 +1034,7 @@ func TestFetchNotificationsDo(t *testing.T) { }) Convey("Test delayed notifications with ts and trigger on maintenance", func() { - var triggerMaintenance int64 = 130 + var triggerMaintenance int64 = time.Now().Add(time.Hour).Unix() dataBase.SetTriggerCheckMaintenance("test1", map[string]int64{}, &triggerMaintenance, "test", 100) //nolint defer func() { triggerMaintenance = 0 diff --git a/datatypes.go b/datatypes.go index 98d62e514..51c15f9ab 100644 --- a/datatypes.go +++ b/datatypes.go @@ -413,7 +413,7 @@ type CheckData struct { State State `json:"state" example:"OK"` Maintenance int64 `json:"maintenance,omitempty" example:"0" format:"int64"` MaintenanceInfo MaintenanceInfo `json:"maintenance_info"` - // Timestamp - time, which means when the checker last checked this trigger, updated every checkInterval seconds + // Timestamp - time, which means when the checker last checked this trigger, this value stops updating if the trigger does not receive metrics Timestamp int64 `json:"timestamp,omitempty" example:"1590741916" format:"int64"` EventTimestamp int64 `json:"event_timestamp,omitempty" example:"1590741878" format:"int64"` // LastSuccessfulCheckTimestamp - time of the last check of the trigger, during which there were no errors @@ -435,7 +435,7 @@ func (checkData *CheckData) RemoveMetricsToTargetRelation() { // IsTriggerOnMaintenance checks if the trigger is on Maintenance func (checkData *CheckData) IsTriggerOnMaintenance() bool { - return checkData.Timestamp <= checkData.Maintenance + return time.Now().Unix() <= checkData.Maintenance } // IsMetricOnMaintenance checks if the metric of the given trigger is on Maintenance @@ -449,7 +449,7 @@ func (checkData *CheckData) IsMetricOnMaintenance(metric string) bool { return false } - return checkData.Timestamp <= metricState.Maintenance + return time.Now().Unix() <= metricState.Maintenance } // MetricState represents metric state data for given timestamp diff --git a/datatypes_test.go b/datatypes_test.go index 724bf3285..2d7b33472 100644 --- a/datatypes_test.go +++ b/datatypes_test.go @@ -313,26 +313,22 @@ func TestScheduledNotification_GetState(t *testing.T) { state := notification.GetState(&CheckData{ Metrics: map[string]MetricState{ "test": { - Maintenance: 100, + Maintenance: time.Now().Add(time.Hour).Unix(), }, }, - Timestamp: 80, }) So(state, ShouldEqual, IgnoredNotification) }) Convey("Get Ignored state with trigger on maintenance", func() { state := notification.GetState(&CheckData{ - Maintenance: 100, - Timestamp: 80, + Maintenance: time.Now().Add(time.Hour).Unix(), }) So(state, ShouldEqual, IgnoredNotification) }) Convey("Get Valid state with trigger without metrics", func() { - state := notification.GetState(&CheckData{ - Timestamp: 80, - }) + state := notification.GetState(&CheckData{}) So(state, ShouldEqual, ValidNotification) }) @@ -341,7 +337,6 @@ func TestScheduledNotification_GetState(t *testing.T) { Metrics: map[string]MetricState{ "test": {}, }, - Timestamp: 80, }) So(state, ShouldEqual, ValidNotification) }) @@ -428,19 +423,18 @@ func TestTrigger_IsSimple(t *testing.T) { func TestCheckData_IsTriggerOnMaintenance(t *testing.T) { Convey("IsTriggerOnMaintenance manipulations", t, func() { checkData := &CheckData{ - Maintenance: 120, - Timestamp: 100, + Maintenance: time.Now().Add(time.Hour).Unix(), } - Convey("Test with trigger check Maintenance more than last check timestamp", func() { + Convey("Test with trigger check Maintenance more than time now", func() { actual := checkData.IsTriggerOnMaintenance() So(actual, ShouldBeTrue) }) - Convey("Test with trigger check Maintenance less than last check timestamp", func() { - checkData.Timestamp = 150 + Convey("Test with trigger check Maintenance less than time now", func() { + checkData.Maintenance = time.Now().Add(-time.Hour).Unix() defer func() { - checkData.Timestamp = 100 + checkData.Maintenance = time.Now().Add(time.Hour).Unix() }() actual := checkData.IsTriggerOnMaintenance() @@ -454,11 +448,10 @@ func TestCheckData_IsMetricOnMaintenance(t *testing.T) { checkData := &CheckData{ Metrics: map[string]MetricState{ "test1": { - Maintenance: 110, + Maintenance: time.Now().Add(time.Hour).Unix(), }, "test2": {}, }, - Timestamp: 100, } Convey("Test with a metric that is not in the trigger", func() { @@ -476,10 +469,14 @@ func TestCheckData_IsMetricOnMaintenance(t *testing.T) { So(actual, ShouldBeTrue) }) - Convey("Test with the metric that is in the trigger, but the last successful check of the trigger is more than Maintenance", func() { - checkData.Timestamp = 120 + Convey("Test with the metric that is in the trigger, but the time now is more than Maintenance", func() { + metric := checkData.Metrics["test1"] + metric.Maintenance = time.Now().Add(-time.Hour).Unix() + checkData.Metrics["test1"] = metric defer func() { - checkData.Timestamp = 100 + metric := checkData.Metrics["test1"] + metric.Maintenance = time.Now().Add(time.Hour).Unix() + checkData.Metrics["test1"] = metric }() actual := checkData.IsMetricOnMaintenance("test1") @@ -491,7 +488,7 @@ func TestCheckData_IsMetricOnMaintenance(t *testing.T) { defer func() { checkData.Metrics = map[string]MetricState{ "test1": { - Maintenance: 110, + Maintenance: time.Now().Add(time.Hour).Unix(), }, "test2": {}, } From 4776d94e19ce517125c1123120358699042c863f Mon Sep 17 00:00:00 2001 From: almostinf Date: Mon, 30 Oct 2023 14:53:44 +0300 Subject: [PATCH 10/17] fix notifications cycle --- database/redis/notification.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index f7a343f72..a62e72c08 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -196,13 +196,13 @@ func (connector *DbConnector) filterNotificationsByState(notifications []*moira. return nil, nil, fmt.Errorf("failed to get notifications trigger checks: %w", err) } - for i := range notifications { - switch notifications[i].GetState(triggerChecks[i]) { + for i, notification := range notifications { + switch notification.GetState(triggerChecks[i]) { case moira.ValidNotification: - validNotifications = append(validNotifications, notifications[i]) + validNotifications = append(validNotifications, notification) case moira.RemovedNotification: - toRemoveNotifications = append(toRemoveNotifications, notifications[i]) + toRemoveNotifications = append(toRemoveNotifications, notification) } } From 46678d8f47a2574735c570869d1d356a309c5c5b Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 31 Oct 2023 15:19:09 +0300 Subject: [PATCH 11/17] change pointer on limit to -1 and add removeNotifications tests --- database/redis/notification.go | 22 ++--- database/redis/notification_test.go | 132 ++++++++++++++++++++++------ 2 files changed, 115 insertions(+), 39 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index a62e72c08..06e52fcc2 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -250,7 +250,7 @@ func (connector *DbConnector) FetchNotifications(to int64, limit int64) ([]*moir // No limit if limit == notifier.NotificationsLimitUnlimited { - return connector.fetchNotifications(to, nil) + return connector.fetchNotifications(to, notifier.NotificationsLimitUnlimited) } count, err := connector.notificationsCount(to) @@ -260,10 +260,10 @@ func (connector *DbConnector) FetchNotifications(to int64, limit int64) ([]*moir // Hope count will be not greater then limit when we call fetchNotificationsNoLimit if limit > connector.notification.TransactionHeuristicLimit && count < limit/2 { - return connector.fetchNotifications(to, nil) + return connector.fetchNotifications(to, notifier.NotificationsLimitUnlimited) } - return connector.fetchNotifications(to, &limit) + return connector.fetchNotifications(to, limit) } func (connector *DbConnector) notificationsCount(to int64) (int64, error) { @@ -280,7 +280,7 @@ func (connector *DbConnector) notificationsCount(to int64) (int64, error) { } // fetchNotificationsWithLimit reads and drops notifications from DB with limit -func (connector *DbConnector) fetchNotifications(to int64, limit *int64) ([]*moira.ScheduledNotification, error) { +func (connector *DbConnector) fetchNotifications(to int64, limit int64) ([]*moira.ScheduledNotification, error) { // fetchNotificationsDo uses WATCH, so transaction may fail and will retry it // see https://redis.io/topics/transactions @@ -307,10 +307,10 @@ func (connector *DbConnector) fetchNotifications(to int64, limit *int64) ([]*moi // getNotificationsInTxWithLimit receives notifications from the database by a certain time // sorted by timestamp in one transaction with or without limit, depending on whether limit is nil -func getNotificationsInTxWithLimit(ctx context.Context, tx *redis.Tx, to int64, limit *int64) ([]*moira.ScheduledNotification, error) { +func getNotificationsInTxWithLimit(ctx context.Context, tx *redis.Tx, to int64, limit int64) ([]*moira.ScheduledNotification, error) { var rng *redis.ZRangeBy - if limit != nil { - rng = &redis.ZRangeBy{Min: "-inf", Max: strconv.FormatInt(to, 10), Offset: 0, Count: *limit} + if limit != notifier.NotificationsLimitUnlimited { + rng = &redis.ZRangeBy{Min: "-inf", Max: strconv.FormatInt(to, 10), Offset: 0, Count: limit} } else { rng = &redis.ZRangeBy{Min: "-inf", Max: strconv.FormatInt(to, 10)} } @@ -342,7 +342,7 @@ This is to ensure that notifications with the same timestamp are always clumped func getLimitedNotifications( ctx context.Context, tx *redis.Tx, - limit *int64, + limit int64, notifications []*moira.ScheduledNotification, ) ([]*moira.ScheduledNotification, error) { if len(notifications) == 0 { @@ -351,7 +351,7 @@ func getLimitedNotifications( limitedNotifications := notifications - if limit != nil { + if limit != notifier.NotificationsLimitUnlimited { limitedNotifications = limitNotifications(notifications) lastTs := limitedNotifications[len(limitedNotifications)-1].Timestamp @@ -359,7 +359,7 @@ func getLimitedNotifications( // this means that all notifications have same timestamp, // we hope that all notifications with same timestamp should fit our memory var err error - limitedNotifications, err = getNotificationsInTxWithLimit(ctx, tx, lastTs, nil) + limitedNotifications, err = getNotificationsInTxWithLimit(ctx, tx, lastTs, notifier.NotificationsLimitUnlimited) if err != nil { return nil, fmt.Errorf("failed to get notification without limit in transaction: %w", err) } @@ -370,7 +370,7 @@ func getLimitedNotifications( } // fetchNotificationsDo performs fetching of notifications within a single transaction -func (connector *DbConnector) fetchNotificationsDo(to int64, limit *int64) ([]*moira.ScheduledNotification, error) { +func (connector *DbConnector) fetchNotificationsDo(to int64, limit int64) ([]*moira.ScheduledNotification, error) { // See https://redis.io/topics/transactions ctx := connector.context diff --git a/database/redis/notification_test.go b/database/redis/notification_test.go index 105a54851..6fe3da03e 100644 --- a/database/redis/notification_test.go +++ b/database/redis/notification_test.go @@ -330,7 +330,7 @@ func TestGetNotificationsInTxWithLimit(t *testing.T) { Convey("Test with zero notifications without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{}) err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, nil) + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{}) return nil @@ -344,7 +344,7 @@ func TestGetNotificationsInTxWithLimit(t *testing.T) { Convey("Test all notifications without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, nil) + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) return nil @@ -359,7 +359,7 @@ func TestGetNotificationsInTxWithLimit(t *testing.T) { limit = 1 addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, &limit) + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, limit) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld}) return nil @@ -374,7 +374,7 @@ func TestGetNotificationsInTxWithLimit(t *testing.T) { limit = 3 addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, &limit) + actual, err := getNotificationsInTxWithLimit(ctx, tx, now+dataBase.GetDelayedTimeInSeconds()*2, limit) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) return nil @@ -418,7 +418,7 @@ func TestGetLimitedNotifications(t *testing.T) { Convey("Test all notifications with different timestamps without limit", func() { notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getLimitedNotifications(ctx, tx, nil, notifications) + actual, err := getLimitedNotifications(ctx, tx, notifier.NotificationsLimitUnlimited, notifications) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) return nil @@ -432,7 +432,7 @@ func TestGetLimitedNotifications(t *testing.T) { Convey("Test all notifications with different timestamps and limit", func() { notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getLimitedNotifications(ctx, tx, &limit, notifications) + actual, err := getLimitedNotifications(ctx, tx, limit, notifications) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification}) return nil @@ -456,7 +456,7 @@ func TestGetLimitedNotifications(t *testing.T) { notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} expected := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getLimitedNotifications(ctx, tx, &limit, notifications) + actual, err := getLimitedNotifications(ctx, tx, limit, notifications) So(err, ShouldBeNil) assert.ElementsMatch(t, actual, expected) return nil @@ -480,7 +480,7 @@ func TestGetLimitedNotifications(t *testing.T) { notifications := []*moira.ScheduledNotification{¬ificationOld, ¬ification} expected := []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew} err := client.Watch(ctx, func(tx *redis.Tx) error { - actual, err := getLimitedNotifications(ctx, tx, &limit, notifications) + actual, err := getLimitedNotifications(ctx, tx, limit, notifications) So(err, ShouldBeNil) assert.ElementsMatch(t, actual, expected) return nil @@ -569,8 +569,8 @@ func TestFilterNotificationsByState(t *testing.T) { }) Convey("With metric on maintenance", func() { - dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint - defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint + defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint validNotifications, removedNotifications, err := dataBase.filterNotificationsByState([]*moira.ScheduledNotification{notificationOld, notification, notificationNew}) So(err, ShouldBeNil) @@ -696,8 +696,8 @@ func TestHandleNotifications(t *testing.T) { }) Convey("With both delayed and not delayed valid notifications and metric on maintenance", func() { - dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint - defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint + defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test": 0}, nil, "test", 100) //nolint validNotifications, removedNotifications, err := dataBase.handleNotifications([]*moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) So(err, ShouldBeNil) @@ -870,7 +870,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("With limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) limit = 1 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), limit) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld}) @@ -880,7 +880,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), nil) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification, ¬ificationNew}) @@ -893,7 +893,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("With limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{}) limit = 10 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), limit) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{}) @@ -903,7 +903,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{}) - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), nil) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{}) @@ -914,7 +914,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Test all notification with ts and without limit in db", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld, notification4}) - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), nil) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification4, ¬ification, ¬ificationNew}) @@ -925,7 +925,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Test all notifications with ts and big limit in db", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) limit = 100 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) //nolint + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), limit) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification}) @@ -936,7 +936,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Test notifications with ts and small limit in db", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld, notification4}) limit = 3 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) //nolint + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), limit) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification4}) @@ -947,7 +947,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Test notifications with ts and limit = count", func() { addNotifications(dataBase, []moira.ScheduledNotification{notification, notificationNew, notificationOld}) limit = 3 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), &limit) //nolint + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds(), limit) //nolint So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ification}) @@ -968,7 +968,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("With big limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) limit = 100 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, &limit) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, limit) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew}) @@ -983,7 +983,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, nil) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew3}) @@ -998,13 +998,13 @@ func TestFetchNotificationsDo(t *testing.T) { }) Convey("Test notifications with ts and metric on maintenance", func() { - dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint - defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": 0}, nil, "test", 100) //nolint + dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": time.Now().Add(time.Hour).Unix()}, nil, "test", 100) //nolint + defer dataBase.SetTriggerCheckMaintenance("test2", map[string]int64{"test2": 0}, nil, "test", 100) //nolint Convey("With limit = count", func() { addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) limit = 6 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, &limit) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, limit) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew2}) @@ -1019,7 +1019,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("Without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, nil) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew2}) @@ -1044,7 +1044,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("With small limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) limit = 3 - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, &limit) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, limit) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2}) @@ -1059,7 +1059,7 @@ func TestFetchNotificationsDo(t *testing.T) { Convey("without limit", func() { addNotifications(dataBase, []moira.ScheduledNotification{notificationOld, notificationOld2, notification, notificationNew, notificationNew2, notificationNew3}) - actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, nil) + actual, err := dataBase.fetchNotificationsDo(now+dataBase.GetDelayedTimeInSeconds()+3, notifier.NotificationsLimitUnlimited) So(err, ShouldBeNil) So(actual, ShouldResemble, []*moira.ScheduledNotification{¬ificationOld, ¬ificationOld2, ¬ification, ¬ificationNew, ¬ificationNew3}) @@ -1270,3 +1270,79 @@ func TestGetNotificationsTriggerChecks(t *testing.T) { }) }) } + +func TestRemoveNotifications(t *testing.T) { + logger, _ := logging.GetLogger("dataBase") + dataBase := NewTestDatabase(logger) + dataBase.Flush() + defer dataBase.Flush() + + client := dataBase.client + ctx := dataBase.context + pipe := (*client).TxPipeline() + + notification1 := &moira.ScheduledNotification{ + Timestamp: 1, + } + notification2 := &moira.ScheduledNotification{ + Timestamp: 2, + } + notification3 := &moira.ScheduledNotification{ + Timestamp: 3, + } + + Convey("Test removeNotifications", t, func() { + Convey("Test remove empty notifications", func() { + count, err := dataBase.removeNotifications(ctx, pipe, []*moira.ScheduledNotification{}) + So(err, ShouldBeNil) + So(count, ShouldEqual, 0) + + allNotifications, countAllNotifications, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(countAllNotifications, ShouldEqual, 0) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + }) + + Convey("Test remove one notification", func() { + addNotifications(dataBase, []moira.ScheduledNotification{*notification1, *notification2, *notification3}) + + count, err := dataBase.removeNotifications(ctx, pipe, []*moira.ScheduledNotification{notification2}) + So(err, ShouldBeNil) + So(count, ShouldEqual, 1) + + allNotifications, countAllNotifications, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(countAllNotifications, ShouldEqual, 2) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{notification1, notification3}) + }) + + Convey("Test remove all notifications", func() { + addNotifications(dataBase, []moira.ScheduledNotification{*notification1, *notification2, *notification3}) + + count, err := dataBase.removeNotifications(ctx, pipe, []*moira.ScheduledNotification{notification1, notification2, notification3}) + So(err, ShouldBeNil) + So(count, ShouldEqual, 3) + + allNotifications, countAllNotifications, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(countAllNotifications, ShouldEqual, 0) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{}) + }) + + Convey("Test remove a nonexistent notification", func() { + notification4 := &moira.ScheduledNotification{ + Timestamp: 4, + } + addNotifications(dataBase, []moira.ScheduledNotification{*notification1, *notification2, *notification3}) + + count, err := dataBase.removeNotifications(ctx, pipe, []*moira.ScheduledNotification{notification4}) + So(err, ShouldBeNil) + So(count, ShouldEqual, 0) + + allNotifications, countAllNotifications, err := dataBase.GetNotifications(0, -1) + So(err, ShouldBeNil) + So(countAllNotifications, ShouldEqual, 3) + So(allNotifications, ShouldResemble, []*moira.ScheduledNotification{notification1, notification2, notification3}) + }) + }) +} From 1073d5da20daac330abc8f1e7b159b5d57ec4c8b Mon Sep 17 00:00:00 2001 From: almostinf Date: Thu, 2 Nov 2023 18:16:35 +0300 Subject: [PATCH 12/17] simple refactor --- database/redis/notification.go | 6 +++--- database/redis/notification_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index 06e52fcc2..263aec3b9 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -178,9 +178,9 @@ func (connector *DbConnector) getNotificationsTriggerChecks(notifications []*moi checkDataMap[triggerID] = triggersLastCheck[i] } - result := make([]*moira.CheckData, len(notifications)) - for i, notification := range notifications { - result[i] = checkDataMap[notification.Trigger.ID] + result := make([]*moira.CheckData, 0, len(notifications)) + for _, notification := range notifications { + result = append(result, checkDataMap[notification.Trigger.ID]) } return result, nil diff --git a/database/redis/notification_test.go b/database/redis/notification_test.go index 6fe3da03e..9cacc9f3f 100644 --- a/database/redis/notification_test.go +++ b/database/redis/notification_test.go @@ -397,7 +397,7 @@ func TestGetLimitedNotifications(t *testing.T) { ctx := dataBase.context Convey("Test getLimitedNotifications", t, func() { - var limit int64 = 0 + var limit int64 now = time.Now().Unix() notificationNew := moira.ScheduledNotification{ SendFail: 1, From 229857288f7460218a425cc9f70432aa15c4bd78 Mon Sep 17 00:00:00 2001 From: almostinf Date: Fri, 3 Nov 2023 19:17:55 +0300 Subject: [PATCH 13/17] add named return parameters --- database/redis/notification.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index 263aec3b9..56d0959e3 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -140,9 +140,9 @@ func (connector *DbConnector) GetDelayedTimeInSeconds() int64 { } // filterNotificationsByDelay filters notifications into delayed and not delayed notifications -func filterNotificationsByDelay(notifications []*moira.ScheduledNotification, delayedTime int64) ([]*moira.ScheduledNotification, []*moira.ScheduledNotification) { - delayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) - notDelayedNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) +func filterNotificationsByDelay(notifications []*moira.ScheduledNotification, delayedTime int64) (delayedNotifications []*moira.ScheduledNotification, notDelayedNotifications []*moira.ScheduledNotification) { + delayedNotifications = make([]*moira.ScheduledNotification, 0, len(notifications)) + notDelayedNotifications = make([]*moira.ScheduledNotification, 0, len(notifications)) for _, notification := range notifications { if notification.IsDelayed(delayedTime) { @@ -187,9 +187,9 @@ func (connector *DbConnector) getNotificationsTriggerChecks(notifications []*moi } // filterNotificationsByState filters notifications based on their state to the corresponding arrays -func (connector *DbConnector) filterNotificationsByState(notifications []*moira.ScheduledNotification) ([]*moira.ScheduledNotification, []*moira.ScheduledNotification, error) { - validNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) - toRemoveNotifications := make([]*moira.ScheduledNotification, 0, len(notifications)) +func (connector *DbConnector) filterNotificationsByState(notifications []*moira.ScheduledNotification) (validNotifications []*moira.ScheduledNotification, toRemoveNotifications []*moira.ScheduledNotification, err error) { + validNotifications = make([]*moira.ScheduledNotification, 0, len(notifications)) + toRemoveNotifications = make([]*moira.ScheduledNotification, 0, len(notifications)) triggerChecks, err := connector.getNotificationsTriggerChecks(notifications) if err != nil { From d0899527a1072a5054fe3b2279e8279793dd5c9c Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 7 Nov 2023 13:59:16 +0300 Subject: [PATCH 14/17] add logs on removed notifications --- database/redis/notification.go | 37 +++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index 56d0959e3..d0779f2fd 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -145,11 +145,15 @@ func filterNotificationsByDelay(notifications []*moira.ScheduledNotification, de notDelayedNotifications = make([]*moira.ScheduledNotification, 0, len(notifications)) for _, notification := range notifications { + if notification == nil { + continue + } + if notification.IsDelayed(delayedTime) { delayedNotifications = append(delayedNotifications, notification) - continue + } else { + notDelayedNotifications = append(notDelayedNotifications, notification) } - notDelayedNotifications = append(notDelayedNotifications, notification) } return delayedNotifications, notDelayedNotifications @@ -197,12 +201,14 @@ func (connector *DbConnector) filterNotificationsByState(notifications []*moira. } for i, notification := range notifications { - switch notification.GetState(triggerChecks[i]) { - case moira.ValidNotification: - validNotifications = append(validNotifications, notification) + if notification != nil { + switch notification.GetState(triggerChecks[i]) { + case moira.ValidNotification: + validNotifications = append(validNotifications, notification) - case moira.RemovedNotification: - toRemoveNotifications = append(toRemoveNotifications, notification) + case moira.RemovedNotification: + toRemoveNotifications = append(toRemoveNotifications, notification) + } } } @@ -369,6 +375,21 @@ func getLimitedNotifications( return limitedNotifications, nil } +// Helper function for logging information on removed notifications +func logRemovedNotifications(logger moira.Logger, removedNotifications []*moira.ScheduledNotification) { + removedNotificationsTriggerIDs := make([]string, 0, len(removedNotifications)) + for _, removedNotification := range removedNotifications { + if removedNotification != nil { + removedNotificationsTriggerIDs = append(removedNotificationsTriggerIDs, removedNotification.Trigger.ID) + } + } + + logger.Info(). + Interface("removed_notifications_trigger_ids", removedNotificationsTriggerIDs). + Int("removed_count", len(removedNotifications)). + Msg("Remove notifications in transaction") +} + // fetchNotificationsDo performs fetching of notifications within a single transaction func (connector *DbConnector) fetchNotificationsDo(to int64, limit int64) ([]*moira.ScheduledNotification, error) { // See https://redis.io/topics/transactions @@ -414,6 +435,8 @@ func (connector *DbConnector) fetchNotificationsDo(to int64, limit int64) ([]*mo return fmt.Errorf("failed to remove notifications in transaction: %w", err) } + logRemovedNotifications(connector.logger, toRemoveNotifications) + return nil }) From 3c92154563c2e549fbdd1b093fc6a582ef05222e Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 7 Nov 2023 16:48:25 +0300 Subject: [PATCH 15/17] fix removed notification logs --- database/redis/notification.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index d0779f2fd..d52ba2816 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -377,15 +377,24 @@ func getLimitedNotifications( // Helper function for logging information on removed notifications func logRemovedNotifications(logger moira.Logger, removedNotifications []*moira.ScheduledNotification) { - removedNotificationsTriggerIDs := make([]string, 0, len(removedNotifications)) + triggerIDsSet := make(map[string]struct{}, len(removedNotifications)) for _, removedNotification := range removedNotifications { - if removedNotification != nil { - removedNotificationsTriggerIDs = append(removedNotificationsTriggerIDs, removedNotification.Trigger.ID) + if removedNotification == nil { + continue + } + + if _, ok := triggerIDsSet[removedNotification.Trigger.ID]; !ok { + triggerIDsSet[removedNotification.Trigger.ID] = struct{}{} } } + triggerIDs := make([]string, 0, len(triggerIDsSet)) + for triggerID := range triggerIDsSet { + triggerIDs = append(triggerIDs, triggerID) + } + logger.Info(). - Interface("removed_notifications_trigger_ids", removedNotificationsTriggerIDs). + Interface("notification_trigger_ids", triggerIDs). Int("removed_count", len(removedNotifications)). Msg("Remove notifications in transaction") } From eebaa6b808c34a46f56b0f7bb4f070d816985b05 Mon Sep 17 00:00:00 2001 From: almostinf Date: Tue, 7 Nov 2023 16:58:03 +0300 Subject: [PATCH 16/17] fix removed notification logs --- database/redis/notification.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/database/redis/notification.go b/database/redis/notification.go index d52ba2816..12bceb32f 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -388,6 +388,10 @@ func logRemovedNotifications(logger moira.Logger, removedNotifications []*moira. } } + if len(triggerIDsSet) == 0 { + return + } + triggerIDs := make([]string, 0, len(triggerIDsSet)) for triggerID := range triggerIDsSet { triggerIDs = append(triggerIDs, triggerID) From 7acacd89d9f2a2c553bf5d4cb268fca19facc883 Mon Sep 17 00:00:00 2001 From: almostinf Date: Mon, 13 Nov 2023 09:13:25 +0300 Subject: [PATCH 17/17] update log --- database/redis/notification.go | 60 +++++++++++++++++----------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/database/redis/notification.go b/database/redis/notification.go index 12bceb32f..d46a433c5 100644 --- a/database/redis/notification.go +++ b/database/redis/notification.go @@ -215,6 +215,34 @@ func (connector *DbConnector) filterNotificationsByState(notifications []*moira. return validNotifications, toRemoveNotifications, nil } +// Helper function for logging information on to remove notifications +func logToRemoveNotifications(logger moira.Logger, toRemoveNotifications []*moira.ScheduledNotification) { + if len(toRemoveNotifications) == 0 { + return + } + + triggerIDsSet := make(map[string]struct{}, len(toRemoveNotifications)) + for _, removedNotification := range toRemoveNotifications { + if removedNotification == nil { + continue + } + + if _, ok := triggerIDsSet[removedNotification.Trigger.ID]; !ok { + triggerIDsSet[removedNotification.Trigger.ID] = struct{}{} + } + } + + triggerIDs := make([]string, 0, len(triggerIDsSet)) + for triggerID := range triggerIDsSet { + triggerIDs = append(triggerIDs, triggerID) + } + + logger.Info(). + Interface("notification_trigger_ids", triggerIDs). + Int("to_remove_count", len(toRemoveNotifications)). + Msg("To remove notifications") +} + /* handleNotifications filters notifications into delayed and not delayed, then filters delayed notifications by notification state, then merges the 2 arrays @@ -238,6 +266,8 @@ func (connector *DbConnector) handleNotifications(notifications []*moira.Schedul return nil, nil, fmt.Errorf("failed to filter delayed notifications by state: %w", err) } + logToRemoveNotifications(connector.logger, toRemoveNotifications) + validNotifications, err = moira.MergeToSorted[*moira.ScheduledNotification](validNotifications, notDelayedNotifications) if err != nil { return nil, nil, fmt.Errorf("failed to merge valid and not delayed notifications into sorted array: %w", err) @@ -375,34 +405,6 @@ func getLimitedNotifications( return limitedNotifications, nil } -// Helper function for logging information on removed notifications -func logRemovedNotifications(logger moira.Logger, removedNotifications []*moira.ScheduledNotification) { - triggerIDsSet := make(map[string]struct{}, len(removedNotifications)) - for _, removedNotification := range removedNotifications { - if removedNotification == nil { - continue - } - - if _, ok := triggerIDsSet[removedNotification.Trigger.ID]; !ok { - triggerIDsSet[removedNotification.Trigger.ID] = struct{}{} - } - } - - if len(triggerIDsSet) == 0 { - return - } - - triggerIDs := make([]string, 0, len(triggerIDsSet)) - for triggerID := range triggerIDsSet { - triggerIDs = append(triggerIDs, triggerID) - } - - logger.Info(). - Interface("notification_trigger_ids", triggerIDs). - Int("removed_count", len(removedNotifications)). - Msg("Remove notifications in transaction") -} - // fetchNotificationsDo performs fetching of notifications within a single transaction func (connector *DbConnector) fetchNotificationsDo(to int64, limit int64) ([]*moira.ScheduledNotification, error) { // See https://redis.io/topics/transactions @@ -448,8 +450,6 @@ func (connector *DbConnector) fetchNotificationsDo(to int64, limit int64) ([]*mo return fmt.Errorf("failed to remove notifications in transaction: %w", err) } - logRemovedNotifications(connector.logger, toRemoveNotifications) - return nil })