Skip to content

Commit aa1e6d0

Browse files
gaobinlongandreidan
gaobinlong
authored and
andreidan
committed
ILM: Add validation of the number_of_shards parameter in Shrink Action of ILM (elastic#74219)
Add validation of the number_of_shards parameter in Shrink Action of ILM (cherry picked from commit 58feb4e) Signed-off-by: Andrei Dan <[email protected]>
1 parent 5ef2a8a commit aa1e6d0

File tree

7 files changed

+255
-99
lines changed

7 files changed

+255
-99
lines changed

‎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
@@ -147,6 +147,7 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
147147
StepKey checkNotWriteIndex = new StepKey(phase, NAME, CheckNotDataStreamWriteIndexStep.NAME);
148148
StepKey waitForNoFollowerStepKey = new StepKey(phase, NAME, WaitForNoFollowersStep.NAME);
149149
StepKey readOnlyKey = new StepKey(phase, NAME, ReadOnlyAction.NAME);
150+
StepKey checkTargetShardsCountKey = new StepKey(phase, NAME, CheckTargetShardsCountStep.NAME);
150151
StepKey cleanupShrinkIndexKey = new StepKey(phase, NAME, CleanupShrinkIndexStep.NAME);
151152
StepKey generateShrinkIndexNameKey = new StepKey(phase, NAME, GenerateUniqueIndexNameStep.NAME);
152153
StepKey setSingleNodeKey = new StepKey(phase, NAME, SetSingleNodeAllocateStep.NAME);
@@ -177,7 +178,9 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
177178
CheckNotDataStreamWriteIndexStep checkNotWriteIndexStep = new CheckNotDataStreamWriteIndexStep(checkNotWriteIndex,
178179
waitForNoFollowerStepKey);
179180
WaitForNoFollowersStep waitForNoFollowersStep = new WaitForNoFollowersStep(waitForNoFollowerStepKey, readOnlyKey, client);
180-
ReadOnlyStep readOnlyStep = new ReadOnlyStep(readOnlyKey, cleanupShrinkIndexKey, client);
181+
ReadOnlyStep readOnlyStep = new ReadOnlyStep(readOnlyKey, checkTargetShardsCountKey, client);
182+
CheckTargetShardsCountStep checkTargetShardsCountStep = new CheckTargetShardsCountStep(checkTargetShardsCountKey,
183+
cleanupShrinkIndexKey, numberOfShards);
181184
// 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
182185
// 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
183186
// shrink index name)
@@ -221,9 +224,9 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
221224
DeleteStep deleteSourceIndexStep = new DeleteStep(deleteIndexKey, isShrunkIndexKey, client);
222225
ShrunkenIndexCheckStep waitOnShrinkTakeover = new ShrunkenIndexCheckStep(isShrunkIndexKey, nextStepKey);
223226
return Arrays.asList(conditionalSkipShrinkStep, checkNotWriteIndexStep, waitForNoFollowersStep, readOnlyStep,
224-
cleanupShrinkIndexStep, generateUniqueIndexNameStep, setSingleNodeStep, checkShrinkReadyStep, shrink, allocated,
225-
copyMetadata, isDataStreamBranchingStep, aliasSwapAndDelete, waitOnShrinkTakeover, replaceDataStreamBackingIndex,
226-
deleteSourceIndexStep);
227+
checkTargetShardsCountStep, cleanupShrinkIndexStep, generateUniqueIndexNameStep, setSingleNodeStep, checkShrinkReadyStep,
228+
shrink, allocated, copyMetadata, isDataStreamBranchingStep, aliasSwapAndDelete, waitOnShrinkTakeover,
229+
replaceDataStreamBackingIndex, deleteSourceIndexStep);
227230
}
228231

229232
@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)