From f7b4feffd3c8d4570e7daed234c4ba56b9ef8b05 Mon Sep 17 00:00:00 2001 From: Ethan Hunter Date: Wed, 19 Nov 2025 10:23:37 -0700 Subject: [PATCH] reduce the time Dispatch.Group holds the mutex (#4670) * reduce the time Dispatch.Group holds the mutex Signed-off-by: Ethan Hunter * add comment explaining behavior Signed-off-by: Ethan Hunter * switch from manual map clone to maps.Clone Signed-off-by: Ethan Hunter --------- Signed-off-by: Ethan Hunter --- dispatch/dispatch.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/dispatch/dispatch.go b/dispatch/dispatch.go index 9ab347ff4..a25972a12 100644 --- a/dispatch/dispatch.go +++ b/dispatch/dispatch.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "log/slog" + "maps" "sort" "sync" "sync/atomic" @@ -283,8 +284,22 @@ func (d *Dispatcher) Groups(ctx context.Context, routeFilter func(*Route) bool, d.WaitForLoading() groups := AlertGroups{} + // Make a snapshot of the aggrGroupsPerRoute map to use for this function. + // This ensures that we hold the Dispatcher.mtx for as little time as + // possible. + // It also prevents us from holding the any locks in alertFilter or routeFilter + // while we hold the dispatcher lock d.mtx.RLock() - defer d.mtx.RUnlock() + aggrGroupsPerRoute := map[*Route]map[model.Fingerprint]*aggrGroup{} + for route, ags := range d.aggrGroupsPerRoute { + // Since other goroutines could modify d.aggrGroupsPerRoute, we need to + // copy it. We DON'T need to copy the aggrGroup objects because they each + // have a mutex protecting their internal state. + // The aggrGroup methods use the internal lock. It is important to avoid + // accessing internal fields on the aggrGroup objects. + aggrGroupsPerRoute[route] = maps.Clone(ags) + } + d.mtx.RUnlock() // Keep a list of receivers for an alert to prevent checking each alert // again against all routes. The alert has already matched against this @@ -292,7 +307,7 @@ func (d *Dispatcher) Groups(ctx context.Context, routeFilter func(*Route) bool, receivers := map[model.Fingerprint][]string{} now := time.Now() - for route, ags := range d.aggrGroupsPerRoute { + for route, ags := range aggrGroupsPerRoute { if !routeFilter(route) { continue }