From 779d5b9b4e7801e4ee01141fa7909765e75760a7 Mon Sep 17 00:00:00 2001 From: Dmitry Redkin Date: Thu, 28 Jul 2022 11:25:14 +0500 Subject: [PATCH 1/3] fix(test): Fixed test TestCleanupOutdatedMetrics. There was double adding data to DB, because nested Convey run twice (example in docs https://github.com/smartystreets/goconvey/blob/master/convey/isolated_execution_test.go#L51). Fixed it by moving code block outside. --- database/redis/metric_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/database/redis/metric_test.go b/database/redis/metric_test.go index 84e60e4ea..a41a988e5 100644 --- a/database/redis/metric_test.go +++ b/database/redis/metric_test.go @@ -504,6 +504,15 @@ func TestCleanupOutdatedMetrics(t *testing.T) { dataBase.Flush() defer dataBase.Flush() + Convey("When clean up metrics with wrong value of duration was called (positive number)", t, func() { + err := dataBase.CleanUpOutdatedMetrics(time.Hour) + So( + err, + ShouldResemble, + errors.New("clean up duration value must be less than zero, otherwise all metrics will be removed"), + ) + }) + Convey("Given 2 metrics with 2 values older then 1 minute and 2 values younger then 1 minute", t, func() { const ( metric1 = "my.test.super.metric" @@ -617,15 +626,6 @@ func TestCleanupOutdatedMetrics(t *testing.T) { }, }) - Convey("When clean up metrics with wrong value of duration was called (positive number)", func() { - err = dataBase.CleanUpOutdatedMetrics(time.Hour) - So( - err, - ShouldResemble, - errors.New("clean up duration value must be less than zero, otherwise all metrics will be removed"), - ) - }) - Convey("When clean up metrics older then 1 minute was called", func() { err = dataBase.CleanUpOutdatedMetrics(-time.Minute) So(err, ShouldBeNil) From 7f1b48d28629a897546846bdd98b65204f43d7f1 Mon Sep 17 00:00:00 2001 From: Dmitry Redkin Date: Thu, 28 Jul 2022 15:02:08 +0500 Subject: [PATCH 2/3] fix(test): Fixed cluster-specific test TestRemoveAllMetrics. In case of Redis cluster, keys are distributed by nodes, so need to count keys from all masters. Added keys counting for all master nodes of cluster. --- database/redis/metric_test.go | 94 ++++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 12 deletions(-) diff --git a/database/redis/metric_test.go b/database/redis/metric_test.go index a41a988e5..cb88fcdfb 100644 --- a/database/redis/metric_test.go +++ b/database/redis/metric_test.go @@ -1,11 +1,14 @@ package redis import ( + "context" "errors" "fmt" + "sync/atomic" "testing" "time" + "github.com/go-redis/redis/v8" "github.com/moira-alert/moira" logging "github.com/moira-alert/moira/logging/zerolog_adapter" "github.com/patrickmn/go-cache" @@ -933,7 +936,7 @@ func TestRemoveAllMetrics(t *testing.T) { client := *dataBase.client const pattern = "my.test.*.metric*" - Convey("Given metrics with pattern my.test.*", t, func() { + Convey("Given metrics with pattern my.test.*", t, func(c C) { for i := 1; i <= 10; i++ { err := dataBase.SaveMetrics( map[string]*moira.MatchedMetric{ @@ -962,25 +965,92 @@ func TestRemoveAllMetrics(t *testing.T) { So(err, ShouldBeNil) } - result := client.Keys(dataBase.context, "moira-metric-data:my.test*").Val() - So(len(result), ShouldResemble, 20) - result = client.Keys(dataBase.context, "moira-metric-retention:my.test*").Val() - So(len(result), ShouldResemble, 20) + var metricsCount int32 + incrementMetric := func(ctx context.Context, shard redis.UniversalClient) { + result := len(shard.Keys(ctx, "moira-metric-data:my.test*").Val()) + atomic.AddInt32(&metricsCount, int32(result)) + } + switch cl := client.(type) { + case *redis.ClusterClient: + err := cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + incrementMetric(ctx, shard) + return nil + }) + So(err, ShouldBeNil) + default: + incrementMetric(dataBase.context, cl) + } + So(atomic.LoadInt32(&metricsCount), ShouldEqual, 20) + + var retentionsCount int32 + incrementRetention := func(ctx context.Context, shard redis.UniversalClient) { + result := len(shard.Keys(ctx, "moira-metric-retention:my.test*").Val()) + atomic.AddInt32(&retentionsCount, int32(result)) + } + switch cl := client.(type) { + case *redis.ClusterClient: + err := cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + incrementRetention(ctx, shard) + return nil + }) + So(err, ShouldBeNil) + default: + incrementRetention(dataBase.context, cl) + } + So(atomic.LoadInt32(&retentionsCount), ShouldEqual, 20) patternMetricsCount := client.SCard(dataBase.context, "moira-pattern-metrics:my.test.*.metric*").Val() - So(patternMetricsCount, ShouldResemble, int64(20)) + So(patternMetricsCount, ShouldEqual, int64(20)) Convey("When remove all metrics was called", func() { err := dataBase.RemoveAllMetrics() So(err, ShouldBeNil) Convey("No metric data should not exist", func() { - result = client.Keys(dataBase.context, "moira-metric-data:*").Val() - So(len(result), ShouldResemble, 0) - result = client.Keys(dataBase.context, "moira-metric-retention:*").Val() - So(len(result), ShouldResemble, 0) - result = client.Keys(dataBase.context, "moira-pattern-metrics*").Val() - So(len(result), ShouldResemble, 0) + var metricsCount int + switch cl := client.(type) { + case *redis.ClusterClient: + err = cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + result := shard.Keys(ctx, "moira-metric-data:*").Val() + metricsCount += len(result) + return nil + }) + So(err, ShouldBeNil) + default: + result := client.Keys(dataBase.context, "moira-metric-data:*").Val() + metricsCount += len(result) + } + So(metricsCount, ShouldEqual, 0) + + var retentionsCount int + switch cl := client.(type) { + case *redis.ClusterClient: + err = cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + result := shard.Keys(ctx, "moira-metric-retention:*").Val() + retentionsCount += len(result) + return nil + }) + So(err, ShouldBeNil) + default: + result := client.Keys(dataBase.context, "moira-metric-retention:*").Val() + retentionsCount += len(result) + } + So(retentionsCount, ShouldEqual, 0) + + var patternsCount int + switch cl := client.(type) { + case *redis.ClusterClient: + err = cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + result := shard.Keys(ctx, "moira-pattern-metrics*").Val() + patternsCount += len(result) + return nil + }) + So(err, ShouldBeNil) + default: + result := client.Keys(dataBase.context, "moira-pattern-metrics*").Val() + patternsCount += len(result) + } + So(patternsCount, ShouldEqual, 0) }) }) }) From 18f1c59367d65855a7f8a1c64511c6ab2c4abf8a Mon Sep 17 00:00:00 2001 From: Dmitry Redkin Date: Thu, 28 Jul 2022 15:55:10 +0500 Subject: [PATCH 3/3] fix(test): Fixed cluster-specific test TestRemoveMetricsByPrefix. In case of Redis cluster, keys are distributed by nodes, so need to count keys from all masters. Added keys counting for all master nodes of cluster. --- database/redis/metric_test.go | 113 +++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 16 deletions(-) diff --git a/database/redis/metric_test.go b/database/redis/metric_test.go index cb88fcdfb..8780290a2 100644 --- a/database/redis/metric_test.go +++ b/database/redis/metric_test.go @@ -871,7 +871,7 @@ func TestRemoveMetricsByPrefix(t *testing.T) { client := *dataBase.client const pattern = "my.test.*.metric*" - Convey("Given metrics with pattern my.test.*", t, func() { + Convey("Given metrics with pattern my.test.*", t, func(c C) { for i := 1; i <= 10; i++ { err := dataBase.SaveMetrics( map[string]*moira.MatchedMetric{ @@ -900,29 +900,110 @@ func TestRemoveMetricsByPrefix(t *testing.T) { So(err, ShouldBeNil) } - result := client.Keys(dataBase.context, "moira-metric-data:my.test*").Val() - So(len(result), ShouldResemble, 20) - result = client.Keys(dataBase.context, "moira-metric-retention:my.test*").Val() - So(len(result), ShouldResemble, 20) + var metricsCount int32 + incrementMetric := func(ctx context.Context, shard redis.UniversalClient) { + result := len(shard.Keys(ctx, "moira-metric-data:my.test*").Val()) + atomic.AddInt32(&metricsCount, int32(result)) + } + switch cl := client.(type) { + case *redis.ClusterClient: + err := cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + incrementMetric(ctx, shard) + return nil + }) + So(err, ShouldBeNil) + default: + incrementMetric(dataBase.context, cl) + } + So(atomic.LoadInt32(&metricsCount), ShouldEqual, 20) + + var retentionsCount int32 + incrementRetention := func(ctx context.Context, shard redis.UniversalClient) { + result := len(shard.Keys(ctx, "moira-metric-retention:my.test*").Val()) + atomic.AddInt32(&retentionsCount, int32(result)) + } + switch cl := client.(type) { + case *redis.ClusterClient: + err := cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + incrementRetention(ctx, shard) + return nil + }) + So(err, ShouldBeNil) + default: + incrementRetention(dataBase.context, cl) + } + So(atomic.LoadInt32(&retentionsCount), ShouldEqual, 20) patternMetricsCount := client.SCard(dataBase.context, "moira-pattern-metrics:my.test.*.metric*").Val() - So(patternMetricsCount, ShouldResemble, int64(20)) + So(patternMetricsCount, ShouldEqual, int64(20)) Convey("When remove metrics by prefix my.test.super. was called", func() { err := dataBase.RemoveMetricsByPrefix("my.test.super.") So(err, ShouldBeNil) - Convey("No metric data for metrics with this prefix should not exist", func() { - result = client.Keys(dataBase.context, "moira-metric-data:my.test*").Val() - So(len(result), ShouldResemble, 10) - result = client.Keys(dataBase.context, "moira-metric-retention:my.test*").Val() - So(len(result), ShouldResemble, 10) - result = client.Keys(dataBase.context, "moira-metric-data:my.test.mega.*").Val() - So(len(result), ShouldResemble, 10) - result = client.Keys(dataBase.context, "moira-metric-retention:my.test.mega.*").Val() - So(len(result), ShouldResemble, 10) + Convey("No metric data for metrics with this prefix should exist", func() { + var allMetricsCount int + switch cl := client.(type) { + case *redis.ClusterClient: + err = cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + result := shard.Keys(ctx, "moira-metric-data:my.test*").Val() + allMetricsCount += len(result) + return nil + }) + So(err, ShouldBeNil) + default: + result := client.Keys(dataBase.context, "moira-metric-data:my.test*").Val() + allMetricsCount += len(result) + } + So(allMetricsCount, ShouldEqual, 10) + + var allRetentionsCount int + switch cl := client.(type) { + case *redis.ClusterClient: + err = cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + result := shard.Keys(ctx, "moira-metric-retention:my.test*").Val() + allRetentionsCount += len(result) + return nil + }) + So(err, ShouldBeNil) + default: + result := client.Keys(dataBase.context, "moira-metric-retention:my.test*").Val() + allRetentionsCount += len(result) + } + So(allRetentionsCount, ShouldEqual, 10) + + var megaMetricsCount int + switch cl := client.(type) { + case *redis.ClusterClient: + err = cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + result := shard.Keys(ctx, "moira-metric-data:my.test.mega.*").Val() + megaMetricsCount += len(result) + return nil + }) + So(err, ShouldBeNil) + default: + result := client.Keys(dataBase.context, "moira-metric-data:my.test.mega.*").Val() + megaMetricsCount += len(result) + } + So(megaMetricsCount, ShouldEqual, 10) + + var megaRetentionsCount int + switch cl := client.(type) { + case *redis.ClusterClient: + err = cl.ForEachMaster(dataBase.context, func(ctx context.Context, shard *redis.Client) error { + result := shard.Keys(ctx, "moira-metric-retention:my.test.mega.*").Val() + megaRetentionsCount += len(result) + return nil + }) + So(err, ShouldBeNil) + default: + result := client.Keys(dataBase.context, "moira-metric-retention:my.test.mega.*").Val() + megaRetentionsCount += len(result) + } + So(megaRetentionsCount, ShouldEqual, 10) + patternMetricsCount := client.SCard(dataBase.context, "moira-pattern-metrics:my.test.*.metric*").Val() - So(patternMetricsCount, ShouldResemble, int64(10)) + So(patternMetricsCount, ShouldEqual, int64(10)) }) }) })