Commit graph

397 commits

Author SHA1 Message Date
beorn7 c46074f4dd promql: make avg aggregation more precise and less expensive
The basic idea here is that the previous code was always doing
incremental calculation of the mean value, which is more costly and
can be less precise. It protects against overflows, but in most cases,
an overflow doesn't happen anyway.

The other idea applied here is to expand on #14074, where Kahan
summation was applied to sum().

With this commit, the average is calculated in a conventional way
(adding everything up and divide in the end) as long as the sum isn't
overflowing float64. This is combined with Kahan summation so that the
avg aggregation, in most cases, is really equivalent to the sum
aggregation with a following division (which is the user's expectation
as avg is supposed to be syntactic sugar for sum with a following
divison).

If the sum hits ±Inf, the calculation reverts to incremental
calculation of the mean value. Kahan summation is also applied here,
although it cannot fully compensate for the numerical errors
introduced by the incremental mean calculation. (The tests added in
this commit would fail if incremental mean calculation was always
used.)

Signed-off-by: beorn7 <beorn@grafana.com>
2024-07-10 19:20:24 +02:00
beorn7 44d8c1d182 nit: add period at end of sentence
Signed-off-by: beorn7 <beorn@grafana.com>
2024-07-04 17:31:39 +02:00
beorn7 9a837b7f3c promql: Make groupedAggregation.groupCount a float64
It's always used as such. Let's avoid the countless conversions.

Signed-off-by: beorn7 <beorn@grafana.com>
2024-07-04 17:31:34 +02:00
JuanJo Ciarlante c94c5b64c3
feat: add limitk() and limit_ratio() operators (#12503)
* rebase 2024-07-01, picks previous renaming to `limitk()` and `limit_ratio()`

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* gofumpt -d -extra

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* more lint fixes

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* more lint fixes+

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* put limitk() and limit_ratio() behind --enable-feature=promql-experimental-functions

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* EnableExperimentalFunctions for TestConcurrentRangeQueries() also

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* use testutil.RequireEqual to fix tests, WIP equivalent thingie for require.Contains

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* lint fix

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* moar linting

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* rebase 2024-06-19

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* re-add limit(2, metric) testing for N=2 common series subset

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* move `ratio = param` to default switch case, for better readability

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* gofumpt -d -extra util/testutil/cmp.go

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* early break when reaching k elems in limitk(), should have always been so (!)

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* small typo fix

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* no-change small break-loop rearrange for readability

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* remove IsNan(ratio) condition in switch-case, already handled as input validation

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* no-change adding some comments

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* no-change simplify fullMatrix() helper functions used for tests

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* add `limitk(-1, metric)` testcase, which is handled as any k < 1 case

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* engine_test.go: no-change create `requireCommonSeries() helper func (moving code into it) for readability

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* rebase 2024-06-21

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* engine_test.go: HAPPY NOW about its code -> reorg, create and use simpleRangeQuery() function, less lines and more readable ftW \o/

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* move limitk(), limit_ratio() testing to promql/promqltest/testdata/limit.test

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* remove stale leftover after moving tests from engine_test.go to testdata/

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* fix flaky `limit_ratio(0.5, ...)` test case

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* Update promql/engine.go

Co-authored-by: Julius Volz <julius.volz@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* Update promql/engine.go

Co-authored-by: Julius Volz <julius.volz@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* Update promql/engine.go

Co-authored-by: Julius Volz <julius.volz@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* fix AddRatioSample() implementation to use a single conditional (instead of switch/case + fallback return)

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* docs/querying/operators.md: document r < 0

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* add negative limit_ratio() example to docs/querying/examples.md

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* move more extensive docu examples to docs/querying/operators.md

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* typo

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* small docu fix for poor-mans-normality-check, add it to limit.test ;)

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* limit.test: expand "Poor man's normality check" to whole eval range

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* restore mistakenly removed existing small comment

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* expand poors-man-normality-check case(s)

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* Revert "expand poors-man-normality-check case(s)"

This reverts commit f69e1603b2ebe69c0a100197cfbcf6f81644b564, indeed too
flaky 0:)

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* remove humor from docs/querying/operators.md

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* fix signoff

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* add web/ui missing changes

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* expand limit_ratio test cases, cross-fingering they'll not be flaky

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* remove flaky test

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* add missing warnings.Merge(ws) in instant-query return shortcut

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* add missing LimitK||LimitRatio case to codemirror-promql/src/parser/parser.ts

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* fix ui-lint

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

* actually fix returned warnings :]

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>

---------

Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Co-authored-by: Julius Volz <julius.volz@gmail.com>
2024-07-03 22:18:57 +02:00
Björn Rabenstein 2e58d46522
Merge pull request #13662 from prometheus/nhcb
Native histograms custom buckets storage
2024-06-27 21:44:20 +02:00
Bryan Boreham b6aba4ff14
Merge pull request #14074 from bboreham/kahan-sum-sum
[ENHANCEMENT] PromQL: use Kahan summation for sum()
2024-06-24 11:13:26 +01:00
Jeanette Tan 14f8dded39 Merge branch 'main' into nhcb
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-06-07 19:17:14 +08:00
Filip Petkovski 6e68046c25
Implement histogram statistics decoder (#14097)
Implement histogram statistics decoder

This commit speeds up histogram_count and histogram_sum
functions on native histograms. The idea is to have separate decoders which can be
used by the engine to only read count/sum values from histogram objects. This should help
with reducing allocations when decoding histograms, as well as with speeding up aggregations
like sum since they will be done on floats and not on histogram objects.

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>

---------

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
2024-06-06 17:17:13 +02:00
Jeanette Tan f028496133 Merge branch 'main' into nhcb
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-05-14 16:20:15 +08:00
Charles Korn 0e934dba8e
Capture timing information while sorting
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-05-13 19:47:18 +10:00
Charles Korn 036c87223c
Ensure series in matrix values returned for instant queries are always sorted
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-05-13 11:03:15 +10:00
Bryan Boreham ea82b49c33 [ENHANCEMENT] PromQL: use Kahan summation for sum()
This can give a more precise result, by keeping a separate running
compensation value to accumulate small errors.

See https://en.wikipedia.org/wiki/Kahan_summation_algorithm

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-05-09 14:29:38 +01:00
Bryan Boreham 3fd24d1cd7
Merge pull request #13999 from bboreham/extract-promqltest
[Test] Extract most PromQL test code into separate packages
2024-05-09 13:23:11 +01:00
Bryan Boreham e7c77f7b40 promql: export NewTestQuery
So that tests can call it from another package.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-05-08 16:08:04 +01:00
Jeanette Tan 796b1bbfde Merge branch 'main' into nhcb
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-05-08 19:11:39 +08:00
Arve Knudsen a25160e6a4
[REFACTOR] PromQL: simplify rangeEvalTimestampFunctionOverVectorSelector (#14021)
The function `rangeEvalTimestampFunctionOverVectorSelector` appeared to be checking histogram size, however the value it used was always 0 due to subtle variable shadowing.
However we don't need to pass sample values to the `timestamp` function, since the latter only cares about timestamps. This also affects peak sample count in statistics, since we are no longer copying histogram samples.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
2024-05-08 10:39:44 +01:00
György Krajcsovits bcafa5f1f9 Merge remote-tracking branch 'upstream/main' into update-nhcb 2024-04-24 11:06:59 +02:00
Bryan Boreham 12961c6a37 promql: refactor: eliminate one 'else'
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 0ac927515b promql: move group-seen into group struct
Save allocating an auxilliary array.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 7499d90913 promql: remove pointer to aggregation groups
Just allocate in one slice.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham cfbeb6681b promql: re-use one heap for topk and bottomk
Slightly ugly casting saves memory.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 5e3914a27c promql: remove histogramMean from groupedAggregation
Re-use histogramValue since we don't need them separately.

Tidy up initialization.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 2cf3c9de8f promql: store labels per-group only for count_values
This saves memory in other kinds of aggregation.

We don't need `orderedResult` in `aggregationCountValues`; the ordering
is not guaranteed.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 185290a0d2 promql: pull checking of q and k out of loop
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 4584f67e17 promql: inline nextSample function
Move Sample out of loop to reduce allocations, otherwise it escapes to
the heap.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 526ce4ee7a promql: simplify data collection in aggregations
We don't need a Sample, just the float and histogram values.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 2f03acbafc promql: refactor: split topk/bottomk from sum/avg/etc
They aggregate results in different ways.
topk/bottomk don't consider histograms so can simplify data collection.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 74eed67ef6 promql: refactor: pull fetching input data out of rangeEvalAgg
This is a cleaner split of responsibilities.
We now check the sample count after calling rangeEvalAgg.
Changed re-use of samples to use `Clone` and `defer`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 602eb69edf promql: refactor: extract function nextSample
With sub-function nextValues which we shall use shortly.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham eb41e770b7 promql: refactor: extract function addToSeries
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 53a3138eeb promql aggregations: pre-generate mapping from inputs to outputs
So we don't have to re-create it on every time step.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham cb6c4b3092 promql: simplify k/q parameter to topk/bottomk/quantile
Pass it as a float64 not as interface{}.
Make k a simple int, since that is the parameter to make().
Pull invalid quantile warning out of the loop.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham b3bda7df4b promql: aggregations: skip copying input to a Vector
We can work directly from the inputMatrix on each timestep.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham c9b6c4c55a promql: aggregations: output directly to matrix for instant queries
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 3851b74db1 promql: aggregations: skip result vector in range queries
Adjust test to match the lower count, since samples in the vector
are no longer counted.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 59548b8a0b promql: refactor: move collection of results into aggregation()
We don't need to check for duplicates as aggregation cannot generate them.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham bd9bdccb22 promql: refactor: simplify internal data structures
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 5f10d17cef promql: refactor: split out aggregations over range
The new function `rangeEvalAgg` is mostly a copy of `rangeEval`, but
without `initSeries` which we don't need and inlining the callback to
`aggregation()`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham e5f667537c promql: refactor: initialize aggregation before storing in map
This seems more consistent to me.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 29244fb841 promql: refactor: extract count_values implementation
The existing aggregation function is very long and covers very different
cases.

`aggregationCountValues` is just for `count_values`, which differs from
other aggregations in that it outputs as many series per group as there
are values in the input.

Remove the top-level switch on string parameter type; use the same `Op`
check there as elswehere.

Pull checking parameters out to caller, where it is only executed once.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Bryan Boreham 8e04ab6dd4 promql: refactor: extract generateGroupingLabels function
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-04-05 15:47:54 +01:00
Julius Volz 9b7de47787
Remove unused Dmn field on EvalNodeHelper (#13877)
https://github.com/prometheus/prometheus/pull/13446 removed the last usage of
this field, but didn't remove the field.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
2024-04-02 18:45:46 +02:00
György Krajcsovits a3d1a46eda Merge branch 'main' into nhcb 2024-03-22 14:51:48 +01:00
Bartlomiej Plotka 312e3fd728
Merge pull request #13713 from charleskorn/query-engine-interface
rules: allow using alternative PromQL engines for rule evaluation by callers using Prometheus as a lib.
2024-03-13 14:45:42 +01:00
Björn Rabenstein af3618fd35
Merge pull request #13667 from prometheus/beorn7/promql
Improve TestQueryStatistics and fix bugs exposed by it
2024-03-12 16:17:11 +01:00
Charles Korn 26262a1eb7
Remove unnecessary SetQueryLogger method on QueryEngine interface
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-03-12 22:01:01 +11:00
carrychair 856f6e49c8 fix function and struct name
Signed-off-by: carrychair <linghuchong404@gmail.com>
2024-03-09 17:53:17 +08:00
Charles Korn 4e77e8e5ef
Allow using alternative PromQL engines for rule evaluation
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-03-06 14:54:33 +11:00
beorn7 f48c7a5503 promql: Add histograms to TestQueryStatistics
Also, fix the bugs exposed by the tests.

Signed-off-by: beorn7 <beorn@grafana.com>
2024-02-29 19:02:40 +01:00
beorn7 f46dd34982 promql: Add code comment
Signed-off-by: beorn7 <beorn@grafana.com>
2024-02-29 19:02:40 +01:00