From 664c125d87d2664373d9be8653bcef26bae61798 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Wed, 27 Sep 2023 18:33:18 +0200 Subject: [PATCH 1/2] LabelValues() with matchers should use cache When cache was introduced, LabelValues() could never be called with "concurrent" flag so it didn't make sense to use the cached call through the Head. However, since the introduction of forced cache, we should use it, as even with concurrent=false the cache may be used. Signed-off-by: Oleg Zaytsev --- tsdb/querier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tsdb/querier.go b/tsdb/querier.go index fe7f4c4a85..5c4a8a6983 100644 --- a/tsdb/querier.go +++ b/tsdb/querier.go @@ -380,7 +380,7 @@ func inversePostingsForMatcher(ctx context.Context, ix IndexPostingsReader, m *l const maxExpandedPostingsFactor = 100 // Division factor for maximum number of matched series. func labelValuesWithMatchers(ctx context.Context, r IndexReader, name string, matchers ...*labels.Matcher) ([]string, error) { - p, err := PostingsForMatchers(ctx, r, matchers...) + p, err := r.PostingsForMatchers(ctx, false, matchers...) if err != nil { return nil, errors.Wrap(err, "fetching postings for matchers") } From a8c31f279f6224a93fe03571b164e36ef71cef07 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Thu, 28 Sep 2023 10:22:37 +0200 Subject: [PATCH 2/2] Test that IndexReader.PostingsForMatchers is called Signed-off-by: Oleg Zaytsev --- tsdb/querier_test.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tsdb/querier_test.go b/tsdb/querier_test.go index 7250450652..9916febef2 100644 --- a/tsdb/querier_test.go +++ b/tsdb/querier_test.go @@ -2914,9 +2914,25 @@ func TestLabelsValuesWithMatchersOptimization(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - values, err := labelValuesWithMatchers(ctx, ir, c.labelName, c.matchers...) + cir := &indexReaderCountingPostingsForMatchersCalls{IndexReader: ir} + values, err := labelValuesWithMatchers(ctx, cir, c.labelName, c.matchers...) require.NoError(t, err) require.ElementsMatch(t, c.expectedResults, values) + require.Equal(t, 1, cir.postingsForMatchersCalls, + "expected PostingsForMatchers to be called once. "+ + "labelValuesWithMatchers should call the IndexReader.PostingsForMatchers instead of calling the package function PostingsForMatchers "+ + "because IndexReader may use the cached version of the PostingsForMatchers", + ) }) } } + +type indexReaderCountingPostingsForMatchersCalls struct { + IndexReader + postingsForMatchersCalls int +} + +func (f *indexReaderCountingPostingsForMatchersCalls) PostingsForMatchers(ctx context.Context, concurrent bool, ms ...*labels.Matcher) (index.Postings, error) { + f.postingsForMatchersCalls++ + return f.IndexReader.PostingsForMatchers(ctx, concurrent, ms...) +}