From 520c927931796cdeead8761813383fad275f6a44 Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Fri, 1 Dec 2023 10:17:32 +0100 Subject: [PATCH] Alerting: Only warm alert state cache if execute_alerts=true. (#78895) * Alerting: Only warm alert state cache if execute_alerts=true. If the Grafana instance is not executing alerts, then Warm()-ing the state manager is wasteful and could lead to misleading rule status queries, as the status returned will be always based on the state loaded from the database at startup, and not the most recent evaluation state. * Move Warm() down to shared conditional. --- pkg/services/ngalert/ngalert.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 3e7a4dde681..d0eb359484f 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -352,9 +352,7 @@ func (ng *AlertNG) Run(ctx context.Context) error { if !ng.shouldRun() { return nil } - ng.Log.Debug("Starting") - - ng.stateManager.Warm(ctx, ng.store) + ng.Log.Debug("Starting", "execute_alerts", ng.Cfg.UnifiedAlerting.ExecuteAlerts) children, subCtx := errgroup.WithContext(ctx) @@ -366,6 +364,17 @@ func (ng *AlertNG) Run(ctx context.Context) error { }) if ng.Cfg.UnifiedAlerting.ExecuteAlerts { + // Only Warm() the state manager if we are actually executing alerts. + // Doing so when we are not executing alerts is wasteful and could lead + // to misleading rule status queries, as the status returned will be + // always based on the state loaded from the database at startup, and + // not the most recent evaluation state. + // + // Also note that this runs synchronously to ensure state is loaded + // before rule evaluation begins, hence we use ctx and not subCtx. + // + ng.stateManager.Warm(ctx, ng.store) + children.Go(func() error { return ng.schedule.Run(subCtx) })