only increment silences version if a silence is added (prometheus#3961)

Author: Spaceman1701

silence/silence.go

@@ -580,7 +580,8 @@ func (s *Silences) setSilence(msil *pb.MeshSilence, now time.Time) error {
 	if err != nil {
 		return err
 	}
-	if s.st.merge(msil, now) {
+	_, added := s.st.merge(msil, now)
+	if added {
 		s.version++
 	}
 	s.broadcast(b)
@@ -927,8 +928,11 @@ func (s *Silences) Merge(b []byte) error {
 	now := s.nowUTC()
 
 	for _, e := range st {
-		if merged := s.st.merge(e, now); merged {
-			s.version++
+		merged, added := s.st.merge(e, now)
+		if merged {
+			if added {
+				s.version++
+			}
 			if !cluster.OversizedMessage(b) {
 				// If this is the first we've seen the message and it's
 				// not oversized, gossip it to other nodes. We don't
@@ -953,10 +957,13 @@ func (s *Silences) SetBroadcast(f func([]byte)) {
 
 type state map[string]*pb.MeshSilence
 
-func (s state) merge(e *pb.MeshSilence, now time.Time) bool {
+// merge returns two bools: the first is true when merge caused a state change. The second
+// is true if that state change added a new silence. In other words, the second return is
+// true whenever a silence with a new ID has been added to the state as a result of merge.
+func (s state) merge(e *pb.MeshSilence, now time.Time) (bool, bool) {
 	id := e.Silence.Id
 	if e.ExpiresAt.Before(now) {
-		return false
+		return false, false
 	}
 	// Comments list was moved to a single comment. Apply upgrade
 	// on silences received from peers.
@@ -969,9 +976,9 @@ func (s state) merge(e *pb.MeshSilence, now time.Time) bool {
 	prev, ok := s[id]
 	if !ok || prev.Silence.UpdatedAt.Before(e.Silence.UpdatedAt) {
 		s[id] = e
-		return true
+		return true, !ok
 	}
-	return false
+	return false, false
 }
 
 func (s state) MarshalBinary() ([]byte, error) {

silence/silence_test.go

@@ -428,8 +428,10 @@ func TestSilenceSet(t *testing.T) {
 		StartsAt: start1.Add(2 * time.Minute),
 		EndsAt:   start1.Add(5 * time.Minute),
 	}
+	versionBeforeOp := s.Version()
 	require.NoError(t, s.Set(sil1))
 	require.NotEqual(t, "", sil1.Id)
+	require.NotEqual(t, versionBeforeOp, s.Version())
 
 	want := state{
 		sil1.Id: &pb.MeshSilence{
@@ -453,8 +455,10 @@ func TestSilenceSet(t *testing.T) {
 		Matchers: []*pb.Matcher{{Name: "a", Pattern: "b"}},
 		EndsAt:   start2.Add(1 * time.Minute),
 	}
+	versionBeforeOp = s.Version()
 	require.NoError(t, s.Set(sil2))
 	require.NotEqual(t, "", sil2.Id)
+	require.NotEqual(t, versionBeforeOp, s.Version())
 
 	want = state{
 		sil1.Id: want[sil1.Id],
@@ -474,22 +478,27 @@ func TestSilenceSet(t *testing.T) {
 	// Should be able to update silence without modifications. It is expected to
 	// keep the same ID.
 	sil3 := cloneSilence(sil2)
+	versionBeforeOp = s.Version()
 	require.NoError(t, s.Set(sil3))
 	require.Equal(t, sil2.Id, sil3.Id)
+	require.Equal(t, versionBeforeOp, s.Version())
 
 	// Should be able to update silence with comment. It is also expected to
 	// keep the same ID.
 	sil4 := cloneSilence(sil3)
 	sil4.Comment = "c"
+	versionBeforeOp = s.Version()
 	require.NoError(t, s.Set(sil4))
 	require.Equal(t, sil3.Id, sil4.Id)
+	require.Equal(t, versionBeforeOp, s.Version())
 
 	// Extend sil4 to expire at a later time. This should not expire the
 	// existing silence, and so should also keep the same ID.
 	clock.Advance(time.Minute)
 	start5 := s.nowUTC()
 	sil5 := cloneSilence(sil4)
 	sil5.EndsAt = start5.Add(100 * time.Minute)
+	versionBeforeOp = s.Version()
 	require.NoError(t, s.Set(sil5))
 	require.Equal(t, sil4.Id, sil5.Id)
 	want = state{
@@ -507,6 +516,7 @@ func TestSilenceSet(t *testing.T) {
 		},
 	}
 	require.Equal(t, want, s.st, "unexpected state after silence creation")
+	require.Equal(t, versionBeforeOp, s.Version())
 
 	// Replace the silence sil5 with another silence with different matchers.
 	// Unlike previous updates, changing the matchers for an existing silence
@@ -518,6 +528,7 @@ func TestSilenceSet(t *testing.T) {
 
 	sil6 := cloneSilence(sil5)
 	sil6.Matchers = []*pb.Matcher{{Name: "a", Pattern: "c"}}
+	versionBeforeOp = s.Version()
 	require.NoError(t, s.Set(sil6))
 	require.NotEqual(t, sil5.Id, sil6.Id)
 	want = state{
@@ -546,6 +557,7 @@ func TestSilenceSet(t *testing.T) {
 		},
 	}
 	require.Equal(t, want, s.st, "unexpected state after silence creation")
+	require.NotEqual(t, versionBeforeOp, s.Version())
 
 	// Re-create the silence that we just replaced. Changing the start time,
 	// just like changing the matchers, creates a new silence with a different
@@ -555,6 +567,7 @@ func TestSilenceSet(t *testing.T) {
 	sil7 := cloneSilence(sil5)
 	sil7.StartsAt = start1
 	sil7.EndsAt = start1.Add(5 * time.Minute)
+	versionBeforeOp = s.Version()
 	require.NoError(t, s.Set(sil7))
 	require.NotEqual(t, sil2.Id, sil7.Id)
 	want = state{
@@ -574,19 +587,22 @@ func TestSilenceSet(t *testing.T) {
 		},
 	}
 	require.Equal(t, want, s.st, "unexpected state after silence creation")
+	require.NotEqual(t, versionBeforeOp, s.Version())
 
 	// Updating an existing silence with an invalid silence should not expire
 	// the original silence.
 	clock.Advance(time.Millisecond)
 	sil8 := cloneSilence(sil7)
 	sil8.EndsAt = time.Time{}
+	versionBeforeOp = s.Version()
 	require.EqualError(t, s.Set(sil8), "invalid silence: invalid zero end timestamp")
 
 	// sil7 should not be expired because the update failed.
 	clock.Advance(time.Millisecond)
 	sil7, err = s.QueryOne(QIDs(sil7.Id))
 	require.NoError(t, err)
 	require.Equal(t, types.SilenceStateActive, getState(sil7, s.nowUTC()))
+	require.Equal(t, versionBeforeOp, s.Version())
 }
 
 func TestSilenceLimits(t *testing.T) {