Generate random rollup index names for RollupILMAction#69237
Generate random rollup index names for RollupILMAction#69237talevy merged 7 commits intoelastic:masterfrom
Conversation
This commit moves away from the static `rollup-{indexName}` rollup index
naming strategy and moves towards a randomized rollup index name scheme.
This will reduce the complications that exist if the RollupStep fails and retries
in any way. A separate cleanup will still be required for failed temporary indices,
but at least there will not be a conflict.
This commit generates the new rollup index name in the LifecycleExecutionState so
that it can be used in RollupStep and UpdateRollupIndexPolicyStep on a per-index
basis.
talevy
added
>non-issue
:StorageEngine/Rollup
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
talevy
added
the
:Data Management/ILM+SLM
elasticmachine
added
the
Team:Data Management (obsolete)
|
Pinging @elastic/es-core-features (Team:Core/Features) |
csoulios
left a comment
csoulios
left a comment
There was a problem hiding this comment.
Overall, PR looks good. I have only left a comment about index naming in data streams.
The convention is that data stream indices are prefixed with a . and I don't see why we should not follow it.
|
|
||
| // TODO(talevy): find alternative to lowercasing UUID? kind of defeats the expectation of the UUID here. index names must lowercase | ||
| public static String generateRollupIndexName(String indexName) { | ||
| return "rollup-" + indexName + "-" + UUIDs.randomBase64UUID().toLowerCase(Locale.ROOT); |
There was a problem hiding this comment.
I wonder what should happen if index is a member of a data stream. If the name of the index is .ds-index-2022-01-01-000001, rollup index would be rollup-.ds-index-2022-01-01-000001-<random uuid>.
Is this the expected behavior or would we expect the rollup index to be named .rollup-ds-index-2022-01-01-000001-<random uuid> and keep the . prefix?
There was a problem hiding this comment.
I agree, this is a strange departure. I believe Shrink Action in ILM did not use the . syntax because ILM operates more generally on indices, not data streams. So shrunk- as a prefix made sense. For consistency (#65633 (comment)) with both Searchable Snapshot and Shrink, this rollup- prefix was chosen.
If we wish to change this depending on whether ILM is operating on a datastream or not, I think we can bring this up for discussion with the ILM team.
There was a problem hiding this comment.
I'd like to continue this conversation outside of this PR if you think that is reasonable.
It may make sense to update all the actions to follow this pattern.
There was a problem hiding this comment.
I think this is part of a bigger discussion (outside of this PR), we can handle it on the core/features team and we can update this if/when we have a decision on how to treat these
There was a problem hiding this comment.
In this case, I am fine with moving on with this naming now. Thanks
This commit moves away from the static `rollup-{indexName}` rollup index
naming strategy and moves towards a randomized rollup index name scheme.
This will reduce the complications that exist if the RollupStep fails and retries
in any way. A separate cleanup will still be required for failed temporary indices,
but at least there will not be a conflict.
This commit generates the new rollup index name in the LifecycleExecutionState so
that it can be used in RollupStep and UpdateRollupIndexPolicyStep on a per-index
basis.
This commit moves away from the static `rollup-{indexName}` rollup index
naming strategy and moves towards a randomized rollup index name scheme.
This will reduce the complications that exist if the RollupStep fails and retries
in any way. A separate cleanup will still be required for failed temporary indices,
but at least there will not be a conflict.
This commit generates the new rollup index name in the LifecycleExecutionState so
that it can be used in RollupStep and UpdateRollupIndexPolicyStep on a per-index
basis.
5 participants
This commit moves away from the static
rollup-{indexName}rollup indexnaming strategy and moves towards a randomized rollup index name scheme.
This will reduce the complications that exist if the RollupStep fails and retries
in any way. A separate cleanup will still be required for failed temporary indices,
but at least there will not be a conflict.
This commit generates the new rollup index name in the LifecycleExecutionState so
that it can be used in RollupStep and UpdateRollupIndexPolicyStep on a per-index
basis.