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

stop throwing an IllegalStateException for unrecognized steps (#32212) 

Since the reason for a step not being found in a registry may be due to staleness of the
registry between it and the cluster state, we do not want to throw an IllegalStateException.

Staleness is something that will be self-healing after follow-up applications of the cluster state
updates, so this is a recoverable issue that should log a warning instead of throwing an exception

Closes #32181.
This commit is contained in:
Tal Levy 2018-07-31 07:06:52 -07:00 committed by GitHub
parent a314efc920
commit 6e9f33843d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 71 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
x-pack/plugin/index-lifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunner.java
View File

@ -9,6 +9,7 @@ import com.carrotsearch.hppc.cursors.ObjectCursor;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
@ -64,8 +65,11 @@ public class IndexLifecycleRunner {
}
Step currentStep = getCurrentStep(stepRegistry, policy, indexSettings);
if (currentStep == null) {
throw new IllegalStateException(
"current step for index [" + indexMetaData.getIndex().getName() + "] with policy [" + policy + "] is not recognized");
// This may happen in the case that there is invalid ilm-step index settings or the stepRegistry is out of
// sync with the current cluster state
logger.warn("current step [" + getCurrentStepKey(indexSettings) + "] for index [" + indexMetaData.getIndex().getName()
+ "] with policy [" + policy + "] is not recognized");
return;
}
logger.debug("running policy with current-step[" + currentStep.getKey() + "]");
if (currentStep instanceof TerminalPolicyStep) {
@ -164,6 +168,23 @@ public class IndexLifecycleRunner {
}
}
/**
* This method is intended for handling moving to different steps from {@link TransportAction} executions.
* For this reason, it is reasonable to throw {@link IllegalArgumentException} when state is not as expected.
* @param indexName
* The index whose step is to change
* @param currentState
* The current {@link ClusterState}
* @param currentStepKey
* The current {@link StepKey} found for the index in the current cluster state
* @param nextStepKey
* The next step to move the index into
* @param nowSupplier
* The current-time supplier for updating when steps changed
* @param stepRegistry
* The steps registry to check a step-key's existence in the index's current policy
* @return The updated cluster state where the index moved to <code>nextStepKey</code>
*/
static ClusterState moveClusterStateToStep(String indexName, ClusterState currentState, StepKey currentStepKey,
StepKey nextStepKey, LongSupplier nowSupplier,
PolicyStepsRegistry stepRegistry) {
@ -180,10 +201,9 @@ public class IndexLifecycleRunner {
throw new IllegalArgumentException("index [" + indexName + "] is not on current step [" + currentStepKey + "]");
}
try {
stepRegistry.getStep(indexPolicySetting, nextStepKey);
} catch (IllegalStateException e) {
throw new IllegalArgumentException(e.getMessage());
Step nextStep = stepRegistry.getStep(indexPolicySetting, nextStepKey);
if (nextStep == null) {
throw new IllegalArgumentException("step [" + nextStepKey + "] with policy [" + indexPolicySetting + "] does not exist");
}
return IndexLifecycleRunner.moveClusterStateToNextStep(idxMeta.getIndex(), currentState, currentStepKey, nextStepKey, nowSupplier);

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

@ -5,10 +5,12 @@
*/
package org.elasticsearch.xpack.indexlifecycle;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.Diff;
import org.elasticsearch.cluster.DiffableUtils;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.elasticsearch.xpack.core.ClientHelper;
import org.elasticsearch.xpack.core.indexlifecycle.ErrorStep;
import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata;
@ -23,6 +25,8 @@ import java.util.TreeMap;
import java.util.function.LongSupplier;
public class PolicyStepsRegistry {
private static final Logger logger = ESLoggerFactory.getLogger(PolicyStepsRegistry.class);
// keeps track of existing policies in the cluster state
private SortedMap<String, LifecyclePolicyMetadata> lifecyclePolicyMap;
// keeps track of what the first step in a policy is
@ -104,13 +108,9 @@ public class PolicyStepsRegistry {
}
Map<Step.StepKey, Step> steps = stepMap.get(policy);
if (steps == null) {
throw new IllegalStateException("policy [" + policy + "] does not exist");
return null;
}
Step step = steps.get(stepKey);
if (step == null) {
throw new IllegalStateException("step [" + stepKey + "] does not exist");
}
return step;
return steps.get(stepKey);
}
public Step getFirstStep(String policy) {

28
x-pack/plugin/index-lifecycle/src/test/java/org/elasticsearch/xpack/indexlifecycle/ExecuteStepsUpdateTaskTests.java
View File

@ -52,10 +52,12 @@ public class ExecuteStepsUpdateTaskTests extends ESTestCase {
private static final StepKey firstStepKey = new StepKey("phase_1", "action_1", "step_1");
private static final StepKey secondStepKey = new StepKey("phase_1", "action_1", "step_2");
private static final StepKey thirdStepKey = new StepKey("phase_1", "action_1", "step_3");
private static final StepKey invalidStepKey = new StepKey("invalid", "invalid", "invalid");
private ClusterState clusterState;
private PolicyStepsRegistry policyStepsRegistry;
private String mixedPolicyName;
private String allClusterPolicyName;
private String invalidPolicyName;
private Index index;
private MockClusterStateActionStep firstStep;
private MockClusterStateWaitStep secondStep;
@ -75,17 +77,23 @@ public class ExecuteStepsUpdateTaskTests extends ESTestCase {
thirdStep = new MockStep(thirdStepKey, null);
mixedPolicyName = randomAlphaOfLengthBetween(5, 10);
allClusterPolicyName = randomAlphaOfLengthBetween(1, 4);
invalidPolicyName = randomAlphaOfLength(11);
Phase mixedPhase = new Phase("first_phase", TimeValue.ZERO, Collections.singletonMap(MockAction.NAME,
new MockAction(Arrays.asList(firstStep, secondStep, thirdStep))));
Phase allClusterPhase = new Phase("first_phase", TimeValue.ZERO, Collections.singletonMap(MockAction.NAME,
new MockAction(Arrays.asList(firstStep, allClusterSecondStep))));
Phase invalidPhase = new Phase("invalid_phase", TimeValue.ZERO, Collections.singletonMap(MockAction.NAME,
new MockAction(Arrays.asList(new MockClusterStateActionStep(firstStepKey, invalidStepKey)))));
LifecyclePolicy mixedPolicy = new LifecyclePolicy(TestLifecycleType.INSTANCE, mixedPolicyName,
Collections.singletonMap(mixedPhase.getName(), mixedPhase));
LifecyclePolicy allClusterPolicy = new LifecyclePolicy(TestLifecycleType.INSTANCE, allClusterPolicyName,
Collections.singletonMap(allClusterPhase.getName(), allClusterPhase));
LifecyclePolicy invalidPolicy = new LifecyclePolicy(TestLifecycleType.INSTANCE, invalidPolicyName,
Collections.singletonMap(invalidPhase.getName(), invalidPhase));
Map<String, LifecyclePolicyMetadata> policyMap = new HashMap<>();
policyMap.put(mixedPolicyName, new LifecyclePolicyMetadata(mixedPolicy, Collections.emptyMap()));
policyMap.put(allClusterPolicyName, new LifecyclePolicyMetadata(allClusterPolicy, Collections.emptyMap()));
policyMap.put(invalidPolicyName, new LifecyclePolicyMetadata(invalidPolicy, Collections.emptyMap()));
policyStepsRegistry = new PolicyStepsRegistry();
IndexMetaData indexMetadata = IndexMetaData.builder(randomAlphaOfLength(5))
@ -165,6 +173,26 @@ public class ExecuteStepsUpdateTaskTests extends ESTestCase {
assertThat(LifecycleSettings.LIFECYCLE_STEP_INFO_SETTING.get(newState.metaData().index(index).getSettings()), equalTo(""));
}
public void testExecuteInvalidStartStep() throws IOException {
setStateToKey(firstStepKey);
Step startStep = policyStepsRegistry.getStep(mixedPolicyName, firstStepKey);
long now = randomNonNegativeLong();
ExecuteStepsUpdateTask task = new ExecuteStepsUpdateTask(invalidPolicyName, index, startStep, policyStepsRegistry, () -> now);
ClusterState newState = task.execute(clusterState);
assertSame(newState, clusterState);
}
public void testExecuteUntilNullStep() throws IOException {
setStateToKey(firstStepKey);
Step startStep = policyStepsRegistry.getStep(invalidPolicyName, firstStepKey);
long now = randomNonNegativeLong();
ExecuteStepsUpdateTask task = new ExecuteStepsUpdateTask(invalidPolicyName, index, startStep, policyStepsRegistry, () -> now);
ClusterState newState = task.execute(clusterState);
StepKey currentStepKey = IndexLifecycleRunner.getCurrentStepKey(newState.metaData().index(index).getSettings());
assertThat(currentStepKey, equalTo(invalidStepKey));
}
public void testExecuteIncompleteWaitStepNoInfo() throws IOException {
secondStep.setWillComplete(false);
setStateToKey(secondStepKey);

21
x-pack/plugin/index-lifecycle/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunnerTests.java
View File

@ -332,18 +332,13 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
public void testRunPolicyWithNoStepsInRegistry() {
String policyName = "cluster_state_action_policy";
StepKey stepKey = new StepKey("phase", "action", "cluster_state_action_step");
ClusterService clusterService = mock(ClusterService.class);
IndexLifecycleRunner runner = new IndexLifecycleRunner(new PolicyStepsRegistry(), clusterService, () -> 0L);
IndexMetaData indexMetaData = IndexMetaData.builder("my_index").settings(settings(Version.CURRENT))
.numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
IllegalStateException exception = expectThrows(IllegalStateException.class,
() -> runner.runPolicy(policyName, indexMetaData, null, randomBoolean()));
assertEquals("current step for index [my_index] with policy [cluster_state_action_policy] is not recognized",
exception.getMessage());
// verify that no exception is thrown
runner.runPolicy(policyName, indexMetaData, null, randomBoolean());
Mockito.verifyZeroInteractions(clusterService);
}
public void testRunPolicyUnknownStepType() {
@ -524,13 +519,8 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
.put(LifecycleSettings.LIFECYCLE_ACTION, "action_1")
.put(LifecycleSettings.LIFECYCLE_STEP, "step_3")
.build();
IllegalStateException exception = expectThrows(IllegalStateException.class,
() -> IndexLifecycleRunner.getCurrentStep(registry, policyName, invalidIndexSettings));
assertEquals("step [{\"phase\":\"phase_1\",\"action\":\"action_1\",\"name\":\"step_3\"}] does not exist", exception.getMessage());
exception = expectThrows(IllegalStateException.class,
() -> IndexLifecycleRunner.getCurrentStep(registry, "policy_does_not_exist", invalidIndexSettings));
assertEquals("policy [policy_does_not_exist] does not exist", exception.getMessage());
assertNull(IndexLifecycleRunner.getCurrentStep(registry, policyName, invalidIndexSettings));
assertNull(IndexLifecycleRunner.getCurrentStep(registry, "policy_does_not_exist", invalidIndexSettings));
}
public void testMoveClusterStateToNextStep() {
@ -688,7 +678,8 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
() -> IndexLifecycleRunner.moveClusterStateToStep(indexName, clusterState, currentStepKey,
nextStepKey, () -> now, stepRegistry));
assertThat(exception.getMessage(),
equalTo("step [{\"phase\":\"next_phase\",\"action\":\"next_action\",\"name\":\"next_step\"}] does not exist"));
equalTo("step [{\"phase\":\"next_phase\",\"action\":\"next_action\",\"name\":\"next_step\"}] " +
"with policy [my_policy] does not exist"));
}
public void testMoveClusterStateToErrorStep() throws IOException {

6
x-pack/plugin/index-lifecycle/src/test/java/org/elasticsearch/xpack/indexlifecycle/PolicyStepsRegistryTests.java
View File

@ -79,8 +79,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
public void testGetStepUnknownPolicy() {
String policyName = randomAlphaOfLengthBetween(2, 10);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, Collections.emptyMap());
IllegalStateException exception = expectThrows(IllegalStateException.class, () -> registry.getStep(policyName, MOCK_STEP_KEY));
assertThat(exception.getMessage(), equalTo("policy [" + policyName +"] does not exist"));
assertNull(registry.getStep(policyName, MOCK_STEP_KEY));
}
public void testGetStepUnknownStepKey() {
@ -91,8 +90,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, stepMap);
Step.StepKey unknownStepKey = new Step.StepKey(MOCK_STEP_KEY.getPhase(),
MOCK_STEP_KEY.getAction(),MOCK_STEP_KEY.getName() + "not");
IllegalStateException exception = expectThrows(IllegalStateException.class, () -> registry.getStep(policyName, unknownStepKey));
assertThat(exception.getMessage(), equalTo("step [" + unknownStepKey +"] does not exist"));
assertNull(registry.getStep(policyName, unknownStepKey));
}
public void testUpdateFromNothingToSomethingToNothing() {

2
x-pack/plugin/src/test/resources/rest-api-spec/test/index_lifecycle/20_move_to_step.yml
View File

@ -138,7 +138,7 @@ teardown:
action: "invalid"
name: "invalid"
- match: { error.root_cause.0.type: "illegal_argument_exception" }
- match: { error.root_cause.0.reason: "step [{\"phase\":\"invalid\",\"action\":\"invalid\",\"name\":\"invalid\"}] does not exist" }
- match: { error.root_cause.0.reason: "step [{\"phase\":\"invalid\",\"action\":\"invalid\",\"name\":\"invalid\"}] with policy [my_moveable_timeseries_lifecycle] does not exist" }
- do:
acknowledge: true