Skip to content

Commit 0978063

Browse files
nielsbaumannielsbauman
committed
Refactor ILMs UpdateSettingsStep to be more generic
By making the step more generic, we can reuse it in other actions (such as the upcoming improvements to the force-merge action). The change is relatively straightforward as the name of the step is already generic enough.
1 parent 6d5099e commit 0978063

File tree

6 files changed

+46
-44
lines changed

6 files changed

+46
-44
lines changed

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

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,34 @@
1515
import org.elasticsearch.common.settings.Settings;
1616
import org.elasticsearch.core.TimeValue;
1717

18-
import java.util.Objects;
18+
import java.util.function.Function;
1919

2020
/**
2121
* Updates the settings for an index.
2222
*/
2323
public class UpdateSettingsStep extends AsyncActionStep {
2424
public static final String NAME = "update-settings";
2525

26-
private final Settings settings;
26+
private static final Function<IndexMetadata, String> DEFAULT_TARGET_INDEX_NAME_SUPPLIER = indexMetadata -> indexMetadata.getIndex()
27+
.getName();
28+
29+
private final Function<IndexMetadata, String> targetIndexNameSupplier;
30+
private final Function<IndexMetadata, Settings> settingsSupplier;
2731

2832
public UpdateSettingsStep(StepKey key, StepKey nextStepKey, Client client, Settings settings) {
33+
this(key, nextStepKey, client, DEFAULT_TARGET_INDEX_NAME_SUPPLIER, indexMetadata -> settings);
34+
}
35+
36+
public UpdateSettingsStep(
37+
StepKey key,
38+
StepKey nextStepKey,
39+
Client client,
40+
Function<IndexMetadata, String> targetIndexNameSupplier,
41+
Function<IndexMetadata, Settings> settingsSupplier
42+
) {
2943
super(key, nextStepKey, client);
30-
this.settings = settings;
44+
this.targetIndexNameSupplier = targetIndexNameSupplier;
45+
this.settingsSupplier = settingsSupplier;
3146
}
3247

3348
@Override
@@ -42,32 +57,16 @@ public void performAction(
4257
ClusterStateObserver observer,
4358
ActionListener<Void> listener
4459
) {
45-
UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(indexMetadata.getIndex().getName()).masterNodeTimeout(
46-
TimeValue.MAX_VALUE
47-
).settings(settings);
60+
String indexName = targetIndexNameSupplier.apply(indexMetadata);
61+
Settings settings = settingsSupplier.apply(indexMetadata);
62+
UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(indexName).masterNodeTimeout(TimeValue.MAX_VALUE)
63+
.settings(settings);
4864
getClient(currentState.projectId()).admin()
4965
.indices()
5066
.updateSettings(updateSettingsRequest, listener.delegateFailureAndWrap((l, r) -> l.onResponse(null)));
5167
}
5268

53-
public Settings getSettings() {
54-
return settings;
55-
}
56-
57-
@Override
58-
public int hashCode() {
59-
return Objects.hash(super.hashCode(), settings);
60-
}
61-
62-
@Override
63-
public boolean equals(Object obj) {
64-
if (obj == null) {
65-
return false;
66-
}
67-
if (getClass() != obj.getClass()) {
68-
return false;
69-
}
70-
UpdateSettingsStep other = (UpdateSettingsStep) obj;
71-
return super.equals(obj) && Objects.equals(settings, other.settings);
69+
public Function<IndexMetadata, Settings> getSettingsSupplier() {
70+
return settingsSupplier;
7271
}
7372
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ public void testToSteps() {
186186
expectedSettings.put(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), action.getTotalShardsPerNode());
187187
}
188188

189-
assertThat(firstStep.getSettings(), equalTo(expectedSettings.build()));
189+
assertThat(firstStep.getSettingsSupplier().apply(null), equalTo(expectedSettings.build()));
190190
AllocationRoutedStep secondStep = (AllocationRoutedStep) steps.get(1);
191191
assertEquals(expectedSecondStepKey, secondStep.getKey());
192192
assertEquals(nextStepKey, secondStep.getNextStepKey());
@@ -204,13 +204,15 @@ public void testTotalNumberOfShards() throws Exception {
204204
);
205205
List<Step> steps = action.toSteps(null, phase, nextStepKey);
206206
UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0);
207-
assertEquals(totalShardsPerNode, firstStep.getSettings().getAsInt(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), null));
207+
Settings actualSettings = firstStep.getSettingsSupplier().apply(null);
208+
assertEquals(totalShardsPerNode, actualSettings.getAsInt(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), null));
208209

209210
totalShardsPerNode = null;
210211
action = new AllocateAction(numberOfReplicas, totalShardsPerNode, null, null, null);
211212
steps = action.toSteps(null, phase, nextStepKey);
212213
firstStep = (UpdateSettingsStep) steps.get(0);
213-
assertEquals(null, firstStep.getSettings().get(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey()));
214+
actualSettings = firstStep.getSettingsSupplier().apply(null);
215+
assertNull(actualSettings.get(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey()));
214216

215217
// allow an allocate action that only specifies total shards per node (don't expect any exceptions in this case)
216218
action = new AllocateAction(null, 5, null, null, null);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ private void assertBestCompression(ForceMergeAction instance) {
140140

141141
UpdateSettingsStep updateCodecStep = (UpdateSettingsStep) steps.get(5);
142142
assertThat(
143-
updateCodecStep.getSettings().get(EngineConfig.INDEX_CODEC_SETTING.getKey()),
143+
updateCodecStep.getSettingsSupplier().apply(null).get(EngineConfig.INDEX_CODEC_SETTING.getKey()),
144144
equalTo(CodecService.BEST_COMPRESSION_CODEC)
145145
);
146146
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,17 +92,20 @@ public void testMigrateActionsConfiguresTierPreference() {
9292
{
9393
List<Step> steps = MigrateAction.ENABLED.toSteps(null, HOT_PHASE, nextStepKey);
9494
UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1);
95-
assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettings()), is(DATA_HOT));
95+
assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettingsSupplier().apply(null)), is(DATA_HOT));
9696
}
9797
{
9898
List<Step> steps = MigrateAction.ENABLED.toSteps(null, WARM_PHASE, nextStepKey);
9999
UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1);
100-
assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettings()), is(DATA_WARM + "," + DATA_HOT));
100+
assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettingsSupplier().apply(null)), is(DATA_WARM + "," + DATA_HOT));
101101
}
102102
{
103103
List<Step> steps = MigrateAction.ENABLED.toSteps(null, COLD_PHASE, nextStepKey);
104104
UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1);
105-
assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettings()), is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT));
105+
assertThat(
106+
DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettingsSupplier().apply(null)),
107+
is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT)
108+
);
106109
}
107110
}
108111

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ public void testToSteps() {
6969
UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0);
7070
assertThat(firstStep.getKey(), equalTo(expectedFirstStepKey));
7171
assertThat(firstStep.getNextStepKey(), equalTo(nextStepKey));
72-
assertThat(firstStep.getSettings().size(), equalTo(1));
73-
assertEquals(priority, (long) IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettings()));
72+
assertThat(firstStep.getSettingsSupplier().apply(null).size(), equalTo(1));
73+
assertEquals(priority, (long) IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettingsSupplier().apply(null)));
7474
}
7575

7676
public void testNullPriorityStep() {
@@ -88,10 +88,10 @@ public void testNullPriorityStep() {
8888
UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0);
8989
assertThat(firstStep.getKey(), equalTo(expectedFirstStepKey));
9090
assertThat(firstStep.getNextStepKey(), equalTo(nextStepKey));
91-
assertThat(firstStep.getSettings().size(), equalTo(1));
91+
assertThat(firstStep.getSettingsSupplier().apply(null).size(), equalTo(1));
9292
assertThat(
93-
IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettings()),
94-
equalTo(IndexMetadata.INDEX_PRIORITY_SETTING.getDefault(firstStep.getSettings()))
93+
IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettingsSupplier().apply(null)),
94+
equalTo(IndexMetadata.INDEX_PRIORITY_SETTING.getDefault(firstStep.getSettingsSupplier().apply(null)))
9595
);
9696
}
9797

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,14 @@ public UpdateSettingsStep createRandomInstance() {
3232
public UpdateSettingsStep mutateInstance(UpdateSettingsStep instance) {
3333
StepKey key = instance.getKey();
3434
StepKey nextKey = instance.getNextStepKey();
35-
Settings settings = instance.getSettings();
3635

37-
switch (between(0, 2)) {
36+
switch (between(0, 1)) {
3837
case 0 -> key = new StepKey(key.phase(), key.action(), key.name() + randomAlphaOfLength(5));
3938
case 1 -> nextKey = new StepKey(nextKey.phase(), nextKey.action(), nextKey.name() + randomAlphaOfLength(5));
40-
case 2 -> settings = Settings.builder().put(settings).put(randomAlphaOfLength(10), randomInt()).build();
4139
default -> throw new AssertionError("Illegal randomisation branch");
4240
}
4341

44-
return new UpdateSettingsStep(key, nextKey, client, settings);
42+
return new UpdateSettingsStep(key, nextKey, client, instance.getSettingsSupplier().apply(null));
4543
}
4644

4745
@Override
@@ -50,7 +48,7 @@ public UpdateSettingsStep copyInstance(UpdateSettingsStep instance) {
5048
instance.getKey(),
5149
instance.getNextStepKey(),
5250
instance.getClientWithoutProject(),
53-
instance.getSettings()
51+
instance.getSettingsSupplier().apply(null)
5452
);
5553
}
5654

@@ -71,7 +69,7 @@ public void testPerformAction() throws Exception {
7169
UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
7270
@SuppressWarnings("unchecked")
7371
ActionListener<AcknowledgedResponse> listener = (ActionListener<AcknowledgedResponse>) invocation.getArguments()[1];
74-
assertThat(request.settings(), equalTo(step.getSettings()));
72+
assertThat(request.settings(), equalTo(step.getSettingsSupplier().apply(indexMetadata)));
7573
assertThat(request.indices(), equalTo(new String[] { indexMetadata.getIndex().getName() }));
7674
listener.onResponse(AcknowledgedResponse.TRUE);
7775
return null;
@@ -96,7 +94,7 @@ public void testPerformActionFailure() {
9694
UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
9795
@SuppressWarnings("unchecked")
9896
ActionListener<AcknowledgedResponse> listener = (ActionListener<AcknowledgedResponse>) invocation.getArguments()[1];
99-
assertThat(request.settings(), equalTo(step.getSettings()));
97+
assertThat(request.settings(), equalTo(step.getSettingsSupplier().apply(indexMetadata)));
10098
assertThat(request.indices(), equalTo(new String[] { indexMetadata.getIndex().getName() }));
10199
listener.onFailure(exception);
102100
return null;

0 commit comments

Comments
 (0)