Skip to content

Commit 7c15e95

Browse files
talevytalevy
committed
[ILM] reduce time restriction on IndexLifecycleExplainResponse (#35954)
step times were set. The assumption was that these are always set. Tests passed, which led me to believe this was true. There is a time when shrunk indices have their step phase/action/step details set, but with no time information (in the CopyExecutionStateStep). Explain API fails for these
1 parent ac57131 commit 7c15e95

File tree

4 files changed

+26
-26
lines changed

4 files changed

+26
-26
lines changed

‎client/rest-high-level/src/main/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponse.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,15 +124,12 @@ private IndexLifecycleExplainResponse(String index, boolean managedByILM, String
124124
throw new IllegalArgumentException("[" + POLICY_NAME_FIELD.getPreferredName() + "] cannot be null for managed index");
125125
}
126126
// check to make sure that step details are either all null or all set.
127-
long numNull = Stream.of(phase, action, step, phaseTime, actionTime, stepTime).filter(Objects::isNull).count();
128-
if (numNull > 0 && numNull < 6) {
127+
long numNull = Stream.of(phase, action, step).filter(Objects::isNull).count();
128+
if (numNull > 0 && numNull < 3) {
129129
throw new IllegalArgumentException("managed index response must have complete step details [" +
130130
PHASE_FIELD.getPreferredName() + "=" + phase + ", " +
131-
PHASE_TIME_FIELD.getPreferredName() + "=" + phaseTime + ", " +
132131
ACTION_FIELD.getPreferredName() + "=" + action + ", " +
133-
ACTION_TIME_FIELD.getPreferredName() + "=" + actionTime + ", " +
134-
STEP_FIELD.getPreferredName() + "=" + step + ", " +
135-
STEP_TIME_FIELD.getPreferredName() + "=" + stepTime + "]");
132+
STEP_FIELD.getPreferredName() + "=" + step + "]");
136133
}
137134
} else {
138135
if (policyName != null || lifecycleDate != null || phase != null || action != null || step != null || failedStep != null

‎client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ private static IndexLifecycleExplainResponse randomManagedIndexExplainResponse()
6969
}
7070

7171
public void testInvalidStepDetails() {
72-
final int numNull = randomIntBetween(1, 6);
72+
final int numNull = randomIntBetween(1, 3);
7373
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
7474
IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10),
7575
randomAlphaOfLength(10),
@@ -78,9 +78,9 @@ public void testInvalidStepDetails() {
7878
(numNull == 2) ? null : randomAlphaOfLength(10),
7979
(numNull == 3) ? null : randomAlphaOfLength(10),
8080
randomBoolean() ? null : randomAlphaOfLength(10),
81-
(numNull == 4) ? null : randomNonNegativeLong(),
82-
(numNull == 5) ? null : randomNonNegativeLong(),
83-
(numNull == 6) ? null : randomNonNegativeLong(),
81+
randomBoolean() ? null : randomNonNegativeLong(),
82+
randomBoolean() ? null : randomNonNegativeLong(),
83+
randomBoolean() ? null : randomNonNegativeLong(),
8484
randomBoolean() ? null : new BytesArray(new RandomStepInfo(() -> randomAlphaOfLength(10)).toString()),
8585
randomBoolean() ? null : PhaseExecutionInfoTests.randomPhaseExecutionInfo("")));
8686
assertThat(exception.getMessage(), startsWith("managed index response must have complete step details"));

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponse.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,12 @@ private IndexLifecycleExplainResponse(String index, boolean managedByILM, String
113113
throw new IllegalArgumentException("[" + POLICY_NAME_FIELD.getPreferredName() + "] cannot be null for managed index");
114114
}
115115
// check to make sure that step details are either all null or all set.
116-
long numNull = Stream.of(phase, action, step, phaseTime, actionTime, stepTime).filter(Objects::isNull).count();
117-
if (numNull > 0 && numNull < 6) {
116+
long numNull = Stream.of(phase, action, step).filter(Objects::isNull).count();
117+
if (numNull > 0 && numNull < 3) {
118118
throw new IllegalArgumentException("managed index response must have complete step details [" +
119119
PHASE_FIELD.getPreferredName() + "=" + phase + ", " +
120-
PHASE_TIME_FIELD.getPreferredName() + "=" + phaseTime + ", " +
121120
ACTION_FIELD.getPreferredName() + "=" + action + ", " +
122-
ACTION_TIME_FIELD.getPreferredName() + "=" + actionTime + ", " +
123-
STEP_FIELD.getPreferredName() + "=" + step + ", " +
124-
STEP_TIME_FIELD.getPreferredName() + "=" + stepTime + "]");
121+
STEP_FIELD.getPreferredName() + "=" + step + "]");
125122
}
126123
} else {
127124
if (policyName != null || lifecycleDate != null || phase != null || action != null || step != null || failedStep != null

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponseTests.java

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ private static IndexLifecycleExplainResponse randomManagedIndexExplainResponse()
6060
}
6161

6262
public void testInvalidStepDetails() {
63-
final int numNull = randomIntBetween(1, 6);
63+
final int numNull = randomIntBetween(1, 3);
6464
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
6565
IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10),
6666
randomAlphaOfLength(10),
@@ -69,9 +69,9 @@ public void testInvalidStepDetails() {
6969
(numNull == 2) ? null : randomAlphaOfLength(10),
7070
(numNull == 3) ? null : randomAlphaOfLength(10),
7171
randomBoolean() ? null : randomAlphaOfLength(10),
72-
(numNull == 4) ? null : randomNonNegativeLong(),
73-
(numNull == 5) ? null : randomNonNegativeLong(),
74-
(numNull == 6) ? null : randomNonNegativeLong(),
72+
randomBoolean() ? null : randomNonNegativeLong(),
73+
randomBoolean() ? null : randomNonNegativeLong(),
74+
randomBoolean() ? null : randomNonNegativeLong(),
7575
randomBoolean() ? null : new BytesArray(new RandomStepInfo(() -> randomAlphaOfLength(10)).toString()),
7676
randomBoolean() ? null : PhaseExecutionInfoTests.randomPhaseExecutionInfo("")));
7777
assertThat(exception.getMessage(), startsWith("managed index response must have complete step details"));
@@ -109,7 +109,7 @@ protected IndexLifecycleExplainResponse mutateInstance(IndexLifecycleExplainResp
109109
BytesReference stepInfo = instance.getStepInfo();
110110
PhaseExecutionInfo phaseExecutionInfo = instance.getPhaseExecutionInfo();
111111
if (managed) {
112-
switch (between(0, 7)) {
112+
switch (between(0, 10)) {
113113
case 0:
114114
index = index + randomAlphaOfLengthBetween(1, 5);
115115
break;
@@ -120,11 +120,17 @@ protected IndexLifecycleExplainResponse mutateInstance(IndexLifecycleExplainResp
120120
phase = randomAlphaOfLengthBetween(1, 5);
121121
action = randomAlphaOfLengthBetween(1, 5);
122122
step = randomAlphaOfLengthBetween(1, 5);
123+
break;
124+
case 3:
123125
phaseTime = randomValueOtherThan(phaseTime, () -> randomLongBetween(0, 100000));
126+
break;
127+
case 4:
124128
actionTime = randomValueOtherThan(actionTime, () -> randomLongBetween(0, 100000));
129+
break;
130+
case 5:
125131
stepTime = randomValueOtherThan(stepTime, () -> randomLongBetween(0, 100000));
126132
break;
127-
case 3:
133+
case 6:
128134
if (Strings.hasLength(failedStep) == false) {
129135
failedStep = randomAlphaOfLength(10);
130136
} else if (randomBoolean()) {
@@ -133,10 +139,10 @@ protected IndexLifecycleExplainResponse mutateInstance(IndexLifecycleExplainResp
133139
failedStep = null;
134140
}
135141
break;
136-
case 4:
142+
case 7:
137143
policyTime = randomValueOtherThan(policyTime, () -> randomLongBetween(0, 100000));
138144
break;
139-
case 5:
145+
case 8:
140146
if (Strings.hasLength(stepInfo) == false) {
141147
stepInfo = new BytesArray(randomByteArrayOfLength(100));
142148
} else if (randomBoolean()) {
@@ -146,10 +152,10 @@ protected IndexLifecycleExplainResponse mutateInstance(IndexLifecycleExplainResp
146152
stepInfo = null;
147153
}
148154
break;
149-
case 6:
155+
case 9:
150156
phaseExecutionInfo = randomValueOtherThan(phaseExecutionInfo, () -> PhaseExecutionInfoTests.randomPhaseExecutionInfo(""));
151157
break;
152-
case 7:
158+
case 10:
153159
return IndexLifecycleExplainResponse.newUnmanagedIndexResponse(index);
154160
default:
155161
throw new AssertionError("Illegal randomisation branch");

0 commit comments

Comments
 (0)