From db3a88f4872ea22a9c708c20447aa393d52c3cff Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 16 Jan 2018 18:08:05 -0800 Subject: [PATCH] fix TimeseriesLifecycle ordering behavior --- .../TimeseriesLifecycleType.java | 16 +++- .../TimeseriesLifecycleTypeTests.java | 76 +++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleType.java index 3b4690cb3d6..78ad57a851a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleType.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleType.java @@ -148,7 +148,9 @@ public class TimeseriesLifecycleType implements LifecycleType { .map(a -> actions.getOrDefault(a, null)) .filter(Objects::nonNull).findFirst().orElse(null); } else if (action instanceof ForceMergeAction) { - return replicasAction; + if (replicaActionFirst == false) { + return replicasAction; + } } return null; }; @@ -156,17 +158,23 @@ public class TimeseriesLifecycleType implements LifecycleType { return (action) -> { ReplicasAction replicasAction = (ReplicasAction) actions.getOrDefault(ReplicasAction.NAME, null); LifecycleAction allocateAction = actions.getOrDefault(AllocateAction.NAME, null); + boolean replicaActionFirst = replicasAction != null + && replicasAction.getNumberOfReplicas() <= context.getNumberOfReplicas(); if (action == null) { - if (replicasAction != null && replicasAction.getNumberOfReplicas() <= context.getNumberOfReplicas()) { + if (replicaActionFirst) { return replicasAction; } else if (allocateAction != null) { return allocateAction; } return replicasAction; } else if (action instanceof ReplicasAction) { - return allocateAction; + if (replicaActionFirst) { + return allocateAction; + } } else if (action instanceof AllocateAction) { - return replicasAction; + if (replicaActionFirst == false) { + return replicasAction; + } } return null; }; diff --git a/x-pack/plugin/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleTypeTests.java b/x-pack/plugin/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleTypeTests.java index 3ea7447c1a2..5a30c82f20a 100644 --- a/x-pack/plugin/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleTypeTests.java +++ b/x-pack/plugin/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleTypeTests.java @@ -192,6 +192,38 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase { assertNull(provider.next(TEST_ROLLOVER_ACTION)); } + public void testWarmActionProviderWithAllActionsAndReplicasFirst() { + String indexName = randomAlphaOfLengthBetween(1, 10); + Map actions = VALID_WARM_ACTIONS + .stream().map(this::getTestAction).collect(Collectors.toMap(LifecycleAction::getWriteableName, Function.identity())); + actions.put(ReplicasAction.NAME, TEST_REPLICAS_ACTION); + Phase warmPhase = new Phase("warm", TimeValue.ZERO, actions); + MockIndexLifecycleContext context =new MockIndexLifecycleContext(indexName, "", "", + TEST_REPLICAS_ACTION.getNumberOfReplicas() + 1) { + + @Override + public boolean canExecute(Phase phase) { + assertSame(warmPhase, phase); + return true; + } + }; + TimeseriesLifecycleType policy = TimeseriesLifecycleType.INSTANCE; + LifecyclePolicy.NextActionProvider provider = policy.getActionProvider(context, warmPhase); + if (actions.size() > 1) { + int actionCount = 1; + LifecycleAction current = provider.next(null); + assertThat(current, equalTo(TEST_REPLICAS_ACTION)); + while (actionCount++ < actions.size()) { + current = provider.next(current); + } + assertNull(provider.next(current)); + assertThat(current, equalTo(TEST_FORCE_MERGE_ACTION)); + } else { + assertThat(provider.next(null), equalTo(TEST_REPLICAS_ACTION)); + } + + } + public void testWarmActionProviderReplicasActionSortOrder() { String indexName = randomAlphaOfLengthBetween(1, 10); Map actions = randomSubsetOf(VALID_WARM_ACTIONS) @@ -234,6 +266,50 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase { } } + public void testColdActionProviderAllActions() { + String indexName = randomAlphaOfLengthBetween(1, 10); + Map actions = VALID_COLD_ACTIONS + .stream().map(this::getTestAction).collect(Collectors.toMap(LifecycleAction::getWriteableName, Function.identity())); + actions.put(ReplicasAction.NAME, TEST_REPLICAS_ACTION); + Phase coldPhase = new Phase("cold", TimeValue.ZERO, actions); + MockIndexLifecycleContext context =new MockIndexLifecycleContext(indexName, "", "", + TEST_REPLICAS_ACTION.getNumberOfReplicas() - 1) { + + @Override + public boolean canExecute(Phase phase) { + assertSame(coldPhase, phase); + return true; + } + }; + TimeseriesLifecycleType policy = TimeseriesLifecycleType.INSTANCE; + LifecyclePolicy.NextActionProvider provider = policy.getActionProvider(context, coldPhase); + if (actions.size() > 1) { + LifecycleAction current = provider.next(null); + assertThat(current, equalTo(TEST_ALLOCATE_ACTION)); + assertThat(provider.next(current), equalTo(TEST_REPLICAS_ACTION)); + } else { + assertThat(provider.next(null), equalTo(TEST_REPLICAS_ACTION)); + } + + context = new MockIndexLifecycleContext(indexName, "", "", + TEST_REPLICAS_ACTION.getNumberOfReplicas() + 1) { + + @Override + public boolean canExecute(Phase phase) { + assertSame(coldPhase, phase); + return true; + } + }; + provider = policy.getActionProvider(context, coldPhase); + if (actions.size() > 1) { + LifecycleAction current = provider.next(null); + assertThat(current, equalTo(TEST_REPLICAS_ACTION)); + assertThat(provider.next(current), equalTo(TEST_ALLOCATE_ACTION)); + } else { + assertThat(provider.next(null), equalTo(TEST_REPLICAS_ACTION)); + } + } + public void testColdActionProviderReplicasActionSortOrder() { String indexName = randomAlphaOfLengthBetween(1, 10); Map actions = randomSubsetOf(VALID_COLD_ACTIONS)