diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java index 62c430a4c790a..0e52235966020 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -61,7 +62,7 @@ public class TimeseriesLifecycleType implements LifecycleType { ForceMergeAction.NAME, RollupILMAction.NAME, SearchableSnapshotAction.NAME); // a set of actions that cannot be defined (executed) after the managed index has been mounted as searchable snapshot static final Set ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT = Sets.newHashSet(ShrinkAction.NAME, ForceMergeAction.NAME, - FreezeAction.NAME, SearchableSnapshotAction.NAME, RollupILMAction.NAME); + FreezeAction.NAME, RollupILMAction.NAME); static { if (RollupV2.isEnabled()) { @@ -294,23 +295,48 @@ public void validate(Collection phases) { } static void validateActionsFollowingSearchableSnapshot(Collection phases) { - boolean hotPhaseContainsSearchableSnapshot = phases.stream() - .filter(phase -> HOT_PHASE.equals(phase.getName())) - .anyMatch(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME)); - if (hotPhaseContainsSearchableSnapshot) { - String phasesDefiningIllegalActions = phases.stream() - // we're looking for prohibited actions in phases other than hot - .filter(phase -> HOT_PHASE.equals(phase.getName()) == false) - // filter the phases that define illegal actions - .filter(phase -> - Collections.disjoint(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, phase.getActions().keySet()) == false) - .map(Phase::getName) - .collect(Collectors.joining(",")); - if (Strings.hasText(phasesDefiningIllegalActions)) { - throw new IllegalArgumentException("phases [" + phasesDefiningIllegalActions + "] define one or more of " + - ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT + " actions which are not allowed after a " + - "managed index is mounted as a searchable snapshot"); + // invalid configurations can occur if searchable_snapshot is defined in the `hot` phase, with subsequent invalid actions + // being defined in the warm/cold/frozen phases, or if it is defined in the `cold` phase with subsequent invalid actions + // being defined in the frozen phase + + Optional hotPhaseWithSearchableSnapshot = phases.stream() + .filter(phase -> phase.getName().equals(HOT_PHASE)) + .filter(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME)) + .findAny(); + + final List phasesFollowingSearchableSnapshot = new ArrayList<>(phases.size()); + if (hotPhaseWithSearchableSnapshot.isPresent()) { + for (Phase phase : phases) { + if (phase.getName().equals(HOT_PHASE) == false) { + phasesFollowingSearchableSnapshot.add(phase); + } } + } else { + // let's see if the cold phase defines `searchable_snapshot` + Optional coldPhaseWithSearchableSnapshot = phases.stream() + .filter(phase -> phase.getName().equals(COLD_PHASE)) + .filter(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME)) + .findAny(); + if (coldPhaseWithSearchableSnapshot.isPresent()) { + for (Phase phase : phases) { + if (phase.getName().equals(FROZEN_PHASE)) { + phasesFollowingSearchableSnapshot.add(phase); + break; + } + } + } + } + + final String phasesDefiningIllegalActions = phasesFollowingSearchableSnapshot.stream() + // filter the phases that define illegal actions + .filter(phase -> + Collections.disjoint(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, phase.getActions().keySet()) == false) + .map(Phase::getName) + .collect(Collectors.joining(",")); + if (Strings.hasText(phasesDefiningIllegalActions)) { + throw new IllegalArgumentException("phases [" + phasesDefiningIllegalActions + "] define one or more of " + + ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT + " actions which are not allowed after a " + + "managed index is mounted as a searchable snapshot"); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java index 5774fd4ff9439..cd585cf1fdf74 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java @@ -136,7 +136,16 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l phaseNames.add(0, TimeseriesLifecycleType.HOT_PHASE); } boolean hotPhaseContainsSearchableSnap = false; - for (String phase : phaseNames) { + boolean coldPhaseContainsSearchableSnap = false; + // let's order the phases so we can reason about actions in a previous phase in order to generate a random *valid* policy + List orderedPhases = new ArrayList<>(phaseNames.size()); + for (String validPhase : TimeseriesLifecycleType.VALID_PHASES) { + if (phaseNames.contains(validPhase)) { + orderedPhases.add(validPhase); + } + } + + for (String phase : orderedPhases) { TimeValue after = TimeValue.parseTimeValue(randomTimeValue(0, 1000000000, "s", "m", "h", "d"), "test_after"); Map actions = new HashMap<>(); List actionNames = randomSubsetOf(validActions.apply(phase)); @@ -150,12 +159,23 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l if (actionNames.contains(SearchableSnapshotAction.NAME)) { hotPhaseContainsSearchableSnap = true; } - } else { + } + if (phase.equals(TimeseriesLifecycleType.COLD_PHASE)) { if (hotPhaseContainsSearchableSnap) { // let's make sure the other phases don't configure actions that conflict with a possible `searchable_snapshot` action // configured in the hot phase actionNames.removeAll(TimeseriesLifecycleType.ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT); } + + if (actionNames.contains(SearchableSnapshotAction.NAME)) { + coldPhaseContainsSearchableSnap = true; + } + } else { + if (hotPhaseContainsSearchableSnap || coldPhaseContainsSearchableSnap) { + // let's make sure the other phases don't configure actions that conflict with a possible `searchable_snapshot` action + // configured in a previous phase (hot/cold) + actionNames.removeAll(TimeseriesLifecycleType.ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT); + } } for (String action : actionNames) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java index bd9fb017d9535..8bd37c85b6fa0 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java @@ -204,21 +204,75 @@ public void testValidateConflictingDataMigrationConfigurations() { } public void testActionsThatCannotFollowSearchableSnapshot() { - assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT.size(), is(5)); + assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT.size(), is(4)); assertThat(ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT, containsInAnyOrder(ShrinkAction.NAME, FreezeAction.NAME, - ForceMergeAction.NAME, RollupILMAction.NAME, SearchableSnapshotAction.NAME)); + ForceMergeAction.NAME, RollupILMAction.NAME)); } public void testValidateActionsFollowingSearchableSnapshot() { - Phase hotPhase = new Phase("hot", TimeValue.ZERO, Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo"))); - Phase warmPhase = new Phase("warm", TimeValue.ZERO, Map.of(ShrinkAction.NAME, new ShrinkAction(1, null))); - Phase coldPhase = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction())); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase))); - assertThat(e.getMessage(), is( - "phases [warm,cold] define one or more of [searchable_snapshot, forcemerge, freeze, shrink, rollup] actions" + - " which are not allowed after a managed index is mounted as a searchable snapshot")); + { + Phase hotPhase = new Phase("hot", TimeValue.ZERO, Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo"))); + Phase warmPhase = new Phase("warm", TimeValue.ZERO, Map.of(ShrinkAction.NAME, new ShrinkAction(1, null))); + Phase coldPhase = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction())); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase))); + assertThat(e.getMessage(), is( + "phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup] actions" + + " which are not allowed after a managed index is mounted as a searchable snapshot")); + } + + { + Phase warmPhase = new Phase("warm", TimeValue.ZERO, + Map.of(ShrinkAction.NAME, new ShrinkAction(1, null))); + Phase coldPhase = new Phase("cold", TimeValue.ZERO, + Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo"))); + Phase frozenPhase = new Phase("frozen", TimeValue.ZERO, + Map.of(FreezeAction.NAME, new FreezeAction())); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(warmPhase, coldPhase, frozenPhase))); + assertThat(e.getMessage(), is( + "phases [frozen] define one or more of [forcemerge, freeze, shrink, rollup] actions" + + " which are not allowed after a managed index is mounted as a searchable snapshot")); + } + + { + Phase hotPhase = new Phase("hot", TimeValue.ZERO, + Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo"))); + Phase warmPhase = new Phase("warm", TimeValue.ZERO, + Map.of(ShrinkAction.NAME, new ShrinkAction(1, null))); + Phase coldPhase = new Phase("cold", TimeValue.ZERO, + Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo"))); + Phase frozenPhase = new Phase("frozen", TimeValue.ZERO, + Map.of(FreezeAction.NAME, new FreezeAction())); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(hotPhase, warmPhase, coldPhase, + frozenPhase))); + assertThat(e.getMessage(), is( + "phases [warm,frozen] define one or more of [forcemerge, freeze, shrink, rollup] actions" + + " which are not allowed after a managed index is mounted as a searchable snapshot")); + } + + { + Phase hot = new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, new RolloverAction(null, null, 1L), + SearchableSnapshotAction.NAME, new SearchableSnapshotAction(randomAlphaOfLengthBetween(4, 10)))); + Phase warm = new Phase("warm", TimeValue.ZERO, Map.of(ForceMergeAction.NAME, new ForceMergeAction(1, null))); + Phase cold = new Phase("cold", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction())); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(warm, hot, cold))); + assertThat(e.getMessage(), is( + "phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup] actions" + + " which are not allowed after a managed index is mounted as a searchable snapshot")); + } + + { + Phase frozenPhase = new Phase("frozen", TimeValue.ZERO, Map.of(FreezeAction.NAME, new FreezeAction(), + SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo"))); + try { + TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(List.of(frozenPhase)); + } catch (Exception e) { + fail("unexpected exception while validating phase [ "+ frozenPhase +" ] but got [" + e.getMessage()+ "]"); + } + } } public void testGetOrderedPhases() { diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java index 86a00fe5c23ba..3841dc82f0617 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java @@ -48,6 +48,7 @@ import static java.util.Collections.singletonMap; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.xpack.TimeSeriesRestDriver.createComposableTemplate; +import static org.elasticsearch.xpack.TimeSeriesRestDriver.createIndexWithSettings; import static org.elasticsearch.xpack.TimeSeriesRestDriver.createNewSingletonPolicy; import static org.elasticsearch.xpack.TimeSeriesRestDriver.createPolicy; import static org.elasticsearch.xpack.TimeSeriesRestDriver.createSnapshotRepo; @@ -224,7 +225,7 @@ public void testCreateInvalidPolicy() { ) ); - assertThat(exception.getMessage(), is("phases [warm,cold] define one or more of [searchable_snapshot, forcemerge, freeze, shrink, rollup]" + + assertThat(exception.getMessage(), is("phases [warm,cold] define one or more of [forcemerge, freeze, shrink, rollup]" + " actions which are not allowed after a managed index is mounted as a searchable snapshot")); } @@ -459,23 +460,32 @@ public void testConvertingSearchableSnapshotFromFullToPartial() throws Exception @SuppressWarnings("unchecked") public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception { - String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT); + String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT) +"-000001"; createSnapshotRepo(client(), snapshotRepo, randomBoolean()); - createPolicy(client(), policy, null, null, - new Phase("cold", TimeValue.ZERO, - singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), - MountSearchableSnapshotRequest.Storage.SHARED_CACHE))), + createPolicy(client(), policy, + new Phase("hot", TimeValue.ZERO, + Map.of( + SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), + MountSearchableSnapshotRequest.Storage.SHARED_CACHE), + RolloverAction.NAME, new RolloverAction(null, null, 1L))), + null, null, new Phase("frozen", TimeValue.ZERO, singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), MountSearchableSnapshotRequest.Storage.FULL_COPY))), null ); - createIndex(index, Settings.builder() - .put(LifecycleSettings.LIFECYCLE_NAME, policy) - .build()); + String alias = "alias-" + randomAlphaOfLengthBetween(5, 10); + createIndexWithSettings(client(), index, alias, Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(LifecycleSettings.LIFECYCLE_NAME, policy) + .put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, alias), + true); + ensureGreen(index); - indexDocument(client(), index, true); + indexDocument(client(), alias, true); + rolloverMaxOneDocCondition(client(), alias); final String searchableSnapMountedIndexName = SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX + SearchableSnapshotAction.PARTIAL_RESTORED_INDEX_PREFIX + index;