[RollupV2] Move rollup metadata to field mappings #69921
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also, added source index uuid in the rollup index metadata. Removed rollup group and rollup metadata as they are not used any more.
csoulios
added
>non-issue
:StorageEngine/Rollup
elasticmachine
added
the
Team:Analytics
Collaborator
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
csoulios
changed the title
talevy
reviewed
Mar 3, 2021
Contributor
talevy
left a comment
talevy
left a comment
There was a problem hiding this comment.
I'm glad to see this change come in! I think it is great to leverage the mapping metadata to hold this information.
...ugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/RollupMetadata.java
Show resolved
Hide resolved
x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java
Show resolved
Hide resolved
talevy
reviewed
Mar 9, 2021
Contributor
talevy
left a comment
talevy
left a comment
There was a problem hiding this comment.
I left some additional comments
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java
Outdated
Show resolved
Hide resolved
...ugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@elasticmachine run elasticsearch-ci/1 |
talevy
approved these changes
Mar 10, 2021
Contributor
talevy
left a comment
talevy
left a comment
There was a problem hiding this comment.
Left one testing comment, but no need to iterate. LGTM
| return response; | ||
| } | ||
|
|
||
| private void assertRollupIndex(RollupActionConfig config) { |
Contributor
There was a problem hiding this comment.
maybe this method should be removed and replaced with direct calls to assertRollupIndex(config, index, rollupIndex) so things are more explicit
Contributor
Author
There was a problem hiding this comment.
Thanks a lot for the feedback and all the help.
I am removing assertRollupIndex(RollupActionConfig config) and also rollup(RollupActionConfig config)
Contributor
Author
|
@elasticmachine run elasticsearch-ci/bwc |
probakowski
pushed a commit
to probakowski/elasticsearch
that referenced
this pull request
Mar 15, 2021
This PR moves field related rollup metadata from the index metadata to the field mapping metadata.
- date_histogram fields is moved to the timestamp field
(fields are fixed_interval or calendar_interval, time_zone)
- histogram fields are moved to the numeric field on which the the histogram is computed
(the field is named interval)
Also, the index uuid has been added to the index rollup settings as index.rollup.source.uuid
and index.rollup.source.name
The rest of the RollupMetadata has been removed and no rollup metadata exists in the global
cluster state.
Co-authored-by: Tal Levy <[email protected]>
csoulios
added a commit
that referenced
this pull request
Mar 22, 2021
Backports #69921 to 7.x This PR moves field related rollup metadata from the index metadata to the field mapping metadata. date_histogram fields is moved to the timestamp field (fields are fixed_interval or calendar_interval, time_zone) histogram fields are moved to the numeric field on which the the histogram is computed (the field is named interval) Also, the index uuid has been added to the index rollup settings as index.rollup.source.uuid and index.rollup.source.name The rest of the RollupMetadata has been removed. Finally, no rollup metadata exists in the global cluster state Co-authored-by: Tal Levy <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR moves field related rollup metadata from the index metadata to the field mapping metadata.
date_histogramfields is moved to the timestamp field (fields arefixed_intervalorcalendar_interval,time_zone)histogramfields are moved to the numeric field on which the the histogram is computed (the field is namedinterval)Also, the index uuid has been added to the index rollup settings as
index.rollup.source.uuidandindex.rollup.source.nameThe rest of the
RollupMetadatahas been removed. Finally, no rollup metadata exists in the global cluster state