From f6c554abe95eaedb2f744c8c325c402e3c73eb8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Fri, 29 Nov 2024 09:42:18 +0100 Subject: [PATCH 1/4] frontend: add new query pruning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current method of excluding/including sub query results in PromQL by comparing to -Inf or +Inf is no longer valid after https://github.com/prometheus/prometheus/pull/15245 due to comparison of native histograms to a float with < or > result in Jeanette's warning, not empty set. The new method uses logical AND operation to intersect the sub query with either a const vector() or an empty vector(). E.g. subquery and on() (vector(1)==1) subquery and on() (vector(-1)==1) which become: subquery (vector(-1)==1) Note that although in theory (vector(-1)==1) could be dropped in some cases, it depends on the context and out of scope for this PR. Signed-off-by: György Krajcsovits --- CHANGELOG.md | 1 + .../querymiddleware/astmapper/pruning.go | 212 +++++------------- .../querymiddleware/astmapper/pruning_test.go | 171 +++++++------- pkg/frontend/querymiddleware/prune_test.go | 17 +- 4 files changed, 140 insertions(+), 261 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 498f1efd813..b2958eeb8eb 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 no longer prune ` < -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..3b94438f2a0 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,89 @@ 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 + e, ok := expr.(*parser.BinaryExpr) + if !ok { + return expr, 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: + 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 } -} -// 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 + isConst, isEmpty := pruner.isConst(e.RHS) + if !isConst { + return expr, false, nil } - 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 + 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 } + // The right hand side is const no empty, so the whole expression is just the + // left side. + return e.LHS, false, nil } -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 +func (pruner *queryPruner) isConst(expr parser.Expr) (isConst, isEmpty bool) { + var lhs, rhs parser.Expr + switch e := expr.(type) { + case *parser.ParenExpr: + return pruner.isConst(e.Expr) + case *parser.BinaryExpr: + if e.Op != parser.EQLC || e.ReturnBool { + return false, false } - } - 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 + lhs = e.LHS + rhs = e.RHS default: - return expr + return false, false } - // 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, + lIsVector, lValue := pruner.isConstVector(lhs) + if lIsVector { + rIsConst, rValue := pruner.isNumber(rhs) + if rIsConst { + return true, rValue != lValue } + return false, false } - - // 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, - } + var lIsConst bool + lIsConst, lValue = pruner.isNumber(lhs) + if !lIsConst { + return false, false } - - return expr + rIsVector, rValue := pruner.isConstVector(rhs) + if !rIsVector { + return false, false + } + return true, lValue != rValue } -// 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) isConstVector(expr parser.Expr) (isVector bool, value float64) { switch e := expr.(type) { case *parser.ParenExpr: - return pruner.isInfinite(e.Expr) - case *parser.NumberLiteral: - if math.IsInf(e.Val, 1) { - return true, true + return pruner.isConstVector(e.Expr) + case *parser.Call: + if e.Func.Name != "vector" || len(e.Args) != 1 { + return false, 0 } - if math.IsInf(e.Val, -1) { - return true, false + lit, ok := e.Args[0].(*parser.NumberLiteral) + if !ok { + return false, 0 } - return false, false - 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 - } - return expr -} - -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 + return true, lit.Val } - return expr + return false, 0 } -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 - } - return expr -} - -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..c014ee9acc0 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) { @@ -76,6 +69,7 @@ func TestQueryPruning(t *testing.T) { require.Nil(t, err) if !template.IsEmpty { + fmt.Printf("query1: %s\n", query) // Ensure the query produces some results. require.NotEmpty(t, expectedRes.(*PrometheusResponse).Data.Result) requireValidSamples(t, expectedRes.(*PrometheusResponse).Data.Result) @@ -87,6 +81,7 @@ func TestQueryPruning(t *testing.T) { if !template.IsEmpty { // Ensure the query produces some results. + fmt.Printf("query2: %s\n", query) require.NotEmpty(t, prunedRes.(*PrometheusResponse).Data.Result) requireValidSamples(t, prunedRes.(*PrometheusResponse).Data.Result) } From ca3a91c2c2f1a9a97c66b237b6b4b2b883da8d93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 2 Dec 2024 07:35:59 +0100 Subject: [PATCH 2/4] updates from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- CHANGELOG.md | 2 +- .../querymiddleware/astmapper/pruning.go | 34 +++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dfbf06e8c7..6a1f2405922 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,7 +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 no longer prune ` < -Inf`. Triggered by https://github.com/prometheus/prometheus/pull/15245. #10026 +* [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 3b94438f2a0..7267e72ab11 100644 --- a/pkg/frontend/querymiddleware/astmapper/pruning.go +++ b/pkg/frontend/querymiddleware/astmapper/pruning.go @@ -51,8 +51,8 @@ func (pruner *queryPruner) MapExpr(expr parser.Expr) (mapped parser.Expr, finish // "and on()", return the right hand side. return e.RHS, false, nil } - // The right hand side is const no empty, so the whole expression is just the - // left side. + // The right hand side is const and not empty, so the whole expression is + // just the left side. return e.LHS, false, nil } @@ -71,24 +71,28 @@ func (pruner *queryPruner) isConst(expr parser.Expr) (isConst, isEmpty bool) { return false, false } - lIsVector, lValue := pruner.isConstVector(lhs) - if lIsVector { - rIsConst, rValue := pruner.isNumber(rhs) - if rIsConst { - return true, rValue != lValue - } - return false, false + if vectorAndConst, equals := pruner.isVectorAndNumberEqual(lhs, rhs); vectorAndConst { + return true, !equals + } + if vectorAndConst, equals := pruner.isVectorAndNumberEqual(rhs, lhs); vectorAndConst { + return true, !equals } - var lIsConst bool - lIsConst, lValue = pruner.isNumber(lhs) - if !lIsConst { + return false, false +} + +// 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 } - rIsVector, rValue := pruner.isConstVector(rhs) - if !rIsVector { + rIsConst, rValue := pruner.isNumber(rhs) + if !rIsConst { return false, false } - return true, lValue != rValue + return true, rValue == lValue } func (pruner *queryPruner) isConstVector(expr parser.Expr) (isVector bool, value float64) { From 77d4a158f6c087e7a2650f4f90a72f008462712c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 2 Dec 2024 07:37:00 +0100 Subject: [PATCH 3/4] remove extra debug printout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- pkg/frontend/querymiddleware/prune_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/frontend/querymiddleware/prune_test.go b/pkg/frontend/querymiddleware/prune_test.go index c014ee9acc0..0ab9301f583 100644 --- a/pkg/frontend/querymiddleware/prune_test.go +++ b/pkg/frontend/querymiddleware/prune_test.go @@ -69,7 +69,6 @@ func TestQueryPruning(t *testing.T) { require.Nil(t, err) if !template.IsEmpty { - fmt.Printf("query1: %s\n", query) // Ensure the query produces some results. require.NotEmpty(t, expectedRes.(*PrometheusResponse).Data.Result) requireValidSamples(t, expectedRes.(*PrometheusResponse).Data.Result) @@ -81,7 +80,6 @@ func TestQueryPruning(t *testing.T) { if !template.IsEmpty { // Ensure the query produces some results. - fmt.Printf("query2: %s\n", query) require.NotEmpty(t, prunedRes.(*PrometheusResponse).Data.Result) requireValidSamples(t, prunedRes.(*PrometheusResponse).Data.Result) } From 24790adeebfa241e0c2b509f278392e701e1f163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 2 Dec 2024 07:44:32 +0100 Subject: [PATCH 4/4] consistent naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- pkg/frontend/querymiddleware/astmapper/pruning.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/frontend/querymiddleware/astmapper/pruning.go b/pkg/frontend/querymiddleware/astmapper/pruning.go index 7267e72ab11..34c969f1dcb 100644 --- a/pkg/frontend/querymiddleware/astmapper/pruning.go +++ b/pkg/frontend/querymiddleware/astmapper/pruning.go @@ -71,10 +71,10 @@ func (pruner *queryPruner) isConst(expr parser.Expr) (isConst, isEmpty bool) { return false, false } - if vectorAndConst, equals := pruner.isVectorAndNumberEqual(lhs, rhs); vectorAndConst { + if vectorAndNumber, equals := pruner.isVectorAndNumberEqual(lhs, rhs); vectorAndNumber { return true, !equals } - if vectorAndConst, equals := pruner.isVectorAndNumberEqual(rhs, lhs); vectorAndConst { + if vectorAndNumber, equals := pruner.isVectorAndNumberEqual(rhs, lhs); vectorAndNumber { return true, !equals } return false, false