Skip to content

Commit 58feb4e

Browse files
gaobinlonggaobinlong
authored
ILM: Add validation of the number_of_shards parameter in Shrink Action of ILM (#74219)
Add validation of the number_of_shards parameter in Shrink Action of ILM
1 parent 5c53a66 commit 58feb4e

7 files changed

Lines changed: 255 additions & 99 deletions

File tree

‎client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ILMDocumentationIT.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -621,9 +621,6 @@ public void testRetryPolicy() throws Exception {
621621
.put("index.lifecycle.name", "my_policy")
622622
.build());
623623
client.indices().create(createIndexRequest, RequestOptions.DEFAULT);
624-
assertBusy(() -> assertNotNull(client.indexLifecycle()
625-
.explainLifecycle(new ExplainLifecycleRequest("my_index"), RequestOptions.DEFAULT)
626-
.getIndexResponses().get("my_index").getFailedStep()), 30, TimeUnit.SECONDS);
627624
}
628625

629626
// tag::ilm-retry-lifecycle-policy-request
@@ -644,8 +641,8 @@ public void testRetryPolicy() throws Exception {
644641

645642
assertTrue(acknowledged);
646643
} catch (ElasticsearchException e) {
647-
// the retry API might fail as the shrink action steps are retryable (so if the retry API reaches ES when ILM is retrying the
648-
// failed `shrink` step, the retry API will fail)
644+
// the retry API might fail as the shrink action steps are retryable (ILM will stuck in the `check-target-shards-count` step
645+
// with no failure, the retry API will fail)
649646
// assert that's the exception we encountered (we want to test to fail if there is an actual error with the retry api)
650647
assertThat(e.getMessage(), containsStringIgnoringCase("reason=cannot retry an action for an index [my_index] that has not " +
651648
"encountered an error when running a Lifecycle Policy"));

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CheckTargetShardsCountStep.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
package org.elasticsearch.xpack.core.ilm;
8+
9+
import org.apache.logging.log4j.LogManager;
10+
import org.apache.logging.log4j.Logger;
11+
import org.elasticsearch.cluster.ClusterState;
12+
import org.elasticsearch.cluster.metadata.IndexMetadata;
13+
import org.elasticsearch.index.Index;
14+
import org.elasticsearch.xpack.core.ilm.step.info.SingleMessageFieldInfo;
15+
16+
import java.util.Locale;
17+
18+
/**
19+
* This step checks whether the new shrunken index's shards count is a factor of the source index's shards count.
20+
*/
21+
public class CheckTargetShardsCountStep extends ClusterStateWaitStep {
22+
23+
public static final String NAME = "check-target-shards-count";
24+
25+
private final Integer numberOfShards;
26+
27+
private static final Logger logger = LogManager.getLogger(CheckTargetShardsCountStep.class);
28+
29+
CheckTargetShardsCountStep(StepKey key, StepKey nextStepKey, Integer numberOfShards) {
30+
super(key, nextStepKey);
31+
this.numberOfShards = numberOfShards;
32+
}
33+
34+
@Override
35+
public boolean isRetryable() {
36+
return true;
37+
}
38+
39+
public Integer getNumberOfShards() {
40+
return numberOfShards;
41+
}
42+
43+
@Override
44+
public Result isConditionMet(Index index, ClusterState clusterState) {
45+
IndexMetadata indexMetadata = clusterState.metadata().index(index);
46+
if (indexMetadata == null) {
47+
// Index must have been since deleted, ignore it
48+
logger.debug("[{}] lifecycle action for index [{}] executed but index no longer exists",
49+
getKey().getAction(), index.getName());
50+
return new Result(false, null);
51+
}
52+
String indexName = indexMetadata.getIndex().getName();
53+
if (numberOfShards != null) {
54+
int sourceNumberOfShards = indexMetadata.getNumberOfShards();
55+
if (sourceNumberOfShards % numberOfShards != 0) {
56+
String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
57+
String errorMessage = String.format(Locale.ROOT, "lifecycle action of policy [%s] for index [%s] cannot make progress " +
58+
"because the target shards count [%d] must be a factor of the source index's shards count [%d]",
59+
policyName, indexName, numberOfShards, sourceNumberOfShards);
60+
logger.debug(errorMessage);
61+
return new Result(false, new SingleMessageFieldInfo(errorMessage));
62+
}
63+
}
64+
65+
return new Result(true, null);
66+
}
67+
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrinkAction.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
137137
StepKey checkNotWriteIndex = new StepKey(phase, NAME, CheckNotDataStreamWriteIndexStep.NAME);
138138
StepKey waitForNoFollowerStepKey = new StepKey(phase, NAME, WaitForNoFollowersStep.NAME);
139139
StepKey readOnlyKey = new StepKey(phase, NAME, ReadOnlyAction.NAME);
140+
StepKey checkTargetShardsCountKey = new StepKey(phase, NAME, CheckTargetShardsCountStep.NAME);
140141
StepKey cleanupShrinkIndexKey = new StepKey(phase, NAME, CleanupShrinkIndexStep.NAME);
141142
StepKey generateShrinkIndexNameKey = new StepKey(phase, NAME, GenerateUniqueIndexNameStep.NAME);
142143
StepKey setSingleNodeKey = new StepKey(phase, NAME, SetSingleNodeAllocateStep.NAME);
@@ -167,7 +168,9 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
167168
CheckNotDataStreamWriteIndexStep checkNotWriteIndexStep = new CheckNotDataStreamWriteIndexStep(checkNotWriteIndex,
168169
waitForNoFollowerStepKey);
169170
WaitForNoFollowersStep waitForNoFollowersStep = new WaitForNoFollowersStep(waitForNoFollowerStepKey, readOnlyKey, client);
170-
ReadOnlyStep readOnlyStep = new ReadOnlyStep(readOnlyKey, cleanupShrinkIndexKey, client);
171+
ReadOnlyStep readOnlyStep = new ReadOnlyStep(readOnlyKey, checkTargetShardsCountKey, client);
172+
CheckTargetShardsCountStep checkTargetShardsCountStep = new CheckTargetShardsCountStep(checkTargetShardsCountKey,
173+
cleanupShrinkIndexKey, numberOfShards);
171174
// we generate a unique shrink index name but we also retry if the allocation of the shrunk index is not possible, so we want to
172175
// delete the "previously generated" shrink index (this is a no-op if it's the first run of the action and he haven't generated a
173176
// shrink index name)
@@ -211,9 +214,9 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
211214
DeleteStep deleteSourceIndexStep = new DeleteStep(deleteIndexKey, isShrunkIndexKey, client);
212215
ShrunkenIndexCheckStep waitOnShrinkTakeover = new ShrunkenIndexCheckStep(isShrunkIndexKey, nextStepKey);
213216
return Arrays.asList(conditionalSkipShrinkStep, checkNotWriteIndexStep, waitForNoFollowersStep, readOnlyStep,
214-
cleanupShrinkIndexStep, generateUniqueIndexNameStep, setSingleNodeStep, checkShrinkReadyStep, shrink, allocated,
215-
copyMetadata, isDataStreamBranchingStep, aliasSwapAndDelete, waitOnShrinkTakeover, replaceDataStreamBackingIndex,
216-
deleteSourceIndexStep);
217+
checkTargetShardsCountStep, cleanupShrinkIndexStep, generateUniqueIndexNameStep, setSingleNodeStep, checkShrinkReadyStep,
218+
shrink, allocated, copyMetadata, isDataStreamBranchingStep, aliasSwapAndDelete, waitOnShrinkTakeover,
219+
replaceDataStreamBackingIndex, deleteSourceIndexStep);
217220
}
218221

219222
@Override

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CheckTargetShardsCountStepTests.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
package org.elasticsearch.xpack.core.ilm;
8+
9+
import org.elasticsearch.Version;
10+
import org.elasticsearch.cluster.ClusterState;
11+
import org.elasticsearch.cluster.metadata.IndexMetadata;
12+
import org.elasticsearch.cluster.metadata.Metadata;
13+
import org.elasticsearch.xpack.core.ilm.Step.StepKey;
14+
import org.elasticsearch.xpack.core.ilm.step.info.SingleMessageFieldInfo;
15+
16+
import static org.hamcrest.Matchers.is;
17+
18+
public class CheckTargetShardsCountStepTests extends AbstractStepTestCase<CheckTargetShardsCountStep> {
19+
20+
@Override
21+
protected CheckTargetShardsCountStep createRandomInstance() {
22+
return new CheckTargetShardsCountStep(randomStepKey(), randomStepKey(), null);
23+
}
24+
25+
@Override
26+
protected CheckTargetShardsCountStep mutateInstance(CheckTargetShardsCountStep instance) {
27+
StepKey key = instance.getKey();
28+
StepKey nextKey = instance.getNextStepKey();
29+
30+
switch (between(0, 1)) {
31+
case 0:
32+
key = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5));
33+
break;
34+
case 1:
35+
nextKey = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5));
36+
break;
37+
default:
38+
throw new AssertionError("Illegal randomisation branch");
39+
}
40+
41+
return new CheckTargetShardsCountStep(key, nextKey, null);
42+
}
43+
44+
@Override
45+
protected CheckTargetShardsCountStep copyInstance(CheckTargetShardsCountStep instance) {
46+
return new CheckTargetShardsCountStep(instance.getKey(), instance.getNextStepKey(), instance.getNumberOfShards());
47+
}
48+
49+
public void testStepCompleteIfTargetShardsCountIsValid() {
50+
String policyName = "test-ilm-policy";
51+
IndexMetadata indexMetadata =
52+
IndexMetadata.builder(randomAlphaOfLength(10)).settings(settings(Version.CURRENT)
53+
.put(LifecycleSettings.LIFECYCLE_NAME, policyName))
54+
.numberOfShards(10).numberOfReplicas(randomIntBetween(0, 5)).build();
55+
56+
ClusterState clusterState = ClusterState.builder(emptyClusterState()).metadata(
57+
Metadata.builder().put(indexMetadata, true).build()).build();
58+
59+
CheckTargetShardsCountStep checkTargetShardsCountStep = new CheckTargetShardsCountStep(randomStepKey(), randomStepKey(), 2);
60+
61+
ClusterStateWaitStep.Result result = checkTargetShardsCountStep.isConditionMet(indexMetadata.getIndex(), clusterState);
62+
assertThat(result.isComplete(), is(true));
63+
}
64+
65+
public void testStepIncompleteIfTargetShardsCountNotValid() {
66+
String indexName = randomAlphaOfLength(10);
67+
String policyName = "test-ilm-policy";
68+
IndexMetadata indexMetadata =
69+
IndexMetadata.builder(indexName).settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
70+
.numberOfShards(10).numberOfReplicas(randomIntBetween(0, 5)).build();
71+
72+
ClusterState clusterState = ClusterState.builder(emptyClusterState()).metadata(
73+
Metadata.builder().put(indexMetadata, true).build()).build();
74+
75+
CheckTargetShardsCountStep checkTargetShardsCountStep = new CheckTargetShardsCountStep(randomStepKey(), randomStepKey(), 3);
76+
77+
ClusterStateWaitStep.Result result = checkTargetShardsCountStep.isConditionMet(indexMetadata.getIndex(), clusterState);
78+
assertThat(result.isComplete(), is(false));
79+
SingleMessageFieldInfo info = (SingleMessageFieldInfo) result.getInfomationContext();
80+
assertThat(info.getMessage(), is("lifecycle action of policy [" + policyName + "] for index [" + indexName +
81+
"] cannot make progress because the target shards count [3] must be a factor of the source index's shards count [10]"));
82+
}
83+
}

0 commit comments

Comments
 (0)