Skip to content

Commit

Permalink
[query] Special case particular Prom matchers (#2479)
Browse files Browse the repository at this point in the history
  • Loading branch information
arnikola authored Jul 23, 2020
1 parent db1f9c5 commit 6a2ee8c
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 24 deletions.
14 changes: 7 additions & 7 deletions scripts/docker-integration-tests/query_fanout/warning.sh
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ function test_range_query {
}

function test_search {
start=$(date -d "$(date +%Y-%m-%dT%H:%M:%SZ) -1 minute" +%Y-%m-%dT%H:%M:%SZ)
end=$(date -d "$(date +%Y-%m-%dT%H:%M:%SZ) +1 minute" +%Y-%m-%dT%H:%M:%SZ)

s=$(( $(date +%s) - 60 ))
start=$(date -r $s +%Y-%m-%dT%H:%M:%SZ)
e=$(( $(date +%s) + 60 ))
end=$(date -r $e +%Y-%m-%dT%H:%M:%SZ)
curl -D headers -X POST 0.0.0.0:7201/search -d '{
"start": "'$start'",
"end": "'$end'",
Expand Down Expand Up @@ -255,14 +256,14 @@ function test_fanout_warning_search {
# write 5 metrics to cluster a
write_metrics coordinator-cluster-a 5
# unlimited query against cluster a has no header
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 15
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 1000
# limited query against cluster a has header
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 4 max_fetch_series_limit_applied

# write 10 metrics to cluster b
write_metrics coordinator-cluster-b 10
# unlimited query against cluster a has no header
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 16
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 1000
# remote limited query against cluster a has header
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 4 max_fetch_series_limit_applied
}
Expand Down Expand Up @@ -357,7 +358,7 @@ function test_fanout_warning_missing_zone {
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_range_query 9 14 max_fetch_series_limit_applied,remote_store_cluster-c_fetch_data_error

METRIC_NAME=$SEARCH_NAME
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 16 remote_store_cluster-c_fetch_data_error
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 1000 remote_store_cluster-c_fetch_data_error
ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_search 4 max_fetch_series_limit_applied,remote_store_cluster-c_fetch_data_error

ATTEMPTS=3 TIMEOUT=1 retry_with_backoff test_labels 100 remote_store_cluster-c_fetch_data_error
Expand Down Expand Up @@ -391,4 +392,3 @@ function test_fanout_warnings {
test_fanout_warning_graphite
test_fanout_warning_missing_zone
}

5 changes: 3 additions & 2 deletions src/query/api/v1/handler/prom/read_instant.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,11 @@ func parseTime(s string) (time.Time, error) {
// Upstream issue: https://github.com/golang/go/issues/20555
switch s {
case minTimeFormatted:
return minTime, nil
return time.Time{}, nil
case maxTimeFormatted:
return maxTime, nil
return time.Now(), nil
}

return time.Time{}, fmt.Errorf("cannot parse %q to a valid timestamp", s)
}

Expand Down
107 changes: 95 additions & 12 deletions src/query/storage/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package storage

import (
"bytes"
"fmt"

"github.com/m3db/m3/src/dbnode/storage/index"
Expand All @@ -31,8 +30,10 @@ import (
"github.com/m3db/m3/src/x/ident"
)

var (
dotStar = []byte(".*")
const (
dot = byte('.')
plus = byte('+')
star = byte('*')
)

// FromM3IdentToMetric converts an M3 ident metric to a coordinator metric.
Expand Down Expand Up @@ -119,6 +120,18 @@ func FetchQueryToM3Query(

// Optimization for single matcher case.
if len(matchers) == 1 {
specialCase := isSpecialCaseMatcher(matchers[0])
if specialCase.skip {
// NB: only matcher has no effect; this is synonymous to an AllQuery.
return index.Query{
Query: idx.NewAllQuery(),
}, nil
}

if specialCase.isSpecial {
return index.Query{Query: specialCase.query}, nil
}

q, err := matcherToQuery(matchers[0])
if err != nil {
return index.Query{}, err
Expand All @@ -127,59 +140,129 @@ func FetchQueryToM3Query(
return index.Query{Query: q}, nil
}

idxQueries := make([]idx.Query, len(matchers))
var err error
for i, matcher := range matchers {
idxQueries[i], err = matcherToQuery(matcher)
idxQueries := make([]idx.Query, 0, len(matchers))
for _, matcher := range matchers {
specialCase := isSpecialCaseMatcher(matcher)
if specialCase.skip {
continue
}

if specialCase.isSpecial {
idxQueries = append(idxQueries, specialCase.query)
continue
}

q, err := matcherToQuery(matcher)
if err != nil {
return index.Query{}, err
}

idxQueries = append(idxQueries, q)
}

q := idx.NewConjunctionQuery(idxQueries...)

return index.Query{Query: q}, nil
}

type specialCase struct {
query idx.Query
isSpecial bool
skip bool
}

func isSpecialCaseMatcher(matcher models.Matcher) specialCase {
if len(matcher.Value) == 0 {
if matcher.Type == models.MatchNotRegexp ||
matcher.Type == models.MatchNotEqual {
query := idx.NewFieldQuery(matcher.Name)
return specialCase{query: query, isSpecial: true}
}

if matcher.Type == models.MatchRegexp ||
matcher.Type == models.MatchEqual {
query := idx.NewNegationQuery(idx.NewFieldQuery(matcher.Name))
return specialCase{query: query, isSpecial: true}
}

return specialCase{}
}

// NB: no special case for regex / not regex here.
isNegatedRegex := matcher.Type == models.MatchNotRegexp
isRegex := matcher.Type == models.MatchRegexp
if !isNegatedRegex && !isRegex {
return specialCase{}
}

if len(matcher.Value) != 2 && matcher.Value[0] != dot {
return specialCase{}
}

if matcher.Value[1] == star {
if isNegatedRegex {
// NB: This should match no results.
query := idx.NewNegationQuery(idx.NewAllQuery())
return specialCase{query: query, isSpecial: true}
}

// NB: this matcher should not affect query results.
return specialCase{skip: true}
}

if matcher.Value[1] == plus {
query := idx.NewFieldQuery(matcher.Name)
if isNegatedRegex {
query = idx.NewNegationQuery(query)
}

return specialCase{query: query, isSpecial: true}
}

return specialCase{}
}

func matcherToQuery(matcher models.Matcher) (idx.Query, error) {
negate := false
switch matcher.Type {
// Support for Regexp types
case models.MatchNotRegexp:
negate = true
fallthrough

case models.MatchRegexp:
var (
query idx.Query
err error
)
if bytes.Equal(dotStar, matcher.Value) {
query = idx.NewFieldQuery(matcher.Name)
} else {
query, err = idx.NewRegexpQuery(matcher.Name, matcher.Value)
}
query, err = idx.NewRegexpQuery(matcher.Name, matcher.Value)
if err != nil {
return idx.Query{}, err
}

if negate {
query = idx.NewNegationQuery(query)
}

return query, nil

// Support exact matches
case models.MatchNotEqual:
negate = true
fallthrough

case models.MatchEqual:
query := idx.NewTermQuery(matcher.Name, matcher.Value)
if negate {
query = idx.NewNegationQuery(query)
}

return query, nil

case models.MatchNotField:
negate = true
fallthrough

case models.MatchField:
query := idx.NewFieldQuery(matcher.Name)
if negate {
Expand Down
17 changes: 14 additions & 3 deletions src/query/storage/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ func TestFetchQueryToM3Query(t *testing.T) {
},
},
{
name: "regexp match -> field",
expected: "field(t1)",
name: "regexp match dot star -> field",
expected: "all()",
matchers: models.Matchers{
{
Type: models.MatchRegexp,
Expand All @@ -122,6 +122,17 @@ func TestFetchQueryToM3Query(t *testing.T) {
},
},
},
{
name: "regexp match dot plus -> field",
expected: "field(t1)",
matchers: models.Matchers{
{
Type: models.MatchRegexp,
Name: []byte("t1"),
Value: []byte(".+"),
},
},
},
{
name: "regexp match negated",
expected: "negation(regexp(t1, v1))",
Expand All @@ -135,7 +146,7 @@ func TestFetchQueryToM3Query(t *testing.T) {
},
{
name: "regexp match negated",
expected: "negation(field(t1))",
expected: "negation(all())",
matchers: models.Matchers{
{
Type: models.MatchNotRegexp,
Expand Down
43 changes: 43 additions & 0 deletions src/query/test/compatibility/testdata/selectors.test
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,46 @@ eval instant at 0s http_requests{foo!~"bar", job="api-server"}
eval instant at 0s http_requests{foo!~"bar", job="api-server", instance="1", x!="y", z="", group!=""}
http_requests{job="api-server", instance="1", group="production"} 0
http_requests{job="api-server", instance="1", group="canary"} 0

# check special casing for existing label
eval instant at 0s http_requests{job="", instance="0", group="production"}

eval instant at 0s http_requests{job!="", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{job=~"", instance="0", group="production"}

eval instant at 0s http_requests{job!~"", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{job=~".+", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{job=~".*", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{job!~".+", instance="0", group="production"}

eval instant at 0s http_requests{job!~".*", instance="0", group="production"}

# check special casing for non-existent label
eval instant at 0s http_requests{foo="", job="api-server", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{foo!="", job="api-server", instance="0", group="production"}

eval instant at 0s http_requests{foo=~"", job="api-server", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{foo!~"", job="api-server", instance="0", group="production"}

eval instant at 0s http_requests{foo=~".+", job="api-server", instance="0", group="production"}

eval instant at 0s http_requests{foo=~".*", job="api-server", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{foo!~".+", job="api-server", instance="0", group="production"}
http_requests{job="api-server", instance="0", group="production"} 0

eval instant at 0s http_requests{foo!~".*", job="api-server", instance="0", group="production"}

This comment has been minimized.

Copy link
@robskillington

robskillington Jul 23, 2020

Collaborator

🎉


0 comments on commit 6a2ee8c

Please sign in to comment.