Browse Source

Use HashMap inside cluster info service (#87899)

The InternalClusterInfoService internally uses ImmutableOpenMap
for keeping track of available space on each node. This commit converts
those usages to use HashMap. Note that an unmodifiableMap wrapper is
used because updates to this (from each node) are likely to happen often
as disk is used.

relates #86239
Ryan Ernst 3 years ago
parent
commit
f627bdd733

+ 31 - 38
server/src/main/java/org/elasticsearch/cluster/InternalClusterInfoService.java

@@ -27,7 +27,6 @@ import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.routing.ShardRouting;
 import org.elasticsearch.cluster.routing.ShardRouting;
 import org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings;
 import org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.cluster.service.ClusterService;
-import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Setting.Property;
@@ -40,6 +39,7 @@ import org.elasticsearch.monitor.fs.FsInfo;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.threadpool.ThreadPool;
 
 
 import java.util.ArrayList;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.HashSet;
 import java.util.List;
 import java.util.List;
@@ -83,8 +83,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
     private volatile TimeValue updateFrequency;
     private volatile TimeValue updateFrequency;
     private volatile TimeValue fetchTimeout;
     private volatile TimeValue fetchTimeout;
 
 
-    private volatile ImmutableOpenMap<String, DiskUsage> leastAvailableSpaceUsages;
-    private volatile ImmutableOpenMap<String, DiskUsage> mostAvailableSpaceUsages;
+    private volatile Map<String, DiskUsage> leastAvailableSpaceUsages;
+    private volatile Map<String, DiskUsage> mostAvailableSpaceUsages;
     private volatile IndicesStatsSummary indicesStatsSummary;
     private volatile IndicesStatsSummary indicesStatsSummary;
 
 
     private final ThreadPool threadPool;
     private final ThreadPool threadPool;
@@ -97,8 +97,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
     private RefreshScheduler refreshScheduler;
     private RefreshScheduler refreshScheduler;
 
 
     public InternalClusterInfoService(Settings settings, ClusterService clusterService, ThreadPool threadPool, Client client) {
     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.indicesStatsSummary = IndicesStatsSummary.EMPTY;
         this.threadPool = threadPool;
         this.threadPool = threadPool;
         this.client = client;
         this.client = client;
@@ -181,15 +181,15 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
                         logger.warn(() -> "failed to retrieve stats for node [" + failure.nodeId() + "]", failure.getCause());
                         logger.warn(() -> "failed to retrieve stats for node [" + failure.nodeId() + "]", failure.getCause());
                     }
                     }
 
 
-                    ImmutableOpenMap.Builder<String, DiskUsage> leastAvailableUsagesBuilder = ImmutableOpenMap.builder();
-                    ImmutableOpenMap.Builder<String, DiskUsage> mostAvailableUsagesBuilder = ImmutableOpenMap.builder();
+                    Map<String, DiskUsage> leastAvailableUsagesBuilder = new HashMap<>();
+                    Map<String, DiskUsage> mostAvailableUsagesBuilder = new HashMap<>();
                     fillDiskUsagePerNode(
                     fillDiskUsagePerNode(
                         adjustNodesStats(nodesStatsResponse.getNodes()),
                         adjustNodesStats(nodesStatsResponse.getNodes()),
                         leastAvailableUsagesBuilder,
                         leastAvailableUsagesBuilder,
                         mostAvailableUsagesBuilder
                         mostAvailableUsagesBuilder
                     );
                     );
-                    leastAvailableSpaceUsages = leastAvailableUsagesBuilder.build();
-                    mostAvailableSpaceUsages = mostAvailableUsagesBuilder.build();
+                    leastAvailableSpaceUsages = Collections.unmodifiableMap(leastAvailableUsagesBuilder);
+                    mostAvailableSpaceUsages = Collections.unmodifiableMap(mostAvailableUsagesBuilder);
                 }
                 }
 
 
                 @Override
                 @Override
@@ -199,8 +199,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
                     } else {
                     } else {
                         logger.warn("failed to retrieve node stats", e);
                         logger.warn("failed to retrieve node stats", e);
                     }
                     }
-                    leastAvailableSpaceUsages = ImmutableOpenMap.of();
-                    mostAvailableSpaceUsages = ImmutableOpenMap.of();
+                    leastAvailableSpaceUsages = Map.of();
+                    mostAvailableSpaceUsages = Map.of();
                 }
                 }
             }, this::onStatsProcessed));
             }, this::onStatsProcessed));
 
 
@@ -246,9 +246,9 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
                     }
                     }
 
 
                     final ShardStats[] stats = indicesStatsResponse.getShards();
                     final ShardStats[] stats = indicesStatsResponse.getShards();
-                    final ImmutableOpenMap.Builder<String, Long> shardSizeByIdentifierBuilder = ImmutableOpenMap.builder();
-                    final ImmutableOpenMap.Builder<ShardId, Long> shardDataSetSizeBuilder = ImmutableOpenMap.builder();
-                    final ImmutableOpenMap.Builder<ShardRouting, String> dataPathByShardRoutingBuilder = ImmutableOpenMap.builder();
+                    final Map<String, Long> shardSizeByIdentifierBuilder = new HashMap<>();
+                    final Map<ShardId, Long> shardDataSetSizeBuilder = new HashMap<>();
+                    final Map<ShardRouting, String> dataPathByShardRoutingBuilder = new HashMap<>();
                     final Map<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace.Builder> reservedSpaceBuilders = new HashMap<>();
                     final Map<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace.Builder> reservedSpaceBuilders = new HashMap<>();
                     buildShardLevelInfo(
                     buildShardLevelInfo(
                         stats,
                         stats,
@@ -258,15 +258,14 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
                         reservedSpaceBuilders
                         reservedSpaceBuilders
                     );
                     );
 
 
-                    final ImmutableOpenMap.Builder<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace> rsrvdSpace = ImmutableOpenMap
-                        .builder();
+                    final Map<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace> rsrvdSpace = new HashMap<>();
                     reservedSpaceBuilders.forEach((nodeAndPath, builder) -> rsrvdSpace.put(nodeAndPath, builder.build()));
                     reservedSpaceBuilders.forEach((nodeAndPath, builder) -> rsrvdSpace.put(nodeAndPath, builder.build()));
 
 
                     indicesStatsSummary = new IndicesStatsSummary(
                     indicesStatsSummary = new IndicesStatsSummary(
-                        shardSizeByIdentifierBuilder.build(),
-                        shardDataSetSizeBuilder.build(),
-                        dataPathByShardRoutingBuilder.build(),
-                        rsrvdSpace.build()
+                        Collections.unmodifiableMap(shardSizeByIdentifierBuilder),
+                        Collections.unmodifiableMap(shardDataSetSizeBuilder),
+                        Collections.unmodifiableMap(dataPathByShardRoutingBuilder),
+                        Collections.unmodifiableMap(rsrvdSpace)
                     );
                     );
                 }
                 }
 
 
@@ -342,8 +341,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
             return currentRefresh::execute;
             return currentRefresh::execute;
         } else {
         } else {
             return () -> {
             return () -> {
-                leastAvailableSpaceUsages = ImmutableOpenMap.of();
-                mostAvailableSpaceUsages = ImmutableOpenMap.of();
+                leastAvailableSpaceUsages = Map.of();
+                mostAvailableSpaceUsages = Map.of();
                 indicesStatsSummary = IndicesStatsSummary.EMPTY;
                 indicesStatsSummary = IndicesStatsSummary.EMPTY;
                 thisRefreshListeners.forEach(l -> l.onResponse(ClusterInfo.EMPTY));
                 thisRefreshListeners.forEach(l -> l.onResponse(ClusterInfo.EMPTY));
             };
             };
@@ -413,9 +412,9 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
 
 
     static void buildShardLevelInfo(
     static void buildShardLevelInfo(
         ShardStats[] stats,
         ShardStats[] stats,
-        ImmutableOpenMap.Builder<String, Long> shardSizes,
-        ImmutableOpenMap.Builder<ShardId, Long> shardDataSetSizeBuilder,
-        ImmutableOpenMap.Builder<ShardRouting, String> newShardRoutingToDataPath,
+        Map<String, Long> shardSizes,
+        Map<ShardId, Long> shardDataSetSizeBuilder,
+        Map<ShardRouting, String> newShardRoutingToDataPath,
         Map<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace.Builder> reservedSpaceByShard
         Map<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace.Builder> reservedSpaceByShard
     ) {
     ) {
         for (ShardStats s : stats) {
         for (ShardStats s : stats) {
@@ -448,8 +447,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
 
 
     static void fillDiskUsagePerNode(
     static void fillDiskUsagePerNode(
         List<NodeStats> nodeStatsArray,
         List<NodeStats> nodeStatsArray,
-        ImmutableOpenMap.Builder<String, DiskUsage> newLeastAvailableUsages,
-        ImmutableOpenMap.Builder<String, DiskUsage> newMostAvailableUsages
+        Map<String, DiskUsage> newLeastAvailableUsages,
+        Map<String, DiskUsage> newMostAvailableUsages
     ) {
     ) {
         for (NodeStats nodeStats : nodeStatsArray) {
         for (NodeStats nodeStats : nodeStatsArray) {
             if (nodeStats.getFs() == null) {
             if (nodeStats.getFs() == null) {
@@ -534,18 +533,12 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt
     }
     }
 
 
     private record IndicesStatsSummary(
     private record IndicesStatsSummary(
-        ImmutableOpenMap<String, Long> shardSizes,
-        ImmutableOpenMap<ShardId, Long> shardDataSetSizes,
-        ImmutableOpenMap<ShardRouting, String> shardRoutingToDataPath,
-        ImmutableOpenMap<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace> reservedSpace
+        Map<String, Long> shardSizes,
+        Map<ShardId, Long> shardDataSetSizes,
+        Map<ShardRouting, String> shardRoutingToDataPath,
+        Map<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace> reservedSpace
     ) {
     ) {
-        static final IndicesStatsSummary EMPTY = new IndicesStatsSummary(
-            ImmutableOpenMap.of(),
-            ImmutableOpenMap.of(),
-            ImmutableOpenMap.of(),
-            ImmutableOpenMap.of()
-        );
-
+        static final IndicesStatsSummary EMPTY = new IndicesStatsSummary(Map.of(), Map.of(), Map.of(), Map.of());
     }
     }
 
 
 }
 }

+ 8 - 9
server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java

@@ -17,7 +17,6 @@ import org.elasticsearch.cluster.routing.RecoverySource.PeerRecoverySource;
 import org.elasticsearch.cluster.routing.ShardRouting;
 import org.elasticsearch.cluster.routing.ShardRouting;
 import org.elasticsearch.cluster.routing.ShardRoutingHelper;
 import org.elasticsearch.cluster.routing.ShardRoutingHelper;
 import org.elasticsearch.cluster.routing.UnassignedInfo;
 import org.elasticsearch.cluster.routing.UnassignedInfo;
-import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.index.shard.ShardPath;
 import org.elasticsearch.index.shard.ShardPath;
@@ -29,6 +28,7 @@ import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashMap;
 import java.util.List;
 import java.util.List;
+import java.util.Map;
 
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
 import static java.util.Collections.emptySet;
@@ -116,10 +116,9 @@ public class DiskUsageTests extends ESTestCase {
             new ShardStats(test_0, new ShardPath(false, test0Path, test0Path, test_0.shardId()), commonStats0, null, null, null),
             new ShardStats(test_0, new ShardPath(false, test0Path, test0Path, test_0.shardId()), commonStats0, null, null, null),
             new ShardStats(test_1, new ShardPath(false, test1Path, test1Path, test_1.shardId()), commonStats1, null, null, null),
             new ShardStats(test_1, new ShardPath(false, test1Path, test1Path, test_1.shardId()), commonStats1, null, null, null),
             new ShardStats(test_1, new ShardPath(false, test1Path, test1Path, test_1.shardId()), commonStats2, null, null, null) };
             new ShardStats(test_1, new ShardPath(false, test1Path, test1Path, test_1.shardId()), commonStats2, null, null, null) };
-        ImmutableOpenMap.Builder<String, Long> shardSizes = ImmutableOpenMap.builder();
-        ImmutableOpenMap.Builder<ShardId, Long> shardDataSetSizes = ImmutableOpenMap.builder();
-        ImmutableOpenMap.Builder<ShardRouting, String> routingToPath = ImmutableOpenMap.builder();
-        ClusterState state = ClusterState.builder(new ClusterName("blarg")).version(0).build();
+        Map<String, Long> shardSizes = new HashMap<>();
+        Map<ShardId, Long> shardDataSetSizes = new HashMap<>();
+        Map<ShardRouting, String> routingToPath = new HashMap<>();
         InternalClusterInfoService.buildShardLevelInfo(stats, shardSizes, shardDataSetSizes, routingToPath, new HashMap<>());
         InternalClusterInfoService.buildShardLevelInfo(stats, shardSizes, shardDataSetSizes, routingToPath, new HashMap<>());
         assertEquals(2, shardSizes.size());
         assertEquals(2, shardSizes.size());
         assertTrue(shardSizes.containsKey(ClusterInfo.shardIdentifierFromRouting(test_0)));
         assertTrue(shardSizes.containsKey(ClusterInfo.shardIdentifierFromRouting(test_0)));
@@ -141,8 +140,8 @@ public class DiskUsageTests extends ESTestCase {
     }
     }
 
 
     public void testFillDiskUsage() {
     public void testFillDiskUsage() {
-        ImmutableOpenMap.Builder<String, DiskUsage> newLeastAvaiableUsages = ImmutableOpenMap.builder();
-        ImmutableOpenMap.Builder<String, DiskUsage> newMostAvaiableUsages = ImmutableOpenMap.builder();
+        Map<String, DiskUsage> newLeastAvaiableUsages = new HashMap<>();
+        Map<String, DiskUsage> newMostAvaiableUsages = new HashMap<>();
         FsInfo.Path[] node1FSInfo = new FsInfo.Path[] {
         FsInfo.Path[] node1FSInfo = new FsInfo.Path[] {
             new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80),
             new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80),
             new FsInfo.Path("/least", "/dev/sdb", 200, 190, 70),
             new FsInfo.Path("/least", "/dev/sdb", 200, 190, 70),
@@ -229,8 +228,8 @@ public class DiskUsageTests extends ESTestCase {
     }
     }
 
 
     public void testFillDiskUsageSomeInvalidValues() {
     public void testFillDiskUsageSomeInvalidValues() {
-        ImmutableOpenMap.Builder<String, DiskUsage> newLeastAvailableUsages = ImmutableOpenMap.builder();
-        ImmutableOpenMap.Builder<String, DiskUsage> newMostAvailableUsages = ImmutableOpenMap.builder();
+        Map<String, DiskUsage> newLeastAvailableUsages = new HashMap<>();
+        Map<String, DiskUsage> newMostAvailableUsages = new HashMap<>();
         FsInfo.Path[] node1FSInfo = new FsInfo.Path[] {
         FsInfo.Path[] node1FSInfo = new FsInfo.Path[] {
             new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80),
             new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80),
             new FsInfo.Path("/least", "/dev/sdb", -1, -1, -1),
             new FsInfo.Path("/least", "/dev/sdb", -1, -1, -1),