Skip to content

Commit

Permalink
Make some CS Applier NOOPs Cheaper (#79098)
Browse files Browse the repository at this point in the history
Short-circuiting a few spots where we needlessly diff maps or iterate over all indices.
  • Loading branch information
original-brownbear authored Oct 14, 2021
1 parent dabd10b commit f81fa5e
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.cluster.metadata.DataStream;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -83,12 +84,16 @@ protected void doClose() {
@Override
public void applyClusterState(ClusterChangedEvent event) {
final Metadata metadata = event.state().metadata();
final ImmutableOpenMap<String, IndexMetadata> indices = metadata.indices();
if (indices == event.previousState().metadata().indices()) {
return;
}

// clear out mappers for indices that no longer exist or whose timestamp range is no longer known
fieldTypesByIndex.keySet().removeIf(index -> hasUsefulTimestampField(metadata.index(index)) == false);

// capture mappers for indices that do exist
for (IndexMetadata indexMetadata : metadata.indices().values()) {
for (IndexMetadata indexMetadata : indices.values()) {
final Index index = indexMetadata.getIndex();

if (hasUsefulTimestampField(indexMetadata) && fieldTypesByIndex.containsKey(index) == false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,11 @@ public void clusterChanged(ClusterChangedEvent event) {
public void applyClusterState(ClusterChangedEvent event) {
if (event.localNodeMaster()) { // only act if we are master, otherwise
// keep idle until elected
if (event.state().metadata().custom(IndexLifecycleMetadata.TYPE) != null) {
policyRegistry.update(event.state());
final IndexLifecycleMetadata ilmMetadata = event.state().metadata().custom(IndexLifecycleMetadata.TYPE);
// only update the policy registry if we just became the master node or if the ilm metadata changed
if (ilmMetadata != null && (event.previousState().nodes().isLocalNodeElectedMaster() == false
|| ilmMetadata != event.previousState().metadata().custom(IndexLifecycleMetadata.TYPE))) {
policyRegistry.update(ilmMetadata);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ Map<String, Map<Step.StepKey, Step>> getStepMap() {
return stepMap;
}

public void update(ClusterState clusterState) {
final IndexLifecycleMetadata meta = clusterState.metadata().custom(IndexLifecycleMetadata.TYPE);
public void update(IndexLifecycleMetadata meta) {
assert meta != null : "IndexLifecycleMetadata cannot be null when updating the policy steps registry";

DiffableUtils.MapDiff<String, LifecyclePolicyMetadata, Map<String, LifecyclePolicyMetadata>> mapDiff =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ private IndexMetadata setupIndexPolicy(String policyName) {
.metadata(metadata)
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
.build();
policyStepsRegistry.update(clusterState);
policyStepsRegistry.update(clusterState.metadata().custom(IndexLifecycleMetadata.TYPE));
return indexMetadata;
}

Expand Down Expand Up @@ -197,7 +197,7 @@ public void testExecuteInvalidStartStep() throws Exception {
.put(IndexMetadata.builder(clusterState.getMetadata().index(index))
.putCustom(ILM_CUSTOM_METADATA_KEY, lifecycleState.build().asMap()))).build();

policyStepsRegistry.update(clusterState);
policyStepsRegistry.update(clusterState.metadata().custom(IndexLifecycleMetadata.TYPE));

Step invalidStep = new MockClusterStateActionStep(firstStepKey, secondStepKey);
long now = randomNonNegativeLong();
Expand Down Expand Up @@ -301,6 +301,6 @@ private void setStateToKey(StepKey stepKey) throws IOException {
.metadata(Metadata.builder(clusterState.getMetadata())
.put(IndexMetadata.builder(clusterState.getMetadata().index(index))
.putCustom(ILM_CUSTOM_METADATA_KEY, lifecycleState.build().asMap()))).build();
policyStepsRegistry.update(clusterState);
policyStepsRegistry.update(clusterState.metadata().custom(IndexLifecycleMetadata.TYPE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public void testUpdateFromNothingToSomethingToNothing() throws Exception {
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client, null);

// add new policy
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));

assertThat(registry.getFirstStep(newPolicy.getName()), equalTo(policySteps.get(0)));
assertThat(registry.getLifecyclePolicyMap().size(), equalTo(1));
Expand All @@ -252,15 +252,15 @@ public void testUpdateFromNothingToSomethingToNothing() throws Exception {
.putCustom(ILM_CUSTOM_METADATA_KEY, newIndexState.build().asMap())))
.nodes(DiscoveryNodes.builder().localNodeId(nodeId).masterNodeId(nodeId).add(masterNode).build())
.build();
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));
assertThat(registeredStepsForPolicy.get(step.getKey()), equalTo(step));
assertThat(registry.getStep(metadata.index(index), step.getKey()), equalTo(step));
}

Map<String, LifecyclePolicyMetadata> registryPolicyMap = registry.getLifecyclePolicyMap();
Map<String, Step> registryFirstStepMap = registry.getFirstStepMap();
Map<String, Map<Step.StepKey, Step>> registryStepMap = registry.getStepMap();
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));
assertThat(registry.getLifecyclePolicyMap(), equalTo(registryPolicyMap));
assertThat(registry.getFirstStepMap(), equalTo(registryFirstStepMap));
assertThat(registry.getStepMap(), equalTo(registryStepMap));
Expand All @@ -271,7 +271,7 @@ public void testUpdateFromNothingToSomethingToNothing() throws Exception {
.metadata(
Metadata.builder(metadata)
.putCustom(IndexLifecycleMetadata.TYPE, lifecycleMetadata)).build();
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));
assertTrue(registry.getLifecyclePolicyMap().isEmpty());
assertTrue(registry.getFirstStepMap().isEmpty());
assertTrue(registry.getStepMap().isEmpty());
Expand Down Expand Up @@ -305,7 +305,7 @@ public void testUpdateChangedPolicy() {
.build();
PolicyStepsRegistry registry = new PolicyStepsRegistry(NamedXContentRegistry.EMPTY, client, null);
// add new policy
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));

// swap out policy
newPolicy = LifecyclePolicyTests.randomTestLifecyclePolicy(policyName);
Expand All @@ -314,7 +314,7 @@ public void testUpdateChangedPolicy() {
randomNonNegativeLong(), randomNonNegativeLong())), OperationMode.RUNNING);
currentState = ClusterState.builder(currentState)
.metadata(Metadata.builder(metadata).putCustom(IndexLifecycleMetadata.TYPE, lifecycleMetadata)).build();
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));
// TODO(talevy): assert changes... right now we do not support updates to policies. will require internal cleanup
}

Expand Down Expand Up @@ -383,7 +383,7 @@ public void testUpdatePolicyButNoPhaseChangeIndexStepsDontChange() throws Except
PolicyStepsRegistry registry = new PolicyStepsRegistry(REGISTRY, client, null);

// add new policy
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));

Map<Step.StepKey, Step> registeredStepsForPolicy = registry.getStepMap().get(newPolicy.getName());
Step shrinkStep = registeredStepsForPolicy.entrySet().stream()
Expand All @@ -409,7 +409,7 @@ public void testUpdatePolicyButNoPhaseChangeIndexStepsDontChange() throws Except
currentState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).build();

// Update the policies
registry.update(currentState);
registry.update(currentState.metadata().custom(IndexLifecycleMetadata.TYPE));

registeredStepsForPolicy = registry.getStepMap().get(newPolicy.getName());
shrinkStep = registeredStepsForPolicy.entrySet().stream()
Expand Down

0 comments on commit f81fa5e

Please sign in to comment.