Avoid over allocating strings.Builder when creating cache key for exp…

…anded postings Signed-off-by: alanprot <[email protected]>
cortexproject · Dec 20, 2024 · 22f5ecd · 22f5ecd
1 parent 5a78608
commit 22f5ecd
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/pkg/storage/tsdb/expanded_postings_cache.go b/pkg/storage/tsdb/expanded_postings_cache.go
@@ -208,7 +208,7 @@ func (c *blocksPostingsForMatchersCache) fetchPostings(blockID ulid.ULID, ix tsd
 		return nil, 0, err
 	}
 
-	key := c.cacheKey(seed, blockID, ms...)
+	key := cacheKey(seed, blockID, ms...)
 	promise, loaded := cache.getPromiseForKey(key, fetch)
 	if loaded {
 		c.metrics.CacheHits.WithLabelValues(cache.name).Inc()
@@ -235,7 +235,7 @@ func (c *blocksPostingsForMatchersCache) getSeedForMetricName(metricName string)
 	return c.seedByHash.getSeed(c.userId, metricName)
 }
 
-func (c *blocksPostingsForMatchersCache) cacheKey(seed string, blockID ulid.ULID, ms ...*labels.Matcher) string {
+func cacheKey(seed string, blockID ulid.ULID, ms ...*labels.Matcher) string {
 	slices.SortFunc(ms, func(i, j *labels.Matcher) int {
 		if i.Type != j.Type {
 			return int(i.Type - j.Type)
@@ -254,15 +254,16 @@ func (c *blocksPostingsForMatchersCache) cacheKey(seed string, blockID ulid.ULID
 		sepLen  = 1
 	)
 
-	var size int
+	size := len(seed) + len(blockID.String()) + 2*sepLen
 	for _, m := range ms {
-		size += len(seed) + len(blockID.String()) + len(m.Name) + len(m.Value) + typeLen + 2*sepLen
+		size += len(m.Name) + len(m.Value) + typeLen + sepLen
 	}
 	sb := strings.Builder{}
 	sb.Grow(size)
 	sb.WriteString(seed)
 	sb.WriteByte('|')
 	sb.WriteString(blockID.String())
+	sb.WriteByte('|')
 	for _, m := range ms {
 		sb.WriteString(m.Name)
 		sb.WriteString(m.Type.String())

diff --git a/pkg/storage/tsdb/expanded_postings_cache_test.go b/pkg/storage/tsdb/expanded_postings_cache_test.go
@@ -3,17 +3,48 @@ package tsdb
 import (
 	"bytes"
 	"fmt"
+	"github.com/prometheus/prometheus/model/labels"
 	"strings"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/oklog/ulid"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/atomic"
 )
 
+func TestCacheKey(t *testing.T) {
+	blockID := ulid.MustNew(1, nil)
+	seed := "seed123"
+	matchers := []*labels.Matcher{
+		{
+			Type:  labels.MatchEqual,
+			Name:  "name_1",
+			Value: "value_1",
+		},
+		{
+			Type:  labels.MatchNotEqual,
+			Name:  "name_2",
+			Value: "value_2",
+		},
+		{
+			Type:  labels.MatchRegexp,
+			Name:  "name_3",
+			Value: "value_4",
+		},
+		{
+			Type:  labels.MatchNotRegexp,
+			Name:  "name_5",
+			Value: "value_4",
+		},
+	}
+	r := cacheKey(seed, blockID, matchers...)
+	require.Equal(t, "seed123|00000000010000000000000000|name_1=value_1|name_2!=value_2|name_3=~value_4|name_5!~value_4|", r)
+}
+
 func Test_ShouldFetchPromiseOnlyOnce(t *testing.T) {
 	cfg := PostingsCacheConfig{
 		Enabled:  true,