ILM: searchable snapshot executes before migrate in cold/frozen #68861
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
This moves the execution of the `searchable_snapshot` action before the `migrate` action in the `cold` and `frozen` phases for a more efficient data migration (ie. mounting it as a searchable snapshot directly on the target tier)
andreidan
added
>enhancement
:Data Management/ILM+SLM
elasticmachine
added
the
Team:Data Management
Collaborator
|
Pinging @elastic/es-core-features (Team:Core/Features) |
Contributor
Author
|
Note: I'll have a look over the needed documentation changes (if any) in a subsequent PR. |
Contributor
Author
|
@elasticmachine update branch |
dakrone
requested changes
Feb 10, 2021
Member
dakrone
left a comment
dakrone
left a comment
There was a problem hiding this comment.
I left a couple of small comments, otherwise this looks good!
Comment on lines
136
to
139
| if (phase.getActions().get(SearchableSnapshotAction.NAME) != null) { | ||
| // the `searchable_snapshot` action defines migration rules itself, so no need to inject a migrate action | ||
| return false; | ||
| } |
Member
There was a problem hiding this comment.
I believe we still want to inject a migrate action for the frozen phase, so that a searchable snapshot created in the cold phase transitions to frozen nodes when it reaches the frozen phase, even if there is a searchable snapshot action (because the searchable snapshot action could end up being a noop if the settings are the same)
Contributor
Author
There was a problem hiding this comment.
I think you're right, good shout!
| assert DataTier.validTierName(targetTier) : "invalid data tier name:" + targetTier; | ||
|
|
||
| BranchingStep conditionalSkipActionStep = new BranchingStep(preMigrateBranchingKey, migrationKey, nextStepKey, | ||
| (index, clusterState) -> skipMigrateAction(phase, clusterState.metadata().index(index))); |
Member
There was a problem hiding this comment.
Can you add some debug logging about how we're skipping the migration because of XYZ for this? I think it'd be helpful if someone is trying to diagnose why migrate behaves the way it does.
Contributor
Author
|
@elasticmachine update branch |
Contributor
Author
|
Mentioned the relation between the |
…action Not that searchable_snapshot can precede other actions in the same phase (eg. in frozen it is followed by `migrate`) we need to allow the mounted index to resume executing the ILM policy starting with a step that's part of a new action (ie. migrate). This adds support to resume the execution of the mounted index from another action. With older versions the execution would resume from the PhaseCompleteStep as it was the last action in a phase, which was handled as a special case in the `CopyExecutionStateStep`. This commit generalises the `CopyExecutionStateStep` to be able to resume from any `StepKey`.
andreidan
commented
Feb 11, 2021
Comment on lines
-248
to
-249
| // a policy with only the cold phase will have a null "nextStepKey", hence the "null" nextStepKey passed in below when that's the | ||
| // case |
Contributor
Author
There was a problem hiding this comment.
this is not true anymore. the next step will be the (phase) complete one.
Contributor
Author
dakrone
approved these changes
Feb 11, 2021
Member
dakrone
left a comment
dakrone
left a comment
There was a problem hiding this comment.
LGTM, thanks for changing this!
andreidan
added a commit
to andreidan/elasticsearch
that referenced
this pull request
Feb 15, 2021
…tic#68861) This moves the execution of the `searchable_snapshot` action before the `migrate` action in the `cold` and `frozen` phases for more efficient data migration (ie. mounting it as a searchable snapshot directly on the target tier) Now that searchable_snapshot can precede other actions in the same phase (eg. in frozen it is followed by `migrate`) we need to allow the mounted index to resume executing the ILM policy starting with a step that's part of a new action (ie. migrate). This adds support to resume the execution of the mounted index from another action. With older versions, the execution would resume from the PhaseCompleteStep as it was the last action in a phase, which was handled as a special case in the `CopyExecutionStateStep`. This generalises the `CopyExecutionStateStep` to be able to resume from any `StepKey`. (cherry picked from commit 800ae51) Signed-off-by: Andrei Dan <[email protected]>
andreidan
added a commit
that referenced
this pull request
Feb 15, 2021
…) (#68971) This moves the execution of the `searchable_snapshot` action before the `migrate` action in the `cold` and `frozen` phases for more efficient data migration (ie. mounting it as a searchable snapshot directly on the target tier) Now that searchable_snapshot can precede other actions in the same phase (eg. in frozen it is followed by `migrate`) we need to allow the mounted index to resume executing the ILM policy starting with a step that's part of a new action (ie. migrate). This adds support to resume the execution of the mounted index from another action. With older versions, the execution would resume from the PhaseCompleteStep as it was the last action in a phase, which was handled as a special case in the `CopyExecutionStateStep`. This generalises the `CopyExecutionStateStep` to be able to resume from any `StepKey`. (cherry picked from commit 800ae51) Signed-off-by: Andrei Dan <[email protected]> Co-authored-by: Elastic Machine <[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 moves the execution of the
searchable_snapshotaction before themigrateaction in thecoldandfrozenphases for a more efficientdata migration (ie. mounting it as a searchable snapshot directly on the
target tier)
Resolves #68635