TSDB: Implement downsampling on time-series indices #85708

csoulios · 2022-04-05T20:31:23Z

This PR implements downsampling operation on time series indices.

The PR creates a _rollup endpoint that allows users to downsample an index and can be
accessed by the following call:

POST /<source_index>/_rollup/<rollup_index>
{
    "fixed_interval": "1d"
}

Requirements

An index can be downsampled if all of the following requirements are met:

Must be a time series index (have the index.mode: time_series index setting)
Must not be writeable (have the index.blocks.write: true index setting)
Must have dimension fields marked with mapping parameter time_series_dimension: true
Must have metric fields marked with mapping parameter time_series_metric

Steps

The steps followed by the downsampling action are the following:

Read the index mapping from the source index and extract the dimension and metric fields.
Create a temporary rollup index with the following settings/mapping:
1. The following index settings will be copied from the original index
  - index.mode
  - index.routing_path
  - index.time_series.start_time
  - index.time_series.end_time
2. Set the following index settings to the values of the original index:
  - index.rollup.source.uuid
  - index.rollup.source.name
3. All fields mapped as time_series_dimension will be created in the rollup index with exactly the same mapping.
4. All fields mapped as time_series_metric will be created and their field type will change to aggregate_metric_double.
Use the TimeSeriesIndexSearcher implementation to traverse over the (sorted by _tsid, @timestamp) source index and create the rollup index.
Rename the temporary rollup index to its defined name.
If the source index is a member of a data stream, we must replace the original index with the rollup index. So the rollup index is added and the source index is deleted. The addition and deletion of the indices should be an atomic action.

TODO

Implement logic for storing fields that are neither dimensions nor metrics (aka labels)
Make rollup task cancellable
More testing

Relates to #74660

Fixes #65769
Fixes #69799
Finally, this PR is based on the code written for #64900

Not needed for timeseries indices

Removed most of the configuration, which is extracted from the index mapping. Modified TransportRollupAction to extract the rollup config from the field caps

Removed most of the configuration, which is extracted from the index mapping. Modified TransportRollupAction to extract the rollup config from the field caps.

elasticmachine · 2022-04-05T20:31:26Z

Pinging @elastic/es-analytics-geo (Team:Analytics)

elasticsearchmachine · 2022-04-05T20:31:48Z

Hi @csoulios, I've created a changelog YAML for you.

I will deal with them later

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java

nik9000

I looked at it for a while and left some comments. I think I want to have another look on Monday too.

nik9000 · 2022-05-18T18:15:45Z

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/RollupShardIndexer.java

-                numSent.addAndGet(-items);
+                if (failure != null) {
+                    long items = request.numberOfActions();
+                    numSent.addAndGet(-items);


I think this one might still be worth looking at.

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

nik9000 · 2022-05-20T13:44:26Z

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

+                } else if (e.getValue().values().iterator().next().getMetricType() != null) {
+                    metricFieldCaps.put(field, fieldCaps);
+                } else {
+                    // TODO: Field is not a dimension or a metric. Treat it as a tag


For now we just throw tags away, right?

Yes, in a following PR we will add the tags. I left this there as a placeholder

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

nik9000 · 2022-05-20T13:51:49Z

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

-                                        if (resizeResponse.isAcknowledged()) {
-                                            // 6.
-                                            publishMetadata(originalIndexName, tmpIndexName, rollupIndexName, listener);
+                                    // 5. Add rollup index to data stream and publish rollup metadata


Should we wait for all of the replicas to recover? I think a health check for green on the index is enough for that. I'm not sure that's a thing we always want, but it seems like a reasonable default.

It can wait for a follow up if you'd like.

Not sure why we should wait for the replicas to recover. I was thinking that this can happen in parallel. Any concerns we may lose the primary shard while deleting the source index?

I am afraid that waiting for replicas to recover may take lots of time.

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

weizijun · 2022-05-21T00:19:06Z

I added a PR(#82944) in January, and discuss with @csoulios about the downsampling design several times. Now, I'm waiting for this PR to be merged. Then I can contribute my code together to the downsampling requirement.

- Set refresh_interval to -1 - Refresh index before adding to data stream - Force merge index in the end

nik9000

Let's get this in. There's certainly more to do but I feel like this is a good start for rollups.

nik9000 · 2022-05-23T17:53:49Z

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

+                                    // 5. Refresh rollup index
+                                    refreshIndex(rollupIndexName, parentTask, ActionListener.wrap(refreshIndexResponse -> {
+                                        if (refreshIndexResponse.getFailedShards() == 0) {
+                                            // 6. Add rollup index to data stream and publish rollup metadata


Maybe label it 6-8?

Done in 393d7c2

nik9000 · 2022-05-23T18:37:36Z

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

+                                                    );
+                                                }
+                                            },
+                                                e -> deleteRollupIndex(


I wonder if there is a good way to reduce the duplicate calls here. Oh well, in a follow up.

nik9000 · 2022-05-23T18:39:17Z

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

+                                                                new ElasticsearchException(
+                                                                    "Failed to force-merge index [" + rollupIndexName + "]",
+                                                                    e
+                                                                )


Will this fail the request but not cause the destination index to be deleted? What will ILM think of that?

I was thinking about it. If we reach at this point, rollup will be successful even if force-merge fails. Also, at this point we have delete the source index, so there's no way to rollback and restart the operation.

On the other hand, if force-merge fails we end up with a perfectly healthy rollup index with multiple segments. So, do you think at this point we can log and swallow the exception?

In 393d7c2, I changed it so that it does not fail the rollup action.

nik9000 · 2022-05-24T15:43:33Z

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

+                                                                 * operation.
+                                                                 */
+                                                                logger.error(
+                                                                    "Failed to force-merge rollup index [" + rollupIndexName + "]",


👍

It's probably worth a test that fails here and makes sure we don't fall over. I think this will do the job but it's probably worth tracing this path. Both as a "for now" thing and to make sure that changes in the future don't cause us to lose this property.

This PR implements downsampling operation on time series indices. The PR creates a _rollup endpoint that allows users to downsample an index and can be accessed by the following call: POST /<source_index>/_rollup/<rollup_index> { "fixed_interval": "1d" } Requirements An index can be downsampled if all of the following requirements are met: Must be a time series index (have the index.mode: time_series index setting) Must not be writeable (have the index.blocks.write: true index setting) Must have dimension fields marked with mapping parameter time_series_dimension: true Must have metric fields marked with mapping parameter time_series_metric Relates to elastic#74660 Fixes elastic#65769 Fixes elastic#69799 Finally, this PR is based on the code written for elastic#64900

…88565) This PR modified the rollup action so that all index settings are copied from the source index to the rollup index. Relates to #85708

This PR adds support for an ILM action that downsamples a time-series index by invoking the _rollup endpoint (#85708) A policy that includes the rollup action will look like the following PUT _ilm/policy/my_policy { "policy": { "phases": { "warm": { "actions": { "rollup": { "fixed_interval": "24h" } } } } } } Relates to #74660 Fixes #68609

csoulios added 13 commits April 5, 2022 23:13

Remove feature flag es.rollup_v2_feature_flag_enabled

94fb431

RollupShardIndexer

b8e9d2b

Cleanup

7b7d06b

Test should not merge segments

75ce70c

Refactoring code

98627b1

Fix wrong timestamp order

3215864

Cleanup

34f42bd

Cleanup

29bc089

Removed CompressingOfflineSorter implementation

939a1c0

Not needed for timeseries indices

class to record

23d39a8

Added more tests

0f40815

Refactored rollup action config

7a44a68

Removed most of the configuration, which is extracted from the index mapping. Modified TransportRollupAction to extract the rollup config from the field caps

Refactored rollup action config

b94e82d

Removed most of the configuration, which is extracted from the index mapping. Modified TransportRollupAction to extract the rollup config from the field caps.

csoulios added >feature :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data :StorageEngine/Metrics You know, for Metrics v8.3.0 labels Apr 5, 2022

elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 5, 2022

csoulios marked this pull request as draft April 5, 2022 20:31

csoulios and others added 8 commits April 5, 2022 23:31

Update docs/changelog/85708.yaml

e514fbb

Version updates

219da73

CI Fixes

1a385e7

checkstyle

633abac

checkstyle

d4daeb8

Skip some failing tests

496e00f

I will deal with them later

Fix broken test with wrong index sort order

5a0642c

Fix ILM test

d0222f0

csoulios commented Apr 11, 2022

View reviewed changes

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java Show resolved Hide resolved

csoulios added 5 commits May 19, 2022 23:04

Added feature flag to security tests

270ea69

Added feature flag to build files

7197a5f

Reverted wrong version bump

8b96364

Add the number of indexed docs to the shard response

839a29d

Added javadoc

de6b766

csoulios requested a review from nik9000 May 20, 2022 13:33

nik9000 reviewed May 20, 2022

View reviewed changes

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java Show resolved Hide resolved

csoulios added 5 commits May 23, 2022 10:45

Merge branch 'master' into tsdb-rollup

0f3f827

Changes to rollup index:

0ff80dc

- Set refresh_interval to -1 - Refresh index before adding to data stream - Force merge index in the end

Remove org.elasticsearch.rollup from module-info

7c4c441

Merge branch 'master' into tsdb-rollup

5856f1d

Added more validation for source index

1dd28fd

csoulios requested a review from nik9000 May 23, 2022 13:18

nik9000 approved these changes May 23, 2022

View reviewed changes

csoulios added 2 commits May 24, 2022 10:41

Do not fail rollup if force-merge fails

393d7c2

Merge branch 'master' into tsdb-rollup

bfd9ce9

nik9000 approved these changes May 24, 2022

View reviewed changes

Merge branch 'master' into tsdb-rollup

0a06f41

csoulios merged commit d539957 into elastic:master May 25, 2022

csoulios deleted the tsdb-rollup branch May 25, 2022 14:16

This was referenced May 31, 2022

TSDB: Implement Downsampling ILM Action for time-series indices #87269

Merged

[RollupV2]: make RollupAction available and improve some features #82944

Closed

add rollup index check before doing rollup #79596

Closed

csoulios mentioned this pull request Jun 9, 2022

[WIP] Add DownSampleAction to downsampling time_series index #83771

Closed

csoulios mentioned this pull request Jul 15, 2022

TSDB: Downsampling copies all settings from source to rollup index #88565

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TSDB: Implement downsampling on time-series indices #85708

TSDB: Implement downsampling on time-series indices #85708

csoulios commented Apr 5, 2022 •

edited by nik9000

Loading

elasticmachine commented Apr 5, 2022

elasticsearchmachine commented Apr 5, 2022

nik9000 left a comment

nik9000 May 18, 2022

nik9000 May 20, 2022

csoulios May 20, 2022

nik9000 May 20, 2022

nik9000 May 20, 2022

csoulios May 20, 2022

weizijun commented May 21, 2022

nik9000 left a comment

nik9000 May 23, 2022

csoulios May 24, 2022

nik9000 May 23, 2022

nik9000 May 23, 2022

csoulios May 24, 2022

csoulios May 24, 2022

nik9000 May 24, 2022

TSDB: Implement downsampling on time-series indices #85708

TSDB: Implement downsampling on time-series indices #85708

Conversation

csoulios commented Apr 5, 2022 • edited by nik9000 Loading

Requirements

Steps

TODO

elasticmachine commented Apr 5, 2022

elasticsearchmachine commented Apr 5, 2022

nik9000 left a 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

weizijun commented May 21, 2022

nik9000 left a 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

csoulios commented Apr 5, 2022 •

edited by nik9000

Loading