Limits should include expired silences (prometheus#3862)

* Limits should include expired silences Signed-off-by: George Robinson <[email protected]> * Fix docs Signed-off-by: George Robinson <[email protected]> --------- Signed-off-by: George Robinson <[email protected]>
grafana · Jun 3, 2024 · 9f1bbb0 · 9f1bbb0
1 parent 6ad94e4
commit 9f1bbb0
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 17 deletions.
diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go
@@ -145,7 +145,7 @@ func run() int {
 		dataDir             = kingpin.Flag("storage.path", "Base path for data storage.").Default("data/").String()
 		retention           = kingpin.Flag("data.retention", "How long to keep data for.").Default("120h").Duration()
 		maintenanceInterval = kingpin.Flag("data.maintenance-interval", "Interval between garbage collection and snapshotting to disk of the silences and the notification logs.").Default("15m").Duration()
-		maxSilences         = kingpin.Flag("silences.max-silences", "Maximum number of active and pending silences, excluding expired silences. If negative or zero, no limit is set.").Default("0").Int()
+		maxSilences         = kingpin.Flag("silences.max-silences", "Maximum number of silences, including expired silences. If negative or zero, no limit is set.").Default("0").Int()
 		maxPerSilenceBytes  = kingpin.Flag("silences.max-per-silence-bytes", "Maximum per silence size in bytes. If negative or zero, no limit is set.").Default("0").Int()
 		alertGCInterval     = kingpin.Flag("alerts.gc-interval", "Interval between alert GC.").Default("30m").Duration()
 

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -26,7 +26,7 @@ sending an HTTP POST request to the `/-/reload` endpoint.
 
 Alertmanager supports a number of configurable limits via command-line flags.
 
-To limit the maximum number of active and pending silences, excluding expired ones,
+To limit the maximum number of silences, including expired ones,
 use the `--silences.max-silences` flag.
 You can limit the maximum size of individual silences with `--silences.max-per-silence-bytes`,
 where the unit is in bytes.

diff --git a/silence/silence.go b/silence/silence.go
@@ -204,8 +204,8 @@ type Silences struct {
 
 // Limits contains the limits for silences.
 type Limits struct {
-	// MaxSilences limits the maximum number active and pending silences.
-	// It does not include expired silences.
+	// MaxSilences limits the maximum number of silences, including expired
+	// silences.
 	MaxSilences int
 	// MaxPerSilenceBytes is the maximum size of an individual silence as
 	// stored on disk.
@@ -646,18 +646,8 @@ func (s *Silences) set(sil *pb.Silence) (string, error) {
 
 	// If we got here it's either a new silence or a replacing one.
 	if s.limits.MaxSilences > 0 {
-		// Get the number of active and pending silences to enforce limits.
-		q := &query{}
-		err := QState(types.SilenceStateActive, types.SilenceStatePending)(q)
-		if err != nil {
-			return "", fmt.Errorf("unable to query silences while checking limits: %w", err)
-		}
-		sils, _, err := s.query(q, s.nowUTC())
-		if err != nil {
-			return "", fmt.Errorf("unable to query silences while checking limits: %w", err)
-		}
-		if len(sils)+1 > s.limits.MaxSilences {
-			return "", fmt.Errorf("exceeded maximum number of silences: %d (limit: %d)", len(sils), s.limits.MaxSilences)
+		if len(s.st)+1 > s.limits.MaxSilences {
+			return "", fmt.Errorf("exceeded maximum number of silences: %d (limit: %d)", len(s.st), s.limits.MaxSilences)
 		}
 	}
 

diff --git a/silence/silence_test.go b/silence/silence_test.go
@@ -465,8 +465,12 @@ func TestSilenceLimits(t *testing.T) {
 			MaxSilences:        1,
 			MaxPerSilenceBytes: 2 << 11, // 4KB
 		},
+		Retention: 100 * time.Millisecond,
 	})
 	require.NoError(t, err)
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+	go s.Maintenance(100*time.Millisecond, "", stopCh, nil)
 
 	// Insert sil1 should succeed without error.
 	sil1 := &pb.Silence{
@@ -489,8 +493,10 @@ func TestSilenceLimits(t *testing.T) {
 	require.EqualError(t, err, "exceeded maximum number of silences: 1 (limit: 1)")
 	require.Equal(t, "", id2)
 
-	// Expire sil1. This should allow sil2 to be inserted.
+	// Expire sil1 and wait for the maintenance to finish.
+	// This should allow sil2 to be inserted.
 	require.NoError(t, s.Expire(id1))
+	time.Sleep(150 * time.Millisecond)
 	id2, err = s.Set(sil2)
 	require.NoError(t, err)
 	require.NotEqual(t, "", id2)
@@ -501,6 +507,7 @@ func TestSilenceLimits(t *testing.T) {
 
 	// Expire sil2.
 	require.NoError(t, s.Expire(id2))
+	time.Sleep(150 * time.Millisecond)
 
 	// Insert sil3 should fail because it exceeds maximum size.
 	sil3 := &pb.Silence{