[Metadata Immutability] Change different indices lookup objects from …

…array type to lists

Changed the arrays to immutable List instances, added new versions of the getters which returns List instances.

Resolves opensearch-project#8647

Signed-off-by: Abdul Muneer Kolarkunnu <[email protected]>

server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java

@@ -706,7 +706,7 @@ public Map<String, Set<String>> resolveSearchRoutingAllIndices(Metadata metadata
         if (routing != null) {
             Set<String> r = Sets.newHashSet(Strings.splitStringByCommaToArray(routing));
             Map<String, Set<String>> routings = new HashMap<>();
-            String[] concreteIndices = metadata.getConcreteAllIndices();
+            Set<String> concreteIndices = metadata.getConcreteAllIndicesList();
             for (String index : concreteIndices) {
                 routings.put(index, r);
             }
@@ -746,7 +746,7 @@ static boolean isExplicitAllPattern(Collection<String> aliasesOrIndices) {
      */
     boolean isPatternMatchingAllIndices(Metadata metadata, String[] indicesOrAliases, String[] concreteIndices) {
         // if we end up matching on all indices, check, if its a wildcard parameter, or a "-something" structure
-        if (concreteIndices.length == metadata.getConcreteAllIndices().length && indicesOrAliases.length > 0) {
+        if (concreteIndices.length == metadata.getConcreteAllIndicesList().size() && indicesOrAliases.length > 0) {
 
             // we might have something like /-test1,+test1 that would identify all indices
             // or something like /-test1 with test1 index missing and IndicesOptions.lenient()
@@ -1182,17 +1182,17 @@ private boolean isEmptyOrTrivialWildcard(List<String> expressions) {
 
         private static List<String> resolveEmptyOrTrivialWildcard(IndicesOptions options, Metadata metadata) {
             if (options.expandWildcardsOpen() && options.expandWildcardsClosed() && options.expandWildcardsHidden()) {
-                return Arrays.asList(metadata.getConcreteAllIndices());
+                return new ArrayList<>(metadata.getConcreteAllIndicesList());
             } else if (options.expandWildcardsOpen() && options.expandWildcardsClosed()) {
-                return Arrays.asList(metadata.getConcreteVisibleIndices());
+                return metadata.getConcreteVisibleIndicesList();
             } else if (options.expandWildcardsOpen() && options.expandWildcardsHidden()) {
-                return Arrays.asList(metadata.getConcreteAllOpenIndices());
+                return metadata.getConcreteAllOpenIndicesList();
             } else if (options.expandWildcardsOpen()) {
-                return Arrays.asList(metadata.getConcreteVisibleOpenIndices());
+                return metadata.getConcreteVisibleOpenIndicesList();
             } else if (options.expandWildcardsClosed() && options.expandWildcardsHidden()) {
-                return Arrays.asList(metadata.getConcreteAllClosedIndices());
+                return metadata.getConcreteAllClosedIndicesList();
             } else if (options.expandWildcardsClosed()) {
-                return Arrays.asList(metadata.getConcreteVisibleClosedIndices());
+                return metadata.getConcreteVisibleClosedIndicesList();
             } else {
                 return Collections.emptyList();
             }

server/src/main/java/org/opensearch/cluster/metadata/Metadata.java

@@ -271,12 +271,12 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio
     private final transient int totalNumberOfShards; // Transient ? not serializable anyway?
     private final int totalOpenIndexShards;
 
-    private final String[] allIndices;
-    private final String[] visibleIndices;
-    private final String[] allOpenIndices;
-    private final String[] visibleOpenIndices;
-    private final String[] allClosedIndices;
-    private final String[] visibleClosedIndices;
+    private final Set<String> allIndices;
+    private final List<String> visibleIndices;
+    private final List<String> allOpenIndices;
+    private final List<String> visibleOpenIndices;
+    private final List<String> allClosedIndices;
+    private final List<String> visibleClosedIndices;
 
     private final SortedMap<String, IndexAbstraction> indicesLookup;
 
@@ -291,12 +291,12 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio
         final Map<String, IndexMetadata> indices,
         final Map<String, IndexTemplateMetadata> templates,
         final Map<String, Custom> customs,
-        String[] allIndices,
-        String[] visibleIndices,
-        String[] allOpenIndices,
-        String[] visibleOpenIndices,
-        String[] allClosedIndices,
-        String[] visibleClosedIndices,
+        Set<String> allIndices,
+        List<String> visibleIndices,
+        List<String> allOpenIndices,
+        List<String> visibleOpenIndices,
+        List<String> allClosedIndices,
+        List<String> visibleClosedIndices,
         SortedMap<String, IndexAbstraction> indicesLookup
     ) {
         this.clusterUUID = clusterUUID;
@@ -321,12 +321,12 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio
         this.totalNumberOfShards = totalNumberOfShards;
         this.totalOpenIndexShards = totalOpenIndexShards;
 
-        this.allIndices = allIndices;
-        this.visibleIndices = visibleIndices;
-        this.allOpenIndices = allOpenIndices;
-        this.visibleOpenIndices = visibleOpenIndices;
-        this.allClosedIndices = allClosedIndices;
-        this.visibleClosedIndices = visibleClosedIndices;
+        this.allIndices = Collections.unmodifiableSet(allIndices);
+        this.visibleIndices = Collections.unmodifiableList(visibleIndices);
+        this.allOpenIndices = Collections.unmodifiableList(allOpenIndices);
+        this.visibleOpenIndices = Collections.unmodifiableList(visibleOpenIndices);
+        this.allClosedIndices = Collections.unmodifiableList(allClosedIndices);
+        this.visibleClosedIndices = Collections.unmodifiableList(visibleClosedIndices);
         this.indicesLookup = indicesLookup;
     }
 
@@ -601,44 +601,86 @@ private static String mergePaths(String path, String field) {
     }
 
     /**
-     * Returns all the concrete indices.
+     * Returns all the concrete indices in the format of array of strings.
      */
     public String[] getConcreteAllIndices() {
+        return allIndices.toArray(String[]::new);
+    }
+
+    /**
+     * Returns all the concrete indices in the format of set of strings.
+     */
+    public Set<String> getConcreteAllIndicesList() {
         return allIndices;
     }
 
     /**
-     * Returns all the concrete indices that are not hidden.
+     * Returns all the concrete indices that are not hidden in the format of array of strings.
      */
     public String[] getConcreteVisibleIndices() {
+        return visibleIndices.toArray(String[]::new);
+    }
+
+    /**
+     * Returns all the concrete indices that are not hidden in the format of list of strings.
+     */
+    public List<String> getConcreteVisibleIndicesList() {
         return visibleIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are open.
+     * Returns all of the concrete indices that are open in the format of array of strings.
      */
     public String[] getConcreteAllOpenIndices() {
+        return allOpenIndices.toArray(String[]::new);
+    }
+
+    /**
+     * Returns all of the concrete indices that are open in the format of list of strings.
+     */
+    public List<String> getConcreteAllOpenIndicesList() {
         return allOpenIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are open and not hidden.
+     * Returns all of the concrete indices that are open and not hidden in the format of array of strings.
      */
     public String[] getConcreteVisibleOpenIndices() {
+        return visibleOpenIndices.toArray(String[]::new);
+    }
+
+    /**
+     * Returns all of the concrete indices that are open and not hidden in the format of list of strings.
+     */
+    public List<String> getConcreteVisibleOpenIndicesList() {
         return visibleOpenIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are closed.
+     * Returns all of the concrete indices that are closed in the format of array of strings.
      */
     public String[] getConcreteAllClosedIndices() {
+        return allClosedIndices.toArray(String[]::new);
+    }
+
+    /**
+     * Returns all of the concrete indices that are closed in the format of list of strings.
+     */
+    public List<String> getConcreteAllClosedIndicesList() {
         return allClosedIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are closed and not hidden.
+     * Returns all of the concrete indices that are closed and not hidden in the format of array of strings.
      */
     public String[] getConcreteVisibleClosedIndices() {
+        return visibleClosedIndices.toArray(String[]::new);
+    }
+
+    /**
+     * Returns all of the concrete indices that are closed and not hidden in the format of list of strings.
+     */
+    public List<String> getConcreteVisibleClosedIndicesList() {
         return visibleClosedIndices;
     }
 
@@ -1556,12 +1598,12 @@ protected Metadata buildMetadataWithPreviousIndicesLookups() {
                 indices,
                 templates,
                 customs,
-                Arrays.copyOf(previousMetadata.allIndices, previousMetadata.allIndices.length),
-                Arrays.copyOf(previousMetadata.visibleIndices, previousMetadata.visibleIndices.length),
-                Arrays.copyOf(previousMetadata.allOpenIndices, previousMetadata.allOpenIndices.length),
-                Arrays.copyOf(previousMetadata.visibleOpenIndices, previousMetadata.visibleOpenIndices.length),
-                Arrays.copyOf(previousMetadata.allClosedIndices, previousMetadata.allClosedIndices.length),
-                Arrays.copyOf(previousMetadata.visibleClosedIndices, previousMetadata.visibleClosedIndices.length),
+                previousMetadata.allIndices,
+                previousMetadata.visibleIndices,
+                previousMetadata.allOpenIndices,
+                previousMetadata.visibleOpenIndices,
+                previousMetadata.allClosedIndices,
+                previousMetadata.visibleClosedIndices,
                 Collections.unmodifiableSortedMap(previousMetadata.indicesLookup)
             );
         }
@@ -1657,17 +1699,6 @@ protected Metadata buildMetadataWithRecomputedIndicesLookups() {
 
             validateDataStreams(indicesLookup, (DataStreamMetadata) customs.get(DataStreamMetadata.TYPE));
 
-            // build all concrete indices arrays:
-            // TODO: I think we can remove these arrays. it isn't worth the effort, for operations on all indices.
-            // When doing an operation across all indices, most of the time is spent on actually going to all shards and
-            // do the required operations, the bottleneck isn't resolving expressions into concrete indices.
-            String[] allIndicesArray = allIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] visibleIndicesArray = visibleIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] allOpenIndicesArray = allOpenIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] visibleOpenIndicesArray = visibleOpenIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] allClosedIndicesArray = allClosedIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] visibleClosedIndicesArray = visibleClosedIndices.toArray(Strings.EMPTY_ARRAY);
-
             return new Metadata(
                 clusterUUID,
                 clusterUUIDCommitted,
@@ -1679,12 +1710,12 @@ protected Metadata buildMetadataWithRecomputedIndicesLookups() {
                 indices,
                 templates,
                 customs,
-                allIndicesArray,
-                visibleIndicesArray,
-                allOpenIndicesArray,
-                visibleOpenIndicesArray,
-                allClosedIndicesArray,
-                visibleClosedIndicesArray,
+                allIndices,
+                visibleIndices,
+                allOpenIndices,
+                visibleOpenIndices,
+                allClosedIndices,
+                visibleClosedIndices,
                 indicesLookup
             );
         }

server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java

@@ -1572,11 +1572,17 @@ private static void compareMetadata(
         if (compareIndicesLookups == true) {
             assertEquals(previousMetadata.indices(), newMetadata.indices());
             assertEquals(previousMetadata.getConcreteAllIndices(), newMetadata.getConcreteAllIndices());
+            assertEquals(previousMetadata.getConcreteAllIndicesList(), newMetadata.getConcreteAllIndicesList());
             assertEquals(previousMetadata.getConcreteAllClosedIndices(), newMetadata.getConcreteAllClosedIndices());
+            assertEquals(previousMetadata.getConcreteAllClosedIndicesList(), newMetadata.getConcreteAllClosedIndicesList());
             assertEquals(previousMetadata.getConcreteAllOpenIndices(), newMetadata.getConcreteAllOpenIndices());
+            assertEquals(previousMetadata.getConcreteAllOpenIndicesList(), newMetadata.getConcreteAllOpenIndicesList());
             assertEquals(previousMetadata.getConcreteVisibleIndices(), newMetadata.getConcreteVisibleIndices());
+            assertEquals(previousMetadata.getConcreteVisibleIndicesList(), newMetadata.getConcreteVisibleIndicesList());
             assertEquals(previousMetadata.getConcreteVisibleClosedIndices(), newMetadata.getConcreteVisibleClosedIndices());
+            assertEquals(previousMetadata.getConcreteVisibleClosedIndicesList(), newMetadata.getConcreteVisibleClosedIndicesList());
             assertEquals(previousMetadata.getConcreteVisibleOpenIndices(), newMetadata.getConcreteVisibleOpenIndices());
+            assertEquals(previousMetadata.getConcreteVisibleOpenIndicesList(), newMetadata.getConcreteVisibleOpenIndicesList());
             assertEquals(previousMetadata.getIndicesLookup(), newMetadata.getIndicesLookup());
             assertEquals(previousMetadata.getCustoms().get(DataStreamMetadata.TYPE), newMetadata.getCustoms().get(DataStreamMetadata.TYPE));
         }