Add new sampling method to the Downsample API#136813
Conversation
gmarouli
added
>enhancement
:StorageEngine/Downsampling
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
|
Hi @gmarouli, I've created a changelog YAML for you. |
| if (method != null) { | ||
| return method; | ||
| } | ||
| boolean isIndexDownsampled = indexMetadata.getSettings().get(IndexMetadata.INDEX_DOWNSAMPLE_INTERVAL_KEY) != null; |
There was a problem hiding this comment.
If this is true, wouldn't fromString return a valid method above? Maybe I missed the logic here.
There was a problem hiding this comment.
You are correct, the code as it is now will always have a sampling method. This is for backwards compatibility. Indices before this PR will not have sampling method defined in their metadata and we know that only the aggregate was available, so this is why we specify it here. Even if we ever change the default, this shouldn't change. I will add a comment so it is clear.
There was a problem hiding this comment.
Should we be just returning AGGREGATE then, without checking the setting again?
There was a problem hiding this comment.
I did consider this as well to avoid this check. If we do that we move the responsibility to the caller, it's possible that the caller has already checked that so this is a redundant check. On the other hand, I do think that it is part of this methods responsibility to return null when the index is not downsampled.
If it was on the critical path, I would consider removing the check and adding a warning in the javadoc of this method. However, considering that this is called mainly by the downsample API and by the data stream lifecycle in the future, I think it's a performance penalty we can accept. What do you think?
server/src/main/java/org/elasticsearch/action/downsample/DownsampleConfig.java
Outdated
Show resolved
Hide resolved
...downsample/src/internalClusterTest/java/org/elasticsearch/xpack/downsample/DownsampleIT.java
Outdated
Show resolved
Hide resolved
…ticsearch/xpack/downsample/DownsampleIT.java Co-authored-by: Kostas Krikellas <131142368+kkrik-es@users.noreply.github.com>
|
Thanks @martijnvg , apart from adjusting these tests, I ensured that all tests apart from (ILM, DLM & telemetry) are also using random sampling. |
martijnvg
left a comment
martijnvg
left a comment
There was a problem hiding this comment.
Thanks for adjusting the tests! LGTM
Following #136813, we expose to ILM the new sampling method config in the downsampling API. This will allow users to configure the sampling method in their downsample action of their ILM policies. For example: ``` PUT _ilm/policy/datastream_policy { "policy": { "phases": { "hot": { "actions": { "rollover": { "max_docs": 1 }, "downsample": { "fixed_interval": "1h", "force_merge_index": false, "sampling_method": "aggregate" } } } } } } ```
When downsampling gauge metrics, we create an aggregate object that contains the min, max, sum and value_count values that allow us to respond to the min, max, sum, count and average aggregations without losing any accuracy.
However, we recognise that for certain cases keeping just the last value might be good enough. For this reason, we add one more configuration option in the downsample API that allows a user to choose the sampling method between:
aggregate: when supported, this will downsample a metric by summarising its values in a aggregate document. This remains the default
last_value: this will keep the last value and discard the rest.
```
POST /my-time-series-index/_downsample/my-downsampled-time-series-index
{
"fixed_interval": "1d",
"sampling_method": "last_value"
}
```
This PR introduces the sampling method only in the downsampling API. There will be follow ups that will introduce it in the lifecycle management features.
Relates: elastic#128357.
…37023) Following #136813, we expose to data stream lifecycle the new sampling method config in the downsampling API. This will allow users to configure the sampling method directly in the lifecycle configuration. For example: ``` PUT _data_stream/my-ds/_lifecycle { "data_retention": "10d", "downsampling_method": "last_value", "downsampling": [ { "after": "1d", "fixed_interval": "5m } ] } ```
4 participants
When downsampling gauge metrics, we create an aggregate object that contains the
min,max,sumandvalue_countvalues that allow us to respond to themin,max,sum,countandaverageaggregations without losing any accuracy.However, we recognise that for certain cases keeping just the last value might be good enough. For this reason, we add one more configuration option in the downsample API that allows a user to choose the sampling method between:
aggregate: when supported, this will downsample a metric by summarising its values in a aggregate document. This remains the defaultlast_value: this will keep the last value and discard the rest.This PR introduces the sampling method only in the downsampling API. There will be follow ups that will introduce it in the lifecycle management features.
Relates: #128357.