-
Notifications
You must be signed in to change notification settings - Fork 25.3k
ILM: migrate action configures the _tier_preference setting #62829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ILM: migrate action configures the _tier_preference setting #62829
Conversation
The `migrate` action will now configure the `index.routing.allocation.include._tier_preference` setting to the corresponding tiers. For the HOT phase it will configure `data_hot`, for the WARM phase it will configure `data_warm,data_hot` and for the COLD phase `data_cold,data_warm,data_cold`.
Pinging @elastic/es-core-features (:Core/Features/ILM+SLM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Andrei, I left some comments below but they're pretty minor, otherwise this LGTM
logger.debug("[{}] lifecycle action for index [{}] cannot make progress because not all shards are active", | ||
getKey().getAction(), index.getName()); | ||
} else { | ||
logger.debug("[{}] migration of index [{}] to the [{}] tier cannot progress, as not all shards are active", | ||
getKey().getAction(), index.getName(), destinationTier); | ||
if (Strings.hasText(availableDestinationTier)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this becomes the equivalent of a null check with the .orElse("")
above, maybe we can just leave it in an Optional<String>
above and do availableDestinationTier.isPresent()
, which I think is a bit clearer than checking for text (especially since we don't care what the text is)
.../plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java
Outdated
Show resolved
Hide resolved
statusMessage = String.format(Locale.ROOT, "[%s] lifecycle action [%s] waiting for [%s] shards to be moved to the [%s] " + | ||
"tier", index.getName(), getKey().getAction(), allocationPendingAllShards, availableDestinationTier); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this message we should include the original preference setting, or else I think users will be confused why their cold
phase index is waiting to move to the data_warm
tier (if they see preference [data_cold,data_warm,data_hot]
in the message I hope it will be clearer to them)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great point, updating the message to include the preference. Thanks Lee
.../plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java
Outdated
Show resolved
Hide resolved
throw new IllegalArgumentException("invalid data tier [" + targetTier + "]"); | ||
} | ||
String preferredTiers = COLD_TO_HOT_TIERS.stream().skip(indexOfTargetTier).collect(Collectors.joining(",")); | ||
if (Strings.hasText(preferredTiers) == false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we keep this, we could reverse the double negative with
if (Strings.hasText(preferredTiers) == false) { | |
if (Strings.isEmpty(preferredTiers)) { |
But what I'm trying to understand is how this would actually be empty, even in the hot phase the stream skip above would at least return "data_hot"
, and we check that the index is not -1 above, so I'm not sure how it actually gets into this state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is my paranoia, you're right, I'll drop it
Co-authored-by: Lee Hinman <dakrone@users.noreply.github.com>
Co-authored-by: Lee Hinman <dakrone@users.noreply.github.com>
Co-authored-by: Lee Hinman <dakrone@users.noreply.github.com>
@elasticmachine update branch |
…62829) The `migrate` action will now configure the `index.routing.allocation.include._tier_preference` setting to the corresponding tiers. For the HOT phase it will configure `data_hot`, for the WARM phase it will configure `data_warm,data_hot` and for the COLD phase `data_cold,data_warm,data_cold`. (cherry picked from commit 9dbf0e6) Signed-off-by: Andrei Dan <andrei.dan@elastic.co>
…2829) (#62860) The `migrate` action will now configure the `index.routing.allocation.include._tier_preference` setting to the corresponding tiers. For the HOT phase it will configure `data_hot`, for the WARM phase it will configure `data_warm,data_hot` and for the COLD phase `data_cold,data_warm,data_cold`. (cherry picked from commit 9dbf0e6) Signed-off-by: Andrei Dan <andrei.dan@elastic.co>
The
migrate
action will now configure theindex.routing.allocation.include._tier_preference
setting to the correspondingtiers. For the HOT phase it will configure
data_hot
, for the WARM phase it willconfigure
data_warm,data_hot
and for the COLD phasedata_cold,data_warm,data_cold
.Relates to #60848