From 417104dd0c803b6a94b99a8d3e7b4612dab1a80e Mon Sep 17 00:00:00 2001 From: Pedro Tanaka Date: Mon, 9 Dec 2024 17:37:31 +0100 Subject: [PATCH] Trying to make the cache more reusable introducing interface Signed-off-by: Pedro Tanaka --- pkg/store/cache/matcher_cache_test.go | 12 ++++---- pkg/store/cache/matchers_cache.go | 20 ++++++------- pkg/store/matcher_cache.go | 1 - pkg/store/storepb/custom.go | 42 +++++++++++++++++++++++++-- 4 files changed, 55 insertions(+), 20 deletions(-) delete mode 100644 pkg/store/matcher_cache.go diff --git a/pkg/store/cache/matcher_cache_test.go b/pkg/store/cache/matcher_cache_test.go index c99d4c9cd8..1951d0aaab 100644 --- a/pkg/store/cache/matcher_cache_test.go +++ b/pkg/store/cache/matcher_cache_test.go @@ -17,26 +17,26 @@ func TestMatchersCache(t *testing.T) { cache, err := storecache.NewMatchersCache(storecache.WithSize(2)) testutil.Ok(t, err) - matcher := storepb.LabelMatcher{ + matcher := &storepb.LabelMatcher{ Type: storepb.LabelMatcher_EQ, Name: "key", Value: "val", } - matcher2 := storepb.LabelMatcher{ + matcher2 := &storepb.LabelMatcher{ Type: storepb.LabelMatcher_RE, Name: "key2", Value: "val2|val3", } - matcher3 := storepb.LabelMatcher{ + matcher3 := &storepb.LabelMatcher{ Type: storepb.LabelMatcher_EQ, Name: "key3", Value: "val3", } var cacheHit bool - newItem := func(matcher storepb.LabelMatcher) (*labels.Matcher, error) { + newItem := func(matcher storepb.ConversionLabelMatcher) (*labels.Matcher, error) { cacheHit = false return storepb.MatcherToPromMatcher(matcher) } @@ -92,7 +92,7 @@ func BenchmarkMatchersCache(b *testing.B) { b.Fatalf("failed to create cache: %v", err) } - matchers := []storepb.LabelMatcher{ + matchers := []*storepb.LabelMatcher{ {Type: storepb.LabelMatcher_EQ, Name: "key1", Value: "val1"}, {Type: storepb.LabelMatcher_EQ, Name: "key2", Value: "val2"}, {Type: storepb.LabelMatcher_EQ, Name: "key3", Value: "val3"}, @@ -100,7 +100,7 @@ func BenchmarkMatchersCache(b *testing.B) { {Type: storepb.LabelMatcher_RE, Name: "key5", Value: "^(val5|val6|val7|val8|val9).*$"}, } - newItem := func(matcher storepb.LabelMatcher) (*labels.Matcher, error) { + newItem := func(matcher storepb.ConversionLabelMatcher) (*labels.Matcher, error) { return storepb.MatcherToPromMatcher(matcher) } diff --git a/pkg/store/cache/matchers_cache.go b/pkg/store/cache/matchers_cache.go index 11f44c6bc2..124c12f7ef 100644 --- a/pkg/store/cache/matchers_cache.go +++ b/pkg/store/cache/matchers_cache.go @@ -1,7 +1,7 @@ package storecache import ( - "github.com/hashicorp/golang-lru/v2" + 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" @@ -12,12 +12,12 @@ import ( const DefaultCacheSize = 200 -type NewItemFunc func(matcher storepb.LabelMatcher) (*labels.Matcher, error) +type NewItemFunc func(matcher storepb.ConversionLabelMatcher) (*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 storepb.LabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) + GetOrSet(key storepb.ConversionLabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) } // Ensure implementations satisfy the interface. @@ -35,14 +35,14 @@ func NewNoopMatcherCache() MatchersCache { } // GetOrSet implements MatchersCache by always creating a new matcher without caching. -func (n *NoopMatcherCache) GetOrSet(key storepb.LabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) { +func (n *NoopMatcherCache) GetOrSet(key storepb.ConversionLabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) { return newItem(key) } // LruMatchersCache implements MatchersCache with an LRU cache and metrics. type LruMatchersCache struct { reg prometheus.Registerer - cache *lru.Cache[storepb.LabelMatcher, *labels.Matcher] + cache *lru.Cache[storepb.ConversionLabelMatcher, *labels.Matcher] metrics *matcherCacheMetrics size int sf singleflight.Group @@ -73,7 +73,7 @@ func NewMatchersCache(opts ...MatcherCacheOption) (*LruMatchersCache, error) { } cache.metrics = newMatcherCacheMetrics(cache.reg) - lruCache, err := lru.NewWithEvict[storepb.LabelMatcher, *labels.Matcher](cache.size, cache.onEvict) + lruCache, err := lru.NewWithEvict[storepb.ConversionLabelMatcher, *labels.Matcher](cache.size, cache.onEvict) if err != nil { return nil, err } @@ -82,7 +82,7 @@ func NewMatchersCache(opts ...MatcherCacheOption) (*LruMatchersCache, error) { return cache, nil } -func (c *LruMatchersCache) GetOrSet(key storepb.LabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) { +func (c *LruMatchersCache) GetOrSet(key storepb.ConversionLabelMatcher, newItem NewItemFunc) (*labels.Matcher, error) { c.metrics.requestsTotal.Inc() if item, ok := c.cache.Get(key); ok { c.metrics.hitsTotal.Inc() @@ -110,7 +110,7 @@ func (c *LruMatchersCache) GetOrSet(key storepb.LabelMatcher, newItem NewItemFun return v.(*labels.Matcher), nil } -func (c *LruMatchersCache) onEvict(_ storepb.LabelMatcher, _ *labels.Matcher) { +func (c *LruMatchersCache) onEvict(_ storepb.ConversionLabelMatcher, _ *labels.Matcher) { c.metrics.evicted.Inc() c.metrics.numItems.Set(float64(c.cache.Len())) } @@ -153,8 +153,8 @@ func newMatcherCacheMetrics(reg prometheus.Registerer) *matcherCacheMetrics { // NOTE: It (can) allocate memory. func MatchersToPromMatchersCached(cache MatchersCache, ms ...storepb.LabelMatcher) ([]*labels.Matcher, error) { res := make([]*labels.Matcher, 0, len(ms)) - for _, m := range ms { - pm, err := cache.GetOrSet(m, storepb.MatcherToPromMatcher) + for i := range ms { + pm, err := cache.GetOrSet(&ms[i], storepb.MatcherToPromMatcher) if err != nil { return nil, err } diff --git a/pkg/store/matcher_cache.go b/pkg/store/matcher_cache.go deleted file mode 100644 index 72440ea2a6..0000000000 --- a/pkg/store/matcher_cache.go +++ /dev/null @@ -1 +0,0 @@ -package store diff --git a/pkg/store/storepb/custom.go b/pkg/store/storepb/custom.go index 46ce34ed57..b4a4325bce 100644 --- a/pkg/store/storepb/custom.go +++ b/pkg/store/storepb/custom.go @@ -17,6 +17,7 @@ import ( "google.golang.org/grpc/codes" "github.com/thanos-io/thanos/pkg/store/labelpb" + "github.com/thanos-io/thanos/pkg/store/storepb/prompb" ) var PartialResponseStrategyValues = func() []string { @@ -385,8 +386,8 @@ func PromMatchersToMatchers(ms ...*labels.Matcher) ([]LabelMatcher, error) { // NOTE: It allocates memory. func MatchersToPromMatchers(ms ...LabelMatcher) ([]*labels.Matcher, error) { res := make([]*labels.Matcher, 0, len(ms)) - for _, m := range ms { - pm, err := MatcherToPromMatcher(m) + for i := range ms { + pm, err := MatcherToPromMatcher(&ms[i]) if err != nil { return nil, err } @@ -396,7 +397,11 @@ func MatchersToPromMatchers(ms ...LabelMatcher) ([]*labels.Matcher, error) { } // MatcherToPromMatcher converts a Thanos label matcher to Prometheus label matcher. -func MatcherToPromMatcher(m LabelMatcher) (*labels.Matcher, error) { +func MatcherToPromMatcher(mi ConversionLabelMatcher) (*labels.Matcher, error) { + var m, ok = mi.(*LabelMatcher) + if !ok { + return nil, errors.Errorf("unexpected matcher type %T", mi) + } var t labels.MatchType switch m.Type { @@ -444,6 +449,37 @@ func (m *LabelMatcher) PromString() string { return fmt.Sprintf("%s%s%q", m.Name, m.Type.PromString(), m.Value) } +func (m *LabelMatcher) GetName() string { + return m.Name +} + +func (m *LabelMatcher) GetValue() string { + return m.Value +} + +func (m *LabelMatcher) GetType() prompb.LabelMatcher_Type { + switch m.Type { + case LabelMatcher_EQ: + return prompb.LabelMatcher_EQ + case LabelMatcher_NEQ: + return prompb.LabelMatcher_NEQ + case LabelMatcher_RE: + return prompb.LabelMatcher_RE + case LabelMatcher_NRE: + return prompb.LabelMatcher_NRE + default: + return prompb.LabelMatcher_EQ + } +} + +// ConversionLabelMatcher is a common interface for the Prometheus and Thanos label matchers. +type ConversionLabelMatcher interface { + String() string + GetName() string + GetType() prompb.LabelMatcher_Type + GetValue() string +} + func (x LabelMatcher_Type) PromString() string { typeToStr := map[LabelMatcher_Type]string{ LabelMatcher_EQ: "=",