[Backport 2.x] [Refactor] more ImmutableOpenMap to jdk Map in cluster…

… package (#7301) (#8261) This continues the refactor of ImmutableOpenMap to j.u.Map in the o.o.cluster package in preparation for removal of HPPC dependencies. Signed-off-by: Nicholas Walter Knize <[email protected]>
opensearch-project · Jun 26, 2023 · 7ee220b · 7ee220b
1 parent 78be6ba
commit 7ee220b
Show file tree

Hide file tree

Showing 57 changed files with 510 additions and 570 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -58,6 +58,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - 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))
+- [Refactor] more ImmutableOpenMap to jdk Map in cluster package ([#7301](https://github.com/opensearch-project/OpenSearch/pull/7301))
 
 ### 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
@@ -63,7 +63,7 @@ public class GetIndexResponseTests extends AbstractResponseTestCase<
     protected org.opensearch.action.admin.indices.get.GetIndexResponse createServerTestInstance(XContentType xContentType) {
         String[] indices = generateRandomStringArray(5, 5, false, false);
         final Map<String, MappingMetadata> mappings = new HashMap<>();
-        ImmutableOpenMap.Builder<String, List<AliasMetadata>> aliases = ImmutableOpenMap.builder();
+        final Map<String, List<AliasMetadata>> aliases = new HashMap<>();
         ImmutableOpenMap.Builder<String, Settings> settings = ImmutableOpenMap.builder();
         ImmutableOpenMap.Builder<String, Settings> defaultSettings = ImmutableOpenMap.builder();
         ImmutableOpenMap.Builder<String, String> dataStreams = ImmutableOpenMap.builder();
@@ -95,7 +95,7 @@ protected org.opensearch.action.admin.indices.get.GetIndexResponse createServerT
         return new org.opensearch.action.admin.indices.get.GetIndexResponse(
             indices,
             mappings,
-            aliases.build(),
+            aliases,
             settings.build(),
             defaultSettings.build(),
             dataStreams.build()
@@ -116,7 +116,7 @@ protected void assertInstances(
         assertEquals(serverTestInstance.getMappings(), clientInstance.getMappings());
         assertMapEquals(serverTestInstance.getSettings(), clientInstance.getSettings());
         assertMapEquals(serverTestInstance.defaultSettings(), clientInstance.getDefaultSettings());
-        assertMapEquals(serverTestInstance.getAliases(), clientInstance.getAliases());
+        assertEquals(serverTestInstance.getAliases(), clientInstance.getAliases());
     }
 
     private static MappingMetadata createMappingsForIndex() {

diff --git a/...rTest/java/org/opensearch/action/admin/indices/alias/ValidateIndicesAliasesRequestIT.java b/...rTest/java/org/opensearch/action/admin/indices/alias/ValidateIndicesAliasesRequestIT.java
@@ -107,8 +107,8 @@ public void testAllowed() {
         request.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("index").alias("alias"));
         assertAcked(client().admin().indices().aliases(request).actionGet());
         final GetAliasesResponse response = client().admin().indices().getAliases(new GetAliasesRequest("alias")).actionGet();
-        assertThat(response.getAliases().keys().size(), equalTo(1));
-        assertThat(response.getAliases().keys().iterator().next().value, equalTo("index"));
+        assertThat(response.getAliases().keySet().size(), equalTo(1));
+        assertThat(response.getAliases().keySet().iterator().next(), equalTo("index"));
         final List<AliasMetadata> aliasMetadata = response.getAliases().get("index");
         assertThat(aliasMetadata, hasSize(1));
         assertThat(aliasMetadata.get(0).alias(), equalTo("alias"));

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
@@ -32,7 +32,6 @@
 
 package org.opensearch.action.admin.indices.get;
 
-import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
 import org.opensearch.action.admin.indices.alias.Alias;
 import org.opensearch.action.admin.indices.get.GetIndexRequest.Feature;
 import org.opensearch.action.support.IndicesOptions;
@@ -288,7 +287,7 @@ private void assertEmptyOrOnlyDefaultMappings(GetIndexResponse response, String
     }
 
     private void assertAliases(GetIndexResponse response, String indexName) {
-        ImmutableOpenMap<String, List<AliasMetadata>> aliases = response.aliases();
+        final Map<String, List<AliasMetadata>> aliases = response.aliases();
         assertThat(aliases, notNullValue());
         assertThat(aliases.size(), equalTo(1));
         List<AliasMetadata> indexAliases = aliases.get(indexName);
@@ -311,8 +310,8 @@ private void assertEmptyMappings(GetIndexResponse response) {
 
     private void assertEmptyAliases(GetIndexResponse response) {
         assertThat(response.aliases(), notNullValue());
-        for (final ObjectObjectCursor<String, List<AliasMetadata>> entry : response.getAliases()) {
-            assertTrue(entry.value.isEmpty());
+        for (final List<AliasMetadata> entry : response.getAliases().values()) {
+            assertTrue(entry.isEmpty());
         }
     }
 }
diff --git a/server/src/internalClusterTest/java/org/opensearch/aliases/IndexAliasesIT.java b/server/src/internalClusterTest/java/org/opensearch/aliases/IndexAliasesIT.java
@@ -32,7 +32,6 @@
 
 package org.opensearch.aliases;
 
-import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
 import org.opensearch.action.admin.indices.alias.Alias;
 import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions;
 import org.opensearch.action.admin.indices.alias.get.GetAliasesResponse;
@@ -87,7 +86,6 @@
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_READ_ONLY;
 import static org.opensearch.index.query.QueryBuilders.rangeQuery;
 import static org.opensearch.index.query.QueryBuilders.termQuery;
-import static org.opensearch.test.hamcrest.CollectionAssertions.hasKey;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertBlocked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
@@ -96,6 +94,7 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.emptyArray;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
@@ -1044,8 +1043,8 @@ public void testIndicesGetAliases() throws Exception {
         assertAcked(admin().indices().prepareAliases().removeAlias("foobar", "foo"));
 
         getResponse = admin().indices().prepareGetAliases("foo").addIndices("foobar").get();
-        for (final ObjectObjectCursor<String, List<AliasMetadata>> entry : getResponse.getAliases()) {
-            assertTrue(entry.value.isEmpty());
+        for (final List<AliasMetadata> entry : getResponse.getAliases().values()) {
+            assertTrue(entry.isEmpty());
         }
         assertTrue(admin().indices().prepareGetAliases("foo").addIndices("foobar").get().getAliases().isEmpty());
     }

diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/ClusterStateDiffIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/ClusterStateDiffIT.java
@@ -56,7 +56,6 @@
 import org.opensearch.cluster.routing.UnassignedInfo;
 import org.opensearch.common.UUIDs;
 import org.opensearch.common.bytes.BytesReference;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.io.stream.BytesStreamOutput;
 import org.opensearch.common.io.stream.NamedWriteableAwareStreamInput;
 import org.opensearch.common.io.stream.NamedWriteableRegistry;
@@ -283,7 +282,7 @@ private ClusterState.Builder randomRoutingTable(ClusterState clusterState) {
         if (numberOfIndices > 0) {
             List<String> randomIndices = randomSubsetOf(
                 randomInt(numberOfIndices - 1),
-                clusterState.routingTable().indicesRouting().keys().toArray(String.class)
+                clusterState.routingTable().indicesRouting().keySet().toArray(new String[0])
             );
             for (String index : randomIndices) {
                 if (randomBoolean()) {
@@ -413,7 +412,7 @@ private interface RandomClusterPart<T> {
         /**
          * Returns list of parts from metadata
          */
-        ImmutableOpenMap<String, T> parts(ClusterState clusterState);
+        Map<String, T> parts(ClusterState clusterState);
 
         /**
          * Puts the part back into metadata
@@ -443,12 +442,12 @@ private interface RandomClusterPart<T> {
      */
     private <T> ClusterState randomClusterStateParts(ClusterState clusterState, String prefix, RandomClusterPart<T> randomPart) {
         ClusterState.Builder builder = ClusterState.builder(clusterState);
-        ImmutableOpenMap<String, T> parts = randomPart.parts(clusterState);
+        final Map<String, T> parts = randomPart.parts(clusterState);
         int partCount = parts.size();
         if (partCount > 0) {
             List<String> randomParts = randomSubsetOf(
                 randomInt(partCount - 1),
-                randomPart.parts(clusterState).keys().toArray(String.class)
+                randomPart.parts(clusterState).keySet().toArray(new String[0])
             );
             for (String part : randomParts) {
                 if (randomBoolean()) {
@@ -745,7 +744,7 @@ private ClusterState.Builder randomClusterStateCustoms(final ClusterState cluste
         return ClusterState.builder(randomClusterStateParts(clusterState, "custom", new RandomClusterPart<ClusterState.Custom>() {
 
             @Override
-            public ImmutableOpenMap<String, ClusterState.Custom> parts(ClusterState clusterState) {
+            public Map<String, ClusterState.Custom> parts(ClusterState clusterState) {
                 return clusterState.customs();
             }
 
@@ -769,12 +768,12 @@ public ClusterState.Custom randomCreate(String name) {
                                     new Snapshot(randomName("repo"), new SnapshotId(randomName("snap"), UUIDs.randomBase64UUID())),
                                     randomBoolean(),
                                     randomBoolean(),
-                                    SnapshotsInProgressSerializationTests.randomState(ImmutableOpenMap.of()),
+                                    SnapshotsInProgressSerializationTests.randomState(Map.of()),
                                     Collections.emptyList(),
                                     Collections.emptyList(),
                                     Math.abs(randomLong()),
                                     randomIntBetween(0, 1000),
-                                    ImmutableOpenMap.of(),
+                                    Map.of(),
                                     null,
                                     SnapshotInfoTests.randomUserMetadata(),
                                     randomVersion(random()),
@@ -789,7 +788,7 @@ public ClusterState.Custom randomCreate(String name) {
                                 new Snapshot(randomName("repo"), new SnapshotId(randomName("snap"), UUIDs.randomBase64UUID())),
                                 RestoreInProgress.State.fromValue((byte) randomIntBetween(0, 3)),
                                 emptyList(),
-                                ImmutableOpenMap.of()
+                                Map.of()
                             )
                         ).build();
                     default:

diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/IndicesLifecycleListenerIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/IndicesLifecycleListenerIT.java
@@ -131,7 +131,7 @@ public void beforeIndexCreated(Index index, Settings indexSettings) {
         } catch (Exception e) {
             assertTrue(e.getMessage().contains("failing on purpose"));
             ClusterStateResponse resp = client().admin().cluster().prepareState().get();
-            assertFalse(resp.getState().routingTable().indicesRouting().keys().contains("failed"));
+            assertFalse(resp.getState().routingTable().indicesRouting().keySet().contains("failed"));
         }
     }
 
@@ -181,7 +181,7 @@ public void testIndexStateShardChanged() throws Throwable {
         } catch (OpenSearchException e) {
             assertTrue(e.getMessage().contains("failing on purpose"));
             ClusterStateResponse resp = client().admin().cluster().prepareState().get();
-            assertFalse(resp.getState().routingTable().indicesRouting().keys().contains("failed"));
+            assertFalse(resp.getState().routingTable().indicesRouting().keySet().contains("failed"));
         }
 
         // create an index

diff --git a/...alClusterTest/java/org/opensearch/indices/replication/SegmentReplicationAllocationIT.java b/...alClusterTest/java/org/opensearch/indices/replication/SegmentReplicationAllocationIT.java
@@ -8,7 +8,6 @@
 
 package org.opensearch.indices.replication;
 
-import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.routing.IndexRoutingTable;
@@ -25,6 +24,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
@@ -232,19 +232,19 @@ private void verifyPerIndexPrimaryBalance() throws Exception {
         assertBusy(() -> {
             final ClusterState currentState = client().admin().cluster().prepareState().execute().actionGet().getState();
             RoutingNodes nodes = currentState.getRoutingNodes();
-            for (ObjectObjectCursor<String, IndexRoutingTable> index : currentState.getRoutingTable().indicesRouting()) {
-                final int totalPrimaryShards = index.value.primaryShardsActive();
+            for (final Map.Entry<String, IndexRoutingTable> index : currentState.getRoutingTable().indicesRouting().entrySet()) {
+                final int totalPrimaryShards = index.getValue().primaryShardsActive();
                 final int avgPrimaryShardsPerNode = (int) Math.ceil(totalPrimaryShards * 1f / currentState.getRoutingNodes().size());
                 for (RoutingNode node : nodes) {
-                    final int primaryCount = node.shardsWithState(index.key, STARTED)
+                    final int primaryCount = node.shardsWithState(index.getKey(), STARTED)
                         .stream()
                         .filter(ShardRouting::primary)
                         .collect(Collectors.toList())
                         .size();
                     if (primaryCount > avgPrimaryShardsPerNode) {
                         logger.info(
                             "--> Primary shard balance assertion failure for index {} on node {} {} <= {}",
-                            index.key,
+                            index.getKey(),
                             node.node().getName(),
                             primaryCount,
                             avgPrimaryShardsPerNode
@@ -261,8 +261,8 @@ private void verifyPrimaryBalance() throws Exception {
             final ClusterState currentState = client().admin().cluster().prepareState().execute().actionGet().getState();
             RoutingNodes nodes = currentState.getRoutingNodes();
             int totalPrimaryShards = 0;
-            for (ObjectObjectCursor<String, IndexRoutingTable> index : currentState.getRoutingTable().indicesRouting()) {
-                totalPrimaryShards += index.value.primaryShardsActive();
+            for (final IndexRoutingTable index : currentState.getRoutingTable().indicesRouting().values()) {
+                totalPrimaryShards += index.primaryShardsActive();
             }
             final int avgPrimaryShardsPerNode = (int) Math.ceil(totalPrimaryShards * 1f / currentState.getRoutingNodes().size());
             for (RoutingNode node : nodes) {

diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsIT.java
@@ -31,8 +31,6 @@
 
 package org.opensearch.snapshots;
 
-import com.carrotsearch.hppc.cursors.ObjectCursor;
-
 import org.opensearch.OpenSearchException;
 import org.opensearch.action.ActionFuture;
 import org.opensearch.action.ActionListener;
@@ -1466,8 +1464,8 @@ private void createIndexWithContent(String indexName, String nodeInclude, String
     private static boolean snapshotHasCompletedShard(String snapshot, SnapshotsInProgress snapshotsInProgress) {
         for (SnapshotsInProgress.Entry entry : snapshotsInProgress.entries()) {
             if (entry.snapshot().getSnapshotId().getName().equals(snapshot)) {
-                for (ObjectCursor<SnapshotsInProgress.ShardSnapshotStatus> shard : entry.shards().values()) {
-                    if (shard.value.state().completed()) {
+                for (final SnapshotsInProgress.ShardSnapshotStatus shard : entry.shards().values()) {
+                    if (shard.state().completed()) {
                         return true;
                     }
                 }

diff --git a/...va/org/opensearch/action/admin/cluster/snapshots/restore/RestoreClusterStateListener.java b/...va/org/opensearch/action/admin/cluster/snapshots/restore/RestoreClusterStateListener.java
@@ -40,11 +40,11 @@
 import org.opensearch.cluster.ClusterStateListener;
 import org.opensearch.cluster.RestoreInProgress;
 import org.opensearch.cluster.service.ClusterService;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.index.shard.ShardId;
 import org.opensearch.snapshots.RestoreInfo;
 import org.opensearch.snapshots.RestoreService;
 
+import java.util.Map;
 import java.util.function.Function;
 
 import static org.opensearch.snapshots.RestoreService.restoreInProgress;
@@ -89,7 +89,7 @@ public void clusterChanged(ClusterChangedEvent changedEvent) {
             listener.onResponse(actionResponseFactory.apply(null));
         } else if (newEntry == null) {
             clusterService.removeListener(this);
-            ImmutableOpenMap<ShardId, RestoreInProgress.ShardRestoreStatus> shards = prevEntry.shards();
+            final Map<ShardId, RestoreInProgress.ShardRestoreStatus> shards = prevEntry.shards();
             assert prevEntry.state().completed() : "expected completed snapshot/remote store restore state but was " + prevEntry.state();
             assert RestoreService.completed(shards) : "expected all restore entries to be completed";
             RestoreInfo ri = new RestoreInfo(