From ecc37588b00c5ec1d4f2349266a23b01f6c91743 Mon Sep 17 00:00:00 2001
From: Arve Knudsen <arve.knudsen@gmail.com>
Date: Mon, 27 Nov 2023 16:40:30 +0100
Subject: [PATCH] tsdb: seriesHashmap.set by making receiver a pointer (#13193)

* Fix tsdb.seriesHashmap.set by making receiver a pointer

The method tsdb.seriesHashmap.set currently doesn't set the conflicts
field properly, due to the receiver being a non-pointer. Fix by turning
the receiver into a pointer, and add a corresponding regression test.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
---
 tsdb/head.go      |  4 ++--
 tsdb/head_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/tsdb/head.go b/tsdb/head.go
index bf181a415..25209cd01 100644
--- a/tsdb/head.go
+++ b/tsdb/head.go
@@ -1718,7 +1718,7 @@ func (m *seriesHashmap) get(hash uint64, lset labels.Labels) *memSeries {
 	return nil
 }
 
-func (m seriesHashmap) set(hash uint64, s *memSeries) {
+func (m *seriesHashmap) set(hash uint64, s *memSeries) {
 	if existing, found := m.unique[hash]; !found || labels.Equal(existing.lset, s.lset) {
 		m.unique[hash] = s
 		return
@@ -1736,7 +1736,7 @@ func (m seriesHashmap) set(hash uint64, s *memSeries) {
 	m.conflicts[hash] = append(l, s)
 }
 
-func (m seriesHashmap) del(hash uint64, lset labels.Labels) {
+func (m *seriesHashmap) del(hash uint64, lset labels.Labels) {
 	var rem []*memSeries
 	unique, found := m.unique[hash]
 	switch {
diff --git a/tsdb/head_test.go b/tsdb/head_test.go
index f2325039a..64237e76a 100644
--- a/tsdb/head_test.go
+++ b/tsdb/head_test.go
@@ -5542,3 +5542,54 @@ func TestHeadCompactionWhileAppendAndCommitExemplar(t *testing.T) {
 	app.Commit()
 	h.Close()
 }
+
+func labelsWithHashCollision() (labels.Labels, labels.Labels) {
+	// These two series have the same XXHash; thanks to https://github.com/pstibrany/labels_hash_collisions
+	ls1 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "l6CQ5y")
+	ls2 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "v7uDlF")
+
+	if ls1.Hash() != ls2.Hash() {
+		// These ones are the same when using -tags stringlabels
+		ls1 = labels.FromStrings("__name__", "metric", "lbl", "HFnEaGl")
+		ls2 = labels.FromStrings("__name__", "metric", "lbl", "RqcXatm")
+	}
+
+	if ls1.Hash() != ls2.Hash() {
+		panic("This code needs to be updated: find new labels with colliding hash values.")
+	}
+
+	return ls1, ls2
+}
+
+func TestStripeSeries_getOrSet(t *testing.T) {
+	lbls1, lbls2 := labelsWithHashCollision()
+	ms1 := memSeries{
+		lset: lbls1,
+	}
+	ms2 := memSeries{
+		lset: lbls2,
+	}
+	hash := lbls1.Hash()
+	s := newStripeSeries(1, noopSeriesLifecycleCallback{})
+
+	got, created, err := s.getOrSet(hash, lbls1, func() *memSeries {
+		return &ms1
+	})
+	require.NoError(t, err)
+	require.True(t, created)
+	require.Same(t, &ms1, got)
+
+	// Add a conflicting series
+	got, created, err = s.getOrSet(hash, lbls2, func() *memSeries {
+		return &ms2
+	})
+	require.NoError(t, err)
+	require.True(t, created)
+	require.Same(t, &ms2, got)
+
+	// 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)
+}