Skip to content

Commit

Permalink
ILM: Allow searchable_snapshot to follow searchable_snapshot (ela…
Browse files Browse the repository at this point in the history 
…stic#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.

(cherry picked from commit c10ba9e)
Signed-off-by: Andrei Dan <andrei.dan@elastic.co>

# Conflicts:
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java
  • Loading branch information
@andreidan
andreidan committed Feb 15, 2021
1 parent 664f569 commit 3744d81
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 44 deletions.
60 changes: 43 additions & 17 deletions ...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<String> 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()) {
Expand Down Expand Up @@ -294,23 +295,48 @@ public void validate(Collection<Phase> phases) {
}

static void validateActionsFollowingSearchableSnapshot(Collection<Phase> 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<Phase> hotPhaseWithSearchableSnapshot = phases.stream()
.filter(phase -> phase.getName().equals(HOT_PHASE))
.filter(phase -> phase.getActions().containsKey(SearchableSnapshotAction.NAME))
.findAny();

final List<Phase> 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<Phase> 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");
}
}

Expand Down
24 changes: 22 additions & 2 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<String, LifecycleAction> actions = new HashMap<>();
List<String> actionNames = randomSubsetOf(validActions.apply(phase));
Expand All @@ -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) {
Expand Down
82 changes: 67 additions & 15 deletions ...gin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,25 +206,77 @@ 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, org.elasticsearch.common.collect.Map.of(
SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
Phase warmPhase = new Phase("warm", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(
ShrinkAction.NAME, new ShrinkAction(1, null)));
Phase coldPhase = new Phase("cold", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(
FreezeAction.NAME, new FreezeAction()));

IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(org.elasticsearch.common.collect.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, org.elasticsearch.common.collect.Map.of(SearchableSnapshotAction.NAME,
new SearchableSnapshotAction("repo")));
Phase warmPhase = new Phase("warm", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(ShrinkAction.NAME,
new ShrinkAction(1, null)));
Phase coldPhase = new Phase("cold", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(FreezeAction.NAME,
new FreezeAction()));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot
(org.elasticsearch.common.collect.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,
org.elasticsearch.common.collect.Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
Phase coldPhase = new Phase("cold", TimeValue.ZERO,
org.elasticsearch.common.collect.Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
Phase frozenPhase = new Phase("frozen", TimeValue.ZERO,
org.elasticsearch.common.collect.Map.of(FreezeAction.NAME, new FreezeAction()));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(
org.elasticsearch.common.collect.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,
org.elasticsearch.common.collect.Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
Phase warmPhase = new Phase("warm", TimeValue.ZERO,
org.elasticsearch.common.collect.Map.of(ShrinkAction.NAME, new ShrinkAction(1, null)));
Phase coldPhase = new Phase("cold", TimeValue.ZERO,
org.elasticsearch.common.collect.Map.of(SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
Phase frozenPhase = new Phase("frozen", TimeValue.ZERO,
org.elasticsearch.common.collect.Map.of(FreezeAction.NAME, new FreezeAction()));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(
org.elasticsearch.common.collect.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, org.elasticsearch.common.collect.Map.of(RolloverAction.NAME, new RolloverAction(null, null, 1L),
SearchableSnapshotAction.NAME, new SearchableSnapshotAction(randomAlphaOfLengthBetween(4, 10))));
Phase warm = new Phase("warm", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(ForceMergeAction.NAME, new ForceMergeAction(1, null)));
Phase cold = new Phase("cold", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(FreezeAction.NAME, new FreezeAction()));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(
org.elasticsearch.common.collect.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, org.elasticsearch.common.collect.Map.of(FreezeAction.NAME,
new FreezeAction(), SearchableSnapshotAction.NAME, new SearchableSnapshotAction("repo")));
try {
TimeseriesLifecycleType.validateActionsFollowingSearchableSnapshot(org.elasticsearch.common.collect.List.of(frozenPhase));
} catch (Exception e) {
fail("unexpected exception while validating phase [ "+ frozenPhase +" ] but got [" + e.getMessage()+ "]");
}
}
}

public void testGetOrderedPhases() {
Expand Down
30 changes: 20 additions & 10 deletions ...src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -218,7 +219,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"));
}

Expand Down Expand Up @@ -454,23 +455,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,
org.elasticsearch.common.collect.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;
Expand Down

0 comments on commit 3744d81

Please sign in to comment.