From fd0e25d2a2f3e893f9ae6a2cbf2e8aa372de3cbf Mon Sep 17 00:00:00 2001 From: Nick Knize Date: Thu, 13 Apr 2023 10:39:15 -0500 Subject: [PATCH] [Refactor] ClusterInfo to use j.util.Map instead of ImmutableOpenMap (#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. Signed-off-by: Nicholas Walter Knize --- .../cluster/ClusterInfoServiceIT.java | 27 ++- .../decider/DiskThresholdDeciderIT.java | 2 +- .../org/opensearch/cluster/ClusterInfo.java | 82 ++++---- .../cluster/InternalClusterInfoService.java | 72 +++---- .../allocation/DiskThresholdMonitor.java | 17 +- .../decider/DiskThresholdDecider.java | 19 +- .../opensearch/cluster/ClusterInfoTests.java | 28 +-- .../opensearch/cluster/DiskUsageTests.java | 15 +- .../allocation/DiskThresholdMonitorTests.java | 121 +++++------ ...dexShardConstraintDeciderOverlapTests.java | 54 +++-- .../RemoteShardsBalancerBaseTestCase.java | 10 +- .../decider/DiskThresholdDeciderTests.java | 188 ++++++++---------- .../DiskThresholdDeciderUnitTests.java | 63 ++---- 13 files changed, 309 insertions(+), 389 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/ClusterInfoServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/ClusterInfoServiceIT.java index b133b864a6b82..7ec5daf2b908b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/ClusterInfoServiceIT.java +++ b/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 leastUsages = info.getNodeLeastAvailableDiskUsages(); - ImmutableOpenMap mostUsages = info.getNodeMostAvailableDiskUsages(); - ImmutableOpenMap shardSizes = info.shardSizes; + final Map leastUsages = info.getNodeLeastAvailableDiskUsages(); + final Map mostUsages = info.getNodeMostAvailableDiskUsages(); + final Map 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 usage : leastUsages.values()) { - logger.info("--> usage: {}", usage.value); - assertThat("usage has be retrieved", usage.value.getFreeBytes(), greaterThan(0L)); + for (Map.Entry usage : leastUsages.entrySet()) { + logger.info("--> usage: {}", usage.getValue()); + assertThat("usage has be retrieved", usage.getValue().getFreeBytes(), greaterThan(0L)); } - for (ObjectCursor 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 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(); diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java index bbe9db135ff5f..ed8c94d10c36f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java +++ b/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()); } diff --git a/server/src/main/java/org/opensearch/cluster/ClusterInfo.java b/server/src/main/java/org/opensearch/cluster/ClusterInfo.java index eb728e8fb5035..eb3f1527ba326 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterInfo.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterInfo.java @@ -34,9 +34,7 @@ import com.carrotsearch.hppc.ObjectHashSet; import com.carrotsearch.hppc.cursors.ObjectCursor; -import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.io.stream.Writeable; @@ -47,6 +45,7 @@ import org.opensearch.index.store.StoreStats; import java.io.IOException; +import java.util.Collections; import java.util.Map; import java.util.Objects; @@ -59,15 +58,15 @@ * @opensearch.internal */ public class ClusterInfo implements ToXContentFragment, Writeable { - private final ImmutableOpenMap leastAvailableSpaceUsage; - private final ImmutableOpenMap mostAvailableSpaceUsage; - final ImmutableOpenMap shardSizes; + private final Map leastAvailableSpaceUsage; + private final Map mostAvailableSpaceUsage; + final Map shardSizes; // pkg-private for testing only public static final ClusterInfo EMPTY = new ClusterInfo(); - final ImmutableOpenMap routingToDataPath; - final ImmutableOpenMap reservedSpace; + final Map routingToDataPath; + final Map reservedSpace; protected ClusterInfo() { - this(ImmutableOpenMap.of(), ImmutableOpenMap.of(), ImmutableOpenMap.of(), ImmutableOpenMap.of(), ImmutableOpenMap.of()); + this(Map.of(), Map.of(), Map.of(), Map.of(), Map.of()); } /** @@ -81,11 +80,11 @@ protected ClusterInfo() { * @see #shardIdentifierFromRouting */ public ClusterInfo( - ImmutableOpenMap leastAvailableSpaceUsage, - ImmutableOpenMap mostAvailableSpaceUsage, - ImmutableOpenMap shardSizes, - ImmutableOpenMap routingToDataPath, - ImmutableOpenMap reservedSpace + final Map leastAvailableSpaceUsage, + final Map mostAvailableSpaceUsage, + final Map shardSizes, + final Map routingToDataPath, + final Map reservedSpace ) { this.leastAvailableSpaceUsage = leastAvailableSpaceUsage; this.shardSizes = shardSizes; @@ -106,48 +105,39 @@ public ClusterInfo(StreamInput in) throws IOException { reservedSpaceMap = Map.of(); } - ImmutableOpenMap.Builder leastBuilder = ImmutableOpenMap.builder(); - this.leastAvailableSpaceUsage = leastBuilder.putAll(leastMap).build(); - ImmutableOpenMap.Builder mostBuilder = ImmutableOpenMap.builder(); - this.mostAvailableSpaceUsage = mostBuilder.putAll(mostMap).build(); - ImmutableOpenMap.Builder sizeBuilder = ImmutableOpenMap.builder(); - this.shardSizes = sizeBuilder.putAll(sizeMap).build(); - ImmutableOpenMap.Builder routingBuilder = ImmutableOpenMap.builder(); - this.routingToDataPath = routingBuilder.putAll(routingMap).build(); - ImmutableOpenMap.Builder reservedSpaceBuilder = ImmutableOpenMap.builder(); - this.reservedSpace = reservedSpaceBuilder.putAll(reservedSpaceMap).build(); + this.leastAvailableSpaceUsage = Collections.unmodifiableMap(leastMap); + this.mostAvailableSpaceUsage = Collections.unmodifiableMap(mostMap); + this.shardSizes = Collections.unmodifiableMap(sizeMap); + this.routingToDataPath = Collections.unmodifiableMap(routingMap); + this.reservedSpace = Collections.unmodifiableMap(reservedSpaceMap); } @Override public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(this.leastAvailableSpaceUsage.size()); - for (ObjectObjectCursor c : this.leastAvailableSpaceUsage) { - out.writeString(c.key); - c.value.writeTo(out); - } + out.writeMap(this.leastAvailableSpaceUsage, StreamOutput::writeString, (o, v) -> v.writeTo(o)); out.writeMap(this.mostAvailableSpaceUsage, StreamOutput::writeString, (o, v) -> v.writeTo(o)); out.writeMap(this.shardSizes, StreamOutput::writeString, (o, v) -> out.writeLong(v == null ? -1 : v)); out.writeMap(this.routingToDataPath, (o, k) -> k.writeTo(o), StreamOutput::writeString); if (out.getVersion().onOrAfter(StoreStats.RESERVED_BYTES_VERSION)) { - out.writeMap(this.reservedSpace); + out.writeMap(this.reservedSpace, (o, v) -> v.writeTo(o), (o, v) -> v.writeTo(o)); } } public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject("nodes"); { - for (ObjectObjectCursor c : this.leastAvailableSpaceUsage) { - builder.startObject(c.key); + for (Map.Entry c : this.leastAvailableSpaceUsage.entrySet()) { + builder.startObject(c.getKey()); { // node - builder.field("node_name", c.value.getNodeName()); + builder.field("node_name", c.getValue().getNodeName()); builder.startObject("least_available"); { - c.value.toShortXContent(builder); + c.getValue().toShortXContent(builder); } builder.endObject(); // end "least_available" builder.startObject("most_available"); { - DiskUsage most = this.mostAvailableSpaceUsage.get(c.key); + DiskUsage most = this.mostAvailableSpaceUsage.get(c.getKey()); if (most != null) { most.toShortXContent(builder); } @@ -160,26 +150,26 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); // end "nodes" builder.startObject("shard_sizes"); { - for (ObjectObjectCursor c : this.shardSizes) { - builder.humanReadableField(c.key + "_bytes", c.key, new ByteSizeValue(c.value)); + for (Map.Entry c : this.shardSizes.entrySet()) { + builder.humanReadableField(c.getKey() + "_bytes", c.getKey(), new ByteSizeValue(c.getValue())); } } builder.endObject(); // end "shard_sizes" builder.startObject("shard_paths"); { - for (ObjectObjectCursor c : this.routingToDataPath) { - builder.field(c.key.toString(), c.value); + for (Map.Entry c : this.routingToDataPath.entrySet()) { + builder.field(c.getKey().toString(), c.getValue()); } } builder.endObject(); // end "shard_paths" builder.startArray("reserved_sizes"); { - for (ObjectObjectCursor c : this.reservedSpace) { + for (Map.Entry c : this.reservedSpace.entrySet()) { builder.startObject(); { - builder.field("node_id", c.key.nodeId); - builder.field("path", c.key.path); - c.value.toXContent(builder, params); + builder.field("node_id", c.getKey().nodeId); + builder.field("path", c.getKey().path); + c.getValue().toXContent(builder, params); } builder.endObject(); // NodeAndPath } @@ -192,16 +182,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws * Returns a node id to disk usage mapping for the path that has the least available space on the node. * Note that this does not take account of reserved space: there may be another path with less available _and unreserved_ space. */ - public ImmutableOpenMap getNodeLeastAvailableDiskUsages() { - return this.leastAvailableSpaceUsage; + public Map getNodeLeastAvailableDiskUsages() { + return Collections.unmodifiableMap(this.leastAvailableSpaceUsage); } /** * Returns a node id to disk usage mapping for the path that has the most available space on the node. * Note that this does not take account of reserved space: there may be another path with more available _and unreserved_ space. */ - public ImmutableOpenMap getNodeMostAvailableDiskUsages() { - return this.mostAvailableSpaceUsage; + public Map getNodeMostAvailableDiskUsages() { + return Collections.unmodifiableMap(this.mostAvailableSpaceUsage); } /** diff --git a/server/src/main/java/org/opensearch/cluster/InternalClusterInfoService.java b/server/src/main/java/org/opensearch/cluster/InternalClusterInfoService.java index 052c320e9b268..0acc7bece439f 100644 --- a/server/src/main/java/org/opensearch/cluster/InternalClusterInfoService.java +++ b/server/src/main/java/org/opensearch/cluster/InternalClusterInfoService.java @@ -51,7 +51,6 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.allocation.DiskThresholdSettings; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; @@ -64,6 +63,7 @@ import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.ReceiveTimeoutTransportException; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -108,8 +108,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt private volatile TimeValue updateFrequency; - private volatile ImmutableOpenMap leastAvailableSpaceUsages; - private volatile ImmutableOpenMap mostAvailableSpaceUsages; + private volatile Map leastAvailableSpaceUsages; + private volatile Map mostAvailableSpaceUsages; private volatile IndicesStatsSummary indicesStatsSummary; // null if this node is not currently the cluster-manager private final AtomicReference refreshAndRescheduleRunnable = new AtomicReference<>(); @@ -120,8 +120,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt private final List> listeners = new CopyOnWriteArrayList<>(); public InternalClusterInfoService(Settings settings, ClusterService clusterService, ThreadPool threadPool, Client client) { - this.leastAvailableSpaceUsages = ImmutableOpenMap.of(); - this.mostAvailableSpaceUsages = ImmutableOpenMap.of(); + this.leastAvailableSpaceUsages = Map.of(); + this.mostAvailableSpaceUsages = Map.of(); this.indicesStatsSummary = IndicesStatsSummary.EMPTY; this.threadPool = threadPool; this.client = client; @@ -180,14 +180,14 @@ public void clusterChanged(ClusterChangedEvent event) { if (removedNode.isDataNode()) { logger.trace("Removing node from cluster info: {}", removedNode.getId()); if (leastAvailableSpaceUsages.containsKey(removedNode.getId())) { - ImmutableOpenMap.Builder newMaxUsages = ImmutableOpenMap.builder(leastAvailableSpaceUsages); + Map newMaxUsages = new HashMap<>(leastAvailableSpaceUsages); newMaxUsages.remove(removedNode.getId()); - leastAvailableSpaceUsages = newMaxUsages.build(); + leastAvailableSpaceUsages = Collections.unmodifiableMap(newMaxUsages); } if (mostAvailableSpaceUsages.containsKey(removedNode.getId())) { - ImmutableOpenMap.Builder newMinUsages = ImmutableOpenMap.builder(mostAvailableSpaceUsages); + Map newMinUsages = new HashMap<>(mostAvailableSpaceUsages); newMinUsages.remove(removedNode.getId()); - mostAvailableSpaceUsages = newMinUsages.build(); + mostAvailableSpaceUsages = Collections.unmodifiableMap(newMinUsages); } } } @@ -254,16 +254,16 @@ public final ClusterInfo refresh() { final CountDownLatch nodeLatch = updateNodeStats(new ActionListener() { @Override public void onResponse(NodesStatsResponse nodesStatsResponse) { - ImmutableOpenMap.Builder leastAvailableUsagesBuilder = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder mostAvailableUsagesBuilder = ImmutableOpenMap.builder(); + final Map leastAvailableUsagesBuilder = new HashMap<>(); + final Map mostAvailableUsagesBuilder = new HashMap<>(); fillDiskUsagePerNode( logger, adjustNodesStats(nodesStatsResponse.getNodes()), leastAvailableUsagesBuilder, mostAvailableUsagesBuilder ); - leastAvailableSpaceUsages = leastAvailableUsagesBuilder.build(); - mostAvailableSpaceUsages = mostAvailableUsagesBuilder.build(); + leastAvailableSpaceUsages = Collections.unmodifiableMap(leastAvailableUsagesBuilder); + mostAvailableSpaceUsages = Collections.unmodifiableMap(mostAvailableUsagesBuilder); } @Override @@ -279,29 +279,25 @@ public void onFailure(Exception e) { logger.warn("Failed to execute NodeStatsAction for ClusterInfoUpdateJob", e); } // we empty the usages list, to be safe - we don't know what's going on. - leastAvailableSpaceUsages = ImmutableOpenMap.of(); - mostAvailableSpaceUsages = ImmutableOpenMap.of(); + leastAvailableSpaceUsages = Map.of(); + mostAvailableSpaceUsages = Map.of(); } } }); - final CountDownLatch indicesLatch = updateIndicesStats(new ActionListener() { + final CountDownLatch indicesLatch = updateIndicesStats(new ActionListener<>() { @Override public void onResponse(IndicesStatsResponse indicesStatsResponse) { final ShardStats[] stats = indicesStatsResponse.getShards(); - final ImmutableOpenMap.Builder shardSizeByIdentifierBuilder = ImmutableOpenMap.builder(); - final ImmutableOpenMap.Builder dataPathByShardRoutingBuilder = ImmutableOpenMap.builder(); + final Map shardSizeByIdentifierBuilder = new HashMap<>(); + final Map dataPathByShardRoutingBuilder = new HashMap<>(); final Map reservedSpaceBuilders = new HashMap<>(); buildShardLevelInfo(logger, stats, shardSizeByIdentifierBuilder, dataPathByShardRoutingBuilder, reservedSpaceBuilders); - final ImmutableOpenMap.Builder rsrvdSpace = ImmutableOpenMap.builder(); + final Map rsrvdSpace = new HashMap<>(); reservedSpaceBuilders.forEach((nodeAndPath, builder) -> rsrvdSpace.put(nodeAndPath, builder.build())); - indicesStatsSummary = new IndicesStatsSummary( - shardSizeByIdentifierBuilder.build(), - dataPathByShardRoutingBuilder.build(), - rsrvdSpace.build() - ); + indicesStatsSummary = new IndicesStatsSummary(shardSizeByIdentifierBuilder, dataPathByShardRoutingBuilder, rsrvdSpace); } @Override @@ -360,9 +356,9 @@ public void addListener(Consumer clusterInfoConsumer) { static void buildShardLevelInfo( Logger logger, ShardStats[] stats, - ImmutableOpenMap.Builder shardSizes, - ImmutableOpenMap.Builder newShardRoutingToDataPath, - Map reservedSpaceByShard + final Map shardSizes, + final Map newShardRoutingToDataPath, + final Map reservedSpaceByShard ) { for (ShardStats s : stats) { final ShardRouting shardRouting = s.getShardRouting(); @@ -392,8 +388,8 @@ static void buildShardLevelInfo( static void fillDiskUsagePerNode( Logger logger, List nodeStatsArray, - ImmutableOpenMap.Builder newLeastAvailableUsages, - ImmutableOpenMap.Builder newMostAvailableUsages + final Map newLeastAvailableUsages, + final Map newMostAvailableUsages ) { for (NodeStats nodeStats : nodeStatsArray) { if (nodeStats.getFs() == null) { @@ -475,20 +471,16 @@ static void fillDiskUsagePerNode( * @opensearch.internal */ private static class IndicesStatsSummary { - static final IndicesStatsSummary EMPTY = new IndicesStatsSummary( - ImmutableOpenMap.of(), - ImmutableOpenMap.of(), - ImmutableOpenMap.of() - ); + static final IndicesStatsSummary EMPTY = new IndicesStatsSummary(Map.of(), Map.of(), Map.of()); - final ImmutableOpenMap shardSizes; - final ImmutableOpenMap shardRoutingToDataPath; - final ImmutableOpenMap reservedSpace; + final Map shardSizes; + final Map shardRoutingToDataPath; + final Map reservedSpace; IndicesStatsSummary( - ImmutableOpenMap shardSizes, - ImmutableOpenMap shardRoutingToDataPath, - ImmutableOpenMap reservedSpace + final Map shardSizes, + final Map shardRoutingToDataPath, + final Map reservedSpace ) { this.shardSizes = shardSizes; this.shardRoutingToDataPath = shardRoutingToDataPath; diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitor.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitor.java index 6f63aff2f3a90..5bf1a3b199919 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitor.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitor.java @@ -32,8 +32,6 @@ package org.opensearch.cluster.routing.allocation; -import com.carrotsearch.hppc.ObjectLookupContainer; -import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; @@ -63,6 +61,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; @@ -145,7 +144,7 @@ public void onNewInfo(ClusterInfo info) { return; } - final ImmutableOpenMap usages = info.getNodeLeastAvailableDiskUsages(); + final Map usages = info.getNodeLeastAvailableDiskUsages(); if (usages == null) { logger.trace("skipping monitor as no disk usage information is available"); checkFinished(); @@ -159,7 +158,7 @@ public void onNewInfo(ClusterInfo info) { final long currentTimeMillis = currentTimeMillisSupplier.getAsLong(); // Clean up nodes that have been removed from the cluster - final ObjectLookupContainer nodes = usages.keys(); + final Set nodes = usages.keySet(); cleanUpRemovedNodes(nodes, nodesOverLowThreshold); cleanUpRemovedNodes(nodes, nodesOverHighThreshold); cleanUpRemovedNodes(nodes, nodesOverHighThresholdAndRelocating); @@ -172,9 +171,9 @@ public void onNewInfo(ClusterInfo info) { final List usagesOverHighThreshold = new ArrayList<>(); - for (final ObjectObjectCursor entry : usages) { - final String node = entry.key; - final DiskUsage usage = entry.value; + for (final Map.Entry entry : usages.entrySet()) { + final String node = entry.getKey(); + final DiskUsage usage = entry.getValue(); final RoutingNode routingNode = routingNodes.node(node); if (usage.getFreeBytes() < diskThresholdSettings.getFreeBytesThresholdFloodStage().getBytes() @@ -428,7 +427,7 @@ long sizeOfRelocatingShards(RoutingNode routingNode, DiskUsage diskUsage, Cluste private void markNodesMissingUsageIneligibleForRelease( RoutingNodes routingNodes, - ImmutableOpenMap usages, + Map usages, Set indicesToMarkIneligibleForAutoRelease ) { for (RoutingNode routingNode : routingNodes) { @@ -488,7 +487,7 @@ protected void updateIndicesReadOnly(Set indicesToUpdate, ActionListener .execute(ActionListener.map(wrappedListener, r -> null)); } - private static void cleanUpRemovedNodes(ObjectLookupContainer nodesToKeep, Set nodesToCleanUp) { + private static void cleanUpRemovedNodes(Set nodesToKeep, Set nodesToCleanUp) { for (String node : nodesToCleanUp) { if (nodesToKeep.contains(node) == false) { nodesToCleanUp.remove(node); diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDecider.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDecider.java index ffc80fbc973cb..ddd5e9274f08b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDecider.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDecider.java @@ -32,7 +32,6 @@ package org.opensearch.cluster.routing.allocation.decider; -import com.carrotsearch.hppc.cursors.ObjectCursor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.Version; @@ -49,7 +48,6 @@ import org.opensearch.cluster.routing.allocation.DiskThresholdSettings; import org.opensearch.cluster.routing.allocation.RoutingAllocation; import org.opensearch.common.Strings; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; @@ -59,6 +57,7 @@ import org.opensearch.snapshots.SnapshotShardSizeInfo; import java.util.List; +import java.util.Map; import java.util.Set; import static org.opensearch.cluster.routing.allocation.DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING; @@ -168,7 +167,7 @@ public static long sizeOfRelocatingShards( @Override public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) { ClusterInfo clusterInfo = allocation.clusterInfo(); - ImmutableOpenMap usages = clusterInfo.getNodeMostAvailableDiskUsages(); + Map usages = clusterInfo.getNodeMostAvailableDiskUsages(); final Decision decision = earlyTerminate(allocation, usages); if (decision != null) { return decision; @@ -424,7 +423,7 @@ public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAl throw new IllegalArgumentException("Shard [" + shardRouting + "] is not allocated on node: [" + node.nodeId() + "]"); } final ClusterInfo clusterInfo = allocation.clusterInfo(); - final ImmutableOpenMap usages = clusterInfo.getNodeLeastAvailableDiskUsages(); + final Map usages = clusterInfo.getNodeLeastAvailableDiskUsages(); final Decision decision = earlyTerminate(allocation, usages); if (decision != null) { return decision; @@ -520,7 +519,7 @@ public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAl private DiskUsageWithRelocations getDiskUsage( RoutingNode node, RoutingAllocation allocation, - ImmutableOpenMap usages, + final Map usages, boolean subtractLeavingShards ) { DiskUsage usage = usages.get(node.nodeId()); @@ -566,15 +565,15 @@ private DiskUsageWithRelocations getDiskUsage( * @param usages Map of nodeId to DiskUsage for all known nodes * @return DiskUsage representing given node using the average disk usage */ - DiskUsage averageUsage(RoutingNode node, ImmutableOpenMap usages) { + DiskUsage averageUsage(RoutingNode node, final Map usages) { if (usages.size() == 0) { return new DiskUsage(node.nodeId(), node.node().getName(), "_na_", 0, 0); } long totalBytes = 0; long freeBytes = 0; - for (ObjectCursor du : usages.values()) { - totalBytes += du.value.getTotalBytes(); - freeBytes += du.value.getFreeBytes(); + for (DiskUsage du : usages.values()) { + totalBytes += du.getTotalBytes(); + freeBytes += du.getFreeBytes(); } return new DiskUsage(node.nodeId(), node.node().getName(), "_na_", totalBytes / usages.size(), freeBytes / usages.size()); } @@ -598,7 +597,7 @@ DiskUsage averageUsage(RoutingNode node, ImmutableOpenMap usa return newUsage.getFreeDiskAsPercentage(); } - private Decision earlyTerminate(RoutingAllocation allocation, ImmutableOpenMap usages) { + private Decision earlyTerminate(RoutingAllocation allocation, final Map usages) { // Always allow allocation if the decider is disabled if (diskThresholdSettings.isEnabled() == false) { return allocation.decision(Decision.YES, NAME, "the disk threshold decider is disabled"); diff --git a/server/src/test/java/org/opensearch/cluster/ClusterInfoTests.java b/server/src/test/java/org/opensearch/cluster/ClusterInfoTests.java index 4abbef0c19374..a32d6e35d0182 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterInfoTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterInfoTests.java @@ -34,11 +34,13 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.cluster.routing.TestShardRouting; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.index.shard.ShardId; import org.opensearch.test.OpenSearchTestCase; +import java.util.HashMap; +import java.util.Map; + public class ClusterInfoTests extends OpenSearchTestCase { public void testSerialization() throws Exception { @@ -60,9 +62,9 @@ public void testSerialization() throws Exception { assertEquals(clusterInfo.reservedSpace, result.reservedSpace); } - private static ImmutableOpenMap randomDiskUsage() { + private static Map randomDiskUsage() { int numEntries = randomIntBetween(0, 128); - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(numEntries); + final Map builder = new HashMap<>(numEntries); for (int i = 0; i < numEntries; i++) { String key = randomAlphaOfLength(32); DiskUsage diskUsage = new DiskUsage( @@ -74,34 +76,34 @@ private static ImmutableOpenMap randomDiskUsage() { ); builder.put(key, diskUsage); } - return builder.build(); + return builder; } - private static ImmutableOpenMap randomShardSizes() { + private static Map randomShardSizes() { int numEntries = randomIntBetween(0, 128); - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(numEntries); + final Map builder = new HashMap<>(numEntries); for (int i = 0; i < numEntries; i++) { String key = randomAlphaOfLength(32); long shardSize = randomIntBetween(0, Integer.MAX_VALUE); builder.put(key, shardSize); } - return builder.build(); + return builder; } - private static ImmutableOpenMap randomRoutingToDataPath() { + private static Map randomRoutingToDataPath() { int numEntries = randomIntBetween(0, 128); - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(numEntries); + final Map builder = new HashMap<>(numEntries); for (int i = 0; i < numEntries; i++) { ShardId shardId = new ShardId(randomAlphaOfLength(32), randomAlphaOfLength(32), randomIntBetween(0, Integer.MAX_VALUE)); ShardRouting shardRouting = TestShardRouting.newShardRouting(shardId, null, randomBoolean(), ShardRoutingState.UNASSIGNED); builder.put(shardRouting, randomAlphaOfLength(32)); } - return builder.build(); + return builder; } - private static ImmutableOpenMap randomReservedSpace() { + private static Map randomReservedSpace() { int numEntries = randomIntBetween(0, 128); - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(numEntries); + final Map builder = new HashMap<>(numEntries); for (int i = 0; i < numEntries; i++) { final ClusterInfo.NodeAndPath key = new ClusterInfo.NodeAndPath(randomAlphaOfLength(10), randomAlphaOfLength(10)); final ClusterInfo.ReservedSpace.Builder valueBuilder = new ClusterInfo.ReservedSpace.Builder(); @@ -111,7 +113,7 @@ private static ImmutableOpenMap shardSizes = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder routingToPath = ImmutableOpenMap.builder(); - ClusterState state = ClusterState.builder(new ClusterName("blarg")).version(0).build(); + final Map shardSizes = new HashMap<>(); + final Map routingToPath = new HashMap<>(); InternalClusterInfoService.buildShardLevelInfo(logger, stats, shardSizes, routingToPath, new HashMap<>()); assertEquals(2, shardSizes.size()); assertTrue(shardSizes.containsKey(ClusterInfo.shardIdentifierFromRouting(test_0))); @@ -155,8 +154,8 @@ public void testFillShardLevelInfo() { } public void testFillDiskUsage() { - ImmutableOpenMap.Builder newLeastAvaiableUsages = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder newMostAvaiableUsages = ImmutableOpenMap.builder(); + final Map newLeastAvaiableUsages = new HashMap<>(); + final Map newMostAvaiableUsages = new HashMap<>(); FsInfo.Path[] node1FSInfo = new FsInfo.Path[] { new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80), new FsInfo.Path("/least", "/dev/sdb", 200, 190, 70), @@ -261,8 +260,8 @@ public void testFillDiskUsage() { } public void testFillDiskUsageSomeInvalidValues() { - ImmutableOpenMap.Builder newLeastAvailableUsages = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder newMostAvailableUsages = ImmutableOpenMap.builder(); + final Map newLeastAvailableUsages = new HashMap<>(); + final Map newMostAvailableUsages = new HashMap<>(); FsInfo.Path[] node1FSInfo = new FsInfo.Path[] { new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80), new FsInfo.Path("/least", "/dev/sdb", -1, -1, -1), diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitorTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitorTests.java index e4f3c4eeeb903..21d891bdbc317 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitorTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitorTests.java @@ -49,7 +49,6 @@ import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.common.Priority; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.index.shard.ShardId; @@ -58,7 +57,9 @@ import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -137,19 +138,19 @@ protected void setIndexCreateBlock(ActionListener listener, boolean indexC } }; - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(); + Map builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, 4)); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, 30)); - monitor.onNewInfo(clusterInfo(builder.build())); + monitor.onNewInfo(clusterInfo(builder)); assertFalse(reroute.get()); assertEquals(new HashSet<>(Arrays.asList("test_1", "test_2")), indices.get()); indices.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, 4)); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, 5)); currentTime.addAndGet(randomLongBetween(60001, 120000)); - monitor.onNewInfo(clusterInfo(builder.build())); + monitor.onNewInfo(clusterInfo(builder)); assertTrue(reroute.get()); assertEquals(new HashSet<>(Arrays.asList("test_1", "test_2")), indices.get()); IndexMetadata indexMetadata = IndexMetadata.builder(clusterState.metadata().index("test_2")) @@ -200,10 +201,10 @@ protected void setIndexCreateBlock(ActionListener listener, boolean indexC indices.set(null); reroute.set(false); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, 4)); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, 5)); - monitor.onNewInfo(clusterInfo(builder.build())); + monitor.onNewInfo(clusterInfo(builder)); assertTrue(reroute.get()); assertEquals(Collections.singleton("test_1"), indices.get()); } @@ -232,16 +233,13 @@ protected void updateIndicesReadOnly(Set indicesToMarkReadOnly, ActionLi } }; - final ImmutableOpenMap.Builder allDisksOkBuilder; - allDisksOkBuilder = ImmutableOpenMap.builder(); - allDisksOkBuilder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, 50)); - allDisksOkBuilder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, 50)); - final ImmutableOpenMap allDisksOk = allDisksOkBuilder.build(); + final Map allDisksOk = new HashMap<>(); + allDisksOk.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, 50)); + allDisksOk.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, 50)); - final ImmutableOpenMap.Builder oneDiskAboveWatermarkBuilder = ImmutableOpenMap.builder(); - oneDiskAboveWatermarkBuilder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(5, 9))); - oneDiskAboveWatermarkBuilder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, 50)); - final ImmutableOpenMap oneDiskAboveWatermark = oneDiskAboveWatermarkBuilder.build(); + final Map oneDiskAboveWatermark = new HashMap<>(); + oneDiskAboveWatermark.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(5, 9))); + oneDiskAboveWatermark.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, 50)); // should not reroute when all disks are ok currentTime.addAndGet(randomLongBetween(0, 120000)); @@ -308,12 +306,11 @@ protected void updateIndicesReadOnly(Set indicesToMarkReadOnly, ActionLi assertNull(listenerReference.get()); // should reroute again when one disk has reserved space that pushes it over the high watermark - final ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(1); - builder.put( + final Map reservedSpaces = new HashMap<>(1); + reservedSpaces.put( new ClusterInfo.NodeAndPath("node1", "/foo/bar"), new ClusterInfo.ReservedSpace.Builder().add(new ShardId("baz", "quux", 0), between(41, 100)).build() ); - final ImmutableOpenMap reservedSpaces = builder.build(); currentTime.addAndGet( randomLongBetween( @@ -324,7 +321,6 @@ protected void updateIndicesReadOnly(Set indicesToMarkReadOnly, ActionLi monitor.onNewInfo(clusterInfo(allDisksOk, reservedSpaces)); assertNotNull(listenerReference.get()); listenerReference.getAndSet(null).onResponse(null); - } public void testAutoReleaseIndices() { @@ -348,19 +344,17 @@ public void testAutoReleaseIndices() { ); assertThat(clusterState.getRoutingTable().shardsWithState(ShardRoutingState.STARTED).size(), equalTo(8)); - final ImmutableOpenMap.Builder reservedSpacesBuilder = ImmutableOpenMap - .builder(); + final Map reservedSpaces = new HashMap<>(); final int reservedSpaceNode1 = between(0, 10); - reservedSpacesBuilder.put( + reservedSpaces.put( new ClusterInfo.NodeAndPath("node1", "/foo/bar"), new ClusterInfo.ReservedSpace.Builder().add(new ShardId("", "", 0), reservedSpaceNode1).build() ); final int reservedSpaceNode2 = between(0, 10); - reservedSpacesBuilder.put( + reservedSpaces.put( new ClusterInfo.NodeAndPath("node2", "/foo/bar"), new ClusterInfo.ReservedSpace.Builder().add(new ShardId("", "", 0), reservedSpaceNode2).build() ); - ImmutableOpenMap reservedSpaces = reservedSpacesBuilder.build(); DiskThresholdMonitor monitor = new DiskThresholdMonitor( Settings.EMPTY, @@ -392,20 +386,20 @@ protected void setIndexCreateBlock(ActionListener listener, boolean indexC }; indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(); + Map builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(0, 4))); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, between(0, 4))); - monitor.onNewInfo(clusterInfo(builder.build(), reservedSpaces)); + monitor.onNewInfo(clusterInfo(builder, reservedSpaces)); assertEquals(new HashSet<>(Arrays.asList("test_1", "test_2")), indicesToMarkReadOnly.get()); assertNull(indicesToRelease.get()); // Reserved space is ignored when applying block indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(5, 90))); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, between(5, 90))); - monitor.onNewInfo(clusterInfo(builder.build(), reservedSpaces)); + monitor.onNewInfo(clusterInfo(builder, reservedSpaces)); assertNull(indicesToMarkReadOnly.get()); assertNull(indicesToRelease.get()); @@ -454,66 +448,66 @@ protected void setIndexCreateBlock(ActionListener listener, boolean indexC // When free disk on any of node1 or node2 goes below 5% flood watermark, then apply index block on indices not having the block indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(0, 100))); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, between(0, 4))); - monitor.onNewInfo(clusterInfo(builder.build(), reservedSpaces)); + monitor.onNewInfo(clusterInfo(builder, reservedSpaces)); assertThat(indicesToMarkReadOnly.get(), contains("test_1")); assertNull(indicesToRelease.get()); // When free disk on node1 and node2 goes above 10% high watermark then release index block, ignoring reserved space indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(10, 100))); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, between(10, 100))); - monitor.onNewInfo(clusterInfo(builder.build(), reservedSpaces)); + monitor.onNewInfo(clusterInfo(builder, reservedSpaces)); assertNull(indicesToMarkReadOnly.get()); assertThat(indicesToRelease.get(), contains("test_2")); // When no usage information is present for node2, we don't release the block indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(0, 4))); - monitor.onNewInfo(clusterInfo(builder.build())); + monitor.onNewInfo(clusterInfo(builder)); assertThat(indicesToMarkReadOnly.get(), contains("test_1")); assertNull(indicesToRelease.get()); // When disk usage on one node is between the high and flood-stage watermarks, nothing changes indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(5, 9))); builder.put("node2", new DiskUsage("node2", "node2", "/foo/bar", 100, between(5, 100))); if (randomBoolean()) { builder.put("node3", new DiskUsage("node3", "node3", "/foo/bar", 100, between(0, 100))); } - monitor.onNewInfo(clusterInfo(builder.build())); + monitor.onNewInfo(clusterInfo(builder)); assertNull(indicesToMarkReadOnly.get()); assertNull(indicesToRelease.get()); // When disk usage on one node is missing and the other is below the high watermark, nothing changes indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(5, 100))); if (randomBoolean()) { builder.put("node3", new DiskUsage("node3", "node3", "/foo/bar", 100, between(0, 100))); } - monitor.onNewInfo(clusterInfo(builder.build())); + monitor.onNewInfo(clusterInfo(builder)); assertNull(indicesToMarkReadOnly.get()); assertNull(indicesToRelease.get()); // When disk usage on one node is missing and the other is above the flood-stage watermark, affected indices are blocked indicesToMarkReadOnly.set(null); indicesToRelease.set(null); - builder = ImmutableOpenMap.builder(); + builder = new HashMap<>(); builder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(0, 4))); if (randomBoolean()) { builder.put("node3", new DiskUsage("node3", "node3", "/foo/bar", 100, between(0, 100))); } - monitor.onNewInfo(clusterInfo(builder.build())); + monitor.onNewInfo(clusterInfo(builder)); assertThat(indicesToMarkReadOnly.get(), contains("test_1")); assertNull(indicesToRelease.get()); } @@ -565,22 +559,18 @@ protected void setIndexCreateBlock(ActionListener listener, boolean indexC } }; - final ImmutableOpenMap.Builder allDisksOkBuilder; - allDisksOkBuilder = ImmutableOpenMap.builder(); - allDisksOkBuilder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(15, 100))); - final ImmutableOpenMap allDisksOk = allDisksOkBuilder.build(); + final Map allDisksOk; + allDisksOk = new HashMap<>(); + allDisksOk.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(15, 100))); - final ImmutableOpenMap.Builder aboveLowWatermarkBuilder = ImmutableOpenMap.builder(); - aboveLowWatermarkBuilder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(10, 14))); - final ImmutableOpenMap aboveLowWatermark = aboveLowWatermarkBuilder.build(); + final Map aboveLowWatermark = new HashMap<>(); + aboveLowWatermark.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(10, 14))); - final ImmutableOpenMap.Builder aboveHighWatermarkBuilder = ImmutableOpenMap.builder(); - aboveHighWatermarkBuilder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(5, 9))); - final ImmutableOpenMap aboveHighWatermark = aboveHighWatermarkBuilder.build(); + final Map aboveHighWatermark = new HashMap<>(); + aboveHighWatermark.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(5, 9))); - final ImmutableOpenMap.Builder aboveFloodStageWatermarkBuilder = ImmutableOpenMap.builder(); - aboveFloodStageWatermarkBuilder.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(0, 4))); - final ImmutableOpenMap aboveFloodStageWatermark = aboveFloodStageWatermarkBuilder.build(); + final Map aboveFloodStageWatermark = new HashMap<>(); + aboveFloodStageWatermark.put("node1", new DiskUsage("node1", "node1", "/foo/bar", 100, between(0, 4))); assertNoLogging(monitor, allDisksOk); @@ -727,13 +717,12 @@ protected void setIndexCreateBlock(ActionListener listener, boolean indexC } }; - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(); - monitor.onNewInfo(clusterInfo(builder.build())); + final Map builder = new HashMap<>(); + monitor.onNewInfo(clusterInfo(builder)); assertTrue(countBlocksCalled.get() == 0); } - private void assertNoLogging(DiskThresholdMonitor monitor, ImmutableOpenMap diskUsages) - throws IllegalAccessException { + private void assertNoLogging(DiskThresholdMonitor monitor, final Map diskUsages) throws IllegalAccessException { try (MockLogAppender mockAppender = MockLogAppender.createForLoggers(LogManager.getLogger(DiskThresholdMonitor.class))) { mockAppender.addExpectation( new MockLogAppender.UnseenEventExpectation( @@ -760,26 +749,26 @@ private void assertNoLogging(DiskThresholdMonitor monitor, ImmutableOpenMap diskUsages, String message) + private void assertRepeatedWarningMessages(DiskThresholdMonitor monitor, final Map diskUsages, String message) throws IllegalAccessException { for (int i = between(1, 3); i >= 0; i--) { assertLogging(monitor, diskUsages, Level.WARN, message); } } - private void assertSingleWarningMessage(DiskThresholdMonitor monitor, ImmutableOpenMap diskUsages, String message) + private void assertSingleWarningMessage(DiskThresholdMonitor monitor, final Map diskUsages, String message) throws IllegalAccessException { assertLogging(monitor, diskUsages, Level.WARN, message); assertNoLogging(monitor, diskUsages); } - private void assertSingleInfoMessage(DiskThresholdMonitor monitor, ImmutableOpenMap diskUsages, String message) + private void assertSingleInfoMessage(DiskThresholdMonitor monitor, final Map diskUsages, String message) throws IllegalAccessException { assertLogging(monitor, diskUsages, Level.INFO, message); assertNoLogging(monitor, diskUsages); } - private void assertLogging(DiskThresholdMonitor monitor, ImmutableOpenMap diskUsages, Level level, String message) + private void assertLogging(DiskThresholdMonitor monitor, final Map diskUsages, Level level, String message) throws IllegalAccessException { try (MockLogAppender mockAppender = MockLogAppender.createForLoggers(LogManager.getLogger(DiskThresholdMonitor.class))) { mockAppender.start(); @@ -801,13 +790,13 @@ private void assertLogging(DiskThresholdMonitor monitor, ImmutableOpenMap diskUsages) { - return clusterInfo(diskUsages, ImmutableOpenMap.of()); + private static ClusterInfo clusterInfo(final Map diskUsages) { + return clusterInfo(diskUsages, Map.of()); } private static ClusterInfo clusterInfo( - ImmutableOpenMap diskUsages, - ImmutableOpenMap reservedSpace + final Map diskUsages, + final Map reservedSpace ) { return new ClusterInfo(diskUsages, null, null, null, reservedSpace); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/IndexShardConstraintDeciderOverlapTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/IndexShardConstraintDeciderOverlapTests.java index 24c6dcff42849..7112af6b4efc0 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/IndexShardConstraintDeciderOverlapTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/IndexShardConstraintDeciderOverlapTests.java @@ -15,7 +15,6 @@ import org.opensearch.cluster.OpenSearchAllocationWithConstraintsTestCase; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.Settings; import org.opensearch.index.shard.ShardId; import org.opensearch.test.VersionUtils; @@ -50,28 +49,22 @@ public void testHighWatermarkBreachWithLowShardCount() { .build(); // Build Shard size and disk usages - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node_0", new DiskUsage("node_0", "node_0", "/dev/null", 100, 80)); // 20% used - usagesBuilder.put("node_1", new DiskUsage("node_1", "node_1", "/dev/null", 100, 55)); // 45% used - usagesBuilder.put("node_2", new DiskUsage("node_2", "node_2", "/dev/null", 100, 35)); // 65% used - usagesBuilder.put("high_watermark_node_0", new DiskUsage("high_watermark_node_0", "high_watermark_node_0", "/dev/null", 100, 10)); // 90% - // used - - ImmutableOpenMap usages = usagesBuilder.build(); - ImmutableOpenMap.Builder shardSizesBuilder = ImmutableOpenMap.builder(); - clusterState.getRoutingTable().allShards().forEach(shard -> shardSizesBuilder.put(shardIdentifierFromRouting(shard), 1L)); // Each - // shard - // is 1 - // byte - ImmutableOpenMap shardSizes = shardSizesBuilder.build(); - - final ImmutableOpenMap reservedSpace = new ImmutableOpenMap.Builder< - ClusterInfo.NodeAndPath, - ClusterInfo.ReservedSpace>().fPut(getNodeAndDevNullPath("node_0"), getReservedSpace()) - .fPut(getNodeAndDevNullPath("node_1"), getReservedSpace()) - .fPut(getNodeAndDevNullPath("node_2"), getReservedSpace()) - .fPut(getNodeAndDevNullPath("high_watermark_node_0"), getReservedSpace()) - .build(); + final Map usages = new HashMap<>(); + usages.put("node_0", new DiskUsage("node_0", "node_0", "/dev/null", 100, 80)); // 20% used + usages.put("node_1", new DiskUsage("node_1", "node_1", "/dev/null", 100, 55)); // 45% used + usages.put("node_2", new DiskUsage("node_2", "node_2", "/dev/null", 100, 35)); // 65% used + usages.put("high_watermark_node_0", new DiskUsage("high_watermark_node_0", "high_watermark_node_0", "/dev/null", 100, 10)); // 90% + // used + final Map shardSizes = new HashMap<>(); + clusterState.getRoutingTable().allShards().forEach(shard -> shardSizes.put(shardIdentifierFromRouting(shard), 1L)); // Each + // shard + // is 1 + // byte + final Map reservedSpace = new HashMap<>(); + reservedSpace.put(getNodeAndDevNullPath("node_0"), getReservedSpace()); + reservedSpace.put(getNodeAndDevNullPath("node_1"), getReservedSpace()); + reservedSpace.put(getNodeAndDevNullPath("node_2"), getReservedSpace()); + reservedSpace.put(getNodeAndDevNullPath("high_watermark_node_0"), getReservedSpace()); final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes, reservedSpace); ClusterInfoService cis = () -> clusterInfo; allocation = createAllocationService(settings, cis); @@ -84,10 +77,9 @@ public void testHighWatermarkBreachWithLowShardCount() { /* Shard sizes that would breach high watermark on node_2 if allocated. */ addIndices("big_index_", 1, 10, 0); - ImmutableOpenMap.Builder bigIndexShardSizeBuilder = ImmutableOpenMap.builder(shardSizes); - clusterState.getRoutingNodes().unassigned().forEach(shard -> bigIndexShardSizeBuilder.put(shardIdentifierFromRouting(shard), 20L)); - shardSizes = bigIndexShardSizeBuilder.build(); - final ClusterInfo bigIndexClusterInfo = new DevNullClusterInfo(usages, usages, shardSizes, reservedSpace); + final Map bigIndexShardSizes = new HashMap<>(shardSizes); + clusterState.getRoutingNodes().unassigned().forEach(shard -> bigIndexShardSizes.put(shardIdentifierFromRouting(shard), 20L)); + final ClusterInfo bigIndexClusterInfo = new DevNullClusterInfo(usages, usages, bigIndexShardSizes, reservedSpace); cis = () -> bigIndexClusterInfo; allocation = createAllocationService(settings, cis); @@ -179,10 +171,10 @@ public void testZoneUnbalanced() { */ public static class DevNullClusterInfo extends ClusterInfo { public DevNullClusterInfo( - ImmutableOpenMap leastAvailableSpaceUsage, - ImmutableOpenMap mostAvailableSpaceUsage, - ImmutableOpenMap shardSizes, - ImmutableOpenMap reservedSpace + final Map leastAvailableSpaceUsage, + final Map mostAvailableSpaceUsage, + final Map shardSizes, + final Map reservedSpace ) { super(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, null, reservedSpace); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java index 67278c56b2f78..9d7d0ebc5b2b1 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java @@ -29,7 +29,6 @@ import org.opensearch.cluster.routing.allocation.decider.AllocationDecider; import org.opensearch.cluster.routing.allocation.decider.AllocationDeciders; import org.opensearch.common.SuppressForbidden; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.index.IndexModule; @@ -38,6 +37,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; import static org.opensearch.cluster.routing.ShardRoutingState.INITIALIZING; @@ -235,11 +235,11 @@ public ShardsAllocator createShardAllocator(Settings settings) { */ public static class DevNullClusterInfo extends ClusterInfo { public DevNullClusterInfo( - ImmutableOpenMap leastAvailableSpaceUsage, - ImmutableOpenMap mostAvailableSpaceUsage, - ImmutableOpenMap shardSizes + final Map leastAvailableSpaceUsage, + final Map mostAvailableSpaceUsage, + final Map shardSizes ) { - super(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, null, ImmutableOpenMap.of()); + super(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, null, Map.of()); } @Override diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java index c3f54fa7580ac..da50dd53b7d54 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java @@ -113,17 +113,15 @@ public void testDiskThreshold() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), 0.8) .build(); - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node1", new DiskUsage("node1", "node1", "/dev/null", 100, 10)); // 90% used - usagesBuilder.put("node2", new DiskUsage("node2", "node2", "/dev/null", 100, 35)); // 65% used - usagesBuilder.put("node3", new DiskUsage("node3", "node3", "/dev/null", 100, 60)); // 40% used - usagesBuilder.put("node4", new DiskUsage("node4", "node4", "/dev/null", 100, 80)); // 20% used - ImmutableOpenMap usages = usagesBuilder.build(); - - ImmutableOpenMap.Builder shardSizesBuilder = ImmutableOpenMap.builder(); - shardSizesBuilder.put("[test][0][p]", 10L); // 10 bytes - shardSizesBuilder.put("[test][0][r]", 10L); - ImmutableOpenMap shardSizes = shardSizesBuilder.build(); + final Map usages = new HashMap<>(); + usages.put("node1", new DiskUsage("node1", "node1", "/dev/null", 100, 10)); // 90% used + usages.put("node2", new DiskUsage("node2", "node2", "/dev/null", 100, 35)); // 65% used + usages.put("node3", new DiskUsage("node3", "node3", "/dev/null", 100, 60)); // 40% used + usages.put("node4", new DiskUsage("node4", "node4", "/dev/null", 100, 80)); // 20% used + + final Map shardSizes = new HashMap<>(); + shardSizes.put("[test][0][p]", 10L); // 10 bytes + shardSizes.put("[test][0][r]", 10L); final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); @@ -294,18 +292,16 @@ public void testDiskThresholdWithAbsoluteSizes() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "5b") .build(); - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 10)); // 90% used - usagesBuilder.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 10)); // 90% used - usagesBuilder.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 60)); // 40% used - usagesBuilder.put("node4", new DiskUsage("node4", "n4", "/dev/null", 100, 80)); // 20% used - usagesBuilder.put("node5", new DiskUsage("node5", "n5", "/dev/null", 100, 85)); // 15% used - ImmutableOpenMap usages = usagesBuilder.build(); - - ImmutableOpenMap.Builder shardSizesBuilder = ImmutableOpenMap.builder(); - shardSizesBuilder.put("[test][0][p]", 10L); // 10 bytes - shardSizesBuilder.put("[test][0][r]", 10L); - ImmutableOpenMap shardSizes = shardSizesBuilder.build(); + final Map usages = new HashMap<>(); + usages.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 10)); // 90% used + usages.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 10)); // 90% used + usages.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 60)); // 40% used + usages.put("node4", new DiskUsage("node4", "n4", "/dev/null", 100, 80)); // 20% used + usages.put("node5", new DiskUsage("node5", "n5", "/dev/null", 100, 85)); // 15% used + + final Map shardSizes = new HashMap<>(); + shardSizes.put("[test][0][p]", 10L); // 10 bytes + shardSizes.put("[test][0][r]", 10L); final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); @@ -360,9 +356,7 @@ public void testDiskThresholdWithAbsoluteSizes() { logger.info("--> nodeWithoutPrimary: {}", nodeWithoutPrimary); // Make node without the primary now habitable to replicas - usagesBuilder = ImmutableOpenMap.builder(usages); - usagesBuilder.put(nodeWithoutPrimary, new DiskUsage(nodeWithoutPrimary, "", "/dev/null", 100, 35)); // 65% used - usages = usagesBuilder.build(); + usages.put(nodeWithoutPrimary, new DiskUsage(nodeWithoutPrimary, "", "/dev/null", 100, 35)); // 65% used final ClusterInfo clusterInfo2 = new DevNullClusterInfo(usages, usages, shardSizes); cis = () -> { logger.info("--> calling fake getClusterInfo"); @@ -540,14 +534,15 @@ public void testDiskThresholdWithShardSizes() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "71%") .build(); - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 31)); // 69% used - usagesBuilder.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 1)); // 99% used - ImmutableOpenMap usages = usagesBuilder.build(); + final Map usages = Map.of( + "node1", + new DiskUsage("node1", "n1", "/dev/null", 100, 31), // 69% used + "node2", + new DiskUsage("node2", "n2", "/dev/null", 100, 1) + ); // 99% used - ImmutableOpenMap.Builder shardSizesBuilder = ImmutableOpenMap.builder(); - shardSizesBuilder.put("[test][0][p]", 10L); // 10 bytes - ImmutableOpenMap shardSizes = shardSizesBuilder.build(); + final Map shardSizes = new HashMap<>(); + shardSizes.put("[test][0][p]", 10L); // 10 bytes final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); AllocationDeciders deciders = new AllocationDeciders( @@ -612,15 +607,13 @@ public void testUnknownDiskUsage() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), 0.85) .build(); - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node2", new DiskUsage("node2", "node2", "/dev/null", 100, 50)); // 50% used - usagesBuilder.put("node3", new DiskUsage("node3", "node3", "/dev/null", 100, 0)); // 100% used - ImmutableOpenMap usages = usagesBuilder.build(); + final Map usages = new HashMap<>(); + usages.put("node2", new DiskUsage("node2", "node2", "/dev/null", 100, 50)); // 50% used + usages.put("node3", new DiskUsage("node3", "node3", "/dev/null", 100, 0)); // 100% used - ImmutableOpenMap.Builder shardSizesBuilder = ImmutableOpenMap.builder(); - shardSizesBuilder.put("[test][0][p]", 10L); // 10 bytes - shardSizesBuilder.put("[test][0][r]", 10L); // 10 bytes - ImmutableOpenMap shardSizes = shardSizesBuilder.build(); + final Map shardSizes = new HashMap<>(); + shardSizes.put("[test][0][p]", 10L); // 10 bytes + shardSizes.put("[test][0][r]", 10L); // 10 bytes final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); AllocationDeciders deciders = new AllocationDeciders( @@ -688,11 +681,11 @@ public void testAverageUsage() { RoutingNode rn = new RoutingNode("node1", newNode("node1")); DiskThresholdDecider decider = makeDecider(Settings.EMPTY); - ImmutableOpenMap.Builder usages = ImmutableOpenMap.builder(); + final Map usages = new HashMap<>(); usages.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 50)); // 50% used usages.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 0)); // 100% used - DiskUsage node1Usage = decider.averageUsage(rn, usages.build()); + DiskUsage node1Usage = decider.averageUsage(rn, usages); assertThat(node1Usage.getTotalBytes(), equalTo(100L)); assertThat(node1Usage.getFreeBytes(), equalTo(25L)); } @@ -718,18 +711,16 @@ public void testShardRelocationsTakenIntoAccount() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), 0.8) .build(); - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 40)); // 60% used - usagesBuilder.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 40)); // 60% used - usagesBuilder.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 40)); // 60% used - ImmutableOpenMap usages = usagesBuilder.build(); - - ImmutableOpenMap.Builder shardSizesBuilder = ImmutableOpenMap.builder(); - shardSizesBuilder.put("[test][0][p]", 14L); // 14 bytes - shardSizesBuilder.put("[test][0][r]", 14L); - shardSizesBuilder.put("[test2][0][p]", 1L); // 1 bytes - shardSizesBuilder.put("[test2][0][r]", 1L); - ImmutableOpenMap shardSizes = shardSizesBuilder.build(); + final Map usages = new HashMap<>(); + usages.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 40)); // 60% used + usages.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 40)); // 60% used + usages.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 40)); // 60% used + + final Map shardSizes = new HashMap<>(); + shardSizes.put("[test][0][p]", 14L); // 14 bytes + shardSizes.put("[test][0][r]", 14L); + shardSizes.put("[test2][0][p]", 1L); // 1 bytes + shardSizes.put("[test2][0][r]", 1L); final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); DiskThresholdDecider decider = makeDecider(diskSettings); @@ -801,18 +792,16 @@ public void testShardRelocationsTakenIntoAccount() { logShardStates(clusterState); } - final ImmutableOpenMap.Builder overfullUsagesBuilder = ImmutableOpenMap.builder(); - overfullUsagesBuilder.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 40)); // 60% used - overfullUsagesBuilder.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 40)); // 60% used - overfullUsagesBuilder.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 0)); // 100% used - final ImmutableOpenMap overfullUsages = overfullUsagesBuilder.build(); + final Map overfullUsages = new HashMap<>(); + overfullUsages.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 40)); // 60% used + overfullUsages.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 40)); // 60% used + overfullUsages.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 0)); // 100% used - final ImmutableOpenMap.Builder largerShardSizesBuilder = ImmutableOpenMap.builder(); - largerShardSizesBuilder.put("[test][0][p]", 14L); - largerShardSizesBuilder.put("[test][0][r]", 14L); - largerShardSizesBuilder.put("[test2][0][p]", 2L); - largerShardSizesBuilder.put("[test2][0][r]", 2L); - final ImmutableOpenMap largerShardSizes = largerShardSizesBuilder.build(); + final Map largerShardSizes = new HashMap<>(); + largerShardSizes.put("[test][0][p]", 14L); + largerShardSizes.put("[test][0][r]", 14L); + largerShardSizes.put("[test2][0][p]", 2L); + largerShardSizes.put("[test2][0][r]", 2L); final ClusterInfo overfullClusterInfo = new DevNullClusterInfo(overfullUsages, overfullUsages, largerShardSizes); @@ -872,10 +861,10 @@ public void testShardRelocationsTakenIntoAccount() { usages, usages, shardSizes, - (new ImmutableOpenMap.Builder()).fPut( + Map.of( new ClusterInfo.NodeAndPath("node1", "/dev/null"), new ClusterInfo.ReservedSpace.Builder().add(new ShardId("", "", 0), between(51, 200)).build() - ).build() + ) ) ); clusterState = applyStartedShardsUntilNoChange(clusterState, strategy); @@ -896,16 +885,14 @@ public void testCanRemainWithShardRelocatingAway() { .build(); // We have an index with 2 primary shards each taking 40 bytes. Each node has 100 bytes available - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 20)); // 80% used - usagesBuilder.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 100)); // 0% used - ImmutableOpenMap usages = usagesBuilder.build(); + final Map usages = new HashMap<>(); + usages.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 20)); // 80% used + usages.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 100)); // 0% used - ImmutableOpenMap.Builder shardSizesBuilder = ImmutableOpenMap.builder(); - shardSizesBuilder.put("[test][0][p]", 40L); - shardSizesBuilder.put("[test][1][p]", 40L); - shardSizesBuilder.put("[foo][0][p]", 10L); - ImmutableOpenMap shardSizes = shardSizesBuilder.build(); + final Map shardSizes = new HashMap<>(); + shardSizes.put("[test][0][p]", 40L); + shardSizes.put("[test][1][p]", 40L); + shardSizes.put("[foo][0][p]", 10L); final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); @@ -1053,17 +1040,16 @@ public void testForSingleDataNode() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "70%") .build(); - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 100)); // 0% used - usagesBuilder.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 20)); // 80% used - usagesBuilder.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 100)); // 0% used - ImmutableOpenMap usages = usagesBuilder.build(); + final Map usages = new HashMap<>(); + usages.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 100)); // 0% used + usages.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 20)); // 80% used + usages.put("node3", new DiskUsage("node3", "n3", "/dev/null", 100, 100)); // 0% used // We have an index with 1 primary shards each taking 40 bytes. Each node has 100 bytes available - ImmutableOpenMap.Builder shardSizes = ImmutableOpenMap.builder(); + final Map shardSizes = new HashMap<>(); shardSizes.put("[test][0][p]", 40L); shardSizes.put("[test][1][p]", 40L); - final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes.build()); + final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); DiskThresholdDecider diskThresholdDecider = makeDecider(diskSettings); Metadata metadata = Metadata.builder() @@ -1207,14 +1193,13 @@ public void testWatermarksEnabledForSingleDataNode() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "70%") .build(); - ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("data", new DiskUsage("data", "data", "/dev/null", 100, 20)); // 80% used - ImmutableOpenMap usages = usagesBuilder.build(); + final Map usages = new HashMap<>(); + usages.put("data", new DiskUsage("data", "data", "/dev/null", 100, 20)); // 80% used // We have an index with 1 primary shard, taking 40 bytes. The single data node has only 20 bytes free. - ImmutableOpenMap.Builder shardSizes = ImmutableOpenMap.builder(); + final Map shardSizes = new HashMap<>(); shardSizes.put("[test][0][p]", 40L); - final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes.build()); + final ClusterInfo clusterInfo = new DevNullClusterInfo(usages, usages, shardSizes); DiskThresholdDecider diskThresholdDecider = makeDecider(diskSettings); Metadata metadata = Metadata.builder() @@ -1319,11 +1304,10 @@ public void testDiskThresholdWithSnapshotShardSizes() { .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "95%") .build(); - final ImmutableOpenMap.Builder usagesBuilder = ImmutableOpenMap.builder(); - usagesBuilder.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 21)); // 79% used - usagesBuilder.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 1)); // 99% used - final ImmutableOpenMap usages = usagesBuilder.build(); - final ClusterInfoService clusterInfoService = () -> new DevNullClusterInfo(usages, usages, ImmutableOpenMap.of()); + final Map usages = new HashMap<>(); + usages.put("node1", new DiskUsage("node1", "n1", "/dev/null", 100, 21)); // 79% used + usages.put("node2", new DiskUsage("node2", "n2", "/dev/null", 100, 1)); // 99% used + final ClusterInfoService clusterInfoService = () -> new DevNullClusterInfo(usages, usages, Map.of()); final AllocationDeciders deciders = new AllocationDeciders( new HashSet<>( @@ -1468,18 +1452,18 @@ public void logShardStates(ClusterState state) { */ static class DevNullClusterInfo extends ClusterInfo { DevNullClusterInfo( - ImmutableOpenMap leastAvailableSpaceUsage, - ImmutableOpenMap mostAvailableSpaceUsage, - ImmutableOpenMap shardSizes + final Map leastAvailableSpaceUsage, + final Map mostAvailableSpaceUsage, + final Map shardSizes ) { - this(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, ImmutableOpenMap.of()); + this(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, Map.of()); } DevNullClusterInfo( - ImmutableOpenMap leastAvailableSpaceUsage, - ImmutableOpenMap mostAvailableSpaceUsage, - ImmutableOpenMap shardSizes, - ImmutableOpenMap reservedSpace + final Map leastAvailableSpaceUsage, + final Map mostAvailableSpaceUsage, + final Map shardSizes, + Map reservedSpace ) { super(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, null, reservedSpace); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderUnitTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderUnitTests.java index bbd9658361114..caab381e65e84 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderUnitTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderUnitTests.java @@ -54,14 +54,15 @@ import org.opensearch.cluster.routing.UnassignedInfo; import org.opensearch.cluster.routing.allocation.AllocationService; import org.opensearch.cluster.routing.allocation.RoutingAllocation; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.index.Index; import org.opensearch.index.shard.ShardId; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; @@ -114,25 +115,19 @@ public void testCanAllocateUsesMaxAvailableSpace() { clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder().add(node_0).add(node_1)).build(); // actual test -- after all that bloat :) - ImmutableOpenMap.Builder leastAvailableUsages = ImmutableOpenMap.builder(); + final Map leastAvailableUsages = new HashMap<>(); leastAvailableUsages.put("node_0", new DiskUsage("node_0", "node_0", "_na_", 100, 0)); // all full leastAvailableUsages.put("node_1", new DiskUsage("node_1", "node_1", "_na_", 100, 0)); // all full - ImmutableOpenMap.Builder mostAvailableUsage = ImmutableOpenMap.builder(); + final Map mostAvailableUsage = new HashMap<>(); // 20 - 99 percent since after allocation there must be at least 10% left and shard is 10byte mostAvailableUsage.put("node_0", new DiskUsage("node_0", "node_0", "_na_", 100, randomIntBetween(20, 100))); // this is weird and smells like a bug! it should be up to 20%? mostAvailableUsage.put("node_1", new DiskUsage("node_1", "node_1", "_na_", 100, randomIntBetween(0, 10))); - ImmutableOpenMap.Builder shardSizes = ImmutableOpenMap.builder(); + final Map shardSizes = new HashMap<>(); shardSizes.put("[test][0][p]", 10L); // 10 bytes - final ClusterInfo clusterInfo = new ClusterInfo( - leastAvailableUsages.build(), - mostAvailableUsage.build(), - shardSizes.build(), - ImmutableOpenMap.of(), - ImmutableOpenMap.of() - ); + final ClusterInfo clusterInfo = new ClusterInfo(leastAvailableUsages, mostAvailableUsage, shardSizes, Map.of(), Map.of()); RoutingAllocation allocation = new RoutingAllocation( new AllocationDeciders(Collections.singleton(decider)), clusterState.getRoutingNodes(), @@ -198,23 +193,17 @@ public void testCannotAllocateDueToLackOfDiskResources() { // actual test -- after all that bloat :) - ImmutableOpenMap.Builder leastAvailableUsages = ImmutableOpenMap.builder(); + final Map leastAvailableUsages = new HashMap<>(); leastAvailableUsages.put("node_0", new DiskUsage("node_0", "node_0", "_na_", 100, 0)); // all full - ImmutableOpenMap.Builder mostAvailableUsage = ImmutableOpenMap.builder(); + final Map mostAvailableUsage = new HashMap<>(); final int freeBytes = randomIntBetween(20, 100); mostAvailableUsage.put("node_0", new DiskUsage("node_0", "node_0", "_na_", 100, freeBytes)); - ImmutableOpenMap.Builder shardSizes = ImmutableOpenMap.builder(); + final Map shardSizes = new HashMap<>(); // way bigger than available space final long shardSize = randomIntBetween(110, 1000); shardSizes.put("[test][0][p]", shardSize); - ClusterInfo clusterInfo = new ClusterInfo( - leastAvailableUsages.build(), - mostAvailableUsage.build(), - shardSizes.build(), - ImmutableOpenMap.of(), - ImmutableOpenMap.of() - ); + ClusterInfo clusterInfo = new ClusterInfo(leastAvailableUsages, mostAvailableUsage, shardSizes, Map.of(), Map.of()); RoutingAllocation allocation = new RoutingAllocation( new AllocationDeciders(Collections.singleton(decider)), clusterState.getRoutingNodes(), @@ -245,7 +234,7 @@ public void testCannotAllocateDueToLackOfDiskResources() { public void testCanRemainUsesLeastAvailableSpace() { ClusterSettings nss = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); DiskThresholdDecider decider = new DiskThresholdDecider(Settings.EMPTY, nss); - ImmutableOpenMap.Builder shardRoutingMap = ImmutableOpenMap.builder(); + final Map shardRoutingMap = new HashMap<>(); DiscoveryNode node_0 = new DiscoveryNode( "node_0", @@ -318,26 +307,20 @@ public void testCanRemainUsesLeastAvailableSpace() { clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder().add(node_0).add(node_1)).build(); // actual test -- after all that bloat :) - ImmutableOpenMap.Builder leastAvailableUsages = ImmutableOpenMap.builder(); + final Map leastAvailableUsages = new HashMap<>(); leastAvailableUsages.put("node_0", new DiskUsage("node_0", "node_0", "/node0/least", 100, 10)); // 90% used leastAvailableUsages.put("node_1", new DiskUsage("node_1", "node_1", "/node1/least", 100, 9)); // 91% used - ImmutableOpenMap.Builder mostAvailableUsage = ImmutableOpenMap.builder(); + final Map mostAvailableUsage = new HashMap<>(); mostAvailableUsage.put("node_0", new DiskUsage("node_0", "node_0", "/node0/most", 100, 90)); // 10% used mostAvailableUsage.put("node_1", new DiskUsage("node_1", "node_1", "/node1/most", 100, 90)); // 10% used - ImmutableOpenMap.Builder shardSizes = ImmutableOpenMap.builder(); + final Map shardSizes = new HashMap<>(); shardSizes.put("[test][0][p]", 10L); // 10 bytes shardSizes.put("[test][1][p]", 10L); shardSizes.put("[test][2][p]", 10L); - final ClusterInfo clusterInfo = new ClusterInfo( - leastAvailableUsages.build(), - mostAvailableUsage.build(), - shardSizes.build(), - shardRoutingMap.build(), - ImmutableOpenMap.of() - ); + final ClusterInfo clusterInfo = new ClusterInfo(leastAvailableUsages, mostAvailableUsage, shardSizes, shardRoutingMap, Map.of()); RoutingAllocation allocation = new RoutingAllocation( new AllocationDeciders(Collections.singleton(decider)), clusterState.getRoutingNodes(), @@ -392,16 +375,12 @@ public void testCanRemainUsesLeastAvailableSpace() { } public void testShardSizeAndRelocatingSize() { - ImmutableOpenMap.Builder shardSizes = ImmutableOpenMap.builder(); + final Map shardSizes = new HashMap<>(); shardSizes.put("[test][0][r]", 10L); shardSizes.put("[test][1][r]", 100L); shardSizes.put("[test][2][r]", 1000L); shardSizes.put("[other][0][p]", 10000L); - ClusterInfo info = new DiskThresholdDeciderTests.DevNullClusterInfo( - ImmutableOpenMap.of(), - ImmutableOpenMap.of(), - shardSizes.build() - ); + ClusterInfo info = new DiskThresholdDeciderTests.DevNullClusterInfo(Map.of(), Map.of(), shardSizes); Metadata.Builder metaBuilder = Metadata.builder(); metaBuilder.put( IndexMetadata.builder("test") @@ -520,17 +499,13 @@ public long sizeOfRelocatingShards(RoutingAllocation allocation, RoutingNode nod } public void testSizeShrinkIndex() { - ImmutableOpenMap.Builder shardSizes = ImmutableOpenMap.builder(); + final Map shardSizes = new HashMap<>(); shardSizes.put("[test][0][p]", 10L); shardSizes.put("[test][1][p]", 100L); shardSizes.put("[test][2][p]", 500L); shardSizes.put("[test][3][p]", 500L); - ClusterInfo info = new DiskThresholdDeciderTests.DevNullClusterInfo( - ImmutableOpenMap.of(), - ImmutableOpenMap.of(), - shardSizes.build() - ); + ClusterInfo info = new DiskThresholdDeciderTests.DevNullClusterInfo(Map.of(), Map.of(), shardSizes); Metadata.Builder metaBuilder = Metadata.builder(); metaBuilder.put( IndexMetadata.builder("test")