Use most recent time to remove from ring

Signed-off-by: Daniel Blando <[email protected]>

Author: danielblando

CHANGELOG.md

@@ -60,6 +60,7 @@
 * [BUGFIX] Querier: fixed panic when querying exemplars and using `-distributor.shard-by-all-labels=false`. #4473
 * [BUGFIX] Querier: honor querier minT,maxT if `nil` SelectHints are passed to Select(). #4413
 * [BUGFIX] Compactor: fixed panic while collecting Prometheus metrics. #4483
+* [BUGFIX] Memberlist: Remove entry from the ring even if the timestamp is in the future  #4501
 
 
 ## 1.10.0 / 2021-08-03

pkg/ring/merge_test.go

@@ -356,7 +356,27 @@ func TestMergeRemoveMissing(t *testing.T) {
 			Ingesters: map[string]InstanceDesc{
 				"Ing 1": {Addr: "addr1", Timestamp: now, State: ACTIVE, Tokens: []uint32{30, 40, 50}},
 				"Ing 2": {Addr: "addr2", Timestamp: now + 5, State: ACTIVE, Tokens: []uint32{5, 10, 20, 100, 200}},
-				"Ing 3": {Addr: "addr3", Timestamp: now + 3, State: LEFT}, // When deleting, time depends on value passed to merge function.
+				"Ing 3": {Addr: "addr3", Timestamp: now + 3, State: LEFT},
+			},
+		}
+	}
+
+	thirdRing := func() *Desc {
+		return &Desc{
+			Ingesters: map[string]InstanceDesc{
+				"Ing 1": {Addr: "addr1", Timestamp: now, State: ACTIVE, Tokens: []uint32{30, 40, 50}},
+				"Ing 2": {Addr: "addr2", Timestamp: now, State: JOINING, Tokens: []uint32{5, 10, 20, 100, 200}},
+				"Ing 3": {Addr: "addr3", Timestamp: now + 50, State: LEAVING, Tokens: []uint32{5, 10, 20, 100, 200}},
+			},
+		}
+	}
+
+	expectedThirdSecondMerge := func() *Desc {
+		return &Desc{
+			Ingesters: map[string]InstanceDesc{
+				"Ing 1": {Addr: "addr1", Timestamp: now, State: ACTIVE, Tokens: []uint32{30, 40, 50}},
+				"Ing 2": {Addr: "addr2", Timestamp: now + 5, State: ACTIVE, Tokens: []uint32{5, 10, 20, 100, 200}},
+				"Ing 3": {Addr: "addr3", Timestamp: now + 50, State: LEFT},
 			},
 		}
 	}
@@ -367,7 +387,18 @@ func TestMergeRemoveMissing(t *testing.T) {
 		assert.Equal(t, &Desc{
 			Ingesters: map[string]InstanceDesc{
 				"Ing 2": {Addr: "addr2", Timestamp: now + 5, State: ACTIVE, Tokens: []uint32{5, 10, 20, 100, 200}},
-				"Ing 3": {Addr: "addr3", Timestamp: now + 3, State: LEFT}, // When deleting, time depends on value passed to merge function.
+				"Ing 3": {Addr: "addr3", Timestamp: now + 3, State: LEFT}, // When deleting, time  is recent between now and current timestamp
+			},
+		}, ch) // entire second ring is new
+	}
+
+	{
+		our, ch := mergeLocalCAS(thirdRing(), secondRing(), now+10)
+		assert.Equal(t, expectedThirdSecondMerge(), our)
+		assert.Equal(t, &Desc{
+			Ingesters: map[string]InstanceDesc{
+				"Ing 2": {Addr: "addr2", Timestamp: now + 5, State: ACTIVE, Tokens: []uint32{5, 10, 20, 100, 200}},
+				"Ing 3": {Addr: "addr3", Timestamp: now + 50, State: LEFT}, // When deleting, time  is recent between now and current timestamp
 			},
 		}, ch) // entire second ring is new
 	}

pkg/ring/model.go

@@ -210,7 +210,12 @@ func (d *Desc) mergeWithTime(mergeable memberlist.Mergeable, localCAS bool, now
 				// We are deleting entry "now", and should not keep old timestamp, because there may already be pending
 				// message in the gossip network with newer timestamp (but still older than "now").
 				// Such message would "resurrect" this deleted entry.
-				ting.Timestamp = now.Unix()
+				// If timestamp on the ring is in "the future" we need to use the timestamp of the ring
+				// otherwise the gossip message will be ignored by the peers.
+				// see https://github.com/cortexproject/cortex/blob/d3f46c53616dcdf5c670f3e3a4d371e44b10683e/pkg/ring/model.go#L192-L200
+				if ting.Timestamp < now.Unix() {
+					ting.Timestamp = now.Unix()
+				}
 				thisIngesterMap[name] = ting
 
 				updated = append(updated, name)