From 014d6d130f8779004fa64c0efdbc26c5b855b774 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Thu, 9 Jan 2025 22:25:29 -0500 Subject: [PATCH] Remove sequentialRules, not needed A nil slice will run rules sequentially --- pkg/ruler/rule_concurrency.go | 19 +++++-------------- pkg/ruler/rule_concurrency_test.go | 7 ++++++- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/pkg/ruler/rule_concurrency.go b/pkg/ruler/rule_concurrency.go index 31a38b1e492..6b3babfb1fb 100644 --- a/pkg/ruler/rule_concurrency.go +++ b/pkg/ruler/rule_concurrency.go @@ -181,7 +181,7 @@ func (c *TenantConcurrencyController) Allow(_ context.Context, _ *rules.Group, _ func (c *TenantConcurrencyController) SplitGroupIntoBatches(_ context.Context, g *rules.Group) []rules.ConcurrentRules { if !c.isGroupAtRisk(g) { // If the group is not at risk, we can evaluate the rules sequentially. - return sequentialRules(g) + return nil } logger := log.With(c.logger, "group", g.Name()) @@ -209,7 +209,7 @@ func (c *TenantConcurrencyController) SplitGroupIntoBatches(_ context.Context, g // There are no rules without dependencies. // Fall back to sequential evaluation. level.Info(logger).Log("msg", "No rules without dependencies found, falling back to sequential rule evaluation. This may be due to indeterminate rule dependencies.") - return sequentialRules(g) + return nil } order := []rules.ConcurrentRules{firstBatch} @@ -239,7 +239,7 @@ func (c *TenantConcurrencyController) SplitGroupIntoBatches(_ context.Context, g // We can't evaluate them concurrently. // Fall back to sequential evaluation. level.Warn(logger).Log("msg", "Cyclic rule dependencies detected, falling back to sequential rule evaluation") - return sequentialRules(g) + return nil } order = append(order, batch) @@ -279,19 +279,10 @@ func (n *NoopMultiTenantConcurrencyController) NewTenantConcurrencyControllerFor type NoopTenantConcurrencyController struct{} func (n *NoopTenantConcurrencyController) Done(_ context.Context) {} -func (n *NoopTenantConcurrencyController) SplitGroupIntoBatches(_ context.Context, g *rules.Group) []rules.ConcurrentRules { - return sequentialRules(g) +func (n *NoopTenantConcurrencyController) SplitGroupIntoBatches(_ context.Context, _ *rules.Group) []rules.ConcurrentRules { + return nil } func (n *NoopTenantConcurrencyController) Allow(_ context.Context, _ *rules.Group, _ rules.Rule) bool { return false } - -func sequentialRules(g *rules.Group) []rules.ConcurrentRules { - // Split the group into batches of 1 rule each. - order := make([]rules.ConcurrentRules, len(g.Rules())) - for i := range g.Rules() { - order[i] = []int{i} - } - return order -} diff --git a/pkg/ruler/rule_concurrency_test.go b/pkg/ruler/rule_concurrency_test.go index 8ebd1794047..9d18982ef4d 100644 --- a/pkg/ruler/rule_concurrency_test.go +++ b/pkg/ruler/rule_concurrency_test.go @@ -211,7 +211,7 @@ func TestSplitGroupIntoBatches(t *testing.T) { }, "indeterminates": { inputFile: "fixtures/rules_indeterminates.yaml", - expectedGroups: []rules.ConcurrentRules{{0}, {1}, {2}, {3}, {4}, {5}, {6}}, // Sequential + expectedGroups: nil, }, "all independent": { inputFile: "fixtures/rules_multiple_independent.yaml", @@ -251,6 +251,11 @@ func TestSplitGroupIntoBatches(t *testing.T) { func requireConcurrentRulesEqual(t *testing.T, expected, actual []rules.ConcurrentRules) { t.Helper() + if expected == nil { + require.Nil(t, actual) + return + } + // Like require.Equals but ignores the order of elements in the slices. require.Len(t, actual, len(expected)) for i, expectedBatch := range expected {