Encapsulate Client as class variable for PolicyStepsRegistry (#33529)

Rather than pass in the client on the `update` step, this makes it passed in to
the constructor so it's not required on every update.
This commit is contained in:
Lee Hinman 2018-09-07 16:32:25 -06:00 committed by GitHub
parent e0b99d7ab3
commit 8fa8dea138
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 36 additions and 33 deletions

View File

@ -66,7 +66,7 @@ public class IndexLifecycleService extends AbstractComponent
this.clock = clock;
this.nowSupplier = nowSupplier;
this.scheduledJob = null;
this.policyRegistry = new PolicyStepsRegistry(xContentRegistry);
this.policyRegistry = new PolicyStepsRegistry(xContentRegistry, client);
this.lifecycleRunner = new IndexLifecycleRunner(policyRegistry, clusterService, nowSupplier);
this.pollInterval = LifecycleSettings.LIFECYCLE_POLL_INTERVAL_SETTING.get(settings);
clusterService.addStateApplier(this);
@ -146,7 +146,7 @@ public class IndexLifecycleService extends AbstractComponent
policyRegistry.removeIndices(event.indicesDeleted());
}
if (event.state().metaData().custom(IndexLifecycleMetadata.TYPE) != null) {
policyRegistry.update(event.state(), client);
policyRegistry.update(event.state());
}
}
}

View File

@ -45,6 +45,7 @@ import java.util.stream.Collectors;
public class PolicyStepsRegistry {
private static final Logger logger = LogManager.getLogger(PolicyStepsRegistry.class);
private final Client client;
// keeps track of existing policies in the cluster state
private final SortedMap<String, LifecyclePolicyMetadata> lifecyclePolicyMap;
// keeps track of what the first step in a policy is, the key is policy name
@ -55,22 +56,24 @@ public class PolicyStepsRegistry {
private final Map<Index, List<Step>> indexPhaseSteps;
private final NamedXContentRegistry xContentRegistry;
public PolicyStepsRegistry(NamedXContentRegistry xContentRegistry) {
public PolicyStepsRegistry(NamedXContentRegistry xContentRegistry, Client client) {
this.lifecyclePolicyMap = new TreeMap<>();
this.firstStepMap = new HashMap<>();
this.stepMap = new HashMap<>();
this.indexPhaseSteps = new HashMap<>();
this.xContentRegistry = xContentRegistry;
this.client = client;
}
PolicyStepsRegistry(SortedMap<String, LifecyclePolicyMetadata> lifecyclePolicyMap,
Map<String, Step> firstStepMap, Map<String, Map<Step.StepKey, Step>> stepMap,
Map<Index, List<Step>> indexPhaseSteps, NamedXContentRegistry xContentRegistry) {
Map<Index, List<Step>> indexPhaseSteps, NamedXContentRegistry xContentRegistry, Client client) {
this.lifecyclePolicyMap = lifecyclePolicyMap;
this.firstStepMap = firstStepMap;
this.stepMap = stepMap;
this.indexPhaseSteps = indexPhaseSteps;
this.xContentRegistry = xContentRegistry;
this.client = client;
}
SortedMap<String, LifecyclePolicyMetadata> getLifecyclePolicyMap() {
@ -97,7 +100,7 @@ public class PolicyStepsRegistry {
}
@SuppressWarnings({ "unchecked", "rawtypes" })
public void update(ClusterState clusterState, Client client) {
public void update(ClusterState clusterState) {
final IndexLifecycleMetadata meta = clusterState.metaData().custom(IndexLifecycleMetadata.TYPE);
assert meta != null : "IndexLifecycleMetadata cannot be null when updating the policy steps registry";

View File

@ -100,7 +100,7 @@ public class ExecuteStepsUpdateTaskTests extends ESTestCase {
randomNonNegativeLong(), randomNonNegativeLong()));
policyMap.put(invalidPolicyName, new LifecyclePolicyMetadata(invalidPolicy, Collections.emptyMap(),
randomNonNegativeLong(), randomNonNegativeLong()));
policyStepsRegistry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
policyStepsRegistry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
indexName = randomAlphaOfLength(5);
lifecycleMetadata = new IndexLifecycleMetadata(policyMap, OperationMode.RUNNING);
@ -130,7 +130,7 @@ public class ExecuteStepsUpdateTaskTests extends ESTestCase {
.metaData(metaData)
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
.build();
policyStepsRegistry.update(clusterState, client);
policyStepsRegistry.update(clusterState);
}
public void testNeverExecuteNonClusterStateStep() throws IOException {
@ -165,7 +165,7 @@ public class ExecuteStepsUpdateTaskTests extends ESTestCase {
.put(LifecycleSettings.LIFECYCLE_PHASE, (String) null)
.put(LifecycleSettings.LIFECYCLE_ACTION, (String) null)
.put(LifecycleSettings.LIFECYCLE_STEP, (String) null).build()))).build();
policyStepsRegistry.update(clusterState, client);
policyStepsRegistry.update(clusterState);
Step invalidStep = new MockClusterStateActionStep(firstStepKey, secondStepKey);
long now = randomNonNegativeLong();
@ -230,6 +230,6 @@ public class ExecuteStepsUpdateTaskTests extends ESTestCase {
.put(LifecycleSettings.LIFECYCLE_PHASE, stepKey.getPhase())
.put(LifecycleSettings.LIFECYCLE_ACTION, stepKey.getAction())
.put(LifecycleSettings.LIFECYCLE_STEP, stepKey.getName()).build()))).build();
policyStepsRegistry.update(clusterState, client);
policyStepsRegistry.update(clusterState);
}
}

View File

@ -82,7 +82,7 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
steps.add(step);
Index index = new Index(indexName, indexName + "uuid");
indexSteps.put(index, steps);
return new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY);
return new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
}
public void testRunPolicyTerminalPolicyStep() {
@ -344,7 +344,7 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
public void testRunPolicyWithNoStepsInRegistry() {
String policyName = "cluster_state_action_policy";
ClusterService clusterService = mock(ClusterService.class);
IndexLifecycleRunner runner = new IndexLifecycleRunner(new PolicyStepsRegistry(NamedXContentRegistry.EMPTY),
IndexLifecycleRunner runner = new IndexLifecycleRunner(new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, null),
clusterService, () -> 0L);
IndexMetaData indexMetaData = IndexMetaData.builder("my_index").settings(settings(Version.CURRENT))
.numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
@ -472,7 +472,7 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
Index index = new Index("test", "uuid");
indexSteps.put(index, phase1Steps);
PolicyStepsRegistry registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps,
NamedXContentRegistry.EMPTY);
NamedXContentRegistry.EMPTY, null);
Settings indexSettings = Settings.EMPTY;
Step actualStep = IndexLifecycleRunner.getCurrentStep(registry, policyName, index, indexSettings);
@ -506,7 +506,7 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
// TODO: it'd be nice if we used the actual registry.update method for this
indexSteps.clear();
indexSteps.put(index, Collections.singletonList(fourthStep));
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY);
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
indexSettings = Settings.builder()
.put(LifecycleSettings.LIFECYCLE_PHASE, "phase_2")
@ -527,7 +527,7 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
// Back to phase_1
indexSteps.clear();
indexSteps.put(index, phase1Steps);
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY);
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
indexSettings = Settings.builder()
.put(LifecycleSettings.LIFECYCLE_PHASE, "phase_1")
@ -1080,7 +1080,7 @@ public class IndexLifecycleRunnerTests extends ESTestCase {
Map<String, Map<StepKey, Step>> stepMap = Collections.singletonMap(policyName, policySteps);
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(step));
PolicyStepsRegistry policyStepsRegistry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap,
stepMap, indexSteps, NamedXContentRegistry.EMPTY);
stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
ClusterService clusterService = mock(ClusterService.class);
final AtomicLong now = new AtomicLong(5);
IndexLifecycleRunner runner = new IndexLifecycleRunner(policyStepsRegistry, clusterService, now::get);

View File

@ -54,7 +54,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
String policyName = randomAlphaOfLengthBetween(2, 10);
Step expectedFirstStep = new MockStep(MOCK_STEP_KEY, null);
Map<String, Step> firstStepMap = Collections.singletonMap(policyName, expectedFirstStep);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY, null);
Step actualFirstStep = registry.getFirstStep(policyName);
assertThat(actualFirstStep, sameInstance(expectedFirstStep));
}
@ -63,7 +63,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
String policyName = randomAlphaOfLengthBetween(2, 10);
Step expectedFirstStep = new MockStep(MOCK_STEP_KEY, null);
Map<String, Step> firstStepMap = Collections.singletonMap(policyName, expectedFirstStep);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY, null);
Step actualFirstStep = registry.getFirstStep(policyName + "unknown");
assertNull(actualFirstStep);
}
@ -72,7 +72,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
Step expectedStep = new MockStep(MOCK_STEP_KEY, null);
Index index = new Index("test", "uuid");
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(expectedStep));
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY, null);
Step actualStep = registry.getStep(index, MOCK_STEP_KEY);
assertThat(actualStep, sameInstance(expectedStep));
}
@ -82,13 +82,13 @@ public class PolicyStepsRegistryTests extends ESTestCase {
Step expectedStep = new ErrorStep(errorStepKey);
Index index = new Index("test", "uuid");
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(expectedStep));
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY, null);
Step actualStep = registry.getStep(index, errorStepKey);
assertThat(actualStep, equalTo(expectedStep));
}
public void testGetStepUnknownPolicy() {
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, Collections.emptyMap(), NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, Collections.emptyMap(), NamedXContentRegistry.EMPTY, null);
assertNull(registry.getStep(new Index("test", "uuid"), MOCK_STEP_KEY));
}
@ -96,7 +96,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
Step expectedStep = new MockStep(MOCK_STEP_KEY, null);
Index index = new Index("test", "uuid");
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(expectedStep));
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY, null);
Step.StepKey unknownStepKey = new Step.StepKey(MOCK_STEP_KEY.getPhase(),
MOCK_STEP_KEY.getAction(),MOCK_STEP_KEY.getName() + "not");
assertNull(registry.getStep(index, unknownStepKey));
@ -146,10 +146,10 @@ public class PolicyStepsRegistryTests extends ESTestCase {
.build();
// start with empty registry
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
// add new policy
registry.update(currentState, client);
registry.update(currentState);
assertThat(registry.getFirstStep(newPolicy.getName()), equalTo(policySteps.get(0)));
assertThat(registry.getLifecyclePolicyMap().size(), equalTo(1));
@ -167,7 +167,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
.put(LifecycleSettings.LIFECYCLE_PHASE, step.getKey().getPhase()))))
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
.build();
registry.update(currentState, client);
registry.update(currentState);
assertThat(registeredStepsForPolicy.get(step.getKey()), equalTo(step));
assertThat(registry.getStep(index, step.getKey()), equalTo(step));
}
@ -175,7 +175,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
Map<String, LifecyclePolicyMetadata> registryPolicyMap = registry.getLifecyclePolicyMap();
Map<String, Step> registryFirstStepMap = registry.getFirstStepMap();
Map<String, Map<Step.StepKey, Step>> registryStepMap = registry.getStepMap();
registry.update(currentState, client);
registry.update(currentState);
assertThat(registry.getLifecyclePolicyMap(), equalTo(registryPolicyMap));
assertThat(registry.getFirstStepMap(), equalTo(registryFirstStepMap));
assertThat(registry.getStepMap(), equalTo(registryStepMap));
@ -186,13 +186,13 @@ public class PolicyStepsRegistryTests extends ESTestCase {
.metaData(
MetaData.builder(metaData)
.putCustom(IndexLifecycleMetadata.TYPE, lifecycleMetadata)).build();
registry.update(currentState, client);
registry.update(currentState);
assertTrue(registry.getLifecyclePolicyMap().isEmpty());
assertTrue(registry.getFirstStepMap().isEmpty());
assertTrue(registry.getStepMap().isEmpty());
}
public void testUpdateChangedPolicy() throws Exception {
public void testUpdateChangedPolicy() {
Client client = Mockito.mock(Client.class);
Mockito.when(client.settings()).thenReturn(Settings.EMPTY);
String policyName = randomAlphaOfLengthBetween(5, 10);
@ -217,9 +217,9 @@ public class PolicyStepsRegistryTests extends ESTestCase {
.metaData(metaData)
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
.build();
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
// add new policy
registry.update(currentState, client);
registry.update(currentState);
// swap out policy
newPolicy = LifecyclePolicyTests.randomTestLifecyclePolicy(policyName);
@ -228,7 +228,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
randomNonNegativeLong(), randomNonNegativeLong())), OperationMode.RUNNING);
currentState = ClusterState.builder(currentState)
.metaData(MetaData.builder(metaData).putCustom(IndexLifecycleMetadata.TYPE, lifecycleMetadata)).build();
registry.update(currentState, client);
registry.update(currentState);
// TODO(talevy): assert changes... right now we do not support updates to policies. will require internal cleanup
}
@ -287,10 +287,10 @@ public class PolicyStepsRegistryTests extends ESTestCase {
.build();
// start with empty registry
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
// add new policy
registry.update(currentState, client);
registry.update(currentState);
Map<Step.StepKey, Step> registeredStepsForPolicy = registry.getStepMap().get(newPolicy.getName());
Step shrinkStep = registeredStepsForPolicy.entrySet().stream()
@ -316,7 +316,7 @@ public class PolicyStepsRegistryTests extends ESTestCase {
currentState = ClusterState.builder(ClusterName.DEFAULT).metaData(metaData).build();
// Update the policies
registry.update(currentState, client);
registry.update(currentState);
registeredStepsForPolicy = registry.getStepMap().get(newPolicy.getName());
shrinkStep = registeredStepsForPolicy.entrySet().stream()