Skip to content

Commit e95cc14

Browse files
dakronedakrone
authored
Allow forcemerge in the hot phase for ILM policies (#52073)
* Allow forcemerge in the hot phase for ILM policies This commit changes the `forcemerge` action to also be allowed in the `hot` phase for policies. The forcemerge will occur after a rollover, and allows users to take advantage of higher disk speeds for performing the force merge (on a separate node type, for example). On caveat with this is that a `forcemerge` in the `hot` phase *MUST* be accompanied by a `rollover` action. ILM validates policies to ensure this is the case. Resolves #43165 * Use anyMatch instead of findAny in validation * Make randomTimeseriesLifecyclePolicy single-pass
1 parent 19174d6 commit e95cc14

File tree

5 files changed

+90
-19
lines changed

5 files changed

+90
-19
lines changed

‎docs/reference/ilm/policy-definitions.asciidoc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,11 +292,15 @@ PUT _ilm/policy/my_policy
292292
[[ilm-forcemerge-action]]
293293
==== Force Merge
294294

295-
Phases allowed: warm.
295+
Phases allowed: hot, warm.
296296

297297
NOTE: Index will be be made read-only when this action is run
298298
(see: <<dynamic-index-settings,index.blocks.write>>)
299299

300+
NOTE: If the `forcemerge` action is used in the `hot` phase, the `rollover` action *must* be preset.
301+
ILM validates this predicate and will refuse a policy with a forcemerge in the hot phase without a
302+
rollover action.
303+
300304
The Force Merge Action <<indices-forcemerge,force merges>> the index into at
301305
most a specific number of <<indices-segments,segments>>.
302306

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

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,13 @@ public class TimeseriesLifecycleType implements LifecycleType {
3131
public static final TimeseriesLifecycleType INSTANCE = new TimeseriesLifecycleType();
3232

3333
public static final String TYPE = "timeseries";
34-
static final List<String> VALID_PHASES = Arrays.asList("hot", "warm", "cold", "delete");
35-
static final List<String> ORDERED_VALID_HOT_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, RolloverAction.NAME);
34+
static final String HOT_PHASE = "hot";
35+
static final String WARM_PHASE = "warm";
36+
static final String COLD_PHASE = "cold";
37+
static final String DELETE_PHASE = "delete";
38+
static final List<String> VALID_PHASES = Arrays.asList(HOT_PHASE, WARM_PHASE, COLD_PHASE, DELETE_PHASE);
39+
static final List<String> ORDERED_VALID_HOT_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, RolloverAction.NAME,
40+
ForceMergeAction.NAME);
3641
static final List<String> ORDERED_VALID_WARM_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, ReadOnlyAction.NAME,
3742
AllocateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME);
3843
static final List<String> ORDERED_VALID_COLD_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, AllocateAction.NAME,
@@ -45,10 +50,10 @@ public class TimeseriesLifecycleType implements LifecycleType {
4550
private static Map<String, Set<String>> ALLOWED_ACTIONS = new HashMap<>();
4651

4752
static {
48-
ALLOWED_ACTIONS.put("hot", VALID_HOT_ACTIONS);
49-
ALLOWED_ACTIONS.put("warm", VALID_WARM_ACTIONS);
50-
ALLOWED_ACTIONS.put("cold", VALID_COLD_ACTIONS);
51-
ALLOWED_ACTIONS.put("delete", VALID_DELETE_ACTIONS);
53+
ALLOWED_ACTIONS.put(HOT_PHASE, VALID_HOT_ACTIONS);
54+
ALLOWED_ACTIONS.put(WARM_PHASE, VALID_WARM_ACTIONS);
55+
ALLOWED_ACTIONS.put(COLD_PHASE, VALID_COLD_ACTIONS);
56+
ALLOWED_ACTIONS.put(DELETE_PHASE, VALID_DELETE_ACTIONS);
5257
}
5358

5459
private TimeseriesLifecycleType() {
@@ -126,16 +131,16 @@ public String getPreviousPhaseName(String currentPhaseName, Map<String, Phase> p
126131
public List<LifecycleAction> getOrderedActions(Phase phase) {
127132
Map<String, LifecycleAction> actions = phase.getActions();
128133
switch (phase.getName()) {
129-
case "hot":
134+
case HOT_PHASE:
130135
return ORDERED_VALID_HOT_ACTIONS.stream().map(a -> actions.getOrDefault(a, null))
131136
.filter(Objects::nonNull).collect(Collectors.toList());
132-
case "warm":
137+
case WARM_PHASE:
133138
return ORDERED_VALID_WARM_ACTIONS.stream() .map(a -> actions.getOrDefault(a, null))
134139
.filter(Objects::nonNull).collect(Collectors.toList());
135-
case "cold":
140+
case COLD_PHASE:
136141
return ORDERED_VALID_COLD_ACTIONS.stream().map(a -> actions.getOrDefault(a, null))
137142
.filter(Objects::nonNull).collect(Collectors.toList());
138-
case "delete":
143+
case DELETE_PHASE:
139144
return ORDERED_VALID_DELETE_ACTIONS.stream().map(a -> actions.getOrDefault(a, null))
140145
.filter(Objects::nonNull).collect(Collectors.toList());
141146
default:
@@ -147,20 +152,20 @@ public List<LifecycleAction> getOrderedActions(Phase phase) {
147152
public String getNextActionName(String currentActionName, Phase phase) {
148153
List<String> orderedActionNames;
149154
switch (phase.getName()) {
150-
case "hot":
155+
case HOT_PHASE:
151156
orderedActionNames = ORDERED_VALID_HOT_ACTIONS;
152157
break;
153-
case "warm":
158+
case WARM_PHASE:
154159
orderedActionNames = ORDERED_VALID_WARM_ACTIONS;
155160
break;
156-
case "cold":
161+
case COLD_PHASE:
157162
orderedActionNames = ORDERED_VALID_COLD_ACTIONS;
158163
break;
159-
case "delete":
164+
case DELETE_PHASE:
160165
orderedActionNames = ORDERED_VALID_DELETE_ACTIONS;
161166
break;
162167
default:
163-
throw new IllegalArgumentException("lifecycle type[" + TYPE + "] does not support phase[" + phase.getName() + "]");
168+
throw new IllegalArgumentException("lifecycle type [" + TYPE + "] does not support phase [" + phase.getName() + "]");
164169
}
165170

166171
int index = orderedActionNames.indexOf(currentActionName);
@@ -195,5 +200,19 @@ public void validate(Collection<Phase> phases) {
195200
}
196201
});
197202
});
203+
204+
// Check for forcemerge in 'hot' without a rollover action
205+
if (phases.stream()
206+
// Is there a hot phase
207+
.filter(phase -> HOT_PHASE.equals(phase.getName()))
208+
// That contains the 'forcemerge' action
209+
.filter(phase -> phase.getActions().containsKey(ForceMergeAction.NAME))
210+
// But does *not* contain the 'rollover' action?
211+
.anyMatch(phase -> phase.getActions().containsKey(RolloverAction.NAME) == false)) {
212+
// If there is, throw an exception
213+
throw new IllegalArgumentException("the [" + ForceMergeAction.NAME +
214+
"] action may not be used in the [" + HOT_PHASE +
215+
"] phase without an accompanying [" + RolloverAction.NAME + "] action");
216+
}
198217
}
199218
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,12 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
190190
TimeValue after = TimeValue.parseTimeValue(randomTimeValue(0, 1000000000, "s", "m", "h", "d"), "test_after");
191191
Map<String, LifecycleAction> actions = new HashMap<>();
192192
List<String> actionNames = randomSubsetOf(validActions.apply(phase));
193+
194+
// If the hot phase contains a forcemerge, also make sure to add a rollover, or else the policy will not validate
195+
if (phase.equals(TimeseriesLifecycleType.HOT_PHASE) && actionNames.contains(ForceMergeAction.NAME)) {
196+
actionNames.add(RolloverAction.NAME);
197+
}
198+
193199
for (String action : actionNames) {
194200
actions.put(action, randomAction.apply(action));
195201
}

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@
1010
import org.elasticsearch.test.ESTestCase;
1111

1212
import java.util.Arrays;
13+
import java.util.Collection;
1314
import java.util.Collections;
1415
import java.util.HashMap;
1516
import java.util.List;
1617
import java.util.Map;
1718
import java.util.concurrent.ConcurrentMap;
19+
import java.util.function.Consumer;
1820
import java.util.function.Function;
1921
import java.util.stream.Collectors;
2022

@@ -27,6 +29,7 @@
2729
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.VALID_HOT_ACTIONS;
2830
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.VALID_PHASES;
2931
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.VALID_WARM_ACTIONS;
32+
import static org.hamcrest.Matchers.containsString;
3033
import static org.hamcrest.Matchers.equalTo;
3134

3235
public class TimeseriesLifecycleTypeTests extends ESTestCase {
@@ -64,7 +67,7 @@ public void testValidateHotPhase() {
6467
Map<String, LifecycleAction> actions = VALID_HOT_ACTIONS
6568
.stream().map(this::getTestAction).collect(Collectors.toMap(LifecycleAction::getWriteableName, Function.identity()));
6669
if (randomBoolean()) {
67-
invalidAction = getTestAction(randomFrom("allocate", "forcemerge", "delete", "shrink", "freeze"));
70+
invalidAction = getTestAction(randomFrom("allocate", "delete", "shrink", "freeze"));
6871
actions.put(invalidAction.getWriteableName(), invalidAction);
6972
}
7073
Map<String, Phase> hotPhase = Collections.singletonMap("hot",
@@ -78,6 +81,22 @@ public void testValidateHotPhase() {
7881
} else {
7982
TimeseriesLifecycleType.INSTANCE.validate(hotPhase.values());
8083
}
84+
85+
{
86+
final Consumer<Collection<String>> validateHotActions = hotActions -> {
87+
final Map<String, LifecycleAction> hotActionMap = hotActions.stream()
88+
.map(this::getTestAction)
89+
.collect(Collectors.toMap(LifecycleAction::getWriteableName, Function.identity()));
90+
TimeseriesLifecycleType.INSTANCE.validate(Collections.singleton(new Phase("hot", TimeValue.ZERO, hotActionMap)));
91+
};
92+
93+
validateHotActions.accept(Arrays.asList(RolloverAction.NAME));
94+
validateHotActions.accept(Arrays.asList(RolloverAction.NAME, ForceMergeAction.NAME));
95+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
96+
() -> validateHotActions.accept(Arrays.asList(ForceMergeAction.NAME)));
97+
assertThat(e.getMessage(),
98+
containsString("the [forcemerge] action may not be used in the [hot] phase without an accompanying [rollover] action"));
99+
}
81100
}
82101

83102
public void testValidateWarmPhase() {
@@ -340,11 +359,12 @@ public void testGetNextActionName() {
340359

341360
assertNextActionName("hot", RolloverAction.NAME, null, new String[] {});
342361
assertNextActionName("hot", RolloverAction.NAME, null, new String[] { RolloverAction.NAME });
362+
assertNextActionName("hot", RolloverAction.NAME, ForceMergeAction.NAME, ForceMergeAction.NAME, RolloverAction.NAME);
363+
assertNextActionName("hot", ForceMergeAction.NAME, null, RolloverAction.NAME, ForceMergeAction.NAME);
343364

344365
assertInvalidAction("hot", "foo", new String[] { RolloverAction.NAME });
345366
assertInvalidAction("hot", AllocateAction.NAME, new String[] { RolloverAction.NAME });
346367
assertInvalidAction("hot", DeleteAction.NAME, new String[] { RolloverAction.NAME });
347-
assertInvalidAction("hot", ForceMergeAction.NAME, new String[] { RolloverAction.NAME });
348368
assertInvalidAction("hot", ReadOnlyAction.NAME, new String[] { RolloverAction.NAME });
349369
assertInvalidAction("hot", ShrinkAction.NAME, new String[] { RolloverAction.NAME });
350370

@@ -465,7 +485,7 @@ public void testGetNextActionName() {
465485
Phase phase = new Phase("foo", TimeValue.ZERO, Collections.emptyMap());
466486
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
467487
() -> TimeseriesLifecycleType.INSTANCE.getNextActionName(ShrinkAction.NAME, phase));
468-
assertEquals("lifecycle type[" + TimeseriesLifecycleType.TYPE + "] does not support phase[" + phase.getName() + "]",
488+
assertEquals("lifecycle type [" + TimeseriesLifecycleType.TYPE + "] does not support phase [" + phase.getName() + "]",
469489
exception.getMessage());
470490
}
471491

‎x-pack/plugin/ilm/qa/rest/src/test/resources/rest-api-spec/test/ilm/10_basic.yml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,25 @@ setup:
216216
catch: missing
217217
ilm.get_lifecycle:
218218
policy: "my_timeseries_lifecycle"
219+
220+
---
221+
"Test Invalid Policy":
222+
- do:
223+
catch: bad_request
224+
ilm.put_lifecycle:
225+
policy: "my_invalid_lifecycle"
226+
body: |
227+
{
228+
"policy": {
229+
"phases": {
230+
"hot": {
231+
"min_age": "0s",
232+
"actions": {
233+
"forcemerge": {
234+
"max_num_segments": 1
235+
}
236+
}
237+
}
238+
}
239+
}
240+
}

0 commit comments

Comments
 (0)