From 688c76a4a30ec64766aad7fd8fe3a2200c5e8983 Mon Sep 17 00:00:00 2001 From: Abdul Muneer Kolarkunnu Date: Wed, 26 Jun 2024 17:51:50 +0530 Subject: [PATCH] [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 #8647 Signed-off-by: Abdul Muneer Kolarkunnu --- .../metadata/IndexNameExpressionResolver.java | 16 +-- .../opensearch/cluster/metadata/Metadata.java | 125 +++++++++++------- .../cluster/metadata/MetadataTests.java | 6 + 3 files changed, 92 insertions(+), 55 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java index 24ff83d638d4b..786be0a7f45be 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java @@ -706,7 +706,7 @@ public Map> resolveSearchRoutingAllIndices(Metadata metadata if (routing != null) { Set r = Sets.newHashSet(Strings.splitStringByCommaToArray(routing)); Map> routings = new HashMap<>(); - String[] concreteIndices = metadata.getConcreteAllIndices(); + Set concreteIndices = metadata.getConcreteAllIndicesList(); for (String index : concreteIndices) { routings.put(index, r); } @@ -746,7 +746,7 @@ static boolean isExplicitAllPattern(Collection 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 expressions) { private static List 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(); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index a0ef8de07fbf2..cdaf9c49ebf32 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/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 allIndices; + private final List visibleIndices; + private final List allOpenIndices; + private final List visibleOpenIndices; + private final List allClosedIndices; + private final List visibleClosedIndices; private final SortedMap indicesLookup; @@ -291,12 +291,12 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio final Map indices, final Map templates, final Map customs, - String[] allIndices, - String[] visibleIndices, - String[] allOpenIndices, - String[] visibleOpenIndices, - String[] allClosedIndices, - String[] visibleClosedIndices, + Set allIndices, + List visibleIndices, + List allOpenIndices, + List visibleOpenIndices, + List allClosedIndices, + List visibleClosedIndices, SortedMap 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 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 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 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 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 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 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 ); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java index 618fcb923bc60..221d8150d9913 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java +++ b/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)); }