Fix tsdb.stripeSeries.gc so it handles conflicts properly (#13195)

* Fix tsdb.stripeSeries.gc so it handles conflicts properly

tsdb.stripeSeries.gc needs to prune seriesHashmap.conflicts first,
otherwise seriesHashmap replaces the unique field with the first among
the conflicts. Also add regression test.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* TestStripeSeries_gc: Support stringlabels, don't use internals

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

---------

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

tsdb/head.go

@@ -1886,14 +1886,16 @@ func (s *stripeSeries) gc(mint int64, minOOOMmapRef chunks.ChunkDiskMapperRef) (
 		deletedForCallback := make(map[chunks.HeadSeriesRef]labels.Labels, deletedFromPrevStripe)
 		s.locks[i].Lock()
 
-		for hash, series := range s.hashes[i].unique {
-			check(i, hash, series, deletedForCallback)
-		}
+		// Delete conflicts first so seriesHashmap.del doesn't move them to the `unique` field,
+		// after deleting `unique`.
 		for hash, all := range s.hashes[i].conflicts {
 			for _, series := range all {
 				check(i, hash, series, deletedForCallback)
 			}
 		}
+		for hash, series := range s.hashes[i].unique {
+			check(i, hash, series, deletedForCallback)
+		}
 
 		s.locks[i].Unlock()
 

tsdb/head_test.go

@@ -5563,7 +5563,10 @@ func labelsWithHashCollision() (labels.Labels, labels.Labels) {
 	return ls1, ls2
 }
 
-func TestStripeSeries_getOrSet(t *testing.T) {
+// stripeSeriesWithCollidingSeries returns a stripeSeries with two memSeries having the same, colliding, hash.
+func stripeSeriesWithCollidingSeries(t *testing.T) (*stripeSeries, *memSeries, *memSeries) {
+	t.Helper()
+
 	lbls1, lbls2 := labelsWithHashCollision()
 	ms1 := memSeries{
 		lset: lbls1,
@@ -5589,9 +5592,29 @@ func TestStripeSeries_getOrSet(t *testing.T) {
 	require.True(t, created)
 	require.Same(t, &ms2, got)
 
+	return s, &ms1, &ms2
+}
+
+func TestStripeSeries_getOrSet(t *testing.T) {
+	s, ms1, ms2 := stripeSeriesWithCollidingSeries(t)
+	hash := ms1.lset.Hash()
+
 	// Verify that we can get both of the series despite the hash collision
-	got = s.getByHash(hash, lbls1)
-	require.Same(t, &ms1, got)
-	got = s.getByHash(hash, lbls2)
-	require.Same(t, &ms2, got)
+	got := s.getByHash(hash, ms1.lset)
+	require.Same(t, ms1, got)
+	got = s.getByHash(hash, ms2.lset)
+	require.Same(t, ms2, got)
+}
+
+func TestStripeSeries_gc(t *testing.T) {
+	s, ms1, ms2 := stripeSeriesWithCollidingSeries(t)
+	hash := ms1.lset.Hash()
+
+	s.gc(0, 0)
+
+	// Verify that we can get neither ms1 nor ms2 after gc-ing corresponding series
+	got := s.getByHash(hash, ms1.lset)
+	require.Nil(t, got)
+	got = s.getByHash(hash, ms2.lset)
+	require.Nil(t, got)
 }