Skip to content

Commit

Permalink
Update ILM message for out of order phases error (elastic#75099)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Esduard @danhermann
Esduard authored and danhermann committed Jul 14, 2021
1 parent 51e18cf commit 639948b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 12 deletions.
34 changes: 29 additions & 5 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 @@ -8,8 +8,8 @@

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.rollup.RollupV2;

import java.io.IOException;
Expand All @@ -19,6 +19,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -399,10 +400,33 @@ public static String validateMonotonicallyIncreasingPhaseTimings(Collection<Phas
.collect(Collectors.toSet());
if (phasesWithBadAges.size() > 0) {
phasesWithBadAges.forEach(p -> invalidPhases.add(p.getName()));
errors.add("phases [" + phasesWithBadAges.stream().map(Phase::getName).collect(Collectors.joining(",")) +
"] configure a [min_age] value less than the [min_age] of [" + phase.getMinimumAge() +
"] for the [" + phaseName + "] phase, configuration: " +
phasesWithBadAges.stream().collect(Collectors.toMap(Phase::getName, Phase::getMinimumAge)));

//build an error message string
Iterator<Phase> it = phasesWithBadAges.iterator();
Phase badPhase = it.next();

String error = "Your policy is configured to run the "
+ badPhase.getName() + " phase (min_age: " + badPhase.getMinimumAge() + ")";

if (phasesWithBadAges.size() > 1) {
while (it.hasNext()) {
badPhase = it.next();
error = error + ", the " + badPhase.getName() + " phase (min_age: "
+ badPhase.getMinimumAge()+ ")";
}
//if multiple phases are cited
//replace last occurrence of "," with " and"
StringBuilder builder = new StringBuilder();
int last_comma_index = error.lastIndexOf(",");
builder.append(error, 0, last_comma_index);
builder.append(" and");
builder.append(error.substring(last_comma_index + 1));
error = builder.toString();
}
error = error + " before the " + phaseName + " phase (min_age: " + phase.getMinimumAge() +
"). You should change the phase timing so that the phases will execute in the order of hot, warm, then cold.";

errors.add(error);
}
}
}
Expand Down
47 changes: 41 additions & 6 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 @@ -760,8 +760,9 @@ public void testValidatingIncreasingAges() {
validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));

assertThat(err,
containsString("phases [cold] configure a [min_age] value less than the" +
" [min_age] of [1d] for the [hot] phase, configuration: {cold=12h}"));
containsString("Your policy is configured to run the cold phase "+
"(min_age: 12h) before the hot phase (min_age: 1d). You should change "+
"the phase timing so that the phases will execute in the order of hot, warm, then cold."));
}

{
Expand All @@ -775,8 +776,9 @@ public void testValidatingIncreasingAges() {
validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));

assertThat(err,
containsString("phases [frozen,delete] configure a [min_age] value less " +
"than the [min_age] of [3d] for the [warm] phase, configuration: {frozen=1d, delete=2d}"));
containsString("Your policy is configured to run the frozen phase "+
"(min_age: 1d) and the delete phase (min_age: 2d) before the warm phase (min_age: 3d)."+
" You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
}

{
Expand All @@ -790,8 +792,41 @@ public void testValidatingIncreasingAges() {
validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));

assertThat(err,
containsString("phases [frozen,delete] configure a [min_age] value less than " +
"the [min_age] of [3d] for the [warm] phase, configuration: {frozen=2d, delete=1d}"));
containsString("Your policy is configured to run the frozen phase "+
"(min_age: 2d) and the delete phase (min_age: 1d) before the warm phase (min_age: 3d)."+
" You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
}

{
Phase hotPhase = new Phase(HOT_PHASE, TimeValue.timeValueDays(3), Collections.emptyMap());
Phase warmPhase = new Phase(WARM_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
Phase coldPhase = new Phase(COLD_PHASE, null, Collections.emptyMap());
Phase frozenPhase = new Phase(FROZEN_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
Phase deletePhase = new Phase(DELETE_PHASE, TimeValue.timeValueDays(1), Collections.emptyMap());

String err =
validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));

assertThat(err,
containsString("Your policy is configured to run the frozen phase "+
"(min_age: 2d), the delete phase (min_age: 1d) and the warm phase (min_age: 2d) before the hot phase (min_age: 3d)."+
" You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
}

{
Phase hotPhase = new Phase(HOT_PHASE, TimeValue.timeValueDays(3), Collections.emptyMap());
Phase warmPhase = new Phase(WARM_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
Phase coldPhase = new Phase(COLD_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
Phase frozenPhase = new Phase(FROZEN_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
Phase deletePhase = new Phase(DELETE_PHASE, TimeValue.timeValueDays(1), Collections.emptyMap());

String err =
validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));

assertThat(err,
containsString("Your policy is configured to run the cold phase (min_age: 2d), the frozen phase "+
"(min_age: 2d), the delete phase (min_age: 1d) and the warm phase (min_age: 2d) before the hot phase (min_age: 3d)."+
" You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
}
}

Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugin/ilm/qa/rest/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,10 @@ testClusters.all {
setting 'xpack.license.self_generated.type', 'trial'
user clusterCredentials
}

tasks.named("yamlRestCompatTest").configure {
systemProperty 'tests.rest.blacklist', [
//TODO: remove once #75099 is back ported to 7.x
'ilm/10_basic/Test increasing phase timings validated'
].join(',')
}
2 changes: 1 addition & 1 deletion x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ setup:
"Test increasing phase timings validated":

- do:
catch: /phases \[delete\] configure a \[min_age\] value less than the \[min_age\] of \[10s\] for the \[warm\] phase/
catch: /Your policy is configured to run the delete phase \(min_age\:\ 5s\) before the warm phase \(min_age\:\ 10s\). You should change the phase timing so that the phases will execute in the order of hot, warm, then cold./
ilm.put_lifecycle:
policy: "bad_policy"
body: |
Expand Down

0 comments on commit 639948b

Please sign in to comment.