From f81fa5efba0b2a251ae004b39d077624ce02b05b Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 14 Oct 2021 09:42:47 +0200 Subject: [PATCH] Make some CS Applier NOOPs Cheaper (#79098) Short-circuiting a few spots where we needlessly diff maps or iterate over all indices. --- .../indices/TimestampFieldMapperService.java | 7 ++++++- .../xpack/ilm/IndexLifecycleService.java | 7 +++++-- .../xpack/ilm/PolicyStepsRegistry.java | 3 +-- .../xpack/ilm/ExecuteStepsUpdateTaskTests.java | 6 +++--- .../xpack/ilm/PolicyStepsRegistryTests.java | 16 ++++++++-------- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/TimestampFieldMapperService.java b/server/src/main/java/org/elasticsearch/indices/TimestampFieldMapperService.java index 080eb51c0eeab..849ba13866e70 100644 --- a/server/src/main/java/org/elasticsearch/indices/TimestampFieldMapperService.java +++ b/server/src/main/java/org/elasticsearch/indices/TimestampFieldMapperService.java @@ -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; @@ -83,12 +84,16 @@ protected void doClose() { @Override public void applyClusterState(ClusterChangedEvent event) { final Metadata metadata = event.state().metadata(); + final ImmutableOpenMap 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) { diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleService.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleService.java index 1400b3f68acd2..cbd5bb852b6cc 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleService.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleService.java @@ -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); } } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistry.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistry.java index 26f77f3964231..73f8c695b3609 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistry.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistry.java @@ -91,8 +91,7 @@ Map> 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> mapDiff = diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTaskTests.java index dc85fe5944483..2af56b1c31d96 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTaskTests.java @@ -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; } @@ -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(); @@ -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)); } } diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistryTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistryTests.java index 81cf02f3fdb76..9dcd515d4d49d 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistryTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/PolicyStepsRegistryTests.java @@ -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)); @@ -252,7 +252,7 @@ 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)); } @@ -260,7 +260,7 @@ public void testUpdateFromNothingToSomethingToNothing() throws Exception { Map registryPolicyMap = registry.getLifecyclePolicyMap(); Map registryFirstStepMap = registry.getFirstStepMap(); Map> 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)); @@ -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()); @@ -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); @@ -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 } @@ -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 registeredStepsForPolicy = registry.getStepMap().get(newPolicy.getName()); Step shrinkStep = registeredStepsForPolicy.entrySet().stream() @@ -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()