improve

weizijun · Oct 11, 2021 · 5bf270b · 5bf270b
1 parent 6ba29fb
commit 5bf270b
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 16 deletions.
diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java
@@ -67,6 +67,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.IntConsumer;
+import java.util.stream.Collectors;
 
 /**
  * Holder class for several ingest related services.
@@ -295,30 +296,36 @@ static ClusterState innerDelete(DeletePipelineRequest request, ClusterState curr
     }
 
     static void validateNotInUse(String pipeline, ImmutableOpenMap<String, IndexMetadata> indices) {
+        List<String> defaultPipelineIndices = new ArrayList<>();
+        List<String> finalPipelineIndices = new ArrayList<>();
         for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
             IndexMetadata indexMetadata = cursor.value;
             String defaultPipeline = IndexSettings.DEFAULT_PIPELINE.get(indexMetadata.getSettings());
             String finalPipeline = IndexSettings.FINAL_PIPELINE.get(indexMetadata.getSettings());
             if (pipeline.equals(defaultPipeline)) {
-                throw new IllegalArgumentException(
-                    "unable to remove pipeline ["
-                        + pipeline
-                        + "], as it is in use by index ["
-                        + indexMetadata.getIndex().getName()
-                        + "] default pipeline settings"
-                );
+                defaultPipelineIndices.add(indexMetadata.getIndex().getName());
             }
 
             if (pipeline.equals(finalPipeline)) {
-                throw new IllegalArgumentException(
-                    "unable to remove pipeline ["
-                        + pipeline
-                        + "], as it is in use by index ["
-                        + indexMetadata.getIndex().getName()
-                        + "] final pipeline settings"
-                );
+                finalPipelineIndices.add(indexMetadata.getIndex().getName());
             }
         }
+
+        if (defaultPipelineIndices.size() > 0 || finalPipelineIndices.size() > 0) {
+            throw new IllegalArgumentException(
+                "pipeline ["
+                    + pipeline
+                    + "] cannot be deleted because it is the default pipeline for "
+                    + defaultPipelineIndices.size()
+                    + " indices including ["
+                    + defaultPipelineIndices.stream().limit(3).collect(Collectors.joining(","))
+                    + "] and the final pipeline for "
+                    + finalPipelineIndices.size()
+                    + " indices including ["
+                    + finalPipelineIndices.stream().limit(3).collect(Collectors.joining(","))
+                    + "]"
+            );
+        }
     }
 
     /**

diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java
@@ -35,6 +35,7 @@
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.time.DateFormatter;
@@ -78,6 +79,7 @@
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.instanceOf;
@@ -283,6 +285,55 @@ public void testValidateNoIngestInfo() throws Exception {
         ingestService.validatePipeline(Collections.singletonMap(discoveryNode, ingestInfo), putRequest);
     }
 
+    public void testValidateNotInUse() {
+        String pipeline = "pipeline";
+        ImmutableOpenMap.Builder<String, IndexMetadata> indices = ImmutableOpenMap.builder();
+        int defaultPipelineCount = 0;
+        int finalPipelineCount = 0;
+        int indicesCount = randomIntBetween(5, 10);
+        for (int i = 0; i < indicesCount; i++) {
+            IndexMetadata.Builder builder = IndexMetadata.builder("index" + i).numberOfShards(1).numberOfReplicas(1);
+            Settings.Builder settingsBuilder = settings(Version.CURRENT);
+            if (randomBoolean()) {
+                settingsBuilder.put(IndexSettings.DEFAULT_PIPELINE.getKey(), pipeline);
+                defaultPipelineCount++;
+            }
+
+            if (randomBoolean()) {
+                settingsBuilder.put(IndexSettings.FINAL_PIPELINE.getKey(), pipeline);
+                finalPipelineCount++;
+            }
+
+            builder.settings(settingsBuilder);
+            IndexMetadata indexMetadata = builder.settings(settingsBuilder).numberOfShards(1).numberOfReplicas(1).build();
+            indices.put(indexMetadata.getIndex().getName(), indexMetadata);
+        }
+
+        IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> IngestService.validateNotInUse(pipeline, indices.build())
+        );
+        assertThat(e.getMessage(), containsString("default pipeline for " + defaultPipelineCount + " indices including"));
+        assertThat(e.getMessage(), containsString("final pipeline for " + finalPipelineCount + " indices including"));
+        if (defaultPipelineCount >= 3) {
+            // assert index limit
+            String content = "default pipeline for " + defaultPipelineCount + " indices including [";
+            int start = e.getMessage().indexOf(content) + content.length();
+            int end = e.getMessage().indexOf("] and the final pipeline");
+            // indices content length, eg: index0,index1,index2
+            assertEquals(end - start, (6 + 1 + 6 + 1 + 6));
+        }
+
+        if (finalPipelineCount >= 3) {
+            // assert index limit
+            String content = "final pipeline for " + finalPipelineCount + " indices including [";
+            int start = e.getMessage().indexOf(content) + content.length();
+            int end = e.getMessage().lastIndexOf("]");
+            // indices content length, eg: index0,index1,index2
+            assertEquals(end - start, (6 + 1 + 6 + 1 + 6));
+        }
+    }
+
     public void testGetProcessorsInPipeline() throws Exception {
         IngestService ingestService = createWithProcessors();
         String id = "_id";
@@ -579,7 +630,7 @@ public void testDeleteWithIndexUsePipeline() {
                 IllegalArgumentException.class,
                 () -> IngestService.innerDelete(deleteRequest, finalClusterState)
             );
-            assertTrue(e.getMessage().contains("default pipeline settings"));
+            assertThat(e.getMessage(), containsString("default pipeline for 1 indices including [pipeline-index]"));
         }
 
         {
@@ -596,7 +647,7 @@ public void testDeleteWithIndexUsePipeline() {
                 IllegalArgumentException.class,
                 () -> IngestService.innerDelete(deleteRequest, finalClusterState)
             );
-            assertTrue(e.getMessage().contains("final pipeline settings"));
+            assertThat(e.getMessage(), containsString("final pipeline for 1 indices including [pipeline-index]"));
         }
 
         // Delete pipeline: