diff --git a/CHANGELOG.md b/CHANGELOG.md index 25245783749..6a1f2405922 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ * [ENHANCEMENT] Ingester: Add `-blocks-storage.tsdb.bigger-out-of-order-blocks-for-old-samples` to build 24h blocks for out-of-order data belonging to the previous days instead of building smaller 2h blocks. This reduces pressure on compactors and ingesters when the out-of-order samples span multiple days in the past. #9844 #10033 #10035 * [ENHANCEMENT] Distributor: allow a different limit for info series (series ending in `_info`) label count, via `-validation.max-label-names-per-info-series`. #10028 * [ENHANCEMENT] Ingester: do not reuse labels, samples and histograms slices in the write request if there are more entries than 10x the pre-allocated size. This should help to reduce the in-use memory in case of few requests with a very large number of labels, samples or histograms. #10040 +* [ENHANCEMENT] Query-Frontend: prune ` and on() (vector(x)==y)` style queries and stop pruning ` < -Inf`. Triggered by https://github.com/prometheus/prometheus/pull/15245. #10026 * [BUGFIX] Fix issue where functions such as `rate()` over native histograms could return incorrect values if a float stale marker was present in the selected range. #9508 * [BUGFIX] Fix issue where negation of native histograms (eg. `-some_native_histogram_series`) did nothing. #9508 * [BUGFIX] Fix issue where `metric might not be a counter, name does not end in _total/_sum/_count/_bucket` annotation would be emitted even if `rate` or `increase` did not have enough samples to compute a result. #9508 diff --git a/pkg/frontend/querymiddleware/astmapper/pruning.go b/pkg/frontend/querymiddleware/astmapper/pruning.go index 3040e44f83d..34c969f1dcb 100644 --- a/pkg/frontend/querymiddleware/astmapper/pruning.go +++ b/pkg/frontend/querymiddleware/astmapper/pruning.go @@ -4,8 +4,6 @@ package astmapper import ( "context" - "math" - "strconv" "github.com/go-kit/log" "github.com/prometheus/prometheus/promql/parser" @@ -33,185 +31,93 @@ func (pruner *queryPruner) MapExpr(expr parser.Expr) (mapped parser.Expr, finish return nil, false, err } - switch e := expr.(type) { - case *parser.ParenExpr: - mapped, finished, err = pruner.MapExpr(e.Expr) - if err != nil { - return e, false, err - } - return &parser.ParenExpr{Expr: mapped, PosRange: e.PosRange}, finished, nil - case *parser.BinaryExpr: - return pruner.pruneBinOp(e) - default: - return e, false, nil - } -} - -func (pruner *queryPruner) pruneBinOp(expr *parser.BinaryExpr) (mapped parser.Expr, finished bool, err error) { - switch expr.Op { - case parser.MUL: - return pruner.handleMultiplyOp(expr), false, nil - case parser.GTR, parser.LSS: - return pruner.handleCompOp(expr), false, nil - case parser.LOR: - return pruner.handleOrOp(expr), false, nil - case parser.LAND: - return pruner.handleAndOp(expr), false, nil - case parser.LUNLESS: - return pruner.handleUnlessOp(expr), false, nil - default: + e, ok := expr.(*parser.BinaryExpr) + if !ok { return expr, false, nil } -} -// The bool signifies if the number evaluates to infinity, and if it does -// we return the infinity of the correct sign. -func calcInf(isPositive bool, num string) (*parser.NumberLiteral, bool) { - coeff, err := strconv.Atoi(num) - if err != nil || coeff == 0 { - return nil, false - } - switch { - case isPositive && coeff > 0: - return &parser.NumberLiteral{Val: math.Inf(1)}, true - case isPositive && coeff < 0: - return &parser.NumberLiteral{Val: math.Inf(-1)}, true - case !isPositive && coeff > 0: - return &parser.NumberLiteral{Val: math.Inf(-1)}, true - case !isPositive && coeff < 0: - return &parser.NumberLiteral{Val: math.Inf(1)}, true - default: - return nil, false - } -} - -func (pruner *queryPruner) handleMultiplyOp(expr *parser.BinaryExpr) parser.Expr { - isInfR, signR := pruner.isInfinite(expr.RHS) - if isInfR { - newExpr, ok := calcInf(signR, expr.LHS.String()) - if ok { - return newExpr - } - } - isInfL, signL := pruner.isInfinite(expr.LHS) - if isInfL { - newExpr, ok := calcInf(signL, expr.RHS.String()) - if ok { - return newExpr - } - } - return expr -} - -func (pruner *queryPruner) handleCompOp(expr *parser.BinaryExpr) parser.Expr { - var refNeg, refPos parser.Expr - switch expr.Op { - case parser.LSS: - refNeg = expr.RHS - refPos = expr.LHS - case parser.GTR: - refNeg = expr.LHS - refPos = expr.RHS - default: - return expr + if e.Op != parser.LAND || e.VectorMatching == nil || + !e.VectorMatching.On || len(e.VectorMatching.MatchingLabels) != 0 { + // Return if not " and on() " + return expr, false, nil } - // foo < -Inf or -Inf > foo => vector(0) < -Inf - isInf, sign := pruner.isInfinite(refNeg) - if isInf && !sign { - return &parser.BinaryExpr{ - LHS: &parser.Call{ - Func: parser.Functions["vector"], - Args: []parser.Expr{&parser.NumberLiteral{Val: 0}}, - }, - Op: parser.LSS, - RHS: &parser.NumberLiteral{Val: math.Inf(-1)}, - ReturnBool: false, - } + isConst, isEmpty := pruner.isConst(e.RHS) + if !isConst { + return expr, false, nil } - - // foo > +Inf or +Inf < foo => vector(0) > +Inf => vector(0) < -Inf - isInf, sign = pruner.isInfinite(refPos) - if isInf && sign { - return &parser.BinaryExpr{ - LHS: &parser.Call{ - Func: parser.Functions["vector"], - Args: []parser.Expr{&parser.NumberLiteral{Val: 0}}, - }, - Op: parser.LSS, - RHS: &parser.NumberLiteral{Val: math.Inf(-1)}, - ReturnBool: false, - } + if isEmpty { + // The right hand side is empty, so the whole expression is empty due to + // "and on()", return the right hand side. + return e.RHS, false, nil } - - return expr + // The right hand side is const and not empty, so the whole expression is + // just the left side. + return e.LHS, false, nil } -// 1st bool is true if the number is infinite. -// 2nd bool is true if the number is positive infinity. -func (pruner *queryPruner) isInfinite(expr parser.Expr) (bool, bool) { - mapped, _, err := pruner.MapExpr(expr) - if err == nil { - expr = mapped - } +func (pruner *queryPruner) isConst(expr parser.Expr) (isConst, isEmpty bool) { + var lhs, rhs parser.Expr switch e := expr.(type) { case *parser.ParenExpr: - return pruner.isInfinite(e.Expr) - case *parser.NumberLiteral: - if math.IsInf(e.Val, 1) { - return true, true - } - if math.IsInf(e.Val, -1) { - return true, false + return pruner.isConst(e.Expr) + case *parser.BinaryExpr: + if e.Op != parser.EQLC || e.ReturnBool { + return false, false } - return false, false + lhs = e.LHS + rhs = e.RHS default: return false, false } -} -func (pruner *queryPruner) handleOrOp(expr *parser.BinaryExpr) parser.Expr { - switch { - case pruner.isEmpty(expr.LHS): - return expr.RHS - case pruner.isEmpty(expr.RHS): - return expr.LHS + if vectorAndNumber, equals := pruner.isVectorAndNumberEqual(lhs, rhs); vectorAndNumber { + return true, !equals + } + if vectorAndNumber, equals := pruner.isVectorAndNumberEqual(rhs, lhs); vectorAndNumber { + return true, !equals } - return expr + return false, false } -func (pruner *queryPruner) handleAndOp(expr *parser.BinaryExpr) parser.Expr { - switch { - case pruner.isEmpty(expr.LHS): - return expr.LHS - case pruner.isEmpty(expr.RHS): - return expr.RHS +// isVectorAndNumberEqual returns whether the lhs is a const vector like +// "vector(5)"" and the right hand size is a number like "2". Also returns +// if the values are equal. +func (pruner *queryPruner) isVectorAndNumberEqual(lhs, rhs parser.Expr) (bool, bool) { + lIsVector, lValue := pruner.isConstVector(lhs) + if !lIsVector { + return false, false + } + rIsConst, rValue := pruner.isNumber(rhs) + if !rIsConst { + return false, false } - return expr + return true, rValue == lValue } -func (pruner *queryPruner) handleUnlessOp(expr *parser.BinaryExpr) parser.Expr { - switch { - case pruner.isEmpty(expr.LHS): - return expr.LHS - case pruner.isEmpty(expr.RHS): - return expr.LHS +func (pruner *queryPruner) isConstVector(expr parser.Expr) (isVector bool, value float64) { + switch e := expr.(type) { + case *parser.ParenExpr: + return pruner.isConstVector(e.Expr) + case *parser.Call: + if e.Func.Name != "vector" || len(e.Args) != 1 { + return false, 0 + } + lit, ok := e.Args[0].(*parser.NumberLiteral) + if !ok { + return false, 0 + } + return true, lit.Val } - return expr + return false, 0 } -func (pruner *queryPruner) isEmpty(expr parser.Expr) bool { - mapped, _, err := pruner.MapExpr(expr) - if err == nil { - expr = mapped - } +func (pruner *queryPruner) isNumber(expr parser.Expr) (isNumber bool, value float64) { switch e := expr.(type) { case *parser.ParenExpr: - return pruner.isEmpty(e.Expr) - default: - if e.String() == `vector(0) < -Inf` { - return true - } - return false + return pruner.isNumber(e.Expr) + case *parser.NumberLiteral: + return true, e.Val } + return false, 0 } diff --git a/pkg/frontend/querymiddleware/astmapper/pruning_test.go b/pkg/frontend/querymiddleware/astmapper/pruning_test.go index 963c97e29d6..1d62fb321d0 100644 --- a/pkg/frontend/querymiddleware/astmapper/pruning_test.go +++ b/pkg/frontend/querymiddleware/astmapper/pruning_test.go @@ -18,6 +18,23 @@ func TestQueryPruner(t *testing.T) { in string out string }{ + // Non-prunable expressions. + { + `foo`, + `foo`, + }, + { + `foo[1m]`, + `foo[1m]`, + }, + { + `foo{bar="baz"}[1m]`, + `foo{bar="baz"}[1m]`, + }, + { + `foo{bar="baz"}`, + `foo{bar="baz"}`, + }, { `quantile(0.9,foo)`, `quantile(0.9,foo)`, @@ -40,155 +57,119 @@ func TestQueryPruner(t *testing.T) { }, { `up < -Inf`, - `vector(0) < -Inf`, - }, - { - `-Inf > up`, - `vector(0) < -Inf`, - }, - { - `up > +Inf`, - `vector(0) < -Inf`, - }, - { - `+Inf < up`, - `vector(0) < -Inf`, - }, - { - `up < +Inf`, - `up < +Inf`, - }, - { - `+Inf > up`, - `+Inf > up`, - }, - { - `up > -Inf`, - `up > -Inf`, - }, - { - `-Inf < up`, - `-Inf < up`, + `up < -Inf`, }, { `avg(rate(foo[1m])) < (-Inf)`, - `vector(0) < -Inf`, + `avg(rate(foo[1m])) < (-Inf)`, }, { `Inf * -1`, - `-Inf`, - }, - { - `+1 * -Inf`, - `-Inf`, - }, - { - `1 * +Inf`, - `Inf`, - }, - { - `-Inf * -1`, - `+Inf`, + `Inf * -1`, }, { `avg(rate(foo[1m])) < (-1 * +Inf)`, - `vector(0) < -Inf`, - }, - { - `avg(rate(foo[1m])) < (+1 * +Inf)`, - `avg(rate(foo[1m])) < (+Inf)`, - }, - { - `avg(rate(foo[1m])) < (-1 * -Inf)`, - `avg(rate(foo[1m])) < (+Inf)`, - }, - { - `avg(rate(foo[1m])) < (+1 * -Inf)`, - `vector(0) < -Inf`, + `avg(rate(foo[1m])) < (-1 * +Inf)`, }, { `(-1 * -Inf) < avg(rate(foo[1m]))`, - `vector(0) < -Inf`, + `(-1 * -Inf) < avg(rate(foo[1m]))`, }, { `vector(0) < -Inf or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `vector(0) < -Inf or avg(rate(foo[1m]))`, }, { `avg(rate(foo[1m])) or vector(0) < -Inf`, - `avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) or vector(0) < -Inf`, }, { `avg(rate(foo[1m])) or (vector(0) < -Inf)`, - `avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) or (vector(0) < -Inf)`, }, { `((-1 * -Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `((-1 * -Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) <= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) <= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `(avg(rate(foo[2m])) < (+1 * -Inf)) or avg(rate(foo[1m]))`, + `(avg(rate(foo[2m])) < (+1 * -Inf)) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) >= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) >= avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((-1 * -Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((-1 * -Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, + `avg(rate(foo[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, }, { - `((-1 * +Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `((2 * +Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((2 * +Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((+1 * -Inf) > avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `(avg(rate(foo[2m])) < (+1 * -Inf)) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, }, { - `((-1 * -Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) == avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + `avg(rate(foo[1m])) or avg(rate(bar[1m]))`, + `avg(rate(foo[1m])) or avg(rate(bar[1m]))`, }, - { - `((-1 * -Inf) != avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `((+Inf) != avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, + { // The const expression is on the wrong side. + `(vector(0) == 1) and on() (avg(rate(foo[1m])))`, + `(vector(0) == 1) and on() (avg(rate(foo[1m])))`, }, - { - `((-1 * -Inf) < avg(rate(foo[2m]))) and avg(rate(foo[1m]))`, - `(vector(0) < -Inf)`, + { // Matching on labels. + `(avg(rate(foo[1m]))) and on(id) (vector(0) == 1)`, + `(avg(rate(foo[1m]))) and on(id) (vector(0) == 1)`, }, + { // Not "on" expression. + `(avg(rate(foo[1m]))) and ignoring() (vector(0) == 1)`, + `(avg(rate(foo[1m]))) and ignoring() (vector(0) == 1)`, + }, + // Pruned expressions. { - `((-1 * -Inf) < avg(rate(foo[2m]))) unless avg(rate(foo[1m]))`, - `(vector(0) < -Inf)`, + `(avg(rate(foo[1m]))) and on() (vector(0) == 1)`, + `(vector(0) == 1)`, }, { - `avg(rate(foo[1m])) unless ((-1 * -Inf) < avg(rate(foo[2m])))`, - `avg(rate(foo[1m]))`, + `(avg(rate(foo[1m]))) and on() (vector(1) == 1)`, + `(avg(rate(foo[1m])))`, }, { - `((2 * +Inf) < avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `(avg(rate(foo[1m]))) and on() (vector(3) == 4.5)`, + `(vector(3) == 4.5)`, }, { - `(((-1 * -Inf) < avg(rate(foo[3m]))) unless avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + `(avg(rate(foo[1m]))) and on() (vector(5.5) == 5.5)`, + `(avg(rate(foo[1m])))`, }, { - `((((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m]))) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + // "and on()" is not on top level, "or" has lower precedence. + // This could be further reduced by dropping the vector(0)==1 from + // the "or", but it is intentionally not done as that would + // complicate the algorithm and isn't required to reduce chunks + // loading. + `(avg(rate(foo[1m]))) and on() (vector(0) == 1) or avg(rate(bar[1m]))`, + `(vector(0) == 1) or avg(rate(bar[1m]))`, }, { - `(((-1 * -Inf) < avg(rate(foo[4m]))) unless (avg(rate(foo[3m])) and avg(rate(foo[2m])))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + // "and on()" is not on top level, due to left-right associativity. + `(avg(rate(foo[1m]))) and on() (vector(0) == 1) and avg(rate(bar[1m]))`, + `(vector(0) == 1) and avg(rate(bar[1m]))`, }, { - `(((-1 * -Inf) < avg(rate(foo[4m]))) unless avg(rate(foo[3m])) and avg(rate(foo[2m]))) or avg(rate(foo[1m]))`, - `avg(rate(foo[1m]))`, + // "and on()" is not on top level. + // This could be further reduced by dropping the vector(0)==1 from + // the "or", but it is intentionally not done as that would + // complicate the algorithm and isn't required to reduce chunks + // loading. + `(avg(rate(foo[1m]))) and on() (vector(0) == 1) or avg(rate(bar[1m])) and on() (vector(1) == 1)`, + `(vector(0) == 1) or avg(rate(bar[1m]))`, }, } { tt := tt diff --git a/pkg/frontend/querymiddleware/prune_test.go b/pkg/frontend/querymiddleware/prune_test.go index 71cd3b34a0f..0ab9301f583 100644 --- a/pkg/frontend/querymiddleware/prune_test.go +++ b/pkg/frontend/querymiddleware/prune_test.go @@ -46,17 +46,10 @@ func TestQueryPruning(t *testing.T) { } templates := []template{ - {`avg(rate(%s[1m1s])) < (-1 * +Inf)`, true}, - {`avg(rate(%s[1m1s])) < (+1 * +Inf)`, false}, - {`avg(rate(%s[1m1s])) < (-1 * -Inf)`, false}, - {`avg(rate(%s[1m1s])) < (+1 * -Inf)`, true}, - {`(-1 * -Inf) < avg(rate(%s[1m1s]))`, true}, - {`((-1 * -Inf) < avg(rate(foo[2m1s]))) or avg(rate(%s[1m1s]))`, false}, - {`((-1 * -Inf) < avg(rate(foo[2m1s]))) and avg(rate(%s[1m1s]))`, true}, - {`((-1 * -Inf) < avg(rate(foo[2m1s]))) unless avg(rate(%s[1m1s]))`, true}, - {`avg(rate(%s[1m1s])) unless ((-1 * -Inf) < avg(rate(foo[2m1s])))`, false}, - {`(((-1 * -Inf) < avg(rate(foo[3m1s]))) unless avg(rate(foo[2m1s]))) or avg(rate(%s[1m1s]))`, true}, - {`((((-1 * -Inf) < avg(rate(foo[4m1s]))) unless avg(rate(foo[3m1s]))) and avg(rate(foo[2m1s]))) or avg(rate(%s[1m1s]))`, true}, + {`(avg(rate(%s[1m1s])))`, false}, + {`(avg(rate(%s[1m1s]))) and on() (vector(0) == 1)`, true}, + {`(avg(rate(%s[1m1s]))) and on() (vector(1) == 1)`, false}, + {`avg(rate(%s[1m1s])) or (avg(rate(test_float[1m1s]))) and on() (vector(0) == 1)`, false}, } for _, template := range templates { t.Run(template.query, func(t *testing.T) {