Simplifying cache interface

Signed-off-by: alanprot <[email protected]>
thanos-io · Jan 3, 2025 · a5634c8 · a5634c8
1 parent 6b0121c
commit a5634c8
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 54 deletions.
diff --git a/pkg/store/cache/matchers_cache.go b/pkg/store/cache/matchers_cache.go
@@ -4,26 +4,23 @@
 package storecache
 
 import (
-	"fmt"
-
 	lru "github.com/hashicorp/golang-lru/v2"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"github.com/prometheus/prometheus/model/labels"
 	"golang.org/x/sync/singleflight"
 
 	"github.com/thanos-io/thanos/pkg/store/storepb"
-	"github.com/thanos-io/thanos/pkg/store/storepb/prompb"
 )
 
 const DefaultCacheSize = 200
 
-type NewItemFunc func(matcher ConversionLabelMatcher) (*labels.Matcher, error)
+type NewItemFunc func() (*labels.Matcher, error)
 
 type MatchersCache interface {
 	// GetOrSet retrieves a matcher from cache or creates and stores it if not present.
 	// If the matcher is not in cache, it uses the provided newItem function to create it.
-	GetOrSet(key ConversionLabelMatcher, newItem NewItemFunc) (*labels.Matcher, error)
+	GetOrSet(key string, newItem NewItemFunc) (*labels.Matcher, error)
 }
 
 // Ensure implementations satisfy the interface.
@@ -41,8 +38,8 @@ func NewNoopMatcherCache() MatchersCache {
 }
 
 // GetOrSet implements MatchersCache by always creating a new matcher without caching.
-func (n *NoopMatcherCache) GetOrSet(key ConversionLabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) {
-	return newItem(key)
+func (n *NoopMatcherCache) GetOrSet(_ string, newItem NewItemFunc) (*labels.Matcher, error) {
+	return newItem()
 }
 
 // LruMatchersCache implements MatchersCache with an LRU cache and metrics.
@@ -87,20 +84,19 @@ func NewMatchersCache(opts ...MatcherCacheOption) (*LruMatchersCache, error) {
 	return cache, nil
 }
 
-func (c *LruMatchersCache) GetOrSet(key ConversionLabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) {
+func (c *LruMatchersCache) GetOrSet(key string, newItem NewItemFunc) (*labels.Matcher, error) {
 	c.metrics.requestsTotal.Inc()
-	ks := key.String()
-	v, err, _ := c.sf.Do(ks, func() (interface{}, error) {
-		if item, ok := c.cache.Get(ks); ok {
+	v, err, _ := c.sf.Do(key, func() (interface{}, error) {
+		if item, ok := c.cache.Get(key); ok {
 			c.metrics.hitsTotal.Inc()
 			return item, nil
 		}
 
-		item, err := newItem(key)
+		item, err := newItem()
 		if err != nil {
 			return nil, err
 		}
-		c.cache.Add(ks, item)
+		c.cache.Add(key, item)
 		c.metrics.numItems.Set(float64(c.cache.Len()))
 		return item, nil
 	})
@@ -155,32 +151,11 @@ func newMatcherCacheMetrics(reg prometheus.Registerer) *matcherCacheMetrics {
 func MatchersToPromMatchersCached(cache MatchersCache, ms ...storepb.LabelMatcher) ([]*labels.Matcher, error) {
 	res := make([]*labels.Matcher, 0, len(ms))
 	for i := range ms {
-		pm, err := cache.GetOrSet(&ms[i], MatcherToPromMatcher)
+		pm, err := cache.GetOrSet(ms[i].String(), func() (*labels.Matcher, error) { return storepb.MatcherToPromMatcher(ms[i]) })
 		if err != nil {
 			return nil, err
 		}
 		res = append(res, pm)
 	}
 	return res, nil
 }
-
-func MatcherToPromMatcher(m ConversionLabelMatcher) (*labels.Matcher, error) {
-	mi, ok := m.(*storepb.LabelMatcher)
-	if !ok {
-		return nil, fmt.Errorf("invalid matcher type. Got: %T", m)
-	}
-
-	return storepb.MatcherToPromMatcher(*mi)
-}
-
-// ConversionLabelMatcher is a common interface for the Prometheus and Thanos label matchers.
-type ConversionLabelMatcher interface {
-	String() string
-	GetName() string
-	GetValue() string
-}
-
-var (
-	_ ConversionLabelMatcher = (*storepb.LabelMatcher)(nil)
-	_ ConversionLabelMatcher = (*prompb.LabelMatcher)(nil)
-)
diff --git a/pkg/store/cache/matchers_cache_test.go b/pkg/store/cache/matchers_cache_test.go
@@ -36,51 +36,53 @@ func TestMatchersCache(t *testing.T) {
 	}
 
 	var cacheHit bool
-	newItem := func(matcher storecache.ConversionLabelMatcher) (*labels.Matcher, error) {
-		cacheHit = false
-		return storecache.MatcherToPromMatcher(matcher)
+	newItem := func(matcher *storepb.LabelMatcher) func() (*labels.Matcher, error) {
+		return func() (*labels.Matcher, error) {
+			cacheHit = false
+			return storecache.MatcherToPromMatcher(matcher)
+		}
 	}
 	expected := labels.MustNewMatcher(labels.MatchEqual, "key", "val")
 	expected2 := labels.MustNewMatcher(labels.MatchRegexp, "key2", "val2|val3")
 	expected3 := labels.MustNewMatcher(labels.MatchEqual, "key3", "val3")
 
-	item, err := cache.GetOrSet(cloneMatcher(matcher), newItem)
+	item, err := cache.GetOrSet(matcher.String(), newItem(matcher))
 	testutil.Ok(t, err)
 	testutil.Equals(t, false, cacheHit)
 	testutil.Equals(t, expected.String(), item.String())
 
 	cacheHit = true
-	item, err = cache.GetOrSet(cloneMatcher(matcher), newItem)
+	item, err = cache.GetOrSet(matcher.String(), newItem(matcher))
 	testutil.Ok(t, err)
 	testutil.Equals(t, true, cacheHit)
 	testutil.Equals(t, expected.String(), item.String())
 
 	cacheHit = true
-	item, err = cache.GetOrSet(cloneMatcher(matcher2), newItem)
+	item, err = cache.GetOrSet(matcher2.String(), newItem(matcher2))
 	testutil.Ok(t, err)
 	testutil.Equals(t, false, cacheHit)
 	testutil.Equals(t, expected2.String(), item.String())
 
 	cacheHit = true
-	item, err = cache.GetOrSet(cloneMatcher(matcher2), newItem)
+	item, err = cache.GetOrSet(matcher2.String(), newItem(matcher2))
 	testutil.Ok(t, err)
 	testutil.Equals(t, true, cacheHit)
 	testutil.Equals(t, expected2.String(), item.String())
 
 	cacheHit = true
-	item, err = cache.GetOrSet(cloneMatcher(matcher), newItem)
+	item, err = cache.GetOrSet(matcher.String(), newItem(matcher))
 	testutil.Ok(t, err)
 	testutil.Equals(t, true, cacheHit)
 	testutil.Equals(t, expected, item)
 
 	cacheHit = true
-	item, err = cache.GetOrSet(cloneMatcher(matcher3), newItem)
+	item, err = cache.GetOrSet(matcher3.String(), newItem(matcher3))
 	testutil.Ok(t, err)
 	testutil.Equals(t, false, cacheHit)
 	testutil.Equals(t, expected3, item)
 
 	cacheHit = true
-	item, err = cache.GetOrSet(cloneMatcher(matcher2), newItem)
+	item, err = cache.GetOrSet(matcher2.String(), newItem(matcher2))
 	testutil.Ok(t, err)
 	testutil.Equals(t, false, cacheHit)
 	testutil.Equals(t, expected2.String(), item.String())
@@ -104,17 +106,11 @@ func BenchmarkMatchersCache(b *testing.B) {
 	b.ReportAllocs()
 	for i := 0; i < b.N; i++ {
 		matcher := matchers[i%len(matchers)]
-		_, err := cache.GetOrSet(matcher, storecache.MatcherToPromMatcher)
+		_, err := cache.GetOrSet(matcher.String(), func() (*labels.Matcher, error) {
+			return storecache.MatcherToPromMatcher(matcher)
+		})
 		if err != nil {
 			b.Fatalf("failed to get or set cache item: %v", err)
 		}
 	}
 }
-
-func cloneMatcher(m *storepb.LabelMatcher) *storepb.LabelMatcher {
-	return &storepb.LabelMatcher{
-		Type:  m.Type,
-		Name:  m.Name,
-		Value: m.Value,
-	}
-}