optimize regex matching for empty label values in posting match #14075
Conversation
Signed-off-by: Ben Ye <benye@amazon.com>
|
Hmm, this looks good to me at first glance, but I don't get the line of argument that this is only possible because of a prior optimization. Wasn't it always the case that Maybe I'm missing something. @bboreham you probably have more context. |
Without the empty string matcher optimization, I remember we cannot simply convert |
|
Regexp's in PromQL have always been fully anchored. So |
| @@ -354,8 +354,9 @@ func inversePostingsForMatcher(ctx context.Context, ix IndexReader, m *labels.Ma | |||
| } | |||
|
|
|||
| res := vals[:0] | |||
| // If the inverse match is ="", we just want all the values. | |||
| if m.Type == labels.MatchEqual && m.Value == "" { | |||
| // If the inverse match is ="" or =~"", we just want all the values. | |||
There was a problem hiding this comment.
This made me stop and think. Is this clearer?
| // If the inverse match is ="" or =~"", we just want all the values. | |
| // If the match before inversion was !="" or !~"", we just want all the values. |
There was a problem hiding this comment.
Thanks! Addressed in latest commit.
|
Is there a test covering this case? |
I don't think so. |
Then maybe I misunderstood and probably it is unrelated to https://github.com/prometheus/prometheus/blob/main/model/labels/regexp.go#L680. The optimization is always there just we didn't implement at the beginning. Updated: I mixed this one with the
I will update the test |
Signed-off-by: Ben Ye <benye@amazon.com>
|
Added a unit test case. |
I think the issue with those is that the |
Now
=~""is optimized into empty string matcher https://github.com/prometheus/prometheus/blob/main/model/labels/regexp.go#L680, which matches empty string only.Then we can skip matching because all label values should be non empty string.