Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow shrink in the hot phase for ILM policies #64008

Merged
merged 13 commits into from
Oct 29, 2020
Merged

Allow shrink in the hot phase for ILM policies #64008

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7e6e90f
getOrDefault with null is just get
joegallo Oct 8, 2020
8a8b2d9
Map.of makes this immutable
joegallo Oct 8, 2020
10219f0
Refactor a bit
joegallo Oct 21, 2020
bd1bebc
Allow shrink in the hot phase
joegallo Oct 21, 2020
7066d58
Tidy up some whitespace
joegallo Oct 21, 2020
315f569
Update the shrink docs to match
joegallo Oct 21, 2020
4c40a1a
Add a yamlRestTest for the changes
joegallo Oct 21, 2020
fe5c390
Minor refactor
joegallo Oct 28, 2020
0a537aa
Add an integration test
joegallo Oct 28, 2020
c5e71e6
More docs changes
joegallo Oct 29, 2020
6910ceb
Merge branch 'master' into add-shrink-to-hot-phase
elasticmachine Oct 29, 2020
2d9c307
Oops, drop an extraneous assertOK
joegallo Oct 29, 2020
563720a
Revert "Minor refactor"
joegallo Oct 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions docs/reference/ilm/actions/ilm-shrink.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
[[ilm-shrink]]
=== Shrink

Phases allowed: warm
Phases allowed: hot, warm.

Sets an index to <<dynamic-index-settings, read-only>>
and shrinks it into a new index with fewer primary shards.
Expand All @@ -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 <<indices-shrink-index,Shrink API>> to shrink the index.
can call the <<indices-shrink-index,Shrink API>> 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 <<ccr-put-follow,follower index>>,
policy execution waits until the leader index rolls over (or is
Expand Down
6 changes: 4 additions & 2 deletions docs/reference/ilm/ilm-index-lifecycle.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,22 +77,24 @@ the rollover criteria, it could be 20 minutes before the rollover is complete.
* Hot
- <<ilm-set-priority,Set Priority>>
- <<ilm-unfollow,Unfollow>>
- <<ilm-forcemerge,Force Merge>>
- <<ilm-rollover,Rollover>>
- <<ilm-shrink,Shrink>>
- <<ilm-forcemerge,Force Merge>>
* Warm
- <<ilm-set-priority,Set Priority>>
- <<ilm-unfollow,Unfollow>>
- <<ilm-readonly,Read-Only>>
- <<ilm-allocate,Allocate>>
- <<ilm-migrate,Migrate>>
- <<ilm-shrink,Shrink>>
- <<ilm-forcemerge,Force Merge>>
* Cold
- <<ilm-set-priority-action,Set Priority>>
- <<ilm-unfollow-action,Unfollow>>
- <<ilm-allocate,Allocate>>
- <<ilm-migrate,Migrate>>
- <<ilm-freeze,Freeze>>
- <<ilm-searchable-snapshot, Searchable Snapshot>>
* Delete
- <<ilm-wait-for-snapshot-action,Wait For Snapshot>>
- <<ilm-delete,Delete>>

70 changes: 35 additions & 35 deletions ...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class TimeseriesLifecycleType implements LifecycleType {
static final String DELETE_PHASE = "delete";
static final List<String> VALID_PHASES = Arrays.asList(HOT_PHASE, WARM_PHASE, COLD_PHASE, DELETE_PHASE);
static final List<String> ORDERED_VALID_HOT_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, RolloverAction.NAME,
ForceMergeAction.NAME);
ShrinkAction.NAME, ForceMergeAction.NAME);
static final List<String> ORDERED_VALID_WARM_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, ReadOnlyAction.NAME,
AllocateAction.NAME, MigrateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME);
static final List<String> ORDERED_VALID_COLD_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, AllocateAction.NAME,
Expand All @@ -50,14 +50,13 @@ public class TimeseriesLifecycleType implements LifecycleType {
static final Set<String> VALID_WARM_ACTIONS = Sets.newHashSet(ORDERED_VALID_WARM_ACTIONS);
static final Set<String> VALID_COLD_ACTIONS = Sets.newHashSet(ORDERED_VALID_COLD_ACTIONS);
static final Set<String> VALID_DELETE_ACTIONS = Sets.newHashSet(ORDERED_VALID_DELETE_ACTIONS);
private static final Map<String, Set<String>> ALLOWED_ACTIONS = new HashMap<>();
private static final Map<String, Set<String>> ALLOWED_ACTIONS = Map.of(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not compile on backport (which is fine, just wanted to let you know since you asked about this previously)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, I've pregamed that and I'm ready to do the backport dance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have these utility classes in the 7.x branch that provide these functions in a Java8-compatible way so that backports are easier. They're also packaged as a multi-release JAR so that in JVMs that are 9 or later, they use the Java libraries instead, so there's no downside to using them when running with later JVMs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danhermann the utility classes are unfortunately not that helpful, since to use they the Map.of needs to be fully qualified, or java.util.Map<String, Set<String>> has to be specified to fully qualify the jdk classes, which I think is more awkward than just changing on backport.

HOT_PHASE, VALID_HOT_ACTIONS,
WARM_PHASE, VALID_WARM_ACTIONS,
COLD_PHASE, VALID_COLD_ACTIONS,
DELETE_PHASE, VALID_DELETE_ACTIONS);

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);
}
static final Set<String> HOT_ACTIONS_THAT_REQUIRE_ROLLOVER = Sets.newHashSet(ShrinkAction.NAME, ForceMergeAction.NAME);

private TimeseriesLifecycleType() {
}
Expand Down Expand Up @@ -157,16 +156,16 @@ public List<LifecycleAction> getOrderedActions(Phase phase) {
Map<String, LifecycleAction> 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() + "]");
Expand All @@ -177,20 +176,20 @@ public List<LifecycleAction> getOrderedActions(Phase phase) {
public String getNextActionName(String currentActionName, Phase phase) {
List<String> 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);
Expand Down Expand Up @@ -226,17 +225,18 @@ public void validate(Collection<Phase> 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");
}

Expand Down
59 changes: 30 additions & 29 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ protected LifecyclePolicy createTestInstance() {
public static LifecyclePolicy randomTimeseriesLifecyclePolicyWithAllPhases(@Nullable String lifecycleName) {
List<String> phaseNames = TimeseriesLifecycleType.VALID_PHASES;
Map<String, Phase> phases = new HashMap<>(phaseNames.size());
Function<String, Set<String>> validActions = (phase) -> {
Function<String, Set<String>> validActions = (phase) -> {
switch (phase) {
case "hot":
return TimeseriesLifecycleType.VALID_HOT_ACTIONS;
Expand All @@ -112,14 +112,14 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicyWithAllPhases(@Null
default:
throw new IllegalArgumentException("invalid phase [" + phase + "]");
}};
Function<String, LifecycleAction> randomAction = (action) -> {
Function<String, LifecycleAction> 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:
Expand Down Expand Up @@ -157,7 +157,7 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
List<String> phaseNames = randomSubsetOf(
between(0, TimeseriesLifecycleType.VALID_PHASES.size() - 1), TimeseriesLifecycleType.VALID_PHASES);
Map<String, Phase> phases = new HashMap<>(phaseNames.size());
Function<String, Set<String>> validActions = (phase) -> {
Function<String, Set<String>> validActions = (phase) -> {
switch (phase) {
case "hot":
return TimeseriesLifecycleType.VALID_HOT_ACTIONS;
Expand All @@ -170,7 +170,7 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
default:
throw new IllegalArgumentException("invalid phase [" + phase + "]");
}};
Function<String, LifecycleAction> randomAction = (action) -> {
Function<String, LifecycleAction> randomAction = (action) -> {
switch (action) {
case AllocateAction.NAME:
return AllocateActionTests.randomInstance();
Expand Down Expand Up @@ -204,8 +204,9 @@ public static LifecyclePolicy randomTimeseriesLifecyclePolicy(@Nullable String l
Map<String, LifecycleAction> actions = new HashMap<>();
List<String> 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);
}

Expand Down Expand Up @@ -238,16 +239,16 @@ protected LifecyclePolicy mutateInstance(LifecyclePolicy instance) throws IOExce
String name = instance.getName();
Map<String, Phase> 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);
}
Expand Down Expand Up @@ -300,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());
Expand Down Expand Up @@ -352,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));
}
}
3 changes: 1 addition & 2 deletions ...gin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}

Expand Down Expand Up @@ -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,
Expand Down
Loading