Fix memory leak in blocks storage ingesters (#2586)

* Fix memory leak in blocks storage ingesters

Signed-off-by: Marco Pracucci <[email protected]>

* Updated CHANGELOG

Signed-off-by: Marco Pracucci <[email protected]>

Author: pracucci

CHANGELOG.md

@@ -86,6 +86,7 @@
 * [BUGFIX] QueryFrontend: fixed a situation where span context missed when downstream_url is used. #2539
 * [BUGFIX] Querier: Fixed a situation where querier would crash because of an unresponsive frontend instance. #2569
 * [BUGFIX] Fixed collection of tracing spans from Thanos components used internally. #2584
+* [BUGFIX] Experimental TSDB: fixed memory leak in ingesters. #2586
 
 ## 1.0.1 / 2020-04-23
 

pkg/ingester/client/compat.go

@@ -229,17 +229,21 @@ func FromLabelAdaptersToLabels(ls []LabelAdapter) labels.Labels {
 // get in input labels whose data structure is reused.
 func FromLabelAdaptersToLabelsWithCopy(input []LabelAdapter) labels.Labels {
 	result := make(labels.Labels, len(input))
-
 	for i, l := range input {
 		result[i] = labels.Label{
-			Name:  l.Name,
-			Value: l.Value,
+			Name:  copyString(l.Name),
+			Value: copyString(l.Value),
 		}
 	}
-
 	return result
 }
 
+func copyString(in string) string {
+	out := make([]byte, len(in))
+	copy(out, in)
+	return string(out)
+}
+
 // FromLabelsToLabelAdapters casts labels.Labels to []LabelAdapter.
 // It uses unsafe, but as LabelAdapter == labels.Label this should be safe.
 // This allows us to use labels.Labels directly in protos.

pkg/ingester/client/compat_test.go

@@ -5,10 +5,12 @@ import (
 	"reflect"
 	"sort"
 	"testing"
+	"unsafe"
 
 	"github.com/prometheus/common/model"
 	"github.com/prometheus/prometheus/pkg/labels"
 	"github.com/prometheus/prometheus/pkg/textparse"
+	"github.com/stretchr/testify/assert"
 	"github.com/thanos-io/thanos/pkg/testutil"
 )
 
@@ -113,6 +115,30 @@ func TestMetricMetadataToMetricTypeToMetricType(t *testing.T) {
 	}
 }
 
+func TestFromLabelAdaptersToLabels(t *testing.T) {
+	input := []LabelAdapter{{Name: "hello", Value: "world"}}
+	expected := labels.Labels{labels.Label{Name: "hello", Value: "world"}}
+	actual := FromLabelAdaptersToLabels(input)
+
+	assert.Equal(t, expected, actual)
+
+	// All strings must NOT be copied.
+	assert.Equal(t, uintptr(unsafe.Pointer(&input[0].Name)), uintptr(unsafe.Pointer(&actual[0].Name)))
+	assert.Equal(t, uintptr(unsafe.Pointer(&input[0].Value)), uintptr(unsafe.Pointer(&actual[0].Value)))
+}
+
+func TestFromLabelAdaptersToLabelsWithCopy(t *testing.T) {
+	input := []LabelAdapter{{Name: "hello", Value: "world"}}
+	expected := labels.Labels{labels.Label{Name: "hello", Value: "world"}}
+	actual := FromLabelAdaptersToLabelsWithCopy(input)
+
+	assert.Equal(t, expected, actual)
+
+	// All strings must be copied.
+	assert.NotEqual(t, uintptr(unsafe.Pointer(&input[0].Name)), uintptr(unsafe.Pointer(&actual[0].Name)))
+	assert.NotEqual(t, uintptr(unsafe.Pointer(&input[0].Value)), uintptr(unsafe.Pointer(&actual[0].Value)))
+}
+
 func TestQueryResponse(t *testing.T) {
 	want := buildTestMatrix(10, 10, 10)
 	have := FromQueryResponse(ToQueryResponse(want))