TSDB: Downsampling copies all settings from source to rollup index #88565
Conversation
csoulios
added
>non-issue
:StorageEngine/Rollup
elasticmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java
Outdated
Show resolved
Hide resolved
| targetSettings.put(IndexMetadata.INDEX_ROLLUP_SOURCE_NAME.getKey(), sourceIndex.getName()) | ||
| .put(IndexMetadata.INDEX_ROLLUP_SOURCE_UUID.getKey(), sourceIndex.getUUID()); |
There was a problem hiding this comment.
Do we already have a test for doing a rollup-of-a-rollup where the original index is maintained in the rollup source/uuid? If not, could you add one?
There was a problem hiding this comment.
Right now rollup-of-rollups does not fully work. I plan to add support for rollup-of-rollup in a following PR.
I will definitely add tests in that PR
This part is only plumbing that is going to be needed for rollups of rollups
There was a problem hiding this comment.
Right now rollup-of-rollups does not fully work.
As some subsequent work (not in this PR), does it make sense to reject the request if someone tries to rollup an index that's already been rolled up?
There was a problem hiding this comment.
In data streams this cannot happen because no two indices for the same period can co-exist.
When it comes to concrete indices, there are very valid use cases for allowing multiple rollup indices of the same source index. For example having multiple rollup intervals (hourlies, dailies etc) or having multiple timezones (right now we support only UTC, but tin the future we should support more timezones).
| // Number of replicas had been previously set to 0 to speed up index population | ||
| if (sourceIndexMetadata.getNumberOfReplicas() > 0) { | ||
| settings.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, sourceIndexMetadata.getNumberOfReplicas()); | ||
| } |
There was a problem hiding this comment.
I don't understand this piece, why do we copy replicas only if they are greater than 0? How would this behave if someone had something like index.auto_expand_replicas: 0-2 where the number of replicas is variable? Shouldn't we be consistent and maintain the number of replicas as defined?
There was a problem hiding this comment.
The idea behind this is that when we create the rollup index we explicitly set that num_of_replicas=0. We do so, because we don't want the index to replicate while we populate the index.
When all documents have been written in the index, we set the num_of_replicas equal to the num_of_replicas of the source index. Of course, if num_of_replicas of the source index is 0, we don't have to do anything, hence the conditional.
| targetSettings.put(IndexMetadata.INDEX_ROLLUP_SOURCE_NAME.getKey(), sourceIndex.getName()) | ||
| .put(IndexMetadata.INDEX_ROLLUP_SOURCE_UUID.getKey(), sourceIndex.getUUID()); |
There was a problem hiding this comment.
Right now rollup-of-rollups does not fully work.
As some subsequent work (not in this PR), does it make sense to reject the request if someone tries to rollup an index that's already been rolled up?
|
Thanks a lot for the review! |
After merging PR elastic#88565, that delegates this task to the actual rollup step
…#88812) PR #88565 modified the rollup action so that it copies all index settings from the source to the rollup index. However, the rollup action expilitly requires that the source index has the index.block.write: true. Since the rollup index must be writeable when it is created, this PR excludes the index.block.write i ndex setting when creating the rollup index. The index.block.write index setting is explictly set to true at the end of the rollup action (step 4).
This PR modified the rollup action so that all index settings are copied from the source index to the rollup index.
Relates to #85708