Add delete back to remove notifier as memory leak will still occur

Signed-off-by: Rohan Gupta <[email protected]>

Author: rohang98

pkg/ruler/manager.go

@@ -107,7 +107,7 @@ func (r *DefaultMultiTenantManager) SyncRuleGroups(ctx context.Context, ruleGrou
 			go mngr.Stop()
 			delete(r.userManagers, userID)
 
-			r.stopNotifier(userID)
+			r.removeNotifier(userID)
 			r.mapper.cleanupUser(userID)
 			r.lastReloadSuccessful.DeleteLabelValues(userID)
 			r.lastReloadSuccessfulTimestamp.DeleteLabelValues(userID)
@@ -164,29 +164,31 @@ func (r *DefaultMultiTenantManager) syncRulesToManager(ctx context.Context, user
 // newManager creates a prometheus rule manager wrapped with a user id
 // configured storage, appendable, notifier, and instrumentation
 func (r *DefaultMultiTenantManager) newManager(ctx context.Context, userID string) (RulesManager, error) {
-	notifier, err := r.getOrCreateNotifier(userID)
-	if err != nil {
-		return nil, err
-	}
-
 	// Create a new Prometheus registry and register it within
 	// our metrics struct for the provided user if it doesn't already exist.
 	reg := prometheus.NewRegistry()
 	r.userManagerMetrics.AddUserRegistry(userID, reg)
 
+	notifier, err := r.getOrCreateNotifier(userID, reg)
+	if err != nil {
+		return nil, err
+	}
+
 	return r.managerFactory(ctx, userID, notifier, r.logger, reg), nil
 }
 
-func (r *DefaultMultiTenantManager) stopNotifier(userID string) {
+func (r *DefaultMultiTenantManager) removeNotifier(userID string) {
 	r.notifiersMtx.Lock()
 	defer r.notifiersMtx.Unlock()
 
 	if n, ok := r.notifiers[userID]; ok {
 		n.stop()
 	}
+
+	delete(r.notifiers, userID)
 }
 
-func (r *DefaultMultiTenantManager) getOrCreateNotifier(userID string) (*notifier.Manager, error) {
+func (r *DefaultMultiTenantManager) getOrCreateNotifier(userID string, userManagerRegistry prometheus.Registerer) (*notifier.Manager, error) {
 	r.notifiersMtx.Lock()
 	defer r.notifiersMtx.Unlock()
 
@@ -197,7 +199,7 @@ func (r *DefaultMultiTenantManager) getOrCreateNotifier(userID string) (*notifie
 		return n.notifier, nil
 	}
 
-	reg := prometheus.WrapRegistererWith(prometheus.Labels{"user": userID}, r.registry)
+	reg := prometheus.WrapRegistererWith(prometheus.Labels{"user": userID}, userManagerRegistry)
 	reg = prometheus.WrapRegistererWithPrefix("cortex_", reg)
 	n = newRulerNotifier(&notifier.Options{
 		QueueCapacity: r.cfg.NotificationQueueCapacity,

pkg/ruler/ruler_test.go

@@ -274,7 +274,7 @@ func TestNotifierSendsUserIDHeader(t *testing.T) {
 	defer rcleanup()
 	defer manager.Stop()
 
-	n, err := manager.getOrCreateNotifier("1")
+	n, err := manager.getOrCreateNotifier("1", manager.registry)
 	require.NoError(t, err)
 
 	// Loop until notifier discovery syncs up