From 09abc8ae03e58c84a21cbcc5bfeee9b0d3fc7106 Mon Sep 17 00:00:00 2001 From: ZayanKhan-12 <108294002+ZayanKhan-12@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:05:18 -0400 Subject: [PATCH] notifier: take the write lock when updating LastEval in sendEvaluatorRequests sendEvaluatorRequests held only the cluster's read lock while writing groupInfo.LastEval. A read lock does not exclude other readers of the same group state, and two instances of sendEvaluatorRequests can overlap briefly after a ZooKeeper session bounce (manageEvalLoop spawns a new loop once it reacquires the lock, while the previous loop can still be mid-iteration before observing doEvaluations == false), so the write can race. Writes to shared state need the write lock. The lock is only held for the in-memory iteration - the evaluator request send happens in a spawned goroutine - so this does not hold the lock across any channel sends or network calls. Co-Authored-By: Claude Fable 5 --- core/internal/notifier/coordinator.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/internal/notifier/coordinator.go b/core/internal/notifier/coordinator.go index 5cab2906..7cfdd9c8 100644 --- a/core/internal/notifier/coordinator.go +++ b/core/internal/notifier/coordinator.go @@ -364,10 +364,11 @@ func (nc *Coordinator) sendEvaluatorRequests() { timeNow := time.Now() sendBefore := timeNow.Add(-time.Duration(nc.minInterval) * time.Second) - // Fire off evaluation requests for every group we know about + // Fire off evaluation requests for every group we know about. This loop writes groupInfo.LastEval, + // so it needs the write lock on the cluster - a read lock is not enough nc.clusterLock.RLock() for cluster, consumerGroup := range nc.clusters { - consumerGroup.Lock.RLock() + consumerGroup.Lock.Lock() for consumer, groupInfo := range consumerGroup.Groups { if groupInfo.LastEval.Before(sendBefore) { nc.Log.Debug("Evaluating group", zap.String("group", consumer)) @@ -381,7 +382,7 @@ func (nc *Coordinator) sendEvaluatorRequests() { groupInfo.LastEval = timeNow } } - consumerGroup.Lock.RUnlock() + consumerGroup.Lock.Unlock() } nc.clusterLock.RUnlock()