Skip Shrink when numberOfShards not changed #37953
Conversation
talevy
added
>enhancement
:Data Management/ILM+SLM
|
Pinging @elastic/es-core-features |
Previously, ShrinkAction would fail if it was executed on an index that had the same number of shards as the target shrunken number. This PR introduced a new BranchingStep that is used inside of ShrinkAction to branch which step to move to next, depending on the shard values. So no shrink will occur if the shard count is unchanged.
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/BranchingStep.java
Show resolved
Hide resolved
.../plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/ExecuteStepsUpdateTask.java
Outdated
Show resolved
Hide resolved
| logger.trace("[{}] performing cluster state action ({}) [{}], next: [{}]", | ||
| index.getName(), currentStep.getClass().getSimpleName(), currentStep.getKey(), currentStep.getNextStepKey()); | ||
| logger.trace("[{}] performing cluster state action ({}) [{}]", | ||
| index.getName(), currentStep.getClass().getSimpleName(), currentStep.getKey()); | ||
| try { | ||
| state = ((ClusterStateActionStep) currentStep).performAction(index, state); |
There was a problem hiding this comment.
In the future perhaps we will want to return Tuple<ClusterState, Step.SteyKey> from this. The disconnect in terms of the ordering of how things have to be called feels a little weird to me, I'd prefer it to be completely functional.
What do you think?
There was a problem hiding this comment.
I think that would be a reasonable direction to go, yes.
.../plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/ExecuteStepsUpdateTask.java
Show resolved
Hide resolved
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/BranchingStep.java
Outdated
Show resolved
Hide resolved
thanks for the review Lee! I, too, feel uneasy about the brittleness of this. Should we have a path forward in place for |
@talevy - Can you log a refactoring issue with some thoughts on why this is brittle and a potential way to address ? I am putting together a list of the next round changes for ILM and this will likely be on that list. |
There was a problem hiding this comment.
LGTM assuming CI is happy; I agree with @jakelandis for next steps on the brittleness. Open an issue and link this one, we can prioritize the tech debt for the next release.
|
opened #38047 as a reference for future discussion |
|
run elasticsearch-ci/1 |
Previously, ShrinkAction would fail if it was executed on an index that had the same number of shards as the target shrunken number. This PR introduced a new BranchingStep that is used inside of ShrinkAction to branch which step to move to next, depending on the shard values. So no shrink will occur if the shard count is unchanged.
* elastic/master: ILM setPriority corrections for a 0 value (elastic#38001) Temporarily disable BWC for retention lease stats (elastic#38049) Skip Shrink when numberOfShards not changed (elastic#37953) Add dispatching to `HandledTransportAction` (elastic#38050) Update httpclient for JDK 11 TLS engine (elastic#37994) Reduce flaxiness of ccr recovery timeouts test (elastic#38035) Fix ILM status to allow unknown fields (elastic#38043) Fix ILM Lifecycle Policy to allow unknown fields (elastic#38041) Update verify repository to allow unknown fields (elastic#37619) [ML] Datafeed deprecation checks (elastic#38026) Deprecate minimum_master_nodes (elastic#37868) Remove types from watcher docs (elastic#38002) Add test coverage for Painless general casting of boolean and Boolean (elastic#37780) Fixed test bug, lastFollowTime is null if there are no follower indices. Add ECS schema for user-agent ingest processor (elastic#37727) (elastic#37984) Extract TransportRequestDeduplication from ShardStateAction (elastic#37870)
…r-primary-term * elastic/master: Mute failing date index name processor test Reenable BWC testing after retention lease stats (elastic#38062) Move watcher to use seq# and primary term for concurrency control (elastic#37977) ILM setPriority corrections for a 0 value (elastic#38001) Temporarily disable BWC for retention lease stats (elastic#38049) Skip Shrink when numberOfShards not changed (elastic#37953) Add dispatching to `HandledTransportAction` (elastic#38050) Update httpclient for JDK 11 TLS engine (elastic#37994) Reduce flaxiness of ccr recovery timeouts test (elastic#38035) Fix ILM status to allow unknown fields (elastic#38043) Fix ILM Lifecycle Policy to allow unknown fields (elastic#38041) Update verify repository to allow unknown fields (elastic#37619) [ML] Datafeed deprecation checks (elastic#38026) Deprecate minimum_master_nodes (elastic#37868)
Previously, ShrinkAction would fail if
it was executed on an index that had
the same number of shards as the target
shrunken number.
This PR introduced a new BranchingStep that
is used inside of ShrinkAction to branch which
step to move to next, depending on the
shard values. So no shrink will occur if the
shard count is unchanged.
Fixes #33275.