From 22f5ecdff7d55ca0b0aa1e0d24f33a44a2acdd30 Mon Sep 17 00:00:00 2001
From: alanprot <alanprot@gmail.com>
Date: Fri, 20 Dec 2024 15:47:45 -0800
Subject: [PATCH] Avoid over allocating strings.Builder when creating cache key
 for expanded postings

Signed-off-by: alanprot <alanprot@gmail.com>
---
 pkg/storage/tsdb/expanded_postings_cache.go   |  9 +++---
 .../tsdb/expanded_postings_cache_test.go      | 31 +++++++++++++++++++
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/pkg/storage/tsdb/expanded_postings_cache.go b/pkg/storage/tsdb/expanded_postings_cache.go
index 921881b5c5..57027cf2a4 100644
--- 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
index 6a9476072a..1c17a5ac56 100644
--- 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,