From 7e6e90f2e9ec14d93b573f4874389f9b8606548f Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Thu, 8 Oct 2020 10:50:50 -0400 Subject: [PATCH 01/12] getOrDefault with null is just get --- .../xpack/core/ilm/TimeseriesLifecycleType.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 329d930650192..9932cd16c664c 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 @@ -157,16 +157,16 @@ public List getOrderedActions(Phase phase) { Map actions = phase.getActions(); switch (phase.getName()) { case HOT_PHASE: - return ORDERED_VALID_HOT_ACTIONS.stream().map(a -> actions.getOrDefault(a, null)) + return ORDERED_VALID_HOT_ACTIONS.stream().map(actions::get) .filter(Objects::nonNull).collect(toList()); case WARM_PHASE: - return ORDERED_VALID_WARM_ACTIONS.stream().map(a -> actions.getOrDefault(a, null)) + return ORDERED_VALID_WARM_ACTIONS.stream().map(actions::get) .filter(Objects::nonNull).collect(toList()); case COLD_PHASE: - return ORDERED_VALID_COLD_ACTIONS.stream().map(a -> actions.getOrDefault(a, null)) + return ORDERED_VALID_COLD_ACTIONS.stream().map(actions::get) .filter(Objects::nonNull).collect(toList()); case DELETE_PHASE: - return ORDERED_VALID_DELETE_ACTIONS.stream().map(a -> actions.getOrDefault(a, null)) + return ORDERED_VALID_DELETE_ACTIONS.stream().map(actions::get) .filter(Objects::nonNull).collect(toList()); default: throw new IllegalArgumentException("lifecycle type[" + TYPE + "] does not support phase[" + phase.getName() + "]"); From 8a8b2d999f7f84a98c3df612c97221af3c47c9de Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Thu, 8 Oct 2020 10:04:40 -0400 Subject: [PATCH 02/12] Map.of makes this immutable and lets us drop the static initializer --- .../xpack/core/ilm/TimeseriesLifecycleType.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) 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 9932cd16c664c..206ad12b028a1 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 @@ -50,14 +50,11 @@ public class TimeseriesLifecycleType implements LifecycleType { static final Set VALID_WARM_ACTIONS = Sets.newHashSet(ORDERED_VALID_WARM_ACTIONS); static final Set VALID_COLD_ACTIONS = Sets.newHashSet(ORDERED_VALID_COLD_ACTIONS); static final Set VALID_DELETE_ACTIONS = Sets.newHashSet(ORDERED_VALID_DELETE_ACTIONS); - private static final Map> ALLOWED_ACTIONS = new HashMap<>(); - - static { - ALLOWED_ACTIONS.put(HOT_PHASE, VALID_HOT_ACTIONS); - ALLOWED_ACTIONS.put(WARM_PHASE, VALID_WARM_ACTIONS); - ALLOWED_ACTIONS.put(COLD_PHASE, VALID_COLD_ACTIONS); - ALLOWED_ACTIONS.put(DELETE_PHASE, VALID_DELETE_ACTIONS); - } + private static final Map> ALLOWED_ACTIONS = Map.of( + HOT_PHASE, VALID_HOT_ACTIONS, + WARM_PHASE, VALID_WARM_ACTIONS, + COLD_PHASE, VALID_COLD_ACTIONS, + DELETE_PHASE, VALID_DELETE_ACTIONS); private TimeseriesLifecycleType() { } From 10219f0cd5ab599ee62f2973ff4a714d02ef5ed0 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 21 Oct 2020 10:34:10 -0400 Subject: [PATCH 03/12] Refactor a bit to allow the possibility that there could be more actions in the hot phase that require rollover, rather than only just forcemerge. --- .../core/ilm/TimeseriesLifecycleType.java | 21 +++++++++++-------- .../xpack/core/ilm/LifecyclePolicyTests.java | 5 +++-- .../ilm/TimeseriesLifecycleTypeTests.java | 2 +- 3 files changed, 16 insertions(+), 12 deletions(-) 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 206ad12b028a1..4c89a78bde432 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 @@ -56,6 +56,8 @@ public class TimeseriesLifecycleType implements LifecycleType { COLD_PHASE, VALID_COLD_ACTIONS, DELETE_PHASE, VALID_DELETE_ACTIONS); + static final Set HOT_ACTIONS_THAT_REQUIRE_ROLLOVER = Sets.newHashSet(ForceMergeAction.NAME); + private TimeseriesLifecycleType() { } @@ -223,17 +225,18 @@ public void validate(Collection phases) { }); }); - // Check for forcemerge in 'hot' without a rollover action - if (phases.stream() + // Check for actions in the hot phase that require a rollover + String invalidHotPhaseActions = phases.stream() // Is there a hot phase .filter(phase -> HOT_PHASE.equals(phase.getName())) - // That contains the 'forcemerge' action - .filter(phase -> phase.getActions().containsKey(ForceMergeAction.NAME)) - // But does *not* contain the 'rollover' action? - .anyMatch(phase -> phase.getActions().containsKey(RolloverAction.NAME) == false)) { - // If there is, throw an exception - throw new IllegalArgumentException("the [" + ForceMergeAction.NAME + - "] action may not be used in the [" + HOT_PHASE + + // that does *not* contain the 'rollover' action + .filter(phase -> phase.getActions().containsKey(RolloverAction.NAME) == false) + // but that does have actions that require a rollover action? + .flatMap(phase -> Sets.intersection(phase.getActions().keySet(), HOT_ACTIONS_THAT_REQUIRE_ROLLOVER).stream()) + .collect(Collectors.joining(", ")); + if (Strings.hasText(invalidHotPhaseActions)) { + throw new IllegalArgumentException("the [" + invalidHotPhaseActions + + "] action(s) may not be used in the [" + HOT_PHASE + "] phase without an accompanying [" + RolloverAction.NAME + "] action"); } 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 1d6306b11544b..0ccd0f11b63fd 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 @@ -204,8 +204,9 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l Map actions = new HashMap<>(); List actionNames = randomSubsetOf(validActions.apply(phase)); - // If the hot phase contains a forcemerge, also make sure to add a rollover, or else the policy will not validate - if (phase.equals(TimeseriesLifecycleType.HOT_PHASE) && actionNames.contains(ForceMergeAction.NAME)) { + // If the hot phase has any actions that require a rollover, then ensure there is one so that the policy will validate + if (phase.equals(TimeseriesLifecycleType.HOT_PHASE) + && actionNames.stream().anyMatch(TimeseriesLifecycleType.HOT_ACTIONS_THAT_REQUIRE_ROLLOVER::contains)) { actionNames.add(RolloverAction.NAME); } 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 4be976949561c..1b1c6e7c06cdb 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 @@ -104,7 +104,7 @@ public void testValidateHotPhase() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> validateHotActions.accept(Arrays.asList(ForceMergeAction.NAME))); assertThat(e.getMessage(), - containsString("the [forcemerge] action may not be used in the [hot] phase without an accompanying [rollover] action")); + containsString("the [forcemerge] action(s) may not be used in the [hot] phase without an accompanying [rollover] action")); } } From bd1bebc79e8d687b48e97294905e944a54420f11 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 21 Oct 2020 11:05:30 -0400 Subject: [PATCH 04/12] Allow shrink in the hot phase as long as there's an accompanying rollover. --- .../elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java | 4 ++-- .../xpack/core/ilm/TimeseriesLifecycleTypeTests.java | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) 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 4c89a78bde432..a8cf87ccec2b4 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 @@ -40,7 +40,7 @@ public class TimeseriesLifecycleType implements LifecycleType { static final String DELETE_PHASE = "delete"; static final List VALID_PHASES = Arrays.asList(HOT_PHASE, WARM_PHASE, COLD_PHASE, DELETE_PHASE); static final List ORDERED_VALID_HOT_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, RolloverAction.NAME, - ForceMergeAction.NAME); + ShrinkAction.NAME, ForceMergeAction.NAME); static final List ORDERED_VALID_WARM_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, ReadOnlyAction.NAME, AllocateAction.NAME, MigrateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME); static final List ORDERED_VALID_COLD_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, AllocateAction.NAME, @@ -56,7 +56,7 @@ public class TimeseriesLifecycleType implements LifecycleType { COLD_PHASE, VALID_COLD_ACTIONS, DELETE_PHASE, VALID_DELETE_ACTIONS); - static final Set HOT_ACTIONS_THAT_REQUIRE_ROLLOVER = Sets.newHashSet(ForceMergeAction.NAME); + static final Set HOT_ACTIONS_THAT_REQUIRE_ROLLOVER = Sets.newHashSet(ShrinkAction.NAME, ForceMergeAction.NAME); private TimeseriesLifecycleType() { } 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 1b1c6e7c06cdb..29ce111fe7503 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 @@ -407,7 +407,6 @@ public void testGetNextActionName() { assertInvalidAction("hot", AllocateAction.NAME, new String[] { RolloverAction.NAME }); assertInvalidAction("hot", DeleteAction.NAME, new String[] { RolloverAction.NAME }); assertInvalidAction("hot", ReadOnlyAction.NAME, new String[] { RolloverAction.NAME }); - assertInvalidAction("hot", ShrinkAction.NAME, new String[] { RolloverAction.NAME }); // Warm Phase assertNextActionName("warm", SetPriorityAction.NAME, UnfollowAction.NAME, From 7066d58b1b77316cb735f29ea0101b52f9691349 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 21 Oct 2020 11:12:31 -0400 Subject: [PATCH 05/12] Tidy up some whitespace --- .../core/ilm/TimeseriesLifecycleType.java | 28 +++++----- .../xpack/core/ilm/LifecyclePolicyTests.java | 54 +++++++++---------- 2 files changed, 41 insertions(+), 41 deletions(-) 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 a8cf87ccec2b4..492ede06cbd43 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 @@ -176,20 +176,20 @@ public List getOrderedActions(Phase phase) { public String getNextActionName(String currentActionName, Phase phase) { List orderedActionNames; switch (phase.getName()) { - case HOT_PHASE: - orderedActionNames = ORDERED_VALID_HOT_ACTIONS; - break; - case WARM_PHASE: - orderedActionNames = ORDERED_VALID_WARM_ACTIONS; - break; - case COLD_PHASE: - orderedActionNames = ORDERED_VALID_COLD_ACTIONS; - break; - case DELETE_PHASE: - orderedActionNames = ORDERED_VALID_DELETE_ACTIONS; - break; - default: - throw new IllegalArgumentException("lifecycle type [" + TYPE + "] does not support phase [" + phase.getName() + "]"); + case HOT_PHASE: + orderedActionNames = ORDERED_VALID_HOT_ACTIONS; + break; + case WARM_PHASE: + orderedActionNames = ORDERED_VALID_WARM_ACTIONS; + break; + case COLD_PHASE: + orderedActionNames = ORDERED_VALID_COLD_ACTIONS; + break; + case DELETE_PHASE: + orderedActionNames = ORDERED_VALID_DELETE_ACTIONS; + break; + default: + throw new IllegalArgumentException("lifecycle type [" + TYPE + "] does not support phase [" + phase.getName() + "]"); } int index = orderedActionNames.indexOf(currentActionName); 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 0ccd0f11b63fd..916184f2eefc3 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 @@ -99,7 +99,7 @@ protected LifecyclePolicy createTestInstance() { public static LifecyclePolicy randomTimeseriesLifecyclePolicyWithAllPhases(@Nullable String lifecycleName) { List phaseNames = TimeseriesLifecycleType.VALID_PHASES; Map phases = new HashMap<>(phaseNames.size()); - Function> validActions = (phase) -> { + Function> validActions = (phase) -> { switch (phase) { case "hot": return TimeseriesLifecycleType.VALID_HOT_ACTIONS; @@ -112,14 +112,14 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicyWithAllPhases(@Null default: throw new IllegalArgumentException("invalid phase [" + phase + "]"); }}; - Function randomAction = (action) -> { + Function randomAction = (action) -> { switch (action) { case AllocateAction.NAME: return AllocateActionTests.randomInstance(); case DeleteAction.NAME: return new DeleteAction(); case WaitForSnapshotAction.NAME: - return WaitForSnapshotActionTests.randomInstance(); + return WaitForSnapshotActionTests.randomInstance(); case ForceMergeAction.NAME: return ForceMergeActionTests.randomInstance(); case ReadOnlyAction.NAME: @@ -157,7 +157,7 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l List phaseNames = randomSubsetOf( between(0, TimeseriesLifecycleType.VALID_PHASES.size() - 1), TimeseriesLifecycleType.VALID_PHASES); Map phases = new HashMap<>(phaseNames.size()); - Function> validActions = (phase) -> { + Function> validActions = (phase) -> { switch (phase) { case "hot": return TimeseriesLifecycleType.VALID_HOT_ACTIONS; @@ -170,7 +170,7 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l default: throw new IllegalArgumentException("invalid phase [" + phase + "]"); }}; - Function randomAction = (action) -> { + Function randomAction = (action) -> { switch (action) { case AllocateAction.NAME: return AllocateActionTests.randomInstance(); @@ -239,16 +239,16 @@ protected LifecyclePolicy mutateInstance(LifecyclePolicy instance) throws IOExce String name = instance.getName(); Map phases = instance.getPhases(); switch (between(0, 1)) { - case 0: - name = name + randomAlphaOfLengthBetween(1, 5); - break; - case 1: - String phaseName = randomValueOtherThanMany(phases::containsKey, () -> randomFrom(TimeseriesLifecycleType.VALID_PHASES)); - phases = new LinkedHashMap<>(phases); - phases.put(phaseName, new Phase(phaseName, TimeValue.timeValueSeconds(randomIntBetween(1, 1000)), Collections.emptyMap())); - break; - default: - throw new AssertionError("Illegal randomisation branch"); + case 0: + name = name + randomAlphaOfLengthBetween(1, 5); + break; + case 1: + String phaseName = randomValueOtherThanMany(phases::containsKey, () -> randomFrom(TimeseriesLifecycleType.VALID_PHASES)); + phases = new LinkedHashMap<>(phases); + phases.put(phaseName, new Phase(phaseName, TimeValue.timeValueSeconds(randomIntBetween(1, 1000)), Collections.emptyMap())); + break; + default: + throw new AssertionError("Illegal randomisation branch"); } return new LifecyclePolicy(TimeseriesLifecycleType.INSTANCE, name, phases); } @@ -301,7 +301,7 @@ public void testToStepsWithTwoPhases() { MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"), PhaseCompleteStep.finalStep("second_phase").getKey()); MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), - secondActionStep.getKey()); + secondActionStep.getKey()); MockStep firstActionAnotherStep = new MockStep(new StepKey("first_phase", "test", "bar"), secondAfter.getKey()); MockStep firstActionStep = new MockStep(new StepKey("first_phase", "test", "foo"), firstActionAnotherStep.getKey()); MockStep firstAfter = new MockStep(new StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), firstActionStep.getKey()); @@ -353,30 +353,30 @@ public void testIsActionSafe() { assertFalse(policy.isActionSafe(new StepKey("second_phase", MockAction.NAME, randomAlphaOfLength(10)))); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, - () -> policy.isActionSafe(new StepKey("non_existant_phase", MockAction.NAME, randomAlphaOfLength(10)))); + () -> policy.isActionSafe(new StepKey("non_existant_phase", MockAction.NAME, randomAlphaOfLength(10)))); assertEquals("Phase [non_existant_phase] does not exist in policy [" + policy.getName() + "]", exception.getMessage()); exception = expectThrows(IllegalArgumentException.class, - () -> policy.isActionSafe(new StepKey("first_phase", "non_existant_action", randomAlphaOfLength(10)))); + () -> policy.isActionSafe(new StepKey("first_phase", "non_existant_action", randomAlphaOfLength(10)))); assertEquals("Action [non_existant_action] in phase [first_phase] does not exist in policy [" + policy.getName() + "]", - exception.getMessage()); + exception.getMessage()); assertTrue(policy.isActionSafe(new StepKey("new", randomAlphaOfLength(10), randomAlphaOfLength(10)))); } public void testValidatePolicyName() { - expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) + - "," + randomAlphaOfLengthBetween(0,10))); - expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) + - " " + randomAlphaOfLengthBetween(0,10))); + expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0, 10) + + "," + randomAlphaOfLengthBetween(0, 10))); + expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0, 10) + + " " + randomAlphaOfLengthBetween(0, 10))); expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName("_" + randomAlphaOfLengthBetween(1, 20))); expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(256, 1000))); - LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(1,10) + "_" + randomAlphaOfLengthBetween(0,10)); + LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(1, 10) + "_" + randomAlphaOfLengthBetween(0, 10)); - LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) + "-" + randomAlphaOfLengthBetween(0,10)); - LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) + "+" + randomAlphaOfLengthBetween(0,10)); + LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0, 10) + "-" + randomAlphaOfLengthBetween(0, 10)); + LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0, 10) + "+" + randomAlphaOfLengthBetween(0, 10)); - LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(1,255)); + LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(1, 255)); } } From 315f569ac20698d9fdf79056ec9a607b24b786f9 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 21 Oct 2020 13:11:39 -0400 Subject: [PATCH 06/12] Update the shrink docs to match --- docs/reference/ilm/actions/ilm-shrink.asciidoc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/reference/ilm/actions/ilm-shrink.asciidoc b/docs/reference/ilm/actions/ilm-shrink.asciidoc index 8800136abf52b..58d223aa4d09e 100644 --- a/docs/reference/ilm/actions/ilm-shrink.asciidoc +++ b/docs/reference/ilm/actions/ilm-shrink.asciidoc @@ -2,7 +2,7 @@ [[ilm-shrink]] === Shrink -Phases allowed: warm +Phases allowed: hot, warm. Sets an index to <> and shrinks it into a new index with fewer primary shards. @@ -11,9 +11,12 @@ For example, if the name of the source index is _logs_, the name of the shrunken index is _shrink-logs_. The shrink action allocates all primary shards of the index to one node so it -can call the <> to shrink the index. +can call the <> to shrink the index. After shrinking, it swaps aliases that point to the original index to the new shrunken index. +To use the `shrink` action in the `hot` phase, the `rollover` action *must* be present. +If no rollover action is configured, {ilm-init} will reject the policy. + [IMPORTANT] If the shrink action is used on a <>, policy execution waits until the leader index rolls over (or is From 4c40a1ab3abcd34aa41f6c7ce2fb368b8190d86c Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 21 Oct 2020 13:27:48 -0400 Subject: [PATCH 07/12] Add a yamlRestTest for the changes --- .../rest-api-spec/test/ilm/10_basic.yml | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml b/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml index 3dcfb685323c0..fb985d27be8d8 100644 --- a/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml +++ b/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml @@ -107,7 +107,7 @@ setup: body: settings: index.lifecycle.name: "my_moveable_timeseries_lifecycle" - + - do: ilm.put_lifecycle: policy: "my_timeseries_lifecycle" @@ -238,3 +238,23 @@ setup: } } } + + - do: + catch: bad_request + ilm.put_lifecycle: + policy: "my_invalid_lifecycle" + body: | + { + "policy": { + "phases": { + "hot": { + "min_age": "0s", + "actions": { + "shrink": { + "number_of_shards": 1 + } + } + } + } + } + } From fe5c39094ee298b6fea76a014f8230b7f1364442 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 28 Oct 2020 16:58:15 -0400 Subject: [PATCH 08/12] Minor refactor Inline forceMergeActionWithCodec in place of its only caller. --- .../xpack/ilm/TimeSeriesLifecycleActionsIT.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index 9677fbac3a5b5..607b5f3f1e6be 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -529,7 +529,7 @@ public void testReadOnly() throws Exception { }); } - public void forceMergeActionWithCodec(String codec) throws Exception { + public void testForceMergeAction() throws Exception { createIndexWithSettings(client(), index, alias, Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)); for (int i = 0; i < randomIntBetween(2, 10); i++) { @@ -540,7 +540,7 @@ public void forceMergeActionWithCodec(String codec) throws Exception { } assertThat(getNumberOfSegments(client(), index), greaterThan(1)); - createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, codec)); + createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, null)); updatePolicy(index, policy); assertBusy(() -> { @@ -552,10 +552,6 @@ public void forceMergeActionWithCodec(String codec) throws Exception { expectThrows(ResponseException.class, () -> indexDocument(client(), index)); } - public void testForceMergeAction() throws Exception { - forceMergeActionWithCodec(null); - } - public void testShrinkAction() throws Exception { int numShards = 4; int divisor = randomFrom(2, 4); From 0a537aaf5a4245d4b38f269a99495d87a28f588d Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Wed, 28 Oct 2020 16:59:40 -0400 Subject: [PATCH 09/12] Add an integration test --- .../ilm/TimeSeriesLifecycleActionsIT.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index 607b5f3f1e6be..9b7bc051c4011 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -640,6 +640,48 @@ public void testShrinkDuringSnapshot() throws Exception { assertOK(client().performRequest(new Request("DELETE", "/_snapshot/repo/snapshot"))); } + public void testShrinkActionInTheHotPhase() throws Exception { + int numShards = 2; + int expectedFinalShards = 1; + String originalIndex = index + "-000001"; + String shrunkenIndex = ShrinkAction.SHRUNKEN_INDEX_PREFIX + originalIndex; + + // add a policy + Map hotActions = Map.of( + RolloverAction.NAME, new RolloverAction(null, null, 1L), + ShrinkAction.NAME, new ShrinkAction(expectedFinalShards)); + Map phases = Map.of( + "hot", new Phase("hot", TimeValue.ZERO, hotActions)); + LifecyclePolicy lifecyclePolicy = new LifecyclePolicy(policy, phases); + Request createPolicyRequest = new Request("PUT", "_ilm/policy/" + policy); + createPolicyRequest.setJsonEntity("{ \"policy\":" + Strings.toString(lifecyclePolicy) + "}"); + assertOK(client().performRequest(createPolicyRequest)); + + // and a template + Request createTemplateRequest = new Request("PUT", "_template/" + index); + createTemplateRequest.setJsonEntity("{" + + "\"index_patterns\": [\"" + index + "-*\"], \n" + + " \"settings\": {\n" + + " \"number_of_shards\": " + numShards + ",\n" + + " \"number_of_replicas\": 0,\n" + + " \"index.lifecycle.name\": \"" + policy + "\", \n" + + " \"index.lifecycle.rollover_alias\": \"" + alias + "\"\n" + + " }\n" + + "}"); + client().performRequest(createTemplateRequest); + + // then create the index and index a document to trigger rollover + createIndexWithSettings(client(), originalIndex, alias, Settings.builder(), true); + index(client(), originalIndex, "_id", "foo", "bar"); + + assertBusy(() -> assertTrue(indexExists(shrunkenIndex)), 30, TimeUnit.SECONDS); + assertBusy(() -> assertThat(getStepKeyForIndex(client(), shrunkenIndex), equalTo(PhaseCompleteStep.finalStep("hot").getKey()))); + assertBusy(() -> { + Map settings = getOnlyIndexSettings(client(), shrunkenIndex); + assertThat(settings.get(IndexMetadata.SETTING_NUMBER_OF_SHARDS), equalTo(String.valueOf(expectedFinalShards))); + }); + } + public void testSetSingleNodeAllocationRetriesUntilItSucceeds() throws Exception { int numShards = 2; int expectedFinalShards = 1; From c5e71e6efb62c076091fcf224e544a1d126116b5 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Thu, 29 Oct 2020 10:26:48 -0400 Subject: [PATCH 10/12] More docs changes Move forcemerge after rollover, since that's when it'll actually execute. Add shrink to the hot phase. Migrate was missing in the warm and cold phases, fix that. --- docs/reference/ilm/ilm-index-lifecycle.asciidoc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/reference/ilm/ilm-index-lifecycle.asciidoc b/docs/reference/ilm/ilm-index-lifecycle.asciidoc index 8392a30e48e07..a43d979a2ec76 100644 --- a/docs/reference/ilm/ilm-index-lifecycle.asciidoc +++ b/docs/reference/ilm/ilm-index-lifecycle.asciidoc @@ -77,22 +77,24 @@ the rollover criteria, it could be 20 minutes before the rollover is complete. * Hot - <> - <> - - <> - <> + - <> + - <> * Warm - <> - <> - <> - <> + - <> - <> - <> * Cold - <> - <> - <> + - <> - <> - <> * Delete - <> - <> - From 2d9c307e63a94c4eaca1a9e8996459e1b591b869 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Thu, 29 Oct 2020 12:41:02 -0400 Subject: [PATCH 11/12] Oops, drop an extraneous assertOK --- .../elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index 376a53217eea9..35f519dc87678 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -570,7 +570,7 @@ RolloverAction.NAME, new RolloverAction(null, null, 1L), LifecyclePolicy lifecyclePolicy = new LifecyclePolicy(policy, phases); Request createPolicyRequest = new Request("PUT", "_ilm/policy/" + policy); createPolicyRequest.setJsonEntity("{ \"policy\":" + Strings.toString(lifecyclePolicy) + "}"); - assertOK(client().performRequest(createPolicyRequest)); + client().performRequest(createPolicyRequest); // and a template Request createTemplateRequest = new Request("PUT", "_template/" + index); From 563720a2d0030f9592ef81a369d5effa4592d6b5 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Thu, 29 Oct 2020 13:25:13 -0400 Subject: [PATCH 12/12] Revert "Minor refactor" This reverts commit fe5c39094ee298b6fea76a014f8230b7f1364442. --- .../xpack/ilm/TimeSeriesLifecycleActionsIT.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index 35f519dc87678..31543b704ef09 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -444,7 +444,7 @@ public void testReadOnly() throws Exception { }); } - public void testForceMergeAction() throws Exception { + public void forceMergeActionWithCodec(String codec) throws Exception { createIndexWithSettings(client(), index, alias, Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)); for (int i = 0; i < randomIntBetween(2, 10); i++) { @@ -455,7 +455,7 @@ public void testForceMergeAction() throws Exception { } assertThat(getNumberOfSegments(client(), index), greaterThan(1)); - createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, null)); + createNewSingletonPolicy(client(), policy, "warm", new ForceMergeAction(1, codec)); updatePolicy(index, policy); assertBusy(() -> { @@ -467,6 +467,10 @@ public void testForceMergeAction() throws Exception { expectThrows(ResponseException.class, () -> indexDocument(client(), index)); } + public void testForceMergeAction() throws Exception { + forceMergeActionWithCodec(null); + } + public void testShrinkAction() throws Exception { int numShards = 4; int divisor = randomFrom(2, 4);