Rules: Store dependencies instead of boolean

To improve https://github.com/prometheus/prometheus/pull/15681 further, we'll need to store the dependencies and dependents of each

Right now, if a rule has both (at least 1) dependents and dependencies, it is not possible to determine the order to run the rules and they must all run sequentially

This PR only changes the dependents and dependencies attributes of rules, it does not implement a new topological sort algorithm

Signed-off-by: Julien Duchesne <julien.duchesne@grafana.com>
This commit is contained in:
Julien Duchesne 2024-12-17 23:09:17 -05:00
parent 615195372d
commit 6607a2ba7c
No known key found for this signature in database
GPG key ID: C830FBA4FA638D74
9 changed files with 155 additions and 66 deletions

View file

@ -143,8 +143,9 @@ type AlertingRule struct {
logger *slog.Logger logger *slog.Logger
noDependentRules *atomic.Bool dependenciesMutex sync.RWMutex
noDependencyRules *atomic.Bool dependentRules map[string]struct{}
dependencyRules map[string]struct{}
} }
// NewAlertingRule constructs a new AlertingRule. // NewAlertingRule constructs a new AlertingRule.
@ -171,8 +172,6 @@ func NewAlertingRule(
evaluationTimestamp: atomic.NewTime(time.Time{}), evaluationTimestamp: atomic.NewTime(time.Time{}),
evaluationDuration: atomic.NewDuration(0), evaluationDuration: atomic.NewDuration(0),
lastError: atomic.NewError(nil), lastError: atomic.NewError(nil),
noDependentRules: atomic.NewBool(false),
noDependencyRules: atomic.NewBool(false),
} }
} }
@ -316,20 +315,58 @@ func (r *AlertingRule) Restored() bool {
return r.restored.Load() return r.restored.Load()
} }
func (r *AlertingRule) SetNoDependentRules(noDependentRules bool) { func (r *AlertingRule) SetDependentRules(dependents []Rule) {
r.noDependentRules.Store(noDependentRules) r.dependenciesMutex.Lock()
defer r.dependenciesMutex.Unlock()
r.dependentRules = make(map[string]struct{})
for _, dependent := range dependents {
r.dependentRules[dependent.Name()] = struct{}{}
}
} }
func (r *AlertingRule) NoDependentRules() bool { func (r *AlertingRule) NoDependentRules() bool {
return r.noDependentRules.Load() r.dependenciesMutex.RLock()
defer r.dependenciesMutex.RUnlock()
if r.dependentRules == nil {
return false // We don't know if there are dependent rules.
}
return len(r.dependentRules) == 0
} }
func (r *AlertingRule) SetNoDependencyRules(noDependencyRules bool) { func (r *AlertingRule) DependentRules() map[string]struct{} {
r.noDependencyRules.Store(noDependencyRules) r.dependenciesMutex.RLock()
defer r.dependenciesMutex.RUnlock()
return r.dependentRules
}
func (r *AlertingRule) SetDependencyRules(dependencies []Rule) {
r.dependenciesMutex.Lock()
defer r.dependenciesMutex.Unlock()
r.dependencyRules = make(map[string]struct{})
for _, dependency := range dependencies {
r.dependencyRules[dependency.Name()] = struct{}{}
}
} }
func (r *AlertingRule) NoDependencyRules() bool { func (r *AlertingRule) NoDependencyRules() bool {
return r.noDependencyRules.Load() r.dependenciesMutex.RLock()
defer r.dependenciesMutex.RUnlock()
if r.dependencyRules == nil {
return false // We don't know if there are dependency rules.
}
return len(r.dependencyRules) == 0
}
func (r *AlertingRule) DependencyRules() map[string]struct{} {
r.dependenciesMutex.RLock()
defer r.dependenciesMutex.RUnlock()
return r.dependencyRules
} }
// resolvedRetention is the duration for which a resolved alert instance // resolvedRetention is the duration for which a resolved alert instance

View file

@ -998,7 +998,7 @@ func TestAlertingEvalWithOrigin(t *testing.T) {
require.Equal(t, detail, NewRuleDetail(rule)) require.Equal(t, detail, NewRuleDetail(rule))
} }
func TestAlertingRule_SetNoDependentRules(t *testing.T) { func TestAlertingRule_SetDependentRules(t *testing.T) {
rule := NewAlertingRule( rule := NewAlertingRule(
"test", "test",
&parser.NumberLiteral{Val: 1}, &parser.NumberLiteral{Val: 1},
@ -1012,14 +1012,16 @@ func TestAlertingRule_SetNoDependentRules(t *testing.T) {
) )
require.False(t, rule.NoDependentRules()) require.False(t, rule.NoDependentRules())
rule.SetNoDependentRules(false) rule.SetDependentRules([]Rule{NewRecordingRule("test1", nil, labels.EmptyLabels())})
require.False(t, rule.NoDependentRules()) require.False(t, rule.NoDependentRules())
require.Equal(t, map[string]struct{}{"test1": {}}, rule.DependentRules())
rule.SetNoDependentRules(true) rule.SetDependentRules([]Rule{})
require.True(t, rule.NoDependentRules()) require.True(t, rule.NoDependentRules())
require.Empty(t, rule.DependentRules())
} }
func TestAlertingRule_SetNoDependencyRules(t *testing.T) { func TestAlertingRule_SetDependencyRules(t *testing.T) {
rule := NewAlertingRule( rule := NewAlertingRule(
"test", "test",
&parser.NumberLiteral{Val: 1}, &parser.NumberLiteral{Val: 1},
@ -1033,11 +1035,13 @@ func TestAlertingRule_SetNoDependencyRules(t *testing.T) {
) )
require.False(t, rule.NoDependencyRules()) require.False(t, rule.NoDependencyRules())
rule.SetNoDependencyRules(false) rule.SetDependencyRules([]Rule{NewRecordingRule("test1", nil, labels.EmptyLabels())})
require.False(t, rule.NoDependencyRules()) require.False(t, rule.NoDependencyRules())
require.Equal(t, map[string]struct{}{"test1": {}}, rule.DependencyRules())
rule.SetNoDependencyRules(true) rule.SetDependencyRules([]Rule{})
require.True(t, rule.NoDependencyRules()) require.True(t, rule.NoDependencyRules())
require.Empty(t, rule.DependencyRules())
} }
func TestAlertingRule_ActiveAlertsCount(t *testing.T) { func TestAlertingRule_ActiveAlertsCount(t *testing.T) {

View file

@ -1034,27 +1034,25 @@ func NewGroupMetrics(reg prometheus.Registerer) *Metrics {
// output metric produced by another rule in its expression (i.e. as its "input"). // output metric produced by another rule in its expression (i.e. as its "input").
type dependencyMap map[Rule][]Rule type dependencyMap map[Rule][]Rule
// dependents returns the count of rules which use the output of the given rule as one of their inputs. // dependents returns the rules which use the output of the given rule as one of their inputs.
func (m dependencyMap) dependents(r Rule) int { func (m dependencyMap) dependents(r Rule) []Rule {
return len(m[r]) return m[r]
} }
// dependencies returns the count of rules on which the given rule is dependent for input. // dependencies returns the rules on which the given rule is dependent for input.
func (m dependencyMap) dependencies(r Rule) int { func (m dependencyMap) dependencies(r Rule) []Rule {
if len(m) == 0 { if len(m) == 0 {
return 0 return []Rule{}
} }
var count int var dependencies []Rule
for _, children := range m { for rule, dependents := range m {
for _, child := range children { if slices.Contains(dependents, r) {
if child == r { dependencies = append(dependencies, rule)
count++
}
} }
} }
return count return dependencies
} }
// isIndependent determines whether the given rule is not dependent on another rule for its input, nor is any other rule // isIndependent determines whether the given rule is not dependent on another rule for its input, nor is any other rule
@ -1064,7 +1062,7 @@ func (m dependencyMap) isIndependent(r Rule) bool {
return false return false
} }
return m.dependents(r)+m.dependencies(r) == 0 return len(m.dependents(r)) == 0 && len(m.dependencies(r)) == 0
} }
// buildDependencyMap builds a data-structure which contains the relationships between rules within a group. // buildDependencyMap builds a data-structure which contains the relationships between rules within a group.

View file

@ -444,8 +444,8 @@ func SendAlerts(s Sender, externalURL string) NotifyFunc {
// RuleDependencyController controls whether a set of rules have dependencies between each other. // RuleDependencyController controls whether a set of rules have dependencies between each other.
type RuleDependencyController interface { type RuleDependencyController interface {
// AnalyseRules analyses dependencies between the input rules. For each rule that it's guaranteed // AnalyseRules analyses dependencies between the input rules. For each rule that it's guaranteed
// not having any dependants and/or dependency, this function should call Rule.SetNoDependentRules(true) // not having any dependants and/or dependency, this function should call Rule.SetDependentRules(...)
// and/or Rule.SetNoDependencyRules(true). // and/or Rule.SetDependencyRules(...).
AnalyseRules(rules []Rule) AnalyseRules(rules []Rule)
} }
@ -460,8 +460,8 @@ func (c ruleDependencyController) AnalyseRules(rules []Rule) {
} }
for _, r := range rules { for _, r := range rules {
r.SetNoDependentRules(depMap.dependents(r) == 0) r.SetDependentRules(depMap.dependents(r))
r.SetNoDependencyRules(depMap.dependencies(r) == 0) r.SetDependencyRules(depMap.dependencies(r))
} }
} }

View file

@ -1423,8 +1423,6 @@ func TestRuleGroupEvalIterationFunc(t *testing.T) {
evaluationTimestamp: atomic.NewTime(time.Time{}), evaluationTimestamp: atomic.NewTime(time.Time{}),
evaluationDuration: atomic.NewDuration(0), evaluationDuration: atomic.NewDuration(0),
lastError: atomic.NewError(nil), lastError: atomic.NewError(nil),
noDependentRules: atomic.NewBool(false),
noDependencyRules: atomic.NewBool(false),
} }
group := NewGroup(GroupOptions{ group := NewGroup(GroupOptions{
@ -1613,11 +1611,12 @@ func TestDependencyMap(t *testing.T) {
depMap := buildDependencyMap(group.rules) depMap := buildDependencyMap(group.rules)
require.Zero(t, depMap.dependencies(rule)) require.Zero(t, depMap.dependencies(rule))
require.Equal(t, 2, depMap.dependents(rule)) require.Equal(t, []Rule{rule2, rule4}, depMap.dependents(rule))
require.Len(t, depMap.dependents(rule), 2)
require.False(t, depMap.isIndependent(rule)) require.False(t, depMap.isIndependent(rule))
require.Zero(t, depMap.dependents(rule2)) require.Zero(t, depMap.dependents(rule2))
require.Equal(t, 1, depMap.dependencies(rule2)) require.Equal(t, []Rule{rule}, depMap.dependencies(rule2))
require.False(t, depMap.isIndependent(rule2)) require.False(t, depMap.isIndependent(rule2))
require.Zero(t, depMap.dependents(rule3)) require.Zero(t, depMap.dependents(rule3))
@ -1625,7 +1624,7 @@ func TestDependencyMap(t *testing.T) {
require.True(t, depMap.isIndependent(rule3)) require.True(t, depMap.isIndependent(rule3))
require.Zero(t, depMap.dependents(rule4)) require.Zero(t, depMap.dependents(rule4))
require.Equal(t, 1, depMap.dependencies(rule4)) require.Equal(t, []Rule{rule}, depMap.dependencies(rule4))
require.False(t, depMap.isIndependent(rule4)) require.False(t, depMap.isIndependent(rule4))
} }
@ -1958,7 +1957,8 @@ func TestDependencyMapUpdatesOnGroupUpdate(t *testing.T) {
require.NotEqual(t, orig[h], depMap) require.NotEqual(t, orig[h], depMap)
// We expect there to be some dependencies since the new rule group contains a dependency. // We expect there to be some dependencies since the new rule group contains a dependency.
require.NotEmpty(t, depMap) require.NotEmpty(t, depMap)
require.Equal(t, 1, depMap.dependents(rr)) require.Len(t, depMap.dependents(rr), 1)
require.Equal(t, "HighRequestRate", depMap.dependents(rr)[0].Name())
require.Zero(t, depMap.dependencies(rr)) require.Zero(t, depMap.dependencies(rr))
} }
} }

View file

@ -45,10 +45,12 @@ func (u unknownRule) SetEvaluationDuration(time.Duration) {}
func (u unknownRule) GetEvaluationDuration() time.Duration { return 0 } func (u unknownRule) GetEvaluationDuration() time.Duration { return 0 }
func (u unknownRule) SetEvaluationTimestamp(time.Time) {} func (u unknownRule) SetEvaluationTimestamp(time.Time) {}
func (u unknownRule) GetEvaluationTimestamp() time.Time { return time.Time{} } func (u unknownRule) GetEvaluationTimestamp() time.Time { return time.Time{} }
func (u unknownRule) SetNoDependentRules(bool) {} func (u unknownRule) SetDependentRules([]Rule) {}
func (u unknownRule) NoDependentRules() bool { return false } func (u unknownRule) NoDependentRules() bool { return false }
func (u unknownRule) SetNoDependencyRules(bool) {} func (u unknownRule) DependentRules() map[string]struct{} { return nil }
func (u unknownRule) SetDependencyRules([]Rule) {}
func (u unknownRule) NoDependencyRules() bool { return false } func (u unknownRule) NoDependencyRules() bool { return false }
func (u unknownRule) DependencyRules() map[string]struct{} { return nil }
func TestNewRuleDetailPanics(t *testing.T) { func TestNewRuleDetailPanics(t *testing.T) {
require.PanicsWithValue(t, `unknown rule type "rules.unknownRule"`, func() { require.PanicsWithValue(t, `unknown rule type "rules.unknownRule"`, func() {
@ -76,12 +78,12 @@ func TestNewRuleDetail(t *testing.T) {
require.False(t, detail.NoDependentRules) require.False(t, detail.NoDependentRules)
require.False(t, detail.NoDependencyRules) require.False(t, detail.NoDependencyRules)
rule.SetNoDependentRules(true) rule.SetDependentRules([]Rule{})
detail = NewRuleDetail(rule) detail = NewRuleDetail(rule)
require.True(t, detail.NoDependentRules) require.True(t, detail.NoDependentRules)
require.False(t, detail.NoDependencyRules) require.False(t, detail.NoDependencyRules)
rule.SetNoDependencyRules(true) rule.SetDependencyRules([]Rule{})
detail = NewRuleDetail(rule) detail = NewRuleDetail(rule)
require.True(t, detail.NoDependentRules) require.True(t, detail.NoDependentRules)
require.True(t, detail.NoDependencyRules) require.True(t, detail.NoDependencyRules)
@ -104,12 +106,12 @@ func TestNewRuleDetail(t *testing.T) {
require.False(t, detail.NoDependentRules) require.False(t, detail.NoDependentRules)
require.False(t, detail.NoDependencyRules) require.False(t, detail.NoDependencyRules)
rule.SetNoDependentRules(true) rule.SetDependentRules([]Rule{})
detail = NewRuleDetail(rule) detail = NewRuleDetail(rule)
require.True(t, detail.NoDependentRules) require.True(t, detail.NoDependentRules)
require.False(t, detail.NoDependencyRules) require.False(t, detail.NoDependencyRules)
rule.SetNoDependencyRules(true) rule.SetDependencyRules([]Rule{})
detail = NewRuleDetail(rule) detail = NewRuleDetail(rule)
require.True(t, detail.NoDependentRules) require.True(t, detail.NoDependentRules)
require.True(t, detail.NoDependencyRules) require.True(t, detail.NoDependencyRules)

View file

@ -18,6 +18,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"net/url" "net/url"
"sync"
"time" "time"
"go.uber.org/atomic" "go.uber.org/atomic"
@ -43,8 +44,9 @@ type RecordingRule struct {
// Duration of how long it took to evaluate the recording rule. // Duration of how long it took to evaluate the recording rule.
evaluationDuration *atomic.Duration evaluationDuration *atomic.Duration
noDependentRules *atomic.Bool dependenciesMutex sync.RWMutex
noDependencyRules *atomic.Bool dependentRules map[string]struct{}
dependencyRules map[string]struct{}
} }
// NewRecordingRule returns a new recording rule. // NewRecordingRule returns a new recording rule.
@ -57,8 +59,6 @@ func NewRecordingRule(name string, vector parser.Expr, lset labels.Labels) *Reco
evaluationTimestamp: atomic.NewTime(time.Time{}), evaluationTimestamp: atomic.NewTime(time.Time{}),
evaluationDuration: atomic.NewDuration(0), evaluationDuration: atomic.NewDuration(0),
lastError: atomic.NewError(nil), lastError: atomic.NewError(nil),
noDependentRules: atomic.NewBool(false),
noDependencyRules: atomic.NewBool(false),
} }
} }
@ -172,18 +172,56 @@ func (rule *RecordingRule) GetEvaluationTimestamp() time.Time {
return rule.evaluationTimestamp.Load() return rule.evaluationTimestamp.Load()
} }
func (rule *RecordingRule) SetNoDependentRules(noDependentRules bool) { func (rule *RecordingRule) SetDependentRules(dependents []Rule) {
rule.noDependentRules.Store(noDependentRules) rule.dependenciesMutex.Lock()
defer rule.dependenciesMutex.Unlock()
rule.dependentRules = make(map[string]struct{})
for _, r := range dependents {
rule.dependentRules[r.Name()] = struct{}{}
}
} }
func (rule *RecordingRule) NoDependentRules() bool { func (rule *RecordingRule) NoDependentRules() bool {
return rule.noDependentRules.Load() rule.dependenciesMutex.RLock()
defer rule.dependenciesMutex.RUnlock()
if rule.dependentRules == nil {
return false // We don't know if there are dependent rules.
}
return len(rule.dependentRules) == 0
} }
func (rule *RecordingRule) SetNoDependencyRules(noDependencyRules bool) { func (rule *RecordingRule) DependentRules() map[string]struct{} {
rule.noDependencyRules.Store(noDependencyRules) rule.dependenciesMutex.RLock()
defer rule.dependenciesMutex.RUnlock()
return rule.dependentRules
}
func (rule *RecordingRule) SetDependencyRules(dependencies []Rule) {
rule.dependenciesMutex.Lock()
defer rule.dependenciesMutex.Unlock()
rule.dependencyRules = make(map[string]struct{})
for _, r := range dependencies {
rule.dependencyRules[r.Name()] = struct{}{}
}
} }
func (rule *RecordingRule) NoDependencyRules() bool { func (rule *RecordingRule) NoDependencyRules() bool {
return rule.noDependencyRules.Load() rule.dependenciesMutex.RLock()
defer rule.dependenciesMutex.RUnlock()
if rule.dependencyRules == nil {
return false // We don't know if there are dependency rules.
}
return len(rule.dependencyRules) == 0
}
func (rule *RecordingRule) DependencyRules() map[string]struct{} {
rule.dependenciesMutex.RLock()
defer rule.dependenciesMutex.RUnlock()
return rule.dependencyRules
} }

View file

@ -255,24 +255,28 @@ func TestRecordingEvalWithOrigin(t *testing.T) {
require.Equal(t, detail, NewRuleDetail(rule)) require.Equal(t, detail, NewRuleDetail(rule))
} }
func TestRecordingRule_SetNoDependentRules(t *testing.T) { func TestRecordingRule_SetDependentRules(t *testing.T) {
rule := NewRecordingRule("1", &parser.NumberLiteral{Val: 1}, labels.EmptyLabels()) rule := NewRecordingRule("1", &parser.NumberLiteral{Val: 1}, labels.EmptyLabels())
require.False(t, rule.NoDependentRules()) require.False(t, rule.NoDependentRules())
rule.SetNoDependentRules(false) rule.SetDependentRules([]Rule{NewRecordingRule("test1", nil, labels.EmptyLabels())})
require.False(t, rule.NoDependentRules()) require.False(t, rule.NoDependentRules())
require.Equal(t, map[string]struct{}{"test1": {}}, rule.DependentRules())
rule.SetNoDependentRules(true) rule.SetDependentRules([]Rule{})
require.True(t, rule.NoDependentRules()) require.True(t, rule.NoDependentRules())
require.Empty(t, rule.DependentRules())
} }
func TestRecordingRule_SetNoDependencyRules(t *testing.T) { func TestRecordingRule_SetDependencyRules(t *testing.T) {
rule := NewRecordingRule("1", &parser.NumberLiteral{Val: 1}, labels.EmptyLabels()) rule := NewRecordingRule("1", &parser.NumberLiteral{Val: 1}, labels.EmptyLabels())
require.False(t, rule.NoDependencyRules()) require.False(t, rule.NoDependencyRules())
rule.SetNoDependencyRules(false) rule.SetDependencyRules([]Rule{NewRecordingRule("test1", nil, labels.EmptyLabels())})
require.False(t, rule.NoDependencyRules()) require.False(t, rule.NoDependencyRules())
require.Equal(t, map[string]struct{}{"test1": {}}, rule.DependencyRules())
rule.SetNoDependencyRules(true) rule.SetDependencyRules([]Rule{})
require.True(t, rule.NoDependencyRules()) require.True(t, rule.NoDependencyRules())
require.Empty(t, rule.DependencyRules())
} }

View file

@ -62,19 +62,25 @@ type Rule interface {
// NOTE: Used dynamically by rules.html template. // NOTE: Used dynamically by rules.html template.
GetEvaluationTimestamp() time.Time GetEvaluationTimestamp() time.Time
// SetNoDependentRules sets whether there's no other rule in the rule group that depends on this rule. // SetDependentRules sets rules which depend on the output of this rule.
SetNoDependentRules(bool) SetDependentRules(rules []Rule)
// NoDependentRules returns true if it's guaranteed that in the rule group there's no other rule // NoDependentRules returns true if it's guaranteed that in the rule group there's no other rule
// which depends on this one. In case this function returns false there's no such guarantee, which // which depends on this one. In case this function returns false there's no such guarantee, which
// means there may or may not be other rules depending on this one. // means there may or may not be other rules depending on this one.
NoDependentRules() bool NoDependentRules() bool
// SetNoDependencyRules sets whether this rule doesn't depend on the output of any rule in the rule group. // DependentRules returns the rules (names) which depend on the output of this rule.
SetNoDependencyRules(bool) DependentRules() map[string]struct{}
// SetDependencyRules sets rules on which this rule depends.
SetDependencyRules(rules []Rule)
// NoDependencyRules returns true if it's guaranteed that this rule doesn't depend on the output of // NoDependencyRules returns true if it's guaranteed that this rule doesn't depend on the output of
// any other rule in the group. In case this function returns false there's no such guarantee, which // any other rule in the group. In case this function returns false there's no such guarantee, which
// means the rule may or may not depend on other rules. // means the rule may or may not depend on other rules.
NoDependencyRules() bool NoDependencyRules() bool
// DependencyRules returns the rules (names) on which this rule depends.
DependencyRules() map[string]struct{}
} }