[Backport 2.x] [Refactor] Metadata members and ClusterInfo from Immut…

…ableOpenMap to j.u.Map (opensearch-project#7126) (opensearch-project#7165) (opensearch-project#8241)

* [Refactor] ClusterInfo to use j.util.Map instead of ImmutableOpenMap (opensearch-project#7126)

With java.util.Map immutability and collection improvements the
hppc ImmutableOpenMap is not needed in ClusterInfo. This commit
refactors ClusterInfo to use java Maps and Immutable Collections and
further trim the dependency on the aging hppc library.

* [Refactor] Metadata members from ImmutableOpenMap to j.u.Map (opensearch-project#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]>

CHANGELOG.md

@@ -57,6 +57,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - 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] Sets util from server to common lib ([#8230](https://github.com/opensearch-project/OpenSearch/pull/8230))
+- [Refactor] Metadata members from ImmutableOpenMap to j.u.Map ([#7165](https://github.com/opensearch-project/OpenSearch/pull/7165))
 
 ### Deprecated
 

client/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());

client/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() {

server/src/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");

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);

server/src/internalClusterTest/java/org/opensearch/cluster/ClusterInfoServiceIT.java

@@ -32,7 +32,6 @@
 
 package org.opensearch.cluster;
 
-import com.carrotsearch.hppc.cursors.ObjectCursor;
 import org.opensearch.OpenSearchException;
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.ActionRequest;
@@ -47,7 +46,6 @@
 import org.opensearch.cluster.routing.allocation.decider.EnableAllocationDecider;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.Strings;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.index.IndexService;
@@ -69,6 +67,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -174,24 +173,24 @@ public void testClusterInfoServiceCollectsInformation() {
         infoService.setUpdateFrequency(TimeValue.timeValueMillis(200));
         ClusterInfo info = infoService.refresh();
         assertNotNull("info should not be null", info);
-        ImmutableOpenMap<String, DiskUsage> leastUsages = info.getNodeLeastAvailableDiskUsages();
-        ImmutableOpenMap<String, DiskUsage> mostUsages = info.getNodeMostAvailableDiskUsages();
-        ImmutableOpenMap<String, Long> shardSizes = info.shardSizes;
+        final Map<String, DiskUsage> leastUsages = info.getNodeLeastAvailableDiskUsages();
+        final Map<String, DiskUsage> mostUsages = info.getNodeMostAvailableDiskUsages();
+        final Map<String, Long> shardSizes = info.shardSizes;
         assertNotNull(leastUsages);
         assertNotNull(shardSizes);
         assertThat("some usages are populated", leastUsages.values().size(), Matchers.equalTo(2));
         assertThat("some shard sizes are populated", shardSizes.values().size(), greaterThan(0));
-        for (ObjectCursor<DiskUsage> usage : leastUsages.values()) {
-            logger.info("--> usage: {}", usage.value);
-            assertThat("usage has be retrieved", usage.value.getFreeBytes(), greaterThan(0L));
+        for (Map.Entry<String, DiskUsage> usage : leastUsages.entrySet()) {
+            logger.info("--> usage: {}", usage.getValue());
+            assertThat("usage has be retrieved", usage.getValue().getFreeBytes(), greaterThan(0L));
         }
-        for (ObjectCursor<DiskUsage> usage : mostUsages.values()) {
-            logger.info("--> usage: {}", usage.value);
-            assertThat("usage has be retrieved", usage.value.getFreeBytes(), greaterThan(0L));
+        for (DiskUsage usage : mostUsages.values()) {
+            logger.info("--> usage: {}", usage);
+            assertThat("usage has be retrieved", usage.getFreeBytes(), greaterThan(0L));
         }
-        for (ObjectCursor<Long> size : shardSizes.values()) {
-            logger.info("--> shard size: {}", size.value);
-            assertThat("shard size is greater than 0", size.value, greaterThanOrEqualTo(0L));
+        for (Long size : shardSizes.values()) {
+            logger.info("--> shard size: {}", size);
+            assertThat("shard size is greater than 0", size, greaterThanOrEqualTo(0L));
         }
         ClusterService clusterService = internalTestCluster.getInstance(ClusterService.class, internalTestCluster.getClusterManagerName());
         ClusterState state = clusterService.state();

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();
             }
 

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));
         }
     }

server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java

@@ -574,7 +574,7 @@ private void refreshDiskUsage() {
         // if the nodes were all under the low watermark already (but unbalanced) then a change in the disk usage doesn't trigger a reroute
         // even though it's now possible to achieve better balance, so we have to do an explicit reroute. TODO fix this?
         if (StreamSupport.stream(clusterInfoService.getClusterInfo().getNodeMostAvailableDiskUsages().values().spliterator(), false)
-            .allMatch(cur -> cur.value.getFreeBytes() > WATERMARK_BYTES)) {
+            .allMatch(cur -> cur.getFreeBytes() > WATERMARK_BYTES)) {
             assertAcked(client().admin().cluster().prepareReroute());
         }
 

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();
     }

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();

server/src/internalClusterTest/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();

server/src/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());
         }
     }
 }