Add rate aggregation function #106703

dnhatn · 2024-03-23T18:31:16Z

This PR introduces a rate aggregation function to ESQL, with two main changes:

Define of the grouping state for the rate aggregation function:

For raw input, the function expects data to arrive in descending timestamp order without gaps; hence, perform a reduction with each incoming entry. Each grouping should consist of at most two entries: one for the starting time and one for the ending time.
For intermediate input, the function buffers data as they can arrive out of order, although non-overlapping. This shouldn't have significant issues, as we expect at most two entries per participating pipeline.
The intermediate output consists of three blocks: timestamps, values, and compensation. Both timestamps and values can contain multiple entries sorted in descending order by timestamp.
This rate function does not support non-grouping aggregation. However, I can enable it if we think otherwise.

Modifies the GroupingAggregatorImplementer code generator to include the timestamp vector block. I explored several options to generate multiple input blocks. However, both generated and code generator are much more complicated in a generic solution. And it's unlikely that we will need another function requires multiple input blocks. Hence, I decided to tweak this class to append the timestamps long vector block when specified.

I have left out some parts of this PR to minimize its PR to make reviewing easier. I plan to follow-up these items:

Supporting Ordinal Grouping.
Tracking individual states with the CircuitBreaker
Testing with GroupingAggregatorFunctionTestCase: requires some changes around null inserting and input channels.
Integrating into the language.

Relates #106415

elasticsearchmachine · 2024-03-24T18:13:22Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

elasticsearchmachine · 2024-03-24T18:13:22Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

x-pack/plugin/esql/compute/build.gradle

breskeby

small suggestion to keep idiomatic gradle syntaix in build scripts

Co-authored-by: Rene Groeschke <[email protected]>

martijnvg

Nice @dnhatn! I've scrolled through this change and I think this looks in the direction of how rate would be implemented in compute engine.

martijnvg · 2024-03-25T12:50:55Z

...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java

@@ -261,29 +261,20 @@ void consume() throws IOException {
                    }
                } else {
                    int previousTsidOrd = leaf.timeSeriesHashOrd;
-                    boolean breakOnNextTsidChange = false;


I think this will be fixed when the other pr you opened is merged?

Yes, I have merged this fix in another PR.

nik9000 · 2024-03-25T13:34:41Z

Integrating into the language.

The magic that makes it so you don't have to test it all. Neat.

nik9000 · 2024-03-25T13:36:15Z

...mpute/src/main/generated-src/org/elasticsearch/compute/aggregation/RateDoubleAggregator.java

+        @IntermediateState(name = "value", type = "DOUBLE_BLOCK"),
+        @IntermediateState(name = "compensation", type = "DOUBLE") }
+)
+public class RateDoubleAggregator {


I'm somehow totally unsurpised we've started to use one code generation tool to kick off another code generation tool.

I'm somehow totally unsurpised we've started to use one code generation tool to kick off another code generation tool.

Yeah, I was inspired by your values function and I found it's less error-prone. Do you think we should convert existing functions and tests to use templates as well?

Maybe the next time we find something interesting to do there. I'm not sure it's worth doing without some kind of excuse.

...mpute/src/main/generated-src/org/elasticsearch/compute/aggregation/RateDoubleAggregator.java

nik9000 · 2024-03-25T13:45:57Z

.../compute/src/main/generated-src/org/elasticsearch/compute/aggregation/RateIntAggregator.java

+
+        private int dv(int v0, int v1) {
+            return v0 > v1 ? v1 : v1 - v0;
+        }


I'm curious if we'd be better off assuming that a reset is a wrap instead. I'm sure there's literature on the topic. It can wait for later though.

nik9000 · 2024-03-25T13:46:31Z

.../compute/src/main/generated-src/org/elasticsearch/compute/aggregation/RateIntAggregator.java

+    value = {
+        @IntermediateState(name = "timestamp", type = "LONG_BLOCK"),
+        @IntermediateState(name = "value", type = "INT_BLOCK"),
+        @IntermediateState(name = "compensation", type = "DOUBLE") }


Do we need a compensation for int and long rates? It looks a little like a Kahan sum we shouldn't need that for int and long.

I was going to ask if we need a rate flavor for int and long at all but I think it's right and proper to have one, at least for long. Lots of stuff is going to read it's values as long. And if we can avoid having compensation, all the better.

Do we need a compensation for int and long rates?

I might be missing something here. But the compensation is the sum of all reset values within this group's range. Perhaps, the term compensation is confusing? Should we consider renaming it to totalReset?

Yeah! Let's rename it so I don't think it's kahan.

I've renamed compensation to reset in 89880d4.

dnhatn · 2024-03-25T19:23:10Z

@breskeby @martijnvg @nik9000 Thanks for reviews.

We should track the memory usage of the individual state in the rate aggregation function. Relates #106703

Add support for ordinal grouping in the rate aggregation function. Relates #106703

elasticsearchmachine added the v8.14.0 label Mar 23, 2024

dnhatn force-pushed the rate-function branch 2 times, most recently from ab2c377 to c24b30f Compare March 23, 2024 22:33

dnhatn mentioned this pull request Mar 23, 2024

Fix missing docs in time series source reader #106705

Merged

dnhatn force-pushed the rate-function branch 9 times, most recently from e4071c3 to 576ddfe Compare March 24, 2024 05:25

Add rate aggregation function

Loading
Loading status checks…

53cf773

dnhatn force-pushed the rate-function branch from 576ddfe to 53cf773 Compare March 24, 2024 16:46

dnhatn added :StorageEngine/TSDB :Analytics/ES|QL >non-issue labels Mar 24, 2024

dnhatn requested review from nik9000, martijnvg and kkrik-es March 24, 2024 18:12

dnhatn marked this pull request as ready for review March 24, 2024 18:12

dnhatn requested a review from a team as a code owner March 24, 2024 18:12

elasticsearchmachine added Team:Analytics Team:StorageEngine labels Mar 24, 2024

breskeby reviewed Mar 24, 2024

View reviewed changes

x-pack/plugin/esql/compute/build.gradle Outdated Show resolved Hide resolved

breskeby reviewed Mar 24, 2024

View reviewed changes

x-pack/plugin/esql/compute/build.gradle Outdated Show resolved Hide resolved

breskeby approved these changes Mar 24, 2024

View reviewed changes

dnhatn and others added 2 commits March 24, 2024 13:30

Update x-pack/plugin/esql/compute/build.gradle

Loading
Loading status checks…

96a06be

Co-authored-by: Rene Groeschke <[email protected]>

Update x-pack/plugin/esql/compute/build.gradle

Loading
Loading status checks…

b18d113

Co-authored-by: Rene Groeschke <[email protected]>

martijnvg reviewed Mar 25, 2024

View reviewed changes

nik9000 approved these changes Mar 25, 2024

View reviewed changes

dnhatn added 3 commits March 25, 2024 10:29

compensation -> reset

89880d4

comment

a041dd0

Merge branch 'main' into rate-function

Loading
Loading status checks…

942714a

dnhatn force-pushed the rate-function branch from 231696a to 942714a Compare March 25, 2024 17:31

dnhatn merged commit 49cf3cb into elastic:main Mar 25, 2024
14 checks passed

dnhatn deleted the rate-function branch March 25, 2024 19:24

This was referenced Mar 25, 2024

Track memory in rate aggregation function #106730

Merged

Update Aggregator and GroupingAggregator interfaces to accept sorted pages/blocks. #106414

Closed

dnhatn added a commit that referenced this pull request Mar 26, 2024

Track memory in rate aggregation function (#106730)

Loading
Loading status checks…

cdb2e58

We should track the memory usage of the individual state in the rate aggregation function. Relates #106703

dnhatn mentioned this pull request Mar 26, 2024

Support ordinals grouping for rate aggregation #106735

Merged

dnhatn added a commit that referenced this pull request Mar 26, 2024

Support ordinals grouping for rate aggregation (#106735)

Loading
Loading status checks…

0b3382c

Add support for ordinal grouping in the rate aggregation function. Relates #106703

martijnvg mentioned this pull request Apr 4, 2024

Add es|ql rate aggregate function #106415

Closed

idegtiarenko mentioned this pull request Feb 10, 2025

Supply timestamp to aggregate functions #122174

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rate aggregation function #106703

Add rate aggregation function #106703

dnhatn commented Mar 23, 2024 •

edited

Loading

elasticsearchmachine commented Mar 24, 2024

elasticsearchmachine commented Mar 24, 2024

breskeby left a comment

martijnvg left a comment

martijnvg Mar 25, 2024

dnhatn Mar 25, 2024

nik9000 commented Mar 25, 2024

nik9000 Mar 25, 2024

dnhatn Mar 25, 2024

nik9000 Mar 25, 2024

nik9000 Mar 25, 2024

dnhatn Mar 25, 2024

nik9000 Mar 25, 2024

dnhatn Mar 25, 2024

nik9000 Mar 25, 2024

dnhatn Mar 25, 2024

dnhatn commented Mar 25, 2024

Add rate aggregation function #106703

Add rate aggregation function #106703

Conversation

dnhatn commented Mar 23, 2024 • edited Loading

elasticsearchmachine commented Mar 24, 2024

elasticsearchmachine commented Mar 24, 2024

breskeby left a comment

Choose a reason for hiding this comment

martijnvg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Mar 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnhatn commented Mar 25, 2024

dnhatn commented Mar 23, 2024 •

edited

Loading