Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[query] Update Graphite find calls to complete unterminated queries with all possible path values for ** #3593

Merged
merged 10 commits into from
Jul 12, 2021
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[
{
"id": "a.**.daz",
"text": "daz",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "a.**.cake",
"text": "cake",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "a.**.caw",
"text": "caw",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
[
{
"id": "a**.caw",
"text": "caw",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "a**.daz",
"text": "daz",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "a**.bag",
"text": "bag",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "a**.bar",
"text": "bar",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "a**.biz",
"text": "biz",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "a**.cake",
"text": "cake",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[
{
"id": "qux.**.pos2-1",
"text": "pos2-1",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "qux.**.pos2-0",
"text": "pos2-0",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
[
{
"id": "qux**.pos2-1",
"text": "pos2-1",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "qux**.pos1-a",
"text": "pos1-a",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "qux**.pos1-b",
"text": "pos1-b",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "qux**.pos1-c",
"text": "pos1-c",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
},
{
"id": "qux**.pos2-0",
"text": "pos2-0",
"leaf": 0,
"expandable": 1,
"allowChildren": 1
}
]
4 changes: 4 additions & 0 deletions scripts/docker-integration-tests/carbon/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'f.bar.*' fbaz.json"
ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'g.bar.*' gbaz.json"
ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'h.bar*' hbarbaz.json"
ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'i.bar*' ibarbaz.json"
ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'a**.*' a_starstar_dot_star.json"
ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'a.**.*' a_dot_starstar_dot_star.json"
ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'qux**.*' qux_starstar_dot_star.json"
ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'qux.**.*' qux_dot_starstar_dot_star.json"

# Test find limits from config of matching max docs of 200 with:
# carbon:
Expand Down
56 changes: 38 additions & 18 deletions src/query/api/v1/handler/graphite/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,33 @@ func mergeTags(
terminatedResult *consolidators.CompleteTagsResult,
childResult *consolidators.CompleteTagsResult,
) (map[string]nodeDescriptor, error) {
// sanity check the case.
if terminatedResult.CompleteNameOnly {
return nil, errors.New("terminated result is completing name only")
var (
terminatedResultTags = 0
childResultTags = 0
)

// Sanity check the queries.
if terminatedResult != nil {
terminatedResultTags = len(terminatedResult.CompletedTags)
if terminatedResult.CompleteNameOnly {
return nil, errors.New("terminated result is completing name only")
}
}

if childResult.CompleteNameOnly {
childResultTags = len(childResult.CompletedTags)
robskillington marked this conversation as resolved.
Show resolved Hide resolved
return nil, errors.New("child result is completing name only")
}

mapLength := len(terminatedResult.CompletedTags) + len(childResult.CompletedTags)
tagMap := make(map[string]nodeDescriptor, mapLength)

for _, tag := range terminatedResult.CompletedTags {
for _, value := range tag.Values {
descriptor := tagMap[string(value)]
descriptor.isLeaf = true
tagMap[string(value)] = descriptor
size := terminatedResultTags + childResultTags
robskillington marked this conversation as resolved.
Show resolved Hide resolved
tagMap := make(map[string]nodeDescriptor, size)
if terminatedResult != nil {
for _, tag := range terminatedResult.CompletedTags {
for _, value := range tag.Values {
descriptor := tagMap[string(value)]
descriptor.isLeaf = true
tagMap[string(value)] = descriptor
}
}
}

Expand Down Expand Up @@ -120,7 +130,7 @@ func (h *grahiteFindHandler) ServeHTTP(
// NB: need to run two separate queries, one of which will match only the
// provided matchers, and one which will match the provided matchers with at
// least one more child node. For further information, refer to the comment
// for parseFindParamsToQueries
// for parseFindParamsToQueries.
terminatedQuery, childQuery, raw, err := parseFindParamsToQueries(r)
if err != nil {
xhttp.WriteError(w, err)
Expand All @@ -134,11 +144,17 @@ func (h *grahiteFindHandler) ServeHTTP(
cErr error
wg sync.WaitGroup
)
wg.Add(2)
go func() {
terminatedResult, tErr = h.storage.CompleteTags(ctx, terminatedQuery, opts)
wg.Done()
}()
if terminatedQuery != nil {
// Sometimes we only perform the child query, so only perform
// terminated query if not nil.
wg.Add(1)
go func() {
terminatedResult, tErr = h.storage.CompleteTags(ctx, terminatedQuery, opts)
wg.Done()
}()
}
// Always perform child query.
wg.Add(1)
go func() {
childResult, cErr = h.storage.CompleteTags(ctx, childQuery, opts)
wg.Done()
Expand All @@ -152,7 +168,11 @@ func (h *grahiteFindHandler) ServeHTTP(
return
}

meta := terminatedResult.Metadata.CombineMetadata(childResult.Metadata)
meta := childResult.Metadata
if terminatedResult != nil {
meta = terminatedResult.Metadata.CombineMetadata(childResult.Metadata)
}

// NB: merge results from both queries to specify which series have children
seenMap, err := mergeTags(terminatedResult, childResult)
if err != nil {
Expand Down
37 changes: 36 additions & 1 deletion src/query/api/v1/handler/graphite/find_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"fmt"
"io"
"net/http"
"strings"
"time"

"github.com/m3db/m3/src/query/errors"
Expand Down Expand Up @@ -96,12 +97,46 @@ func parseFindParamsToQueries(r *http.Request) (
xerrors.NewInvalidParamsError(fmt.Errorf("invalid 'until': %s", untilString))
}

matchers, err := graphitestorage.TranslateQueryToMatchersWithTerminator(query)
matchers, queryType, err := graphitestorage.TranslateQueryToMatchersWithTerminator(query)
if err != nil {
return nil, nil, "",
xerrors.NewInvalidParamsError(fmt.Errorf("invalid 'query': %s", query))
}

switch queryType {
case graphitestorage.StarStarUnterminatedTranslatedQuery:
// Translated query for "**" has unterminated search for children
// terms, we are only going to do a single search and assume all
// results that come back have also children in the graphite path
// tree (since it's very expensive to check if each result that comes
// back is a child or leaf node and "**"" in a find query is typically
robskillington marked this conversation as resolved.
Show resolved Hide resolved
// only used for template variables rather than searching for metric
// results, which is the only use case isLeaf/hasChildren is useful).
// Note: Filter to all graphite tags that appears at the last node
// or greater than that (we use 100 as an arbitrary upper bound).
maxPathIndexes := 100
filter := make([][]byte, 0, maxPathIndexes)
parts := 1 + strings.Count(query, ".")
firstPathIndex := parts - 1
for i := firstPathIndex; i < firstPathIndex+maxPathIndexes; i++ {
filter = append(filter, graphite.TagName(i))
}
childQuery := &storage.CompleteTagsQuery{
CompleteNameOnly: false,
FilterNameTags: filter,
TagMatchers: matchers,
Start: xtime.ToUnixNano(from),
End: xtime.ToUnixNano(until),
}
return nil, childQuery, query, nil
case graphitestorage.TerminatedTranslatedQuery:
// Default type of translated query, explicitly craft queries for
// a terminated part of the query and a child part of the query.
break
default:
return nil, nil, "", fmt.Errorf("unknown query type: %v", queryType)
}

// NB: Filter will always be the second last term in the matchers, and the
// matchers should always have a length of at least 2 (term + terminator)
// so this is a sanity check and unexpected in actual execution.
Expand Down
32 changes: 24 additions & 8 deletions src/query/graphite/storage/m3_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,31 @@ func NewM3WrappedStorage(
}
}

// TranslatedQueryType describes a translated query type.
type TranslatedQueryType uint

const (
// TerminatedTranslatedQuery is a query that is terminated at an explicit
// leaf node (i.e. specific graphite path index number).
TerminatedTranslatedQuery TranslatedQueryType = iota
// StarStarUnterminatedTranslatedQuery is a query that is not terminated by
// an explicit leaf node since it matches indefinite child nodes due to
// a "**" in the query which matches indefinited child nodes.
StarStarUnterminatedTranslatedQuery
)

// TranslateQueryToMatchersWithTerminator converts a graphite query to tag
// matcher pairs, and adds a terminator matcher to the end.
func TranslateQueryToMatchersWithTerminator(
query string,
) (models.Matchers, error) {
) (models.Matchers, TranslatedQueryType, error) {
translatedQueryType := TerminatedTranslatedQuery
robskillington marked this conversation as resolved.
Show resolved Hide resolved
if strings.Contains(query, "**") {
translatedQueryType = StarStarUnterminatedTranslatedQuery
// First add matcher to ensure it's a graphite metric with __g0__ tag.
hasFirstPathMatcher, err := convertMetricPartToMatcher(0, wildcard)
if err != nil {
return nil, err
return nil, translatedQueryType, err
}
// Need to regexp on the entire ID since ** matches over different
// graphite path dimensions.
Expand All @@ -110,7 +125,7 @@ func TranslateQueryToMatchersWithTerminator(
}
idRegexp, _, err := graphite.ExtendedGlobToRegexPattern(query, globOpts)
if err != nil {
return nil, err
return nil, translatedQueryType, err
}
return models.Matchers{
hasFirstPathMatcher,
Expand All @@ -119,7 +134,7 @@ func TranslateQueryToMatchersWithTerminator(
Name: doc.IDReservedFieldName,
Value: idRegexp,
},
}, nil
}, translatedQueryType, nil
}

metricLength := graphite.CountMetricParts(query)
Expand All @@ -131,19 +146,20 @@ func TranslateQueryToMatchersWithTerminator(
if len(metric) > 0 {
m, err := convertMetricPartToMatcher(i, metric)
if err != nil {
return nil, err
return nil, translatedQueryType, err
}

matchers[i] = m
} else {
return nil, fmt.Errorf("invalid matcher format: %s", query)
err := fmt.Errorf("invalid matcher format: %s", query)
return nil, translatedQueryType, err
}
}

// Add a terminator matcher at the end to ensure expansion is terminated at
// the last given metric part.
matchers[metricLength] = matcherTerminator(metricLength)
return matchers, nil
return matchers, translatedQueryType, nil
}

// GetQueryTerminatorTagName will return the name for the terminator matcher in
Expand All @@ -158,7 +174,7 @@ func translateQuery(
fetchOpts FetchOptions,
opts M3WrappedStorageOptions,
) (*storage.FetchQuery, error) {
matchers, err := TranslateQueryToMatchersWithTerminator(query)
matchers, _, err := TranslateQueryToMatchersWithTerminator(query)
if err != nil {
return nil, err
}
Expand Down