Skip to content

Commit

Permalink
disable absent and absent_over_time for vertical sharding (#6201)
Browse files Browse the repository at this point in the history
  • Loading branch information
yeya24 authored Mar 10, 2023
1 parent 8ff2753 commit c88f5a3
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re

### Changed
- [#6168](https://github.com/thanos-io/thanos/pull/6168) Receiver: Make ketama hashring fail early when configured with number of nodes lower than the replication factor.
- [#6201](https://github.com/thanos-io/thanos/pull/6201) Query-Frontend: Disable absent and absent_over_time for vertical sharding.

### Removed

Expand Down
18 changes: 17 additions & 1 deletion pkg/querysharding/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,17 @@
package querysharding

import (
"fmt"

lru "github.com/hashicorp/golang-lru"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/promql/parser"
)

var (
notShardableErr = fmt.Errorf("expressions are not shardable")
)

type Analyzer interface {
Analyze(string) (QueryAnalysis, error)
}
Expand Down Expand Up @@ -72,7 +78,9 @@ func (a *CachedQueryAnalyzer) Analyze(query string) (QueryAnalysis, error) {

// Analyze uses the following algorithm:
// - if a query has functions which cannot be sharded such as
// label_join or label_replace, then treat the query as non shardable.
// absent or absent_over_time, then treat the query as non shardable.
// - if a query has functions `label_join` or `label_replace`,
// calculate the shard labels based on grouping labels.
// - Walk the query and find the least common labelset
// used in grouping expressions. If non-empty, treat the query
// as shardable by those labels.
Expand All @@ -89,13 +97,17 @@ func (a *QueryAnalyzer) Analyze(query string) (QueryAnalysis, error) {
analysis QueryAnalysis
dynamicLabels []string
)
isShardable := true
parser.Inspect(expr, func(node parser.Node, nodes []parser.Node) error {
switch n := node.(type) {
case *parser.Call:
if n.Func != nil {
if n.Func.Name == "label_join" || n.Func.Name == "label_replace" {
dstLabel := stringFromArg(n.Args[1])
dynamicLabels = append(dynamicLabels, dstLabel)
} else if n.Func.Name == "absent_over_time" || n.Func.Name == "absent" {
isShardable = false
return notShardableErr
}
}
case *parser.BinaryExpr:
Expand All @@ -117,6 +129,10 @@ func (a *QueryAnalyzer) Analyze(query string) (QueryAnalysis, error) {
return nil
})

if !isShardable {
return nonShardableQuery(), nil
}

// If currently it is shard by, it is still shardable if there is
// any label left after removing the dynamic labels.
// If currently it is shard without, it is still shardable if we
Expand Down
8 changes: 8 additions & 0 deletions pkg/querysharding/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ http_requests_total`,
name: "aggregate by expression with label_join, sharding label is dynamic",
expression: `sum by (dst_label) (label_join(metric, "dst_label", ",", "src_label"))`,
},
{
name: "absent_over_time is not shardable",
expression: `sum by (url) (absent_over_time(http_requests_total{code="400"}[5m]))`,
},
{
name: "absent is not shardable",
expression: `sum by (url) (absent(http_requests_total{code="400"}))`,
},
}

shardableByLabels := []testCase{
Expand Down

0 comments on commit c88f5a3

Please sign in to comment.