Fix sizing of buffer and don't nil exemplars buffer. (#2725)

* Fix sizing of buffer and don't nil exemplars buffer. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * CHANGELOG.md Signed-off-by: Peter Štibraný <pstibrany@gmail.com> Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
grafana · Aug 15, 2022 · 70f91b8 · 70f91b8
1 parent efb7119
commit 70f91b8
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,7 @@
 
 ### Grafana Mimir
 
-* [CHANGE] Distributor: if forwarding rules are used to forward samples, exemplars are now removed from the request. #2710
+* [CHANGE] Distributor: if forwarding rules are used to forward samples, exemplars are now removed from the request. #2710 #2725
 * [CHANGE] Limits: change the default value of `max_global_series_per_metric` limit to `0` (disabled). Setting this limit by default does not provide much benefit because series are sharded by all labels. #2714
 * [BUGFIX] Fix reporting of tracing spans from PromQL engine. #2707
 * [BUGFIX] Distributor: Apply distributor instance limits before running HA deduplication. #2709

diff --git a/pkg/mimirpb/timeseries.go b/pkg/mimirpb/timeseries.go
@@ -348,7 +348,7 @@ func DeepCopyTimeseries(dst, src PreallocTimeseries, keepExemplars bool) Preallo
 	dstTs := dst.TimeSeries
 
 	// Prepare a buffer which is large enough to hold all the label names and values of src.
-	requiredYoloSliceCap := countTotalLabelLen(src.TimeSeries)
+	requiredYoloSliceCap := countTotalLabelLen(src.TimeSeries, keepExemplars)
 	dst.yoloSlice = yoloSliceFromPool()
 	buf := ensureCap(dst.yoloSlice, requiredYoloSliceCap)
 
@@ -380,7 +380,7 @@ func DeepCopyTimeseries(dst, src PreallocTimeseries, keepExemplars bool) Preallo
 			dstTs.Exemplars[exemplarIdx].TimestampMs = src.Exemplars[exemplarIdx].TimestampMs
 		}
 	} else {
-		dstTs.Exemplars = nil
+		dstTs.Exemplars = dstTs.Exemplars[:0]
 	}
 
 	return dst
@@ -401,15 +401,17 @@ func ensureCap(bufRef *[]byte, requiredCap int) []byte {
 }
 
 // countTotalLabelLen takes a time series and calculates the sum of the lengths of all label names and values.
-func countTotalLabelLen(ts *TimeSeries) int {
+func countTotalLabelLen(ts *TimeSeries, includeExemplars bool) int {
 	var labelLen int
 	for _, label := range ts.Labels {
 		labelLen += len(label.Name) + len(label.Value)
 	}
 
-	for _, exemplar := range ts.Exemplars {
-		for _, label := range exemplar.Labels {
-			labelLen += len(label.Name) + len(label.Value)
+	if includeExemplars {
+		for _, exemplar := range ts.Exemplars {
+			for _, label := range exemplar.Labels {
+				labelLen += len(label.Name) + len(label.Value)
+			}
 		}
 	}
 

diff --git a/pkg/mimirpb/timeseries_test.go b/pkg/mimirpb/timeseries_test.go
@@ -154,5 +154,41 @@ func TestDeepCopyTimeseries(t *testing.T) {
 
 	dst = PreallocTimeseries{}
 	dst = DeepCopyTimeseries(dst, src, false)
-	assert.Nil(t, dst.Exemplars)
+	assert.NotNil(t, dst.Exemplars)
+	assert.Len(t, dst.Exemplars, 0)
+}
+
+func TestDeepCopyTimeseriesExemplars(t *testing.T) {
+	src := PreallocTimeseries{
+		TimeSeries: &TimeSeries{
+			Labels: []LabelAdapter{
+				{Name: "sampleLabel1", Value: "sampleValue1"},
+				{Name: "sampleLabel2", Value: "sampleValue2"},
+			},
+			Samples: []Sample{
+				{Value: 1, TimestampMs: 2},
+				{Value: 3, TimestampMs: 4},
+			},
+		},
+	}
+
+	for i := 0; i < 100; i++ {
+		src.Exemplars = append(src.Exemplars, Exemplar{
+			Value:       1,
+			TimestampMs: 2,
+			Labels: []LabelAdapter{
+				{Name: "exemplarLabel1", Value: "exemplarValue1"},
+				{Name: "exemplarLabel2", Value: "exemplarValue2"},
+			},
+		})
+	}
+
+	dst1 := PreallocTimeseries{}
+	dst1 = DeepCopyTimeseries(dst1, src, false)
+
+	dst2 := PreallocTimeseries{}
+	dst2 = DeepCopyTimeseries(dst2, src, true)
+
+	// dst1 should use much smaller buffer than dst2.
+	assert.Less(t, cap(*dst1.yoloSlice), cap(*dst2.yoloSlice))
 }