Helper functions for PromQL-like time series data queries#80590
Conversation
|
How about renaming |
AFAIU "I" stands for "instant". |
| } | ||
| else if (timestamp == timestamps[0]) | ||
| { | ||
| /// Replace the value with bigger one |
There was a problem hiding this comment.
Why do we replace the value with bigger one?
There was a problem hiding this comment.
This is done for stability. Normally, there should not be samples with equal timestamps but if for some reason they are present in user data we want to always choose the same one no mater in which order the data is scanned.
| struct Data | ||
| { | ||
| ValueType values[2]; /// In common scenario values are Float64, so put them first as they need 8-byte alignment | ||
| TimestampType timestamps[2]; /// Timestamps might be stored as DateTime64, DateTime32 or even as 16-bit delta from the base timestamp of the grid |
There was a problem hiding this comment.
Is it always true that timestamps[2] < timestamps[1] < timestamps[0]?
There was a problem hiding this comment.
There can't be timestamp[2], as we store no more than 2 samples and yes, timestamps[0] is the latest
| [(1734955380,NULL),(1734955395,NULL),(1734955410,NULL),(1734955425,0),(1734955440,0),(1734955455,0),(1734955470,0),(1734955485,0),(1734955500,0),(1734955515,1),(1734955530,3),(1734955545,5),(1734955560,5),(1734955575,5),(1734955590,5),(1734955605,8),(1734955620,8),(1734955635,8),(1734955650,8),(1734955665,8),(1734955680,8)] | ||
| Row 1: | ||
| ────── | ||
| rate_5m: [(1734955380,NULL),(1734955395,NULL),(1734955410,NULL),(1734955425,NULL),(1734955440,0),(1734955455,0),(1734955470,0),(1734955485,0),(1734955500,0),(1734955515,0.003467629629629629),(1734955530,0.010345333333333333),(1734955545,0.017170277777777774),(1734955560,0.017114320987654322),(1734955575,0.017069555555555557),(1734955590,0.01703292929292929),(1734955605,0.027203851851851854),(1734955620,0.02716252991452992),(1734955635,0.027127111111111112),(1734955650,0.02709641481481482),(1734955665,0.02706955555555555),(1734955680,0.02704585620915033)] |
There was a problem hiding this comment.
I tried to check value (1734955515,0.003467629629629629) here.
The window was 300 s, so the first point in the window was (1734955421.374,0) and the last point was (1734955511.374,1). Thus it seems the rate should be (1 - 0) / (1734955511.374 - 1734955421.374) which is 0.011111111 and not 0.003467629629629629. Did I miss something?
There was a problem hiding this comment.
Prometheus also does some extrapolation. I actually copied the code for rate and delta calculation from Prometheus to make those computation fully compatible. See here https://github.com/ClickHouse/ClickHouse/pull/80590/files#diff-c2739d27c54dfe109d9cf36ed500c9ba18fa1d8eaadc3a7be59558676e64cc7aR147-R198
There was a problem hiding this comment.
Yes, I see it now. It indeed used extrapolation:
range 1734955215..1734955515 (300s):
range_start = 1734955215
range_end = 1734955515
t1 = 1734955421.374
v1 = 0
t2 = 1734955511.374
v2 = 1
average_interval = 15 (average time difference between points)
raw_increase = (v2 - v1) = (1 - 0) = 1
raw_rate = raw_increase / (t2 - t1) = 1 / (1734955511.374 - 1734955421.374) = 0.011111111
post_interpolation = (range_end - t2) * raw_rate = (1734955515 - 1734955511.374) * 0.011111111 = 0.040288888
pre_interpolation = 0 (because v1 = 0 and interpolation doesn't go to negative values)
increase = raw_increase + pre_interpolation + post_interpolation = 1 + 0 + 0.040288888 = 1.040288888
rate = increase / (range_end - range_start) = 1.040288888 / 300 = 0.00346763
| (1734955646.374, 8), | ||
| (1734955661.374, 8), | ||
| (1734955676.374, 8) | ||
| ]); |
There was a problem hiding this comment.
This time series doesn't contain resets. Did you manage to take resets into account when calculating rate() or delta()?
There was a problem hiding this comment.
This test data has resets
And here
rate and delta are calculated|
|
||
| void registerAggregateFunctionTimeseriesToGrid(AggregateFunctionFactory & factory) | ||
| { | ||
| factory.registerFunction("tsToGrid", createAggregateFunctionTimeseriesToGrid); |
There was a problem hiding this comment.
Function tsToGrid() isn't mentioned in the PR's description.
There was a problem hiding this comment.
Removed tsToGrid in favor of timeSeriesResampleToGridWithStaleness
Renamed |
d969ca0 to
6413d61
Compare
Co-authored-by: Alon Tal <alon@alontal.com>
|
|
||
| /// Aggregate function to store last (latest by timestamp) 2 samples with distinct timestamps | ||
| template <typename TimestampType, typename ValueType> | ||
| class AggregateFunctionLast2Samples final : |
There was a problem hiding this comment.
Is this aggregation function used anywhere except that its Data is used to implement timeSeriesInstantRateToGrid()?
There was a problem hiding this comment.
Almost forgot about it.
This is a helper function that is useful for building MV and table with re-sampled data. It helps to reduce the amount of data that has to be read and processed for queries with grid timestamp step much bigger then raw data time resolution.
Added it to the docs:
https://github.com/ClickHouse/ClickHouse/pull/80590/files#diff-ddd55e57d7c3280a3a719f9939480a1971efe8cb51c6200468fc708bdcfa7a21
| } | ||
| } | ||
|
|
||
| static constexpr UInt16 FORMAT_VERSION = 2; |
There was a problem hiding this comment.
When was FORMAT_VERSION equal to 1?
There was a problem hiding this comment.
It was used during testing with an early adopter customer, so for compatibility it makes sense to not reset it.
| namespace DB | ||
| { | ||
|
|
||
| template <bool array_agruments_, typename TimestampType_, typename IntervalType_, typename ValueType_, bool is_rate_> |
There was a problem hiding this comment.
What does array_arguments_ mean? Which test checks such a case?
There was a problem hiding this comment.
timestamp an value parameters of all these functions can be either individual values or arrays. It's described in the docs:
https://github.com/ClickHouse/ClickHouse/pull/80590/files#diff-ede8b2e1838fa4dc7688a22667c5cf49988c061d62c0d81ce4d9f1be21e82e83R54-R64
array_arguments_ template parameter switches the implementation to array params.
Many tests pass timestamps and values as array. Here are some of them:
https://github.com/ClickHouse/ClickHouse/pull/80590/files#diff-e00e69552b275c579e7cd6ff9fde81f5963eb8c0003b9692777b2f63d22bd055R1-R14
|
CH Inc sync — tests failed - test 03145_non_loaded_projection_backup is flaky in private repo Performance Comparison (amd_release,master_head,1/3) performance runs look unstable |
|
Hi @davenger @vitlibar — while reviewing this PR I found the following:
Happy to discuss — close anything that's wrong or already addressed. |
Aggregate functions
timeSeriesResampleToGridWithStaleness- resample time series data to grid with specified steptimeSeriesDeltaToGrid- calculate Prometheusdeltafor a time series on grid with specified steptimeSeriesRateToGrid- calculate Prometheusratefor a time series on grid with specified steptimeSeriesInstantDeltaToGrid- calculate Prometheusideltafor a time series on grid with specified steptimeSeriesInstantRateToGrid- calculate Prometheusiratefor a time series on grid with specified stepAll these functions accept 2 parameters: timestamp and value and return
Array(Nullable(Float64))Example query to resample time series data to grid starting at ts=90 ending at ts=90+120 with step 15: ([90, 105, 120, 135, 150, ... , 210])
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
timeSeries*helper functions to speedup some scenarios when working with time series data:delta,rate,ideltaandirate.Documentation entry for user-facing changes