Skip to content

Commit

Permalink
Skip Shrink when numberOfShards not changed (#37953)
Browse files Browse the repository at this point in the history
Previously, ShrinkAction would fail if
it was executed on an index that had
the same number of shards as the target
shrunken number.

This PR introduced a new BranchingStep that
is used inside of ShrinkAction to branch which
step to move to next, depending on the
shard values. So no shrink will occur if the
shard count is unchanged.
  • Loading branch information
talevy authored Jan 30, 2019
1 parent b88bdfe commit 7c738fd
Show file tree
Hide file tree
Showing 9 changed files with 346 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ public void testRetryPolicy() throws Exception {
{
Map<String, Phase> phases = new HashMap<>();
Map<String, LifecycleAction> warmActions = new HashMap<>();
warmActions.put(ShrinkAction.NAME, new ShrinkAction(1));
warmActions.put(ShrinkAction.NAME, new ShrinkAction(3));
phases.put("warm", new Phase("warm", TimeValue.ZERO, warmActions));

LifecyclePolicy policy = new LifecyclePolicy("my_policy",
Expand All @@ -602,7 +602,7 @@ public void testRetryPolicy() throws Exception {

CreateIndexRequest createIndexRequest = new CreateIndexRequest("my_index")
.settings(Settings.builder()
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 2)
.put("index.lifecycle.name", "my_policy")
.build());
client.indices().create(createIndexRequest, RequestOptions.DEFAULT);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.core.indexlifecycle;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.index.Index;

import java.util.Objects;
import java.util.function.BiPredicate;

/**
* This step changes its {@link #getNextStepKey()} depending on the
* outcome of a defined predicate. It performs no changes to the
* cluster state.
*/
public class BranchingStep extends ClusterStateActionStep {
public static final String NAME = "branch";

private static final Logger logger = LogManager.getLogger(BranchingStep.class);

private StepKey nextStepKeyOnFalse;
private StepKey nextStepKeyOnTrue;
private BiPredicate<Index, ClusterState> predicate;
private SetOnce<Boolean> predicateValue;

/**
* {@link BranchingStep} is a step whose next step is based on
* the return value of a specific predicate.
*
* @param key the step's key
* @param nextStepKeyOnFalse the key of the step to run if predicate returns false
* @param nextStepKeyOnTrue the key of the step to run if predicate returns true
* @param predicate the condition to check when deciding which step to run next
*/
public BranchingStep(StepKey key, StepKey nextStepKeyOnFalse, StepKey nextStepKeyOnTrue, BiPredicate<Index, ClusterState> predicate) {
// super.nextStepKey is set to null since it is not used by this step
super(key, null);
this.nextStepKeyOnFalse = nextStepKeyOnFalse;
this.nextStepKeyOnTrue = nextStepKeyOnTrue;
this.predicate = predicate;
this.predicateValue = new SetOnce<>();
}

@Override
public ClusterState performAction(Index index, ClusterState clusterState) {
IndexMetaData indexMetaData = clusterState.metaData().index(index);
if (indexMetaData == null) {
// Index must have been since deleted, ignore it
logger.debug("[{}] lifecycle action for index [{}] executed but index no longer exists", getKey().getAction(), index.getName());
return clusterState;
}
predicateValue.set(predicate.test(index, clusterState));
return clusterState;
}

/**
* This method returns the next step to execute based on the predicate. If
* the predicate returned true, then nextStepKeyOnTrue is the key of the
* next step to run, otherwise nextStepKeyOnFalse is.
*
* throws {@link UnsupportedOperationException} if performAction was not called yet
*
* @return next step to execute
*/
@Override
public final StepKey getNextStepKey() {
if (predicateValue.get() == null) {
throw new IllegalStateException("Cannot call getNextStepKey before performAction");
}
return predicateValue.get() ? nextStepKeyOnTrue : nextStepKeyOnFalse;
}

/**
* @return the next step if {@code predicate} is false
*/
final StepKey getNextStepKeyOnFalse() {
return nextStepKeyOnFalse;
}

/**
* @return the next step if {@code predicate} is true
*/
final StepKey getNextStepKeyOnTrue() {
return nextStepKeyOnTrue;
}

public final BiPredicate<Index, ClusterState> getPredicate() {
return predicate;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
BranchingStep that = (BranchingStep) o;
return super.equals(o)
&& Objects.equals(nextStepKeyOnFalse, that.nextStepKeyOnFalse)
&& Objects.equals(nextStepKeyOnTrue, that.nextStepKeyOnTrue);
}

@Override
public int hashCode() {
return Objects.hash(super.hashCode(), nextStepKeyOnFalse, nextStepKeyOnTrue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public boolean isSafeAction() {
public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey) {
Settings readOnlySettings = Settings.builder().put(IndexMetaData.SETTING_BLOCKS_WRITE, true).build();

StepKey branchingKey = new StepKey(phase, NAME, BranchingStep.NAME);
StepKey readOnlyKey = new StepKey(phase, NAME, ReadOnlyAction.NAME);
StepKey setSingleNodeKey = new StepKey(phase, NAME, SetSingleNodeAllocateStep.NAME);
StepKey allocationRoutedKey = new StepKey(phase, NAME, CheckShrinkReadyStep.NAME);
Expand All @@ -94,6 +95,8 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
StepKey aliasKey = new StepKey(phase, NAME, ShrinkSetAliasStep.NAME);
StepKey isShrunkIndexKey = new StepKey(phase, NAME, ShrunkenIndexCheckStep.NAME);

BranchingStep conditionalSkipShrinkStep = new BranchingStep(branchingKey, readOnlyKey, nextStepKey,
(index, clusterState) -> clusterState.getMetaData().index(index).getNumberOfShards() == numberOfShards);
UpdateSettingsStep readOnlyStep = new UpdateSettingsStep(readOnlyKey, setSingleNodeKey, client, readOnlySettings);
SetSingleNodeAllocateStep setSingleNodeStep = new SetSingleNodeAllocateStep(setSingleNodeKey, allocationRoutedKey, client);
CheckShrinkReadyStep checkShrinkReadyStep = new CheckShrinkReadyStep(allocationRoutedKey, shrinkKey);
Expand All @@ -102,12 +105,13 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
CopyExecutionStateStep copyMetadata = new CopyExecutionStateStep(copyMetadataKey, aliasKey, SHRUNKEN_INDEX_PREFIX);
ShrinkSetAliasStep aliasSwapAndDelete = new ShrinkSetAliasStep(aliasKey, isShrunkIndexKey, client, SHRUNKEN_INDEX_PREFIX);
ShrunkenIndexCheckStep waitOnShrinkTakeover = new ShrunkenIndexCheckStep(isShrunkIndexKey, nextStepKey, SHRUNKEN_INDEX_PREFIX);
return Arrays.asList(readOnlyStep, setSingleNodeStep, checkShrinkReadyStep, shrink, allocated, copyMetadata,
aliasSwapAndDelete, waitOnShrinkTakeover);
return Arrays.asList(conditionalSkipShrinkStep, readOnlyStep, setSingleNodeStep, checkShrinkReadyStep, shrink, allocated,
copyMetadata, aliasSwapAndDelete, waitOnShrinkTakeover);
}

@Override
public List<StepKey> toStepKeys(String phase) {
StepKey conditionalSkipKey = new StepKey(phase, NAME, BranchingStep.NAME);
StepKey readOnlyKey = new StepKey(phase, NAME, ReadOnlyAction.NAME);
StepKey setSingleNodeKey = new StepKey(phase, NAME, SetSingleNodeAllocateStep.NAME);
StepKey checkShrinkReadyKey = new StepKey(phase, NAME, CheckShrinkReadyStep.NAME);
Expand All @@ -116,7 +120,7 @@ public List<StepKey> toStepKeys(String phase) {
StepKey copyMetadataKey = new StepKey(phase, NAME, CopyExecutionStateStep.NAME);
StepKey aliasKey = new StepKey(phase, NAME, ShrinkSetAliasStep.NAME);
StepKey isShrunkIndexKey = new StepKey(phase, NAME, ShrunkenIndexCheckStep.NAME);
return Arrays.asList(readOnlyKey, setSingleNodeKey, checkShrinkReadyKey, shrinkKey, enoughShardsKey,
return Arrays.asList(conditionalSkipKey, readOnlyKey, setSingleNodeKey, checkShrinkReadyKey, shrinkKey, enoughShardsKey,
copyMetadataKey, aliasKey, isShrunkIndexKey);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public final StepKey getKey() {
return key;
}

public final StepKey getNextStepKey() {
public StepKey getNextStepKey() {
return nextStepKey;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.core.indexlifecycle;

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.index.Index;
import org.elasticsearch.xpack.core.indexlifecycle.Step.StepKey;

import java.util.function.BiPredicate;

import static org.hamcrest.Matchers.equalTo;

public class BranchingStepTests extends AbstractStepTestCase<BranchingStep> {

public void testPredicateNextStepChange() {
String indexName = randomAlphaOfLength(5);
ClusterState state = ClusterState.builder(ClusterName.DEFAULT).metaData(MetaData.builder()
.put(IndexMetaData.builder(indexName).settings(settings(Version.CURRENT))
.numberOfShards(1).numberOfReplicas(0))).build();
StepKey stepKey = new StepKey(randomAlphaOfLength(5), randomAlphaOfLength(5), BranchingStep.NAME);
StepKey nextStepKey = new StepKey(randomAlphaOfLength(6), randomAlphaOfLength(6), BranchingStep.NAME);
StepKey nextSkipKey = new StepKey(randomAlphaOfLength(7), randomAlphaOfLength(7), BranchingStep.NAME);
{
BranchingStep step = new BranchingStep(stepKey, nextStepKey, nextSkipKey, (i, c) -> true);
expectThrows(IllegalStateException.class, step::getNextStepKey);
step.performAction(state.metaData().index(indexName).getIndex(), state);
assertThat(step.getNextStepKey(), equalTo(step.getNextStepKeyOnTrue()));
expectThrows(SetOnce.AlreadySetException.class, () -> step.performAction(state.metaData().index(indexName).getIndex(), state));
}
{
BranchingStep step = new BranchingStep(stepKey, nextStepKey, nextSkipKey, (i, c) -> false);
expectThrows(IllegalStateException.class, step::getNextStepKey);
step.performAction(state.metaData().index(indexName).getIndex(), state);
assertThat(step.getNextStepKey(), equalTo(step.getNextStepKeyOnFalse()));
expectThrows(SetOnce.AlreadySetException.class, () -> step.performAction(state.metaData().index(indexName).getIndex(), state));
}
}

@Override
public BranchingStep createRandomInstance() {
StepKey stepKey = new StepKey(randomAlphaOfLength(5), randomAlphaOfLength(5), BranchingStep.NAME);
StepKey nextStepKey = new StepKey(randomAlphaOfLength(6), randomAlphaOfLength(6), BranchingStep.NAME);
StepKey nextSkipKey = new StepKey(randomAlphaOfLength(7), randomAlphaOfLength(7), BranchingStep.NAME);
return new BranchingStep(stepKey, nextStepKey, nextSkipKey, (i, c) -> randomBoolean());
}

@Override
public BranchingStep mutateInstance(BranchingStep instance) {
StepKey key = instance.getKey();
StepKey nextStepKey = instance.getNextStepKeyOnFalse();
StepKey nextSkipStepKey = instance.getNextStepKeyOnTrue();
BiPredicate<Index, ClusterState> predicate = instance.getPredicate();

switch (between(0, 2)) {
case 0:
key = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5));
break;
case 1:
nextStepKey = new StepKey(nextStepKey.getPhase(), nextStepKey.getAction(), nextStepKey.getName() + randomAlphaOfLength(5));
break;
case 2:
nextSkipStepKey = new StepKey(nextSkipStepKey.getPhase(), nextSkipStepKey.getAction(),
nextSkipStepKey.getName() + randomAlphaOfLength(5));
break;
default:
throw new AssertionError("Illegal randomisation branch");
}

return new BranchingStep(key, nextStepKey, nextSkipStepKey, predicate);
}

@Override
public BranchingStep copyInstance(BranchingStep instance) {
return new BranchingStep(instance.getKey(), instance.getNextStepKeyOnFalse(), instance.getNextStepKeyOnTrue(),
instance.getPredicate());
}
}
Loading

0 comments on commit 7c738fd

Please sign in to comment.