Skip to content

Commit c10ba9e

Browse files
andreidanandreidan
authored
ILM: Allow searchable_snapshot to follow searchable_snapshot (#68864)
We added this validation for the hot phase as that was the only place a searchable_snapshot action could precede another `searchable_snapshot` action. This is not the case anymore and we now support multiple actions in the same policy. This expands the validation to include the cases where the frozen action defines illegal actions after a searchable_snapshot defined in the cold phase.
1 parent 800ae51 commit c10ba9e

File tree

4 files changed

+150
-40
lines changed

4 files changed

+150
-40
lines changed

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

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.List;
2121
import java.util.Map;
2222
import java.util.Objects;
23+
import java.util.Optional;
2324
import java.util.Set;
2425
import java.util.stream.Collectors;
2526

@@ -61,7 +62,7 @@ public class TimeseriesLifecycleType implements LifecycleType {
6162
ForceMergeAction.NAME, RollupILMAction.NAME, SearchableSnapshotAction.NAME);
6263
// a set of actions that cannot be defined (executed) after the managed index has been mounted as searchable snapshot
6364
static final Set<String> ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT = Sets.newHashSet(ShrinkAction.NAME, ForceMergeAction.NAME,
64-
FreezeAction.NAME, SearchableSnapshotAction.NAME, RollupILMAction.NAME);
65+
FreezeAction.NAME, RollupILMAction.NAME);
6566

6667
static {
6768
if (RollupV2.isEnabled()) {
@@ -301,23 +302,48 @@ public void validate(Collection<Phase> phases) {
301302
}
302303

303304
static void validateActionsFollowingSearchableSnapshot(Collection<Phase> phases) {
304-
boolean hotPhaseContainsSearchableSnapshot = phases.stream()
305-
.filter(phase -> HOT_PHASE.equals(phase.getName()))
306-
.anyMatch(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME));
307-
if (hotPhaseContainsSearchableSnapshot) {
308-
String phasesDefiningIllegalActions = phases.stream()
309-
// we're looking for prohibited actions in phases other than hot
310-
.filter(phase -> HOT_PHASE.equals(phase.getName()) == false)
311-
// filter the phases that define illegal actions
312-
.filter(phase ->
313-
Collections.disjoint(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, phase.getActions().keySet()) == false)
314-
.map(Phase::getName)
315-
.collect(Collectors.joining(","));
316-
if (Strings.hasText(phasesDefiningIllegalActions)) {
317-
throw new IllegalArgumentException("phases [" + phasesDefiningIllegalActions + "] define one or more of " +
318-
ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT + " actions which are not allowed after a " +
319-
"managed index is mounted as a searchable snapshot");
305+
// invalid configurations can occur if searchable_snapshot is defined in the `hot` phase, with subsequent invalid actions
306+
// being defined in the warm/cold/frozen phases, or if it is defined in the `cold` phase with subsequent invalid actions
307+
// being defined in the frozen phase
308+
309+
Optional<Phase> hotPhaseWithSearchableSnapshot = phases.stream()
310+
.filter(phase -> phase.getName().equals(HOT_PHASE))
311+
.filter(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME))
312+
.findAny();
313+
314+
final List<Phase> phasesFollowingSearchableSnapshot = new ArrayList<>(phases.size());
315+
if (hotPhaseWithSearchableSnapshot.isPresent()) {
316+
for (Phase phase : phases) {
317+
if (phase.getName().equals(HOT_PHASE) == false) {
318+
phasesFollowingSearchableSnapshot.add(phase);
319+
}
320320
}
321+
} else {
322+
// let's see if the cold phase defines `searchable_snapshot`
323+
Optional<Phase> coldPhaseWithSearchableSnapshot = phases.stream()
324+
.filter(phase -> phase.getName().equals(COLD_PHASE))
325+
.filter(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME))
326+
.findAny();
327+
if (coldPhaseWithSearchableSnapshot.isPresent()) {
328+
for (Phase phase : phases) {
329+
if (phase.getName().equals(FROZEN_PHASE)) {
330+
phasesFollowingSearchableSnapshot.add(phase);
331+
break;
332+
}
333+
}
334+
}
335+
}
336+
337+
final String phasesDefiningIllegalActions = phasesFollowingSearchableSnapshot.stream()
338+
// filter the phases that define illegal actions
339+
.filter(phase ->
340+
Collections.disjoint(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, phase.getActions().keySet()) == false)
341+
.map(Phase::getName)
342+
.collect(Collectors.joining(","));
343+
if (Strings.hasText(phasesDefiningIllegalActions)) {
344+
throw new IllegalArgumentException("phases [" + phasesDefiningIllegalActions + "] define one or more of " +
345+
ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT + " actions which are not allowed after a " +
346+
"managed index is mounted as a searchable snapshot");
321347
}
322348
}
323349

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,16 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
136136
phaseNames.add(0, TimeseriesLifecycleType.HOT_PHASE);
137137
}
138138
boolean hotPhaseContainsSearchableSnap = false;
139-
for (String phase : phaseNames) {
139+
boolean coldPhaseContainsSearchableSnap = false;
140+
// let's order the phases so we can reason about actions in a previous phase in order to generate a random *valid* policy
141+
List<String> orderedPhases = new ArrayList<>(phaseNames.size());
142+
for (String validPhase : TimeseriesLifecycleType.VALID_PHASES) {
143+
if (phaseNames.contains(validPhase)) {
144+
orderedPhases.add(validPhase);
145+
}
146+
}
147+
148+
for (String phase : orderedPhases) {
140149
TimeValue after = TimeValue.parseTimeValue(randomTimeValue(0, 1000000000, "s", "m", "h", "d"), "test_after");
141150
Map<String, LifecycleAction> actions = new HashMap<>();
142151
List<String> actionNames = randomSubsetOf(validActions.apply(phase));
@@ -150,12 +159,23 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
150159
if (actionNames.contains(SearchableSnapshotAction.NAME)) {
151160
hotPhaseContainsSearchableSnap = true;
152161
}
153-
} else {
162+
}
163+
if (phase.equals(TimeseriesLifecycleType.COLD_PHASE)) {
154164
if (hotPhaseContainsSearchableSnap) {
155165
// let's make sure the other phases don't configure actions that conflict with a possible `searchable_snapshot` action
156166
// configured in the hot phase
157167
actionNames.removeAll(TimeseriesLifecycleType.ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT);
158168
}
169+
170+
if (actionNames.contains(SearchableSnapshotAction.NAME)) {
171+
coldPhaseContainsSearchableSnap = true;
172+
}
173+
} else {
174+
if (hotPhaseContainsSearchableSnap || coldPhaseContainsSearchableSnap) {
175+
// let's make sure the other phases don't configure actions that conflict with a possible `searchable_snapshot` action
176+
// configured in a previous phase (hot/cold)
177+
actionNames.removeAll(TimeseriesLifecycleType.ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT);
178+
}
159179
}
160180

161181
for (String action : actionNames) {

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

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -205,21 +205,75 @@ public void testValidateConflictingDataMigrationConfigurations() {
205205
}
206206

207207
public void testActionsThatCannotFollowSearchableSnapshot() {
208-
assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT.size(), is(5));
208+
assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT.size(), is(4));
209209
assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, containsInAnyOrder(ShrinkAction.NAME, FreezeAction.NAME,
210-
ForceMergeAction.NAME, RollupILMAction.NAME, SearchableSnapshotAction.NAME));
210+
ForceMergeAction.NAME, RollupILMAction.NAME));
211211
}
212212

213213
public void testValidateActionsFollowingSearchableSnapshot() {
214-
Phase hotPhase = new Phase("hot", TimeValue.ZERO, Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
215-
Phase warmPhase = new Phase("warm", TimeValue.ZERO, Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
216-
Phase coldPhase = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction()));
217-
218-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
219-
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase)));
220-
assertThat(e.getMessage(), is(
221-
"phases [warm,cold] define one or more of [searchable_snapshot, forcemerge, freeze, shrink, rollup] actions" +
222-
" which are not allowed after a managed index is mounted as a searchable snapshot"));
214+
{
215+
Phase hotPhase = new Phase("hot", TimeValue.ZERO, Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
216+
Phase warmPhase = new Phase("warm", TimeValue.ZERO, Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
217+
Phase coldPhase = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction()));
218+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
219+
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase)));
220+
assertThat(e.getMessage(), is(
221+
"phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
222+
" which are not allowed after a managed index is mounted as a searchable snapshot"));
223+
}
224+
225+
{
226+
Phase warmPhase = new Phase("warm", TimeValue.ZERO,
227+
Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
228+
Phase coldPhase = new Phase("cold", TimeValue.ZERO,
229+
Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
230+
Phase frozenPhase = new Phase("frozen", TimeValue.ZERO,
231+
Map.of(FreezeAction.NAME, new FreezeAction()));
232+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
233+
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(warmPhase, coldPhase, frozenPhase)));
234+
assertThat(e.getMessage(), is(
235+
"phases [frozen] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
236+
" which are not allowed after a managed index is mounted as a searchable snapshot"));
237+
}
238+
239+
{
240+
Phase hotPhase = new Phase("hot", TimeValue.ZERO,
241+
Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
242+
Phase warmPhase = new Phase("warm", TimeValue.ZERO,
243+
Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
244+
Phase coldPhase = new Phase("cold", TimeValue.ZERO,
245+
Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
246+
Phase frozenPhase = new Phase("frozen", TimeValue.ZERO,
247+
Map.of(FreezeAction.NAME, new FreezeAction()));
248+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
249+
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase,
250+
frozenPhase)));
251+
assertThat(e.getMessage(), is(
252+
"phases [warm,frozen] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
253+
" which are not allowed after a managed index is mounted as a searchable snapshot"));
254+
}
255+
256+
{
257+
Phase hot = new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, new RolloverAction(null, null, 1L),
258+
SearchableSnapshotAction.NAME, new SearchableSnapshotAction(randomAlphaOfLengthBetween(4, 10))));
259+
Phase warm = new Phase("warm", TimeValue.ZERO, Map.of(ForceMergeAction.NAME, new ForceMergeAction(1, null)));
260+
Phase cold = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction()));
261+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
262+
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(warm, hot, cold)));
263+
assertThat(e.getMessage(), is(
264+
"phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup] actions" +
265+
" which are not allowed after a managed index is mounted as a searchable snapshot"));
266+
}
267+
268+
{
269+
Phase frozenPhase = new Phase("frozen", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction(),
270+
SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
271+
try {
272+
TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(frozenPhase));
273+
} catch (Exception e) {
274+
fail("unexpected exception while validating phase [ "+ frozenPhase +" ] but got [" + e.getMessage()+ "]");
275+
}
276+
}
223277
}
224278

225279
public void testGetOrderedPhases() {

‎x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import static java.util.Collections.singletonMap;
5252
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
5353
import static org.elasticsearch.xpack.TimeSeriesRestDriver.createComposableTemplate;
54+
import static org.elasticsearch.xpack.TimeSeriesRestDriver.createIndexWithSettings;
5455
import static org.elasticsearch.xpack.TimeSeriesRestDriver.createNewSingletonPolicy;
5556
import static org.elasticsearch.xpack.TimeSeriesRestDriver.createPolicy;
5657
import static org.elasticsearch.xpack.TimeSeriesRestDriver.createSnapshotRepo;
@@ -227,7 +228,7 @@ public void testCreateInvalidPolicy() {
227228
)
228229
);
229230

230-
assertThat(exception.getMessage(), is("phases [warm,cold] define one or more of [searchable_snapshot, forcemerge, freeze, shrink, rollup]" +
231+
assertThat(exception.getMessage(), is("phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup]" +
231232
" actions which are not allowed after a managed index is mounted as a searchable snapshot"));
232233
}
233234

@@ -462,23 +463,32 @@ public void testConvertingSearchableSnapshotFromFullToPartial() throws Exception
462463

463464
@SuppressWarnings("unchecked")
464465
public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception {
465-
String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT);
466+
String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT) +"-000001";
466467
createSnapshotRepo(client(), snapshotRepo, randomBoolean());
467-
createPolicy(client(), policy, null, null,
468-
new Phase("cold", TimeValue.ZERO,
469-
singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
470-
MountSearchableSnapshotRequest.Storage.SHARED_CACHE))),
468+
createPolicy(client(), policy,
469+
new Phase("hot", TimeValue.ZERO,
470+
Map.of(
471+
SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
472+
MountSearchableSnapshotRequest.Storage.SHARED_CACHE),
473+
RolloverAction.NAME, new RolloverAction(null, null, 1L))),
474+
null, null,
471475
new Phase("frozen", TimeValue.ZERO,
472476
singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(),
473477
MountSearchableSnapshotRequest.Storage.FULL_COPY))),
474478
null
475479
);
476480

477-
createIndex(index, Settings.builder()
478-
.put(LifecycleSettings.LIFECYCLE_NAME, policy)
479-
.build());
481+
String alias = "alias-" + randomAlphaOfLengthBetween(5, 10);
482+
createIndexWithSettings(client(), index, alias, Settings.builder()
483+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
484+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
485+
.put(LifecycleSettings.LIFECYCLE_NAME, policy)
486+
.put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, alias),
487+
true);
488+
480489
ensureGreen(index);
481-
indexDocument(client(), index, true);
490+
indexDocument(client(), alias, true);
491+
rolloverMaxOneDocCondition(client(), alias);
482492

483493
final String searchableSnapMountedIndexName = SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX +
484494
SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX + index;

0 commit comments

Comments
 (0)