[TSDB] Rename rollup public API to downsample #89809

csoulios · 2022-09-05T19:17:45Z

This PR renames all public APIs for downsampling so that they contain the downsample keyword instead of the rollup that we had until now.

The API endpoint for the downsampling action is renamed to:

/source-index/_downsample/target-index

The ILM action is renamed to

PUT _ilm/policy/my_policy
{
  "policy": {
    "phases": {
      "warm": {
        "actions": {
          "downsample": {
  	    "fixed_interval": "24h"
  	  }
  	}
      }
    }
  }
}

unsupported_aggregation_on_rollup_index was renamed to unsupported_aggregation_on_downsampled_index
Internal trasport actions were renamed:

indices:admin/xpack/rollup -> indices:admin/xpack/downsample
indices:admin/xpack/rollup_indexer -> indices:admin/xpack/downsample_indexer

Renamed the following index settings:

index.rollup.source.uuid -> index.downsample.source.uuid
index.rollup.source.name -> index.downsample.source.name
index.rollup.status -> index.downsample.status

Finally, we renamed many internal variables and classes from *Rollup* to *Downsample*. However, this effort will be completed in more than one PRs so that we minimize conflicts with other in-flight PRs.

Relates to #74660

Also renamed RollupActionConfig to DownsampleConfig

Added transport actions to operator Constants

csoulios · 2022-09-06T12:15:44Z

cc @ghudgins @Dosant @flash1293 @salvatore-campagna
cc @elastic/es-clients

salvatore-campagna · 2022-09-06T13:11:37Z

Change of unsupported_aggregation_on_rollup_index might break Kibana integration. Maybe this is not an issue if the Kibana feature is still in development.

csoulios · 2022-09-06T13:20:17Z

Change of unsupported_aggregation_on_rollup_index might break Kibana integration. Maybe this is not an issue if the Kibana feature is still in development.

Thanks for the feedback, Salvatore. I have already discussed this with @flash1293 and we have agreed Kibana will change this as soon as this PR is merged.

flash1293 · 2022-09-06T13:21:43Z

We are aware - this integration is still on a PR so no worries.

elasticsearchmachine · 2022-09-06T18:08:28Z

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

nik9000 · 2022-09-06T18:11:42Z

rest-api-spec/src/main/resources/rest-api-spec/api/downsample.downsample.json

@@ -1,8 +1,8 @@
 {
-  "rollup.rollup":{
+  "downsample.downsample":{


What does this do to our client friends?

Clients should probably be modified so that they invokes the downsample endpoint.

I have already tagged the @elastic/es-clients team in this PR. I will reach out to them and give them a heads up about this change.

If the only API rename is rollup.rollup into downsample.downsample this should not be an issue since this API is experimental and we can have BC break.

@ezimuel thanks for confirming. The detailed list of changes introduced can be found in the description of this PR. Regarding the API this is the only change.

nik9000 · 2022-09-06T18:13:45Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/downsample/DownsampleAction.java

-    public static final String NAME = "indices:admin/xpack/rollup";
+public class DownsampleAction extends ActionType<AcknowledgedResponse> {
+    public static final DownsampleAction INSTANCE = new DownsampleAction();
+    public static final String NAME = "indices:admin/xpack/downsample";


Will this break the mixed cluster case? That's fine for the unreleased downsample stuff but not for the old rollup code I think.

Old rollup code is behind a feature flag and has not been released. We will probably need to bump up the supported versions to 8.5.0 for those tests.

sethmlarson

We're removing the rollup.rollup API which is experimental but has been around for a year. Do we know this API isn't being used often so a direct removal without deprecation is acceptable?
We're creating a new namespace here "downsample" which we try to only do if there's a good reason. Do we see more APIs being added to this namespace that don't match other existing namespaces? My initial thought for an existing alternative would be indices.downsample?

csoulios · 2022-09-07T15:02:27Z

@sethmlarson Thank you for the feedback.

We're removing the rollup.rollup API which is experimental but has been around for a year. Do we know this API isn't being used often so a direct removal without deprecation is acceptable?

This API is not used because it has always been hidden behind a feature flag. It is safe to directly remove it.

We're creating a new namespace here "downsample" which we try to only do if there's a good reason. Do we see more APIs being added to this namespace that don't match other existing namespaces? My initial thought for an existing alternative would be indices.downsample?

This is an excellent catch. I will change it to indices.downsample so that we don't create a separate namespace for it.
Update: Implemented this in ac960eb

* main: (175 commits) Fix integration test on Windows (elastic#89894) Avoiding the use of dynamic map keys in the cluster_formation results of the stable master health indicator (elastic#89842) Mute org.elasticsearch.tracing.apm.ApmIT.testCapturesTracesForHttpTraffic (elastic#89891) Fix typos in audit event types (elastic#89886) Synthetic _source: support histogram field (elastic#89833) [TSDB] Rename rollup public API to downsample (elastic#89809) Format script values access (elastic#89780) [DOCS] Simplifies composite aggregation recommendation (elastic#89878) [DOCS] Update CCS compatibility matrix for 8.3 (elastic#88906) Fix memory leak when double invoking RestChannel.sendResponse (elastic#89873) [ML] Add processor autoscaling decider (elastic#89645) Update disk-usage.asciidoc (elastic#89709) (elastic#89874) Add allocation deciders in createComponents (elastic#89836) Mute flaky H3LatLonGeometryTest.testIndexPoints (elastic#89870) Fix typo in get-snapshot-status-api doc (elastic#89865) Picking master eligible node at random in the master stability health indicator (elastic#89841) Do not reuse the client after a disruption elastic#89815 (elastic#89866) [ML] Distribute trained model allocations across availability zones (elastic#89822) Increment clientCalledCount before onResponse (elastic#89858) AwaitsFix for elastic#89867 ...

* main: (175 commits) Fix integration test on Windows (elastic#89894) Avoiding the use of dynamic map keys in the cluster_formation results of the stable master health indicator (elastic#89842) Mute org.elasticsearch.tracing.apm.ApmIT.testCapturesTracesForHttpTraffic (elastic#89891) Fix typos in audit event types (elastic#89886) Synthetic _source: support histogram field (elastic#89833) [TSDB] Rename rollup public API to downsample (elastic#89809) Format script values access (elastic#89780) [DOCS] Simplifies composite aggregation recommendation (elastic#89878) [DOCS] Update CCS compatibility matrix for 8.3 (elastic#88906) Fix memory leak when double invoking RestChannel.sendResponse (elastic#89873) [ML] Add processor autoscaling decider (elastic#89645) Update disk-usage.asciidoc (elastic#89709) (elastic#89874) Add allocation deciders in createComponents (elastic#89836) Mute flaky H3LatLonGeometryTest.testIndexPoints (elastic#89870) Fix typo in get-snapshot-status-api doc (elastic#89865) Picking master eligible node at random in the master stability health indicator (elastic#89841) Do not reuse the client after a disruption elastic#89815 (elastic#89866) [ML] Distribute trained model allocations across availability zones (elastic#89822) Increment clientCalledCount before onResponse (elastic#89858) AwaitsFix for elastic#89867 ... # Conflicts: # x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/downsample/RollupShardIndexer.java

* main: (283 commits) Fix integration test on Windows (elastic#89894) Avoiding the use of dynamic map keys in the cluster_formation results of the stable master health indicator (elastic#89842) Mute org.elasticsearch.tracing.apm.ApmIT.testCapturesTracesForHttpTraffic (elastic#89891) Fix typos in audit event types (elastic#89886) Synthetic _source: support histogram field (elastic#89833) [TSDB] Rename rollup public API to downsample (elastic#89809) Format script values access (elastic#89780) [DOCS] Simplifies composite aggregation recommendation (elastic#89878) [DOCS] Update CCS compatibility matrix for 8.3 (elastic#88906) Fix memory leak when double invoking RestChannel.sendResponse (elastic#89873) [ML] Add processor autoscaling decider (elastic#89645) Update disk-usage.asciidoc (elastic#89709) (elastic#89874) Add allocation deciders in createComponents (elastic#89836) Mute flaky H3LatLonGeometryTest.testIndexPoints (elastic#89870) Fix typo in get-snapshot-status-api doc (elastic#89865) Picking master eligible node at random in the master stability health indicator (elastic#89841) Do not reuse the client after a disruption elastic#89815 (elastic#89866) [ML] Distribute trained model allocations across availability zones (elastic#89822) Increment clientCalledCount before onResponse (elastic#89858) AwaitsFix for elastic#89867 ...

csoulios added 3 commits September 5, 2022 20:23

Rename rollup action to downsample

a6954d2

Rename rollup exception to downsample

9214822

Rename rollup ILM action to downsample

380bd8e

Also renamed RollupActionConfig to DownsampleConfig

csoulios added >non-issue :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data v8.5.0 labels Sep 5, 2022

csoulios added 3 commits September 6, 2022 11:07

Merge branch 'main' into rollups-rename

1438109

checkstyle

f7ba827

Fixed tests

3567e27

Added transport actions to operator Constants

csoulios added the Team:Clients Meta label for clients team label Sep 6, 2022

csoulios added 2 commits September 6, 2022 15:40

Merge branch 'main' into rollups-rename

4d4681e

Fixed tests

f4d489c

Renamed index settings

43e686a

csoulios requested review from salvatore-campagna and nik9000 September 6, 2022 18:07

csoulios marked this pull request as ready for review September 6, 2022 18:08

elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed Team:Clients Meta label for clients team labels Sep 6, 2022

nik9000 reviewed Sep 6, 2022

View reviewed changes

Fix test

0a8b6c2

sethmlarson reviewed Sep 7, 2022

View reviewed changes

salvatore-campagna mentioned this pull request Sep 7, 2022

Include a new downsampling operation elastic/rally#1574

Merged

Change API namespace to indices

ac960eb

nik9000 approved these changes Sep 7, 2022

View reviewed changes

csoulios merged commit f8d1d2a into elastic:main Sep 7, 2022

csoulios deleted the rollups-rename branch September 7, 2022 16:23

tsullivan mentioned this pull request Sep 8, 2022

[Search] Re-enable test on example search app elastic/kibana#139961

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TSDB] Rename rollup public API to downsample #89809

[TSDB] Rename rollup public API to downsample #89809

csoulios commented Sep 5, 2022 •

edited

Loading

csoulios commented Sep 6, 2022 •

edited

Loading

salvatore-campagna commented Sep 6, 2022

csoulios commented Sep 6, 2022

flash1293 commented Sep 6, 2022

elasticsearchmachine commented Sep 6, 2022

nik9000 Sep 6, 2022

csoulios Sep 7, 2022

ezimuel Sep 7, 2022

csoulios Sep 7, 2022

nik9000 Sep 6, 2022

csoulios Sep 7, 2022

sethmlarson left a comment

csoulios commented Sep 7, 2022 •

edited

Loading

[TSDB] Rename rollup public API to downsample #89809

[TSDB] Rename rollup public API to downsample #89809

Conversation

csoulios commented Sep 5, 2022 • edited Loading

csoulios commented Sep 6, 2022 • edited Loading

salvatore-campagna commented Sep 6, 2022

csoulios commented Sep 6, 2022

flash1293 commented Sep 6, 2022

elasticsearchmachine commented Sep 6, 2022

nik9000 Sep 6, 2022

Choose a reason for hiding this comment

csoulios Sep 7, 2022

Choose a reason for hiding this comment

ezimuel Sep 7, 2022

Choose a reason for hiding this comment

csoulios Sep 7, 2022

Choose a reason for hiding this comment

nik9000 Sep 6, 2022

Choose a reason for hiding this comment

csoulios Sep 7, 2022

Choose a reason for hiding this comment

sethmlarson left a comment

Choose a reason for hiding this comment

csoulios commented Sep 7, 2022 • edited Loading

csoulios commented Sep 5, 2022 •

edited

Loading

csoulios commented Sep 6, 2022 •

edited

Loading

csoulios commented Sep 7, 2022 •

edited

Loading