diff --git a/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java b/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java index f2b1e39ff0427..c500ec23d1b0b 100644 --- a/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java +++ b/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java @@ -55,6 +55,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; @@ -195,6 +196,49 @@ public void testSingleNodeCluster() throws Exception { }); } + public void testNoOpPolicyUpdates() throws Exception { + internalCluster().startNode(); + Map phases = new HashMap<>(); + phases.put("hot", new Phase("hot", TimeValue.ZERO, Map.of())); + LifecyclePolicy policy = new LifecyclePolicy("mypolicy", phases); + + PutLifecycleAction.Request putLifecycleRequest = new PutLifecycleAction.Request(policy); + assertAcked(client().execute(PutLifecycleAction.INSTANCE, putLifecycleRequest).get()); + + GetLifecycleAction.Response getLifecycleResponse = client().execute(GetLifecycleAction.INSTANCE, new GetLifecycleAction.Request()) + .get(); + assertThat(getLifecycleResponse.getPolicies().size(), equalTo(1)); + GetLifecycleAction.LifecyclePolicyResponseItem responseItem = getLifecycleResponse.getPolicies().get(0); + assertThat(responseItem.getLifecyclePolicy(), equalTo(policy)); + assertThat(responseItem.getVersion(), equalTo(1L)); + + // Put the same policy in place, which should be a no-op + putLifecycleRequest = new PutLifecycleAction.Request(policy); + assertAcked(client().execute(PutLifecycleAction.INSTANCE, putLifecycleRequest).get()); + + getLifecycleResponse = client().execute(GetLifecycleAction.INSTANCE, new GetLifecycleAction.Request()).get(); + assertThat(getLifecycleResponse.getPolicies().size(), equalTo(1)); + responseItem = getLifecycleResponse.getPolicies().get(0); + assertThat(responseItem.getLifecyclePolicy(), equalTo(policy)); + // Version should still be 1 + assertThat(responseItem.getVersion(), equalTo(1L)); + + // Generate a brand new policy + Map newPhases = new HashMap<>(phases); + newPhases.put("cold", new Phase("cold", TimeValue.timeValueDays(1), Map.of())); + policy = new LifecyclePolicy("mypolicy", newPhases); + + putLifecycleRequest = new PutLifecycleAction.Request(policy); + assertAcked(client().execute(PutLifecycleAction.INSTANCE, putLifecycleRequest).get()); + + getLifecycleResponse = client().execute(GetLifecycleAction.INSTANCE, new GetLifecycleAction.Request()).get(); + assertThat(getLifecycleResponse.getPolicies().size(), equalTo(1)); + responseItem = getLifecycleResponse.getPolicies().get(0); + assertThat(responseItem.getLifecyclePolicy(), equalTo(policy)); + // Version should now be 2 + assertThat(responseItem.getVersion(), equalTo(2L)); + } + public void testExplainExecution() throws Exception { // start node logger.info("Starting server1"); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java index e3599c2341c27..c133e418db84b 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java @@ -24,6 +24,7 @@ import org.elasticsearch.cluster.metadata.RepositoriesMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.core.Nullable; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; @@ -98,20 +99,34 @@ protected void masterOperation(Task task, Request request, ClusterState state, A LifecyclePolicy.validatePolicyName(request.getPolicy().getName()); + { + IndexLifecycleMetadata lifecycleMetadata = state.metadata().custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY); + LifecyclePolicyMetadata existingPolicy = lifecycleMetadata.getPolicyMetadatas().get(request.getPolicy().getName()); + // Make the request a no-op if the policy and filtered headers match exactly + if (isNoopUpdate(existingPolicy, request.getPolicy(), filteredHeaders)) { + listener.onResponse(AcknowledgedResponse.TRUE); + return; + } + } + clusterService.submitStateUpdateTask( "put-lifecycle-" + request.getPolicy().getName(), new AckedClusterStateUpdateTask(request, listener) { @Override public ClusterState execute(ClusterState currentState) throws Exception { + final IndexLifecycleMetadata currentMetadata = currentState.metadata() + .custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY); + final LifecyclePolicyMetadata existingPolicyMetadata = currentMetadata.getPolicyMetadatas() + .get(request.getPolicy().getName()); + + // Double-check for no-op in the state update task, in case it was changed/reset in the meantime + if (isNoopUpdate(existingPolicyMetadata, request.getPolicy(), filteredHeaders)) { + return currentState; + } + validatePrerequisites(request.getPolicy(), currentState); ClusterState.Builder stateBuilder = ClusterState.builder(currentState); - IndexLifecycleMetadata currentMetadata = currentState.metadata().custom(IndexLifecycleMetadata.TYPE); - if (currentMetadata == null) { // first time using index-lifecycle feature, bootstrap metadata - currentMetadata = IndexLifecycleMetadata.EMPTY; - } - LifecyclePolicyMetadata existingPolicyMetadata = currentMetadata.getPolicyMetadatas() - .get(request.getPolicy().getName()); long nextVersion = (existingPolicyMetadata == null) ? 1L : existingPolicyMetadata.getVersion() + 1L; SortedMap newPolicies = new TreeMap<>(currentMetadata.getPolicyMetadatas()); LifecyclePolicyMetadata lifecyclePolicyMetadata = new LifecyclePolicyMetadata( @@ -160,6 +175,21 @@ public ClusterState execute(ClusterState currentState) throws Exception { ); } + /** + * Returns 'true' if the ILM policy is effectually the same (same policy and headers), and thus can be a no-op update. + */ + static boolean isNoopUpdate( + @Nullable LifecyclePolicyMetadata existingPolicy, + LifecyclePolicy newPolicy, + Map filteredHeaders + ) { + if (existingPolicy == null) { + return false; + } else { + return newPolicy.equals(existingPolicy.getPolicy()) && filteredHeaders.equals(existingPolicy.getHeaders()); + } + } + /** * Validate that the license level is compliant for searchable-snapshots, that any referenced snapshot * repositories exist, and that any referenced SLM policies exist. diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java new file mode 100644 index 0000000000000..b2202f0e5126f --- /dev/null +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ilm.action; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.ilm.LifecyclePolicy; +import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; +import org.elasticsearch.xpack.ilm.LifecyclePolicyTestsUtils; + +import java.util.Map; + +public class TransportPutLifecycleActionTests extends ESTestCase { + public void testIsNoop() { + LifecyclePolicy policy1 = LifecyclePolicyTestsUtils.randomTimeseriesLifecyclePolicy("policy"); + LifecyclePolicy policy2 = randomValueOtherThan(policy1, () -> LifecyclePolicyTestsUtils.randomTimeseriesLifecyclePolicy("policy")); + + Map headers1 = Map.of("foo", "bar"); + Map headers2 = Map.of("foo", "eggplant"); + + LifecyclePolicyMetadata existing = new LifecyclePolicyMetadata(policy1, headers1, randomNonNegativeLong(), randomNonNegativeLong()); + + assertTrue(TransportPutLifecycleAction.isNoopUpdate(existing, policy1, headers1)); + assertFalse(TransportPutLifecycleAction.isNoopUpdate(existing, policy2, headers1)); + assertFalse(TransportPutLifecycleAction.isNoopUpdate(existing, policy1, headers2)); + assertFalse(TransportPutLifecycleAction.isNoopUpdate(null, policy1, headers1)); + } +}