honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix infinite loop in test and correct logic in timeseries type

This commit is contained in:
Tal Levy 2017-12-21 13:14:37 -08:00
parent 66af56320e
commit 6a47b4fa6e
2 changed files with 13 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
x-pack/plugin/src/main/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleType.java
View File

@ -124,17 +124,21 @@ public class TimeseriesLifecycleType implements LifecycleType {
case "warm": case "warm":
return (action) -> { return (action) -> {
ReplicasAction replicasAction = (ReplicasAction) actions.getOrDefault(ReplicasAction.NAME, null); ReplicasAction replicasAction = (ReplicasAction) actions.getOrDefault(ReplicasAction.NAME, null);
boolean replicaActionFirst = replicasAction != null
&& replicasAction.getNumberOfReplicas() <= context.getNumberOfReplicas();
if (action == null) { if (action == null) {
if (replicasAction != null && replicasAction.getNumberOfReplicas() <= context.getNumberOfReplicas()) { if (replicaActionFirst) {
return replicasAction; return replicasAction;
} }
return Stream.of(AllocateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME, ReplicasAction.NAME) return Stream.of(AllocateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME, ReplicasAction.NAME)
.map(a -> actions.getOrDefault(a, null)) .map(a -> actions.getOrDefault(a, null))
.filter(Objects::nonNull).findFirst().orElse(null); .filter(Objects::nonNull).findFirst().orElse(null);
} else if (action instanceof ReplicasAction) { } else if (action instanceof ReplicasAction) {
return Stream.of(AllocateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME) if (replicaActionFirst) {
.map(a -> actions.getOrDefault(a, null)) return Stream.of(AllocateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME)
.filter(Objects::nonNull).findFirst().orElse(null); .map(a -> actions.getOrDefault(a, null))
.filter(Objects::nonNull).findFirst().orElse(null);
}
} else if (action instanceof AllocateAction) { } else if (action instanceof AllocateAction) {
return Stream.of(ShrinkAction.NAME, ForceMergeAction.NAME, ReplicasAction.NAME) return Stream.of(ShrinkAction.NAME, ForceMergeAction.NAME, ReplicasAction.NAME)
.map(a -> actions.getOrDefault(a, null)) .map(a -> actions.getOrDefault(a, null))

17
x-pack/plugin/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleTypeTests.java
View File

@ -192,7 +192,6 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
assertNull(provider.next(TEST_ROLLOVER_ACTION)); assertNull(provider.next(TEST_ROLLOVER_ACTION));
} }
@AwaitsFix(bugUrl = "This gets into an infinite loop if there are other actions as well as the replicas action")
public void testWarmActionProviderReplicasActionSortOrder() { public void testWarmActionProviderReplicasActionSortOrder() {
String indexName = randomAlphaOfLengthBetween(1, 10); String indexName = randomAlphaOfLengthBetween(1, 10);
Map<String, LifecycleAction> actions = randomSubsetOf(VALID_WARM_ACTIONS) Map<String, LifecycleAction> actions = randomSubsetOf(VALID_WARM_ACTIONS)
@ -222,20 +221,14 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
}; };
provider = policy.getActionProvider(context, warmPhase); provider = policy.getActionProvider(context, warmPhase);
if (actions.size() > 1) { if (actions.size() > 1) {
int actionCount = 1;
LifecycleAction current = provider.next(null); LifecycleAction current = provider.next(null);
assertThat(current, not(equalTo(TEST_REPLICAS_ACTION))); assertThat(current, not(equalTo(TEST_REPLICAS_ACTION)));
while (true) { while (actionCount++ < actions.size()) {
// NORELEASE This loop never exits as there is no break condition current = provider.next(current);
// also provider.next(current) never evaluates to null because
// when called with the replicas action it always returns a
// non-null action. We should avoid using while true here
// because it means if there is a bug we will hang the build
if (provider.next(current) == null) {
assertThat(current, equalTo(TEST_REPLICAS_ACTION));
} else {
current = provider.next(current);
}
} }
assertNull(provider.next(current));
assertThat(current, equalTo(TEST_REPLICAS_ACTION));
} else { } else {
assertThat(provider.next(null), equalTo(TEST_REPLICAS_ACTION)); assertThat(provider.next(null), equalTo(TEST_REPLICAS_ACTION));
} }