diff --git a/pkg/services/ngalert/schedule/registry.go b/pkg/services/ngalert/schedule/registry.go index ede71c94d75..e4ecc3b3c22 100644 --- a/pkg/services/ngalert/schedule/registry.go +++ b/pkg/services/ngalert/schedule/registry.go @@ -101,13 +101,13 @@ func (e *Evaluation) Fingerprint() fingerprint { type alertRulesRegistry struct { rules map[models.AlertRuleKey]*models.AlertRule folderTitles map[models.FolderKey]string - mu sync.Mutex + mu sync.RWMutex } // all returns all rules in the registry. func (r *alertRulesRegistry) all() ([]*models.AlertRule, map[models.FolderKey]string) { - r.mu.Lock() - defer r.mu.Unlock() + r.mu.RLock() + defer r.mu.RUnlock() result := make([]*models.AlertRule, 0, len(r.rules)) for _, rule := range r.rules { result = append(result, rule) @@ -116,8 +116,8 @@ func (r *alertRulesRegistry) all() ([]*models.AlertRule, map[models.FolderKey]st } func (r *alertRulesRegistry) get(k models.AlertRuleKey) *models.AlertRule { - r.mu.Lock() - defer r.mu.Unlock() + r.mu.RLock() + defer r.mu.RUnlock() return r.rules[k] } @@ -157,12 +157,14 @@ func (r *alertRulesRegistry) del(k models.AlertRuleKey) (*models.AlertRule, bool } func (r *alertRulesRegistry) isEmpty() bool { - r.mu.Lock() - defer r.mu.Unlock() + r.mu.RLock() + defer r.mu.RUnlock() return len(r.rules) == 0 } func (r *alertRulesRegistry) needsUpdate(keys []models.AlertRuleKeyWithVersion) bool { + r.mu.RLock() + defer r.mu.RUnlock() if len(r.rules) != len(keys) { return true } diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index a968921de0a..043d164fb34 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -229,9 +229,7 @@ func (st *Manager) Get(orgID int64, alertRuleUID string, stateId data.Fingerprin return st.cache.get(orgID, alertRuleUID, stateId) } -// DeleteStateByRuleUID removes the rule instances from cache and instanceStore. A closed channel is returned to be able -// to gracefully handle the clear state step in scheduler in case we do not need to use the historian to save state -// history. +// DeleteStateByRuleUID removes the rule instances from cache and instanceStore. func (st *Manager) DeleteStateByRuleUID(ctx context.Context, ruleKey ngModels.AlertRuleKeyWithGroup, reason string) []StateTransition { logger := st.log.FromContext(ctx) logger.Debug("Resetting state of the rule") @@ -289,10 +287,14 @@ func (st *Manager) ForgetStateByRuleUID(ctx context.Context, ruleKey ngModels.Al // ResetStateByRuleUID removes the rule instances from cache and instanceStore and saves state history. If the state // history has to be saved, rule must not be nil. func (st *Manager) ResetStateByRuleUID(ctx context.Context, rule *ngModels.AlertRule, reason string) []StateTransition { + if rule == nil { + return nil + } + ruleKey := rule.GetKeyWithGroup() transitions := st.DeleteStateByRuleUID(ctx, ruleKey, reason) - if rule == nil || st.historian == nil || len(transitions) == 0 { + if st.historian == nil || len(transitions) == 0 { return transitions } diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index 363d7aef78d..6a70cece78c 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/pkg/services/ngalert/state/manager_test.go @@ -2044,6 +2044,39 @@ func TestIntegrationResetStateByRuleUID(t *testing.T) { } } +func TestResetStateByRuleUID(t *testing.T) { + ctx := context.Background() + + setupManager := func(historian state.Historian) *state.Manager { + cfg := state.ManagerCfg{ + Metrics: metrics.NewNGAlert(prometheus.NewPedanticRegistry()).GetStateMetrics(), + ExternalURL: nil, + InstanceStore: &state.FakeInstanceStore{}, + Images: &state.NoopImageService{}, + Clock: clock.NewMock(), + Historian: historian, + Tracer: tracing.InitializeTracerForTest(), + Log: log.New("ngalert.state.manager"), + } + + return state.NewManager(cfg, state.NewNoopPersister()) + } + + t.Run("with nil historian", func(t *testing.T) { + manager := setupManager(nil) + + transitions := manager.ResetStateByRuleUID(ctx, nil, "test reason") + require.Empty(t, transitions) + }) + + t.Run("with historian", func(t *testing.T) { + manager := setupManager(&state.FakeHistorian{}) + + transitions := manager.ResetStateByRuleUID(ctx, nil, "test reason") + require.Empty(t, transitions) + }) +} + func setCacheID(s *state.State) *state.State { if s.CacheID != 0 { return s