Skip to content

Commit

Permalink
Fix regexp set matches for literal matchers
Browse files Browse the repository at this point in the history
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 #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.
  • Loading branch information
Logiraptor committed Jan 2, 2024
1 parent 12d2c10 commit 84841a4
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion model/labels/regexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions model/labels/regexp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
})
}
}
Expand Down

0 comments on commit 84841a4

Please sign in to comment.