[7.8] Make noop template updates be cluster state noops (#57851) (#58677

) Backports the following commits to 7.8: Make noop template updates be cluster state noops (#57851)
elastic · Jun 29, 2020 · 74c31e9 · 74c31e9
1 parent a17f5a4
commit 74c31e9
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 7 deletions.
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java
@@ -179,7 +179,8 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
     // Package visible for testing
     ClusterState addComponentTemplate(final ClusterState currentState, final boolean create,
                                       final String name, final ComponentTemplate template) throws Exception {
-        if (create && currentState.metadata().componentTemplates().containsKey(name)) {
+        final ComponentTemplate existing = currentState.metadata().componentTemplates().get(name);
+        if (create && existing != null) {
             throw new IllegalArgumentException("component template [" + name + "] already exists");
         }
 
@@ -194,8 +195,6 @@ ClusterState addComponentTemplate(final ClusterState currentState, final boolean
                 .build();
         }
 
-        validateTemplate(finalSettings, stringMappings, indicesService, xContentRegistry);
-
         // Collect all the composable (index) templates that use this component template, we'll use
         // this for validating that they're still going to be valid after this component template
         // has been updated
@@ -239,8 +238,15 @@ ClusterState addComponentTemplate(final ClusterState currentState, final boolean
         final Template finalTemplate = new Template(finalSettings,
             stringMappings == null ? null : new CompressedXContent(stringMappings), template.template().aliases());
         final ComponentTemplate finalComponentTemplate = new ComponentTemplate(finalTemplate, template.version(), template.metadata());
+
+        if (finalComponentTemplate.equals(existing)) {
+            return currentState;
+        }
+
+        validateTemplate(finalSettings, stringMappings, indicesService, xContentRegistry);
         validate(name, finalComponentTemplate);
 
+        // Validate all composable index templates that use this component template
         if (templatesUsingComponent.size() > 0) {
             ClusterState tempStateWithComponentTemplateAdded = ClusterState.builder(currentState)
                 .metadata(Metadata.builder(currentState.metadata()).put(name, finalComponentTemplate))
@@ -266,7 +272,7 @@ ClusterState addComponentTemplate(final ClusterState currentState, final boolean
             }
         }
 
-        logger.info("adding component template [{}]", name);
+        logger.info("{} component template [{}]", existing == null ? "adding" : "updating", name);
         return ClusterState.builder(currentState)
             .metadata(Metadata.builder(currentState.metadata()).put(name, finalComponentTemplate))
             .build();
@@ -367,7 +373,8 @@ public static void validateV2TemplateRequest(Metadata metadata, String name, Com
 
     public ClusterState addIndexTemplateV2(final ClusterState currentState, final boolean create,
                                            final String name, final ComposableIndexTemplate template) throws Exception {
-        if (create && currentState.metadata().templatesV2().containsKey(name)) {
+        final ComposableIndexTemplate existing = currentState.metadata().templatesV2().get(name);
+        if (create && existing != null) {
             throw new IllegalArgumentException("index template [" + name + "] already exists");
         }
 
@@ -434,6 +441,10 @@ public ClusterState addIndexTemplateV2(final ClusterState currentState, final bo
                 template.priority(), template.version(), template.metadata());
         }
 
+        if (finalIndexTemplate.equals(existing)) {
+            return currentState;
+        }
+
         validate(name, finalIndexTemplate);
 
         // Finally, right before adding the template, we need to ensure that the composite settings,
@@ -446,7 +457,7 @@ public ClusterState addIndexTemplateV2(final ClusterState currentState, final bo
                 (finalIndexTemplate.composedOf().size() > 0 ? "with component templates " + finalIndexTemplate.composedOf() + " " : "") +
                 "is invalid", e);
         }
-        logger.info("adding index template [{}]", name);
+        logger.info("{} index template [{}]", existing == null ? "adding" : "updating", name);
         return ClusterState.builder(currentState)
             .metadata(Metadata.builder(currentState.metadata()).put(name, finalIndexTemplate))
             .build();

diff --git a/...r/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/...r/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java
@@ -284,7 +284,7 @@ public void testAddComponentTemplate() throws Exception{
         template = new Template(Settings.builder().build(), new CompressedXContent("{\"invalid\"}"),
             ComponentTemplateTests.randomAliases());
         ComponentTemplate componentTemplate2 = new ComponentTemplate(template, 1L, new HashMap<>());
-        expectThrows(MapperParsingException.class,
+        expectThrows(Exception.class,
             () -> metadataIndexTemplateService.addComponentTemplate(throwState, true, "foo2", componentTemplate2));
 
         template = new Template(Settings.builder().build(), new CompressedXContent("{\"invalid\":\"invalid\"}"),
@@ -981,6 +981,28 @@ public void testUpdateComponentTemplateFailsIfResolvedIndexTemplatesWouldBeInval
                 containsString("mapper [field2] of different type, current_type [text], merged_type [ObjectMapper]")));
     }
 
+    public void testPutExistingComponentTemplateIsNoop() throws Exception {
+        MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService();
+        ClusterState state = ClusterState.EMPTY_STATE;
+        ComponentTemplate componentTemplate = ComponentTemplateTests.randomInstance();
+        state = metadataIndexTemplateService.addComponentTemplate(state, false, "foo", componentTemplate);
+
+        assertNotNull(state.metadata().componentTemplates().get("foo"));
+
+        assertThat(metadataIndexTemplateService.addComponentTemplate(state, false, "foo", componentTemplate), equalTo(state));
+    }
+
+    public void testPutExistingComposableTemplateIsNoop() throws Exception {
+        ClusterState state = ClusterState.EMPTY_STATE;
+        final MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService();
+        ComposableIndexTemplate template = ComposableIndexTemplateTests.randomInstance();
+        state = metadataIndexTemplateService.addIndexTemplateV2(state, false, "foo", template);
+
+        assertNotNull(state.metadata().templatesV2().get("foo"));
+
+        assertThat(metadataIndexTemplateService.addIndexTemplateV2(state, false, "foo", template), equalTo(state));
+    }
+
     private static List<Throwable> putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) {
         MetadataCreateIndexService createIndexService = new MetadataCreateIndexService(
                 Settings.EMPTY,