Skip to content

Commit

Permalink
Remove sequentialRules, not needed
Browse files Browse the repository at this point in the history
A nil slice will run rules sequentially
  • Loading branch information
julienduchesne committed Jan 10, 2025
1 parent f13e131 commit dabab47
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 14 deletions.
17 changes: 4 additions & 13 deletions pkg/ruler/rule_concurrency.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -280,18 +280,9 @@ type NoopTenantConcurrencyController struct{}

func (n *NoopTenantConcurrencyController) Done(_ context.Context) {}
func (n *NoopTenantConcurrencyController) SplitGroupIntoBatches(_ context.Context, g *rules.Group) []rules.ConcurrentRules {
return sequentialRules(g)
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
}
7 changes: 6 additions & 1 deletion pkg/ruler/rule_concurrency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit dabab47

Please sign in to comment.