From 84841a4c4ea5ac4ef623eb7ad62ae585d3bf9af8 Mon Sep 17 00:00:00 2001 From: Patrick Oyarzun Date: Tue, 2 Jan 2024 13:42:01 -0600 Subject: [PATCH] Fix regexp set matches for literal matchers I was surprised to find out that posting lookups for `foo=~"(bar|bar)"` are faster than `foo=~"bar"`. It turns out we introduced a performance regression in https://github.com/grafana/mimir-prometheus/pull/463. When we added the `optimizeAlternatingLiterals` function, we subtly broke one edge case. A regexp matcher which matches a single literal, like `foo=~"bar"` used to return `bar` from `SetMatches()`, but currently does not. The implication is that the tsdb will first do a LabelValues call to get all values for `foo`, then match them against the regexp `bar`. This PR restores the previous behavior which is able to directly lookup postings for `foo="bar"` instead. --- model/labels/regexp.go | 2 +- model/labels/regexp_test.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/model/labels/regexp.go b/model/labels/regexp.go index de4cfc3b1c..e6909dbeac 100644 --- a/model/labels/regexp.go +++ b/model/labels/regexp.go @@ -366,7 +366,7 @@ func optimizeAlternatingLiterals(s string) (StringMatcher, []string) { // If there are no alternates, check if the string is a literal if estimatedAlternates == 1 { if regexp.QuoteMeta(s) == s { - return &equalStringMatcher{s: s, caseSensitive: true}, nil + return &equalStringMatcher{s: s, caseSensitive: true}, []string{s} } return nil, nil } diff --git a/model/labels/regexp_test.go b/model/labels/regexp_test.go index f0e21b2d5c..a03bbd261d 100644 --- a/model/labels/regexp_test.go +++ b/model/labels/regexp_test.go @@ -342,6 +342,14 @@ func TestFindSetMatches(t *testing.T) { matches, actualCaseSensitive := findSetMatches(parsed) require.Equal(t, c.expMatches, matches) require.Equal(t, c.expCaseSensitive, actualCaseSensitive) + + if c.expCaseSensitive { + // When the regexp is case sensitive, we want to ensure that the + // set matches are maintained in the final matcher. + r, err := newFastRegexMatcherWithoutCache(c.pattern) + require.NoError(t, err) + require.Equal(t, c.expMatches, r.SetMatches()) + } }) } }