Skip to content

Commit 8fa8dea

Browse files
dakronedakrone
authored
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.
1 parent e0b99d7 commit 8fa8dea

File tree

5 files changed

+36
-33
lines changed

5 files changed

+36
-33
lines changed

‎x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public IndexLifecycleService(Settings settings, Client client, ClusterService cl
6666
this.clock = clock;
6767
this.nowSupplier = nowSupplier;
6868
this.scheduledJob = null;
69-
this.policyRegistry = new PolicyStepsRegistry(xContentRegistry);
69+
this.policyRegistry = new PolicyStepsRegistry(xContentRegistry, client);
7070
this.lifecycleRunner = new IndexLifecycleRunner(policyRegistry, clusterService, nowSupplier);
7171
this.pollInterval = LifecycleSettings.LIFECYCLE_POLL_INTERVAL_SETTING.get(settings);
7272
clusterService.addStateApplier(this);
@@ -146,7 +146,7 @@ public void applyClusterState(ClusterChangedEvent event) {
146146
policyRegistry.removeIndices(event.indicesDeleted());
147147
}
148148
if (event.state().metaData().custom(IndexLifecycleMetadata.TYPE) != null) {
149-
policyRegistry.update(event.state(), client);
149+
policyRegistry.update(event.state());
150150
}
151151
}
152152
}

‎x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/PolicyStepsRegistry.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
public class PolicyStepsRegistry {
4646
private static final Logger logger = LogManager.getLogger(PolicyStepsRegistry.class);
4747

48+
private final Client client;
4849
// keeps track of existing policies in the cluster state
4950
private final SortedMap<String, LifecyclePolicyMetadata> lifecyclePolicyMap;
5051
// keeps track of what the first step in a policy is, the key is policy name
@@ -55,22 +56,24 @@ public class PolicyStepsRegistry {
5556
private final Map<Index, List<Step>> indexPhaseSteps;
5657
private final NamedXContentRegistry xContentRegistry;
5758

58-
public PolicyStepsRegistry(NamedXContentRegistry xContentRegistry) {
59+
public PolicyStepsRegistry(NamedXContentRegistry xContentRegistry, Client client) {
5960
this.lifecyclePolicyMap = new TreeMap<>();
6061
this.firstStepMap = new HashMap<>();
6162
this.stepMap = new HashMap<>();
6263
this.indexPhaseSteps = new HashMap<>();
6364
this.xContentRegistry = xContentRegistry;
65+
this.client = client;
6466
}
6567

6668
PolicyStepsRegistry(SortedMap<String, LifecyclePolicyMetadata> lifecyclePolicyMap,
6769
Map<String, Step> firstStepMap, Map<String, Map<Step.StepKey, Step>> stepMap,
68-
Map<Index, List<Step>> indexPhaseSteps, NamedXContentRegistry xContentRegistry) {
70+
Map<Index, List<Step>> indexPhaseSteps, NamedXContentRegistry xContentRegistry, Client client) {
6971
this.lifecyclePolicyMap = lifecyclePolicyMap;
7072
this.firstStepMap = firstStepMap;
7173
this.stepMap = stepMap;
7274
this.indexPhaseSteps = indexPhaseSteps;
7375
this.xContentRegistry = xContentRegistry;
76+
this.client = client;
7477
}
7578

7679
SortedMap<String, LifecyclePolicyMetadata> getLifecyclePolicyMap() {
@@ -97,7 +100,7 @@ public void removeIndices(List<Index> indices) {
97100
}
98101

99102
@SuppressWarnings({ "unchecked", "rawtypes" })
100-
public void update(ClusterState clusterState, Client client) {
103+
public void update(ClusterState clusterState) {
101104
final IndexLifecycleMetadata meta = clusterState.metaData().custom(IndexLifecycleMetadata.TYPE);
102105

103106
assert meta != null : "IndexLifecycleMetadata cannot be null when updating the policy steps registry";

‎x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/ExecuteStepsUpdateTaskTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public void prepareState() throws IOException {
100100
randomNonNegativeLong(), randomNonNegativeLong()));
101101
policyMap.put(invalidPolicyName, new LifecyclePolicyMetadata(invalidPolicy, Collections.emptyMap(),
102102
randomNonNegativeLong(), randomNonNegativeLong()));
103-
policyStepsRegistry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
103+
policyStepsRegistry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
104104

105105
indexName = randomAlphaOfLength(5);
106106
lifecycleMetadata = new IndexLifecycleMetadata(policyMap, OperationMode.RUNNING);
@@ -130,7 +130,7 @@ private void setupIndexPolicy(String policyName) {
130130
.metaData(metaData)
131131
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
132132
.build();
133-
policyStepsRegistry.update(clusterState, client);
133+
policyStepsRegistry.update(clusterState);
134134
}
135135

136136
public void testNeverExecuteNonClusterStateStep() throws IOException {
@@ -165,7 +165,7 @@ public void testExecuteInvalidStartStep() throws IOException {
165165
.put(LifecycleSettings.LIFECYCLE_PHASE, (String) null)
166166
.put(LifecycleSettings.LIFECYCLE_ACTION, (String) null)
167167
.put(LifecycleSettings.LIFECYCLE_STEP, (String) null).build()))).build();
168-
policyStepsRegistry.update(clusterState, client);
168+
policyStepsRegistry.update(clusterState);
169169

170170
Step invalidStep = new MockClusterStateActionStep(firstStepKey, secondStepKey);
171171
long now = randomNonNegativeLong();
@@ -230,6 +230,6 @@ private void setStateToKey(StepKey stepKey) throws IOException {
230230
.put(LifecycleSettings.LIFECYCLE_PHASE, stepKey.getPhase())
231231
.put(LifecycleSettings.LIFECYCLE_ACTION, stepKey.getAction())
232232
.put(LifecycleSettings.LIFECYCLE_STEP, stepKey.getName()).build()))).build();
233-
policyStepsRegistry.update(clusterState, client);
233+
policyStepsRegistry.update(clusterState);
234234
}
235235
}

‎x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunnerTests.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ private PolicyStepsRegistry createOneStepPolicyStepRegistry(String policyName, S
8282
steps.add(step);
8383
Index index = new Index(indexName, indexName + "uuid");
8484
indexSteps.put(index, steps);
85-
return new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY);
85+
return new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
8686
}
8787

8888
public void testRunPolicyTerminalPolicyStep() {
@@ -344,7 +344,7 @@ public void testRunPolicyAsyncWaitStepClusterStateChangeIgnored() {
344344
public void testRunPolicyWithNoStepsInRegistry() {
345345
String policyName = "cluster_state_action_policy";
346346
ClusterService clusterService = mock(ClusterService.class);
347-
IndexLifecycleRunner runner = new IndexLifecycleRunner(new PolicyStepsRegistry(NamedXContentRegistry.EMPTY),
347+
IndexLifecycleRunner runner = new IndexLifecycleRunner(new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, null),
348348
clusterService, () -> 0L);
349349
IndexMetaData indexMetaData = IndexMetaData.builder("my_index").settings(settings(Version.CURRENT))
350350
.numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
@@ -472,7 +472,7 @@ public void testGetCurrentStep() {
472472
Index index = new Index("test", "uuid");
473473
indexSteps.put(index, phase1Steps);
474474
PolicyStepsRegistry registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps,
475-
NamedXContentRegistry.EMPTY);
475+
NamedXContentRegistry.EMPTY, null);
476476

477477
Settings indexSettings = Settings.EMPTY;
478478
Step actualStep = IndexLifecycleRunner.getCurrentStep(registry, policyName, index, indexSettings);
@@ -506,7 +506,7 @@ public void testGetCurrentStep() {
506506
// TODO: it'd be nice if we used the actual registry.update method for this
507507
indexSteps.clear();
508508
indexSteps.put(index, Collections.singletonList(fourthStep));
509-
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY);
509+
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
510510

511511
indexSettings = Settings.builder()
512512
.put(LifecycleSettings.LIFECYCLE_PHASE, "phase_2")
@@ -527,7 +527,7 @@ public void testGetCurrentStep() {
527527
// Back to phase_1
528528
indexSteps.clear();
529529
indexSteps.put(index, phase1Steps);
530-
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY);
530+
registry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap, stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
531531

532532
indexSettings = Settings.builder()
533533
.put(LifecycleSettings.LIFECYCLE_PHASE, "phase_1")
@@ -1080,7 +1080,7 @@ public void testIsReadyToTransition() {
10801080
Map<String, Map<StepKey, Step>> stepMap = Collections.singletonMap(policyName, policySteps);
10811081
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(step));
10821082
PolicyStepsRegistry policyStepsRegistry = new PolicyStepsRegistry(lifecyclePolicyMap, firstStepMap,
1083-
stepMap, indexSteps, NamedXContentRegistry.EMPTY);
1083+
stepMap, indexSteps, NamedXContentRegistry.EMPTY, null);
10841084
ClusterService clusterService = mock(ClusterService.class);
10851085
final AtomicLong now = new AtomicLong(5);
10861086
IndexLifecycleRunner runner = new IndexLifecycleRunner(policyStepsRegistry, clusterService, now::get);

‎x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/PolicyStepsRegistryTests.java

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public void testGetFirstStep() {
5454
String policyName = randomAlphaOfLengthBetween(2, 10);
5555
Step expectedFirstStep = new MockStep(MOCK_STEP_KEY, null);
5656
Map<String, Step> firstStepMap = Collections.singletonMap(policyName, expectedFirstStep);
57-
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY);
57+
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY, null);
5858
Step actualFirstStep = registry.getFirstStep(policyName);
5959
assertThat(actualFirstStep, sameInstance(expectedFirstStep));
6060
}
@@ -63,7 +63,7 @@ public void testGetFirstStepUnknownPolicy() {
6363
String policyName = randomAlphaOfLengthBetween(2, 10);
6464
Step expectedFirstStep = new MockStep(MOCK_STEP_KEY, null);
6565
Map<String, Step> firstStepMap = Collections.singletonMap(policyName, expectedFirstStep);
66-
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY);
66+
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, firstStepMap, null, null, NamedXContentRegistry.EMPTY, null);
6767
Step actualFirstStep = registry.getFirstStep(policyName + "unknown");
6868
assertNull(actualFirstStep);
6969
}
@@ -72,7 +72,7 @@ public void testGetStep() {
7272
Step expectedStep = new MockStep(MOCK_STEP_KEY, null);
7373
Index index = new Index("test", "uuid");
7474
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(expectedStep));
75-
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY);
75+
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY, null);
7676
Step actualStep = registry.getStep(index, MOCK_STEP_KEY);
7777
assertThat(actualStep, sameInstance(expectedStep));
7878
}
@@ -82,21 +82,21 @@ public void testGetStepErrorStep() {
8282
Step expectedStep = new ErrorStep(errorStepKey);
8383
Index index = new Index("test", "uuid");
8484
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(expectedStep));
85-
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY);
85+
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY, null);
8686
Step actualStep = registry.getStep(index, errorStepKey);
8787
assertThat(actualStep, equalTo(expectedStep));
8888
}
8989

9090
public void testGetStepUnknownPolicy() {
91-
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, Collections.emptyMap(), NamedXContentRegistry.EMPTY);
91+
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, Collections.emptyMap(), NamedXContentRegistry.EMPTY, null);
9292
assertNull(registry.getStep(new Index("test", "uuid"), MOCK_STEP_KEY));
9393
}
9494

9595
public void testGetStepUnknownStepKey() {
9696
Step expectedStep = new MockStep(MOCK_STEP_KEY, null);
9797
Index index = new Index("test", "uuid");
9898
Map<Index, List<Step>> indexSteps = Collections.singletonMap(index, Collections.singletonList(expectedStep));
99-
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY);
99+
PolicyStepsRegistry registry = new PolicyStepsRegistry(null, null, null, indexSteps, NamedXContentRegistry.EMPTY, null);
100100
Step.StepKey unknownStepKey = new Step.StepKey(MOCK_STEP_KEY.getPhase(),
101101
MOCK_STEP_KEY.getAction(),MOCK_STEP_KEY.getName() + "not");
102102
assertNull(registry.getStep(index, unknownStepKey));
@@ -146,10 +146,10 @@ public void testUpdateFromNothingToSomethingToNothing() throws Exception {
146146
.build();
147147

148148
// start with empty registry
149-
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
149+
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
150150

151151
// add new policy
152-
registry.update(currentState, client);
152+
registry.update(currentState);
153153

154154
assertThat(registry.getFirstStep(newPolicy.getName()), equalTo(policySteps.get(0)));
155155
assertThat(registry.getLifecyclePolicyMap().size(), equalTo(1));
@@ -167,15 +167,15 @@ public void testUpdateFromNothingToSomethingToNothing() throws Exception {
167167
.put(LifecycleSettings.LIFECYCLE_PHASE, step.getKey().getPhase()))))
168168
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
169169
.build();
170-
registry.update(currentState, client);
170+
registry.update(currentState);
171171
assertThat(registeredStepsForPolicy.get(step.getKey()), equalTo(step));
172172
assertThat(registry.getStep(index, step.getKey()), equalTo(step));
173173
}
174174

175175
Map<String, LifecyclePolicyMetadata> registryPolicyMap = registry.getLifecyclePolicyMap();
176176
Map<String, Step> registryFirstStepMap = registry.getFirstStepMap();
177177
Map<String, Map<Step.StepKey, Step>> registryStepMap = registry.getStepMap();
178-
registry.update(currentState, client);
178+
registry.update(currentState);
179179
assertThat(registry.getLifecyclePolicyMap(), equalTo(registryPolicyMap));
180180
assertThat(registry.getFirstStepMap(), equalTo(registryFirstStepMap));
181181
assertThat(registry.getStepMap(), equalTo(registryStepMap));
@@ -186,13 +186,13 @@ public void testUpdateFromNothingToSomethingToNothing() throws Exception {
186186
.metaData(
187187
MetaData.builder(metaData)
188188
.putCustom(IndexLifecycleMetadata.TYPE, lifecycleMetadata)).build();
189-
registry.update(currentState, client);
189+
registry.update(currentState);
190190
assertTrue(registry.getLifecyclePolicyMap().isEmpty());
191191
assertTrue(registry.getFirstStepMap().isEmpty());
192192
assertTrue(registry.getStepMap().isEmpty());
193193
}
194194

195-
public void testUpdateChangedPolicy() throws Exception {
195+
public void testUpdateChangedPolicy() {
196196
Client client = Mockito.mock(Client.class);
197197
Mockito.when(client.settings()).thenReturn(Settings.EMPTY);
198198
String policyName = randomAlphaOfLengthBetween(5, 10);
@@ -217,9 +217,9 @@ public void testUpdateChangedPolicy() throws Exception {
217217
.metaData(metaData)
218218
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
219219
.build();
220-
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
220+
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
221221
// add new policy
222-
registry.update(currentState, client);
222+
registry.update(currentState);
223223

224224
// swap out policy
225225
newPolicy = LifecyclePolicyTests.randomTestLifecyclePolicy(policyName);
@@ -228,7 +228,7 @@ public void testUpdateChangedPolicy() throws Exception {
228228
randomNonNegativeLong(), randomNonNegativeLong())), OperationMode.RUNNING);
229229
currentState = ClusterState.builder(currentState)
230230
.metaData(MetaData.builder(metaData).putCustom(IndexLifecycleMetadata.TYPE, lifecycleMetadata)).build();
231-
registry.update(currentState, client);
231+
registry.update(currentState);
232232
// TODO(talevy): assert changes... right now we do not support updates to policies. will require internal cleanup
233233
}
234234

@@ -287,10 +287,10 @@ public void testUpdatePolicyButNoPhaseChangeIndexStepsDontChange() throws Except
287287
.build();
288288

289289
// start with empty registry
290-
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY);
290+
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client);
291291

292292
// add new policy
293-
registry.update(currentState, client);
293+
registry.update(currentState);
294294

295295
Map<Step.StepKey, Step> registeredStepsForPolicy = registry.getStepMap().get(newPolicy.getName());
296296
Step shrinkStep = registeredStepsForPolicy.entrySet().stream()
@@ -316,7 +316,7 @@ public void testUpdatePolicyButNoPhaseChangeIndexStepsDontChange() throws Except
316316
currentState = ClusterState.builder(ClusterName.DEFAULT).metaData(metaData).build();
317317

318318
// Update the policies
319-
registry.update(currentState, client);
319+
registry.update(currentState);
320320

321321
registeredStepsForPolicy = registry.getStepMap().get(newPolicy.getName());
322322
shrinkStep = registeredStepsForPolicy.entrySet().stream()

0 commit comments

Comments
 (0)