[Refactor] Metadata members from ImmutableOpenMap to j.u.Map (#7165)

Refactors Metadata.{indices, templates, customs} member variables from ImmutableOpenMap to use jdk Maps. Usage of these variables across the codebase is also refactored to use jdk maps. Signed-off-by: Nicholas Walter Knize <[email protected]>
opensearch-project · Jun 23, 2023 · 802476f · 802476f
1 parent 5a8e522
commit 802476f
Show file tree

Hide file tree

Showing 56 changed files with 331 additions and 404 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,6 +55,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Allow insecure string settings to warn-log usage and advise to migration of a newer secure variant ([#5496](https://github.com/opensearch-project/OpenSearch/pull/5496))
 - Pass localNode info to all plugins on node start ([#7919](https://github.com/opensearch-project/OpenSearch/pull/7919)
 - Compress and cache cluster state during validate join request ([#7321](https://github.com/opensearch-project/OpenSearch/pull/7321))
+- [Refactor] Metadata members from ImmutableOpenMap to j.u.Map ([#7165](https://github.com/opensearch-project/OpenSearch/pull/7165))
 
 ### Deprecated
 

diff --git a/...nt/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexResponseTests.java b/...nt/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexResponseTests.java
@@ -62,7 +62,7 @@ public class GetIndexResponseTests extends AbstractResponseTestCase<
     @Override
     protected org.opensearch.action.admin.indices.get.GetIndexResponse createServerTestInstance(XContentType xContentType) {
         String[] indices = generateRandomStringArray(5, 5, false, false);
-        ImmutableOpenMap.Builder<String, MappingMetadata> mappings = ImmutableOpenMap.builder();
+        final Map<String, MappingMetadata> mappings = new HashMap<>();
         ImmutableOpenMap.Builder<String, List<AliasMetadata>> aliases = ImmutableOpenMap.builder();
         ImmutableOpenMap.Builder<String, Settings> settings = ImmutableOpenMap.builder();
         ImmutableOpenMap.Builder<String, Settings> defaultSettings = ImmutableOpenMap.builder();
@@ -94,7 +94,7 @@ protected org.opensearch.action.admin.indices.get.GetIndexResponse createServerT
         }
         return new org.opensearch.action.admin.indices.get.GetIndexResponse(
             indices,
-            mappings.build(),
+            mappings,
             aliases.build(),
             settings.build(),
             defaultSettings.build(),
@@ -113,7 +113,7 @@ protected void assertInstances(
         GetIndexResponse clientInstance
     ) {
         assertArrayEquals(serverTestInstance.getIndices(), clientInstance.getIndices());
-        assertMapEquals(serverTestInstance.getMappings(), clientInstance.getMappings());
+        assertEquals(serverTestInstance.getMappings(), clientInstance.getMappings());
         assertMapEquals(serverTestInstance.getSettings(), clientInstance.getSettings());
         assertMapEquals(serverTestInstance.defaultSettings(), clientInstance.getDefaultSettings());
         assertMapEquals(serverTestInstance.getAliases(), clientInstance.getAliases());

diff --git a/...rest-high-level/src/test/java/org/opensearch/client/indices/GetMappingsResponseTests.java b/...rest-high-level/src/test/java/org/opensearch/client/indices/GetMappingsResponseTests.java
@@ -34,7 +34,6 @@
 
 import org.opensearch.client.AbstractResponseTestCase;
 import org.opensearch.cluster.metadata.MappingMetadata;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.core.xcontent.XContentParser;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.index.mapper.MapperService;
@@ -50,12 +49,12 @@ public class GetMappingsResponseTests extends AbstractResponseTestCase<
 
     @Override
     protected org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse createServerTestInstance(XContentType xContentType) {
-        ImmutableOpenMap.Builder<String, MappingMetadata> mappings = ImmutableOpenMap.builder();
+        final Map<String, MappingMetadata> mappings = new HashMap<>();
         int numberOfIndexes = randomIntBetween(1, 5);
         for (int i = 0; i < numberOfIndexes; i++) {
             mappings.put("index-" + randomAlphaOfLength(5), randomMappingMetadata());
         }
-        return new org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse(mappings.build());
+        return new org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse(mappings);
     }
 
     @Override
@@ -68,7 +67,7 @@ protected void assertInstances(
         org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse serverTestInstance,
         GetMappingsResponse clientInstance
     ) {
-        assertMapEquals(serverTestInstance.getMappings(), clientInstance.mappings());
+        assertEquals(serverTestInstance.getMappings(), clientInstance.mappings());
     }
 
     public static MappingMetadata randomMappingMetadata() {

diff --git a/...rc/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java b/...rc/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java
@@ -45,7 +45,6 @@
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.MappingMetadata;
 import org.opensearch.cluster.metadata.Metadata;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.xcontent.XContentFactory;
@@ -59,6 +58,7 @@
 import org.opensearch.test.OpenSearchIntegTestCase.ClusterScope;
 import org.opensearch.test.OpenSearchIntegTestCase.Scope;
 
+import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiFunction;
@@ -100,7 +100,7 @@ public void testCreationDateGenerated() {
         assertThat(state, notNullValue());
         Metadata metadata = state.getMetadata();
         assertThat(metadata, notNullValue());
-        ImmutableOpenMap<String, IndexMetadata> indices = metadata.getIndices();
+        final Map<String, IndexMetadata> indices = metadata.getIndices();
         assertThat(indices, notNullValue());
         assertThat(indices.size(), equalTo(1));
         IndexMetadata index = indices.get("test");

diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/get/GetIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/get/GetIndexIT.java
@@ -46,6 +46,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_METADATA_BLOCK;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_BLOCKS_METADATA;
@@ -271,15 +272,15 @@ private void assertNonEmptySettings(GetIndexResponse response, String indexName)
     }
 
     private void assertMappings(GetIndexResponse response, String indexName) {
-        ImmutableOpenMap<String, MappingMetadata> mappings = response.mappings();
+        final Map<String, MappingMetadata> mappings = response.mappings();
         assertThat(mappings, notNullValue());
         assertThat(mappings.size(), equalTo(1));
         MappingMetadata indexMappings = mappings.get(indexName);
         assertThat(indexMappings, notNullValue());
     }
 
     private void assertEmptyOrOnlyDefaultMappings(GetIndexResponse response, String indexName) {
-        ImmutableOpenMap<String, MappingMetadata> mappings = response.mappings();
+        final Map<String, MappingMetadata> mappings = response.mappings();
         assertThat(mappings, notNullValue());
         assertThat(mappings.size(), equalTo(1));
         MappingMetadata indexMappings = mappings.get(indexName);

diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/ClusterStateDiffIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/ClusterStateDiffIT.java
@@ -75,6 +75,7 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import static java.util.Collections.emptyList;
@@ -526,7 +527,7 @@ private interface RandomPart<T> {
         /**
          * Returns list of parts from metadata
          */
-        ImmutableOpenMap<String, T> parts(Metadata metadata);
+        Map<String, T> parts(Metadata metadata);
 
         /**
          * Puts the part back into metadata
@@ -556,10 +557,10 @@ private interface RandomPart<T> {
      */
     private <T> Metadata randomParts(Metadata metadata, String prefix, RandomPart<T> randomPart) {
         Metadata.Builder builder = Metadata.builder(metadata);
-        ImmutableOpenMap<String, T> parts = randomPart.parts(metadata);
+        final Map<String, T> parts = randomPart.parts(metadata);
         int partCount = parts.size();
         if (partCount > 0) {
-            List<String> randomParts = randomSubsetOf(randomInt(partCount - 1), randomPart.parts(metadata).keys().toArray(String.class));
+            List<String> randomParts = randomSubsetOf(randomInt(partCount - 1), randomPart.parts(metadata).keySet().toArray(new String[0]));
             for (String part : randomParts) {
                 if (randomBoolean()) {
                     randomPart.remove(builder, part);
@@ -583,7 +584,7 @@ private Metadata randomIndices(Metadata metadata) {
         return randomParts(metadata, "index", new RandomPart<IndexMetadata>() {
 
             @Override
-            public ImmutableOpenMap<String, IndexMetadata> parts(Metadata metadata) {
+            public Map<String, IndexMetadata> parts(Metadata metadata) {
                 return metadata.indices();
             }
 
@@ -645,7 +646,7 @@ public IndexMetadata randomChange(IndexMetadata part) {
     private Metadata randomTemplates(Metadata metadata) {
         return randomParts(metadata, "template", new RandomPart<IndexTemplateMetadata>() {
             @Override
-            public ImmutableOpenMap<String, IndexTemplateMetadata> parts(Metadata metadata) {
+            public Map<String, IndexTemplateMetadata> parts(Metadata metadata) {
                 return metadata.templates();
             }
 
@@ -702,7 +703,7 @@ private Metadata randomMetadataCustoms(final Metadata metadata) {
         return randomParts(metadata, "custom", new RandomPart<Metadata.Custom>() {
 
             @Override
-            public ImmutableOpenMap<String, Metadata.Custom> parts(Metadata metadata) {
+            public Map<String, Metadata.Custom> parts(Metadata metadata) {
                 return metadata.customs();
             }
 

diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/SimpleClusterStateIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/SimpleClusterStateIT.java
@@ -47,7 +47,6 @@
 import org.opensearch.common.Priority;
 import org.opensearch.common.Strings;
 import org.opensearch.common.UUIDs;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.io.stream.NamedWriteableRegistry;
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
@@ -65,7 +64,6 @@
 import org.opensearch.repositories.RepositoriesService;
 import org.opensearch.script.ScriptService;
 import org.opensearch.test.OpenSearchIntegTestCase;
-import org.opensearch.test.hamcrest.CollectionAssertions;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.watcher.ResourceWatcherService;
 
@@ -76,13 +74,15 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Supplier;
 
 import static org.opensearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertIndexTemplateExists;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.is;
@@ -238,14 +238,14 @@ private void testFilteringByIndexWorks(String[] indices, String[] expected) {
             .setIndices(indices)
             .get();
 
-        ImmutableOpenMap<String, IndexMetadata> metadata = clusterState.getState().getMetadata().indices();
+        final Map<String, IndexMetadata> metadata = clusterState.getState().getMetadata().indices();
         assertThat(metadata.size(), is(expected.length));
 
         RoutingTable routingTable = clusterState.getState().getRoutingTable();
         assertThat(routingTable.indicesRouting().size(), is(expected.length));
 
-        for (String expectedIndex : expected) {
-            assertThat(metadata, CollectionAssertions.hasKey(expectedIndex));
+        for (final String expectedIndex : expected) {
+            assertThat(metadata, hasKey(expectedIndex));
             assertThat(routingTable.hasIndex(expectedIndex), is(true));
         }
     }

diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/MetadataNodesIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/MetadataNodesIT.java
@@ -36,7 +36,6 @@
 import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse;
 import org.opensearch.cluster.coordination.Coordinator;
 import org.opensearch.cluster.metadata.IndexMetadata;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.discovery.Discovery;
 import org.opensearch.env.NodeEnvironment;
@@ -152,7 +151,7 @@ public void testMetaWrittenWhenIndexIsClosedAndMetaUpdated() throws Exception {
         );
 
         // make sure it was also written on red node although index is closed
-        ImmutableOpenMap<String, IndexMetadata> indicesMetadata = getIndicesMetadataOnNode(dataNode);
+        Map<String, IndexMetadata> indicesMetadata = getIndicesMetadataOnNode(dataNode);
         assertNotNull(((Map<String, ?>) (indicesMetadata.get(index).mapping().getSourceAsMap().get("properties"))).get("integer_field"));
         assertThat(indicesMetadata.get(index).getState(), equalTo(IndexMetadata.State.CLOSE));
 
@@ -239,7 +238,7 @@ private boolean indexDirectoryExists(String nodeName, Index index) {
         return false;
     }
 
-    private ImmutableOpenMap<String, IndexMetadata> getIndicesMetadataOnNode(String nodeName) {
+    private Map<String, IndexMetadata> getIndicesMetadataOnNode(String nodeName) {
         final Coordinator coordinator = (Coordinator) internalCluster().getInstance(Discovery.class, nodeName);
         return coordinator.getApplierState().getMetadata().getIndices();
     }

diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/RecoveryFromGatewayIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/RecoveryFromGatewayIT.java
@@ -32,8 +32,6 @@
 
 package org.opensearch.gateway;
 
-import com.carrotsearch.hppc.cursors.ObjectCursor;
-
 import org.opensearch.action.admin.cluster.configuration.AddVotingConfigExclusionsAction;
 import org.opensearch.action.admin.cluster.configuration.AddVotingConfigExclusionsRequest;
 import org.opensearch.action.admin.cluster.configuration.ClearVotingConfigExclusionsAction;
@@ -181,8 +179,7 @@ private Map<String, long[]> assertAndCapturePrimaryTerms(Map<String, long[]> pre
         }
         final Map<String, long[]> result = new HashMap<>();
         final ClusterState state = client().admin().cluster().prepareState().get().getState();
-        for (ObjectCursor<IndexMetadata> cursor : state.metadata().indices().values()) {
-            final IndexMetadata indexMetadata = cursor.value;
+        for (final IndexMetadata indexMetadata : state.metadata().indices().values()) {
             final String index = indexMetadata.getIndex().getName();
             final long[] previous = previousTerms.get(index);
             final long[] current = IntStream.range(0, indexMetadata.getNumberOfShards()).mapToLong(indexMetadata::primaryTerm).toArray();

diff --git a/...alClusterTest/java/org/opensearch/indices/replication/SegmentReplicationAllocationIT.java b/...alClusterTest/java/org/opensearch/indices/replication/SegmentReplicationAllocationIT.java
@@ -61,7 +61,6 @@ public void enablePreferPrimaryBalance() {
     /**
      * This test verifies that the overall primary balance is attained during allocation. This test verifies primary
      * balance per index and across all indices is maintained.
-     * @throws Exception
      */
     public void testGlobalPrimaryAllocation() throws Exception {
         internalCluster().startClusterManagerOnlyNode();

diff --git a/...rc/internalClusterTest/java/org/opensearch/operateAllIndices/DestructiveOperationsIT.java b/...rc/internalClusterTest/java/org/opensearch/operateAllIndices/DestructiveOperationsIT.java
@@ -32,7 +32,6 @@
 
 package org.opensearch.operateAllIndices;
 
-import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
 import org.opensearch.action.support.DestructiveOperations;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.IndexMetadata;
@@ -109,8 +108,8 @@ public void testCloseIndexDefaultBehaviour() throws Exception {
         }
 
         ClusterState state = client().admin().cluster().prepareState().get().getState();
-        for (ObjectObjectCursor<String, IndexMetadata> indexMetadataObjectObjectCursor : state.getMetadata().indices()) {
-            assertEquals(IndexMetadata.State.CLOSE, indexMetadataObjectObjectCursor.value.getState());
+        for (final IndexMetadata indexMetadataObjectObjectCursor : state.getMetadata().indices().values()) {
+            assertEquals(IndexMetadata.State.CLOSE, indexMetadataObjectObjectCursor.getState());
         }
     }
 
@@ -141,8 +140,8 @@ public void testOpenIndexDefaultBehaviour() throws Exception {
         }
 
         ClusterState state = client().admin().cluster().prepareState().get().getState();
-        for (ObjectObjectCursor<String, IndexMetadata> indexMetadataObjectObjectCursor : state.getMetadata().indices()) {
-            assertEquals(IndexMetadata.State.OPEN, indexMetadataObjectObjectCursor.value.getState());
+        for (final IndexMetadata indexMetadataObjectObjectCursor : state.getMetadata().indices().values()) {
+            assertEquals(IndexMetadata.State.OPEN, indexMetadataObjectObjectCursor.getState());
         }
     }
 }
diff --git a/server/src/internalClusterTest/java/org/opensearch/recovery/RelocationIT.java b/server/src/internalClusterTest/java/org/opensearch/recovery/RelocationIT.java
@@ -32,7 +32,6 @@
 
 package org.opensearch.recovery;
 
-import com.carrotsearch.hppc.cursors.ObjectCursor;
 import org.apache.lucene.index.IndexFileNames;
 import org.apache.lucene.tests.util.English;
 import org.opensearch.action.ActionFuture;
@@ -771,8 +770,8 @@ public void testRelocationEstablishedPeerRecoveryRetentionLeases() throws Except
 
     private void assertActiveCopiesEstablishedPeerRecoveryRetentionLeases() throws Exception {
         assertBusy(() -> {
-            for (ObjectCursor<String> it : client().admin().cluster().prepareState().get().getState().metadata().indices().keys()) {
-                Map<ShardId, List<ShardStats>> byShardId = Stream.of(client().admin().indices().prepareStats(it.value).get().getShards())
+            for (final String it : client().admin().cluster().prepareState().get().getState().metadata().indices().keySet()) {
+                Map<ShardId, List<ShardStats>> byShardId = Stream.of(client().admin().indices().prepareStats(it).get().getShards())
                     .collect(Collectors.groupingBy(l -> l.getShardRouting().shardId()));
                 for (List<ShardStats> shardStats : byShardId.values()) {
                     Set<String> expectedLeaseIds = shardStats.stream()

diff --git a/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java b/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java
@@ -663,7 +663,6 @@ public void testStrictWeightedRoutingWithCustomString_FailOpenDisabled() throws
 
     /**
      * Should failopen shards even if failopen enabled with custom search preference.
-     * @throws Exception
      */
     public void testStrictWeightedRoutingWithShardPrefNetworkDisruption_FailOpenEnabled() throws Exception {
         Settings commonSettings = Settings.builder()

diff --git a/.../src/main/java/org/opensearch/action/admin/cluster/state/TransportClusterStateAction.java b/.../src/main/java/org/opensearch/action/admin/cluster/state/TransportClusterStateAction.java
@@ -57,6 +57,7 @@
 
 import java.io.IOException;
 import java.util.function.Predicate;
+import java.util.Map;
 
 /**
  * Transport action for obtaining cluster state
@@ -212,9 +213,9 @@ private ClusterStateResponse buildResponse(final ClusterStateRequest request, fi
             }
 
             // filter out metadata that shouldn't be returned by the API
-            for (ObjectObjectCursor<String, Custom> custom : currentState.metadata().customs()) {
-                if (custom.value.context().contains(Metadata.XContentContext.API) == false) {
-                    mdBuilder.removeCustom(custom.key);
+            for (final Map.Entry<String, Custom> custom : currentState.metadata().customs().entrySet()) {
+                if (custom.getValue().context().contains(Metadata.XContentContext.API) == false) {
+                    mdBuilder.removeCustom(custom.getKey());
                 }
             }
         }

diff --git a/...g/opensearch/action/admin/indices/dangling/delete/TransportDeleteDanglingIndexAction.java b/...g/opensearch/action/admin/indices/dangling/delete/TransportDeleteDanglingIndexAction.java
@@ -32,7 +32,6 @@
 
 package org.opensearch.action.admin.indices.dangling.delete;
 
-import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.OpenSearchException;
@@ -186,8 +185,8 @@ public void onFailure(Exception e) {
     private ClusterState deleteDanglingIndex(ClusterState currentState, Index indexToDelete) {
         final Metadata metaData = currentState.getMetadata();
 
-        for (ObjectObjectCursor<String, IndexMetadata> each : metaData.indices()) {
-            if (indexToDelete.getUUID().equals(each.value.getIndexUUID())) {
+        for (final IndexMetadata each : metaData.indices().values()) {
+            if (indexToDelete.getUUID().equals(each.getIndexUUID())) {
                 throw new IllegalArgumentException(
                     "Refusing to delete dangling index "
                         + indexToDelete