Skip to content

Commit 9ebbe1c

Browse files
original-brownbearoriginal-brownbear
authored
Make ClusterInfo use immutable maps in all cases (#88447)
This class's maps are used very hot in the disk threshold allocation decider. Moving them from hppc maps to unmodifiable map wrapping `HashMap` has led to a measurable slowdown in the many-shards benchmark bootstrapping. Lets use immutable map copies here exclusively to make performance outright better and more predictable via a single implementation.
1 parent 47510ad commit 9ebbe1c

File tree

9 files changed

+32
-26
lines changed

9 files changed

+32
-26
lines changed

‎server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,12 @@ public ClusterInfo(
6868
Map<ShardRouting, String> routingToDataPath,
6969
Map<NodeAndPath, ReservedSpace> reservedSpace
7070
) {
71-
this.leastAvailableSpaceUsage = leastAvailableSpaceUsage;
72-
this.shardSizes = shardSizes;
73-
this.shardDataSetSizes = shardDataSetSizes;
74-
this.mostAvailableSpaceUsage = mostAvailableSpaceUsage;
75-
this.routingToDataPath = routingToDataPath;
76-
this.reservedSpace = reservedSpace;
71+
this.leastAvailableSpaceUsage = Map.copyOf(leastAvailableSpaceUsage);
72+
this.shardSizes = Map.copyOf(shardSizes);
73+
this.shardDataSetSizes = Map.copyOf(shardDataSetSizes);
74+
this.mostAvailableSpaceUsage = Map.copyOf(mostAvailableSpaceUsage);
75+
this.routingToDataPath = Map.copyOf(routingToDataPath);
76+
this.reservedSpace = Map.copyOf(reservedSpace);
7777
}
7878

7979
public ClusterInfo(StreamInput in) throws IOException {

‎server/src/main/java/org/elasticsearch/cluster/InternalClusterInfoService.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.elasticsearch.threadpool.ThreadPool;
4040

4141
import java.util.ArrayList;
42-
import java.util.Collections;
4342
import java.util.HashMap;
4443
import java.util.HashSet;
4544
import java.util.List;
@@ -188,8 +187,8 @@ public void onResponse(NodesStatsResponse nodesStatsResponse) {
188187
leastAvailableUsagesBuilder,
189188
mostAvailableUsagesBuilder
190189
);
191-
leastAvailableSpaceUsages = Collections.unmodifiableMap(leastAvailableUsagesBuilder);
192-
mostAvailableSpaceUsages = Collections.unmodifiableMap(mostAvailableUsagesBuilder);
190+
leastAvailableSpaceUsages = Map.copyOf(leastAvailableUsagesBuilder);
191+
mostAvailableSpaceUsages = Map.copyOf(mostAvailableUsagesBuilder);
193192
}
194193

195194
@Override
@@ -262,10 +261,10 @@ public void onResponse(IndicesStatsResponse indicesStatsResponse) {
262261
reservedSpaceBuilders.forEach((nodeAndPath, builder) -> rsrvdSpace.put(nodeAndPath, builder.build()));
263262

264263
indicesStatsSummary = new IndicesStatsSummary(
265-
Collections.unmodifiableMap(shardSizeByIdentifierBuilder),
266-
Collections.unmodifiableMap(shardDataSetSizeBuilder),
267-
Collections.unmodifiableMap(dataPathByShardRoutingBuilder),
268-
Collections.unmodifiableMap(rsrvdSpace)
264+
Map.copyOf(shardSizeByIdentifierBuilder),
265+
Map.copyOf(shardDataSetSizeBuilder),
266+
Map.copyOf(dataPathByShardRoutingBuilder),
267+
Map.copyOf(rsrvdSpace)
269268
);
270269
}
271270

‎server/src/test/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdMonitorTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ private static ClusterInfo clusterInfo(
964964
Map<String, DiskUsage> diskUsages,
965965
Map<ClusterInfo.NodeAndPath, ClusterInfo.ReservedSpace> reservedSpace
966966
) {
967-
return new ClusterInfo(diskUsages, null, null, null, null, reservedSpace);
967+
return new ClusterInfo(diskUsages, Map.of(), Map.of(), Map.of(), Map.of(), reservedSpace);
968968
}
969969

970970
private static DiscoveryNode newFrozenOnlyNode(String nodeId) {

‎server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1308,7 +1308,7 @@ static class DevNullClusterInfo extends ClusterInfo {
13081308
Map<String, Long> shardSizes,
13091309
Map<NodeAndPath, ReservedSpace> reservedSpace
13101310
) {
1311-
super(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, null, null, reservedSpace);
1311+
super(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, Map.of(), Map.of(), reservedSpace);
13121312
}
13131313

13141314
@Override

‎server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderUnitTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public void testCanAllocateUsesMaxAvailableSpace() {
107107
leastAvailableUsages,
108108
mostAvailableUsage,
109109
Map.of("[test][0][p]", 10L), // 10 bytes,
110-
null,
110+
Map.of(),
111111
Map.of(),
112112
Map.of()
113113
);
@@ -185,7 +185,7 @@ public void testCannotAllocateDueToLackOfDiskResources() {
185185
leastAvailableUsages,
186186
mostAvailableUsage,
187187
Map.of("[test][0][p]", shardSize),
188-
null,
188+
Map.of(),
189189
Map.of(),
190190
Map.of()
191191
);
@@ -307,7 +307,7 @@ public void testCanRemainUsesLeastAvailableSpace() {
307307
leastAvailableUsages,
308308
mostAvailableUsage,
309309
shardSizes,
310-
null,
310+
Map.of(),
311311
shardRoutingMap,
312312
Map.of()
313313
);
@@ -745,7 +745,7 @@ public void testDecidesYesIfWatermarksIgnored() {
745745
allFullUsages,
746746
allFullUsages,
747747
Map.of("[test][0][p]", 10L),
748-
null,
748+
Map.of(),
749749
Map.of(),
750750
Map.of()
751751
);
@@ -815,7 +815,7 @@ public void testCannotForceAllocateOver100PercentUsage() {
815815
// bigger than available space
816816
final long shardSize = randomIntBetween(1, 10);
817817
shardSizes.put("[test][0][p]", shardSize);
818-
ClusterInfo clusterInfo = new ClusterInfo(leastAvailableUsages, mostAvailableUsage, shardSizes, null, Map.of(), Map.of());
818+
ClusterInfo clusterInfo = new ClusterInfo(leastAvailableUsages, mostAvailableUsage, shardSizes, Map.of(), Map.of(), Map.of());
819819
RoutingAllocation allocation = new RoutingAllocation(
820820
new AllocationDeciders(Collections.singleton(decider)),
821821
clusterState,

‎x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/storage/ReactiveStorageDeciderService.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,14 @@ private static class ExtendedClusterInfo extends ClusterInfo {
793793
private final ClusterInfo delegate;
794794

795795
private ExtendedClusterInfo(Map<String, Long> extraShardSizes, ClusterInfo info) {
796-
super(info.getNodeLeastAvailableDiskUsages(), info.getNodeMostAvailableDiskUsages(), extraShardSizes, Map.of(), null, null);
796+
super(
797+
info.getNodeLeastAvailableDiskUsages(),
798+
info.getNodeMostAvailableDiskUsages(),
799+
extraShardSizes,
800+
Map.of(),
801+
Map.of(),
802+
Map.of()
803+
);
797804
this.delegate = info;
798805
}
799806

‎x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityServiceTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ public void testContext() {
260260
}
261261
}
262262
state = ClusterState.builder(ClusterName.DEFAULT).nodes(nodes).build();
263-
info = new ClusterInfo(leastUsages, mostUsages, null, null, null, null);
263+
info = new ClusterInfo(leastUsages, mostUsages, Map.of(), Map.of(), Map.of(), Map.of());
264264
context = new AutoscalingCalculateCapacityService.DefaultAutoscalingDeciderContext(
265265
roleNames,
266266
state,
@@ -306,7 +306,7 @@ public void testContext() {
306306
)
307307
);
308308

309-
info = new ClusterInfo(leastUsages, mostUsages, null, null, null, null);
309+
info = new ClusterInfo(leastUsages, mostUsages, Map.of(), Map.of(), Map.of(), Map.of());
310310
context = new AutoscalingCalculateCapacityService.DefaultAutoscalingDeciderContext(
311311
roleNames,
312312
state,

‎x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/storage/FrozenStorageDeciderServiceTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public Tuple<Long, ClusterInfo> sizeAndClusterInfo(IndexMetadata indexMetadata)
109109
// add irrelevant shards noise for completeness (should not happen IRL).
110110
sizes.put(new ShardId(index, i), randomLongBetween(0, Integer.MAX_VALUE));
111111
}
112-
ClusterInfo info = new ClusterInfo(null, null, null, sizes, null, null);
112+
ClusterInfo info = new ClusterInfo(Map.of(), Map.of(), Map.of(), sizes, Map.of(), Map.of());
113113
return Tuple.tuple(totalSize, info);
114114
}
115115
}

‎x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/storage/ReactiveStorageDeciderServiceTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ public void validateSizeOf(ClusterState clusterState, ShardRouting subjectShard,
403403
}
404404

405405
private ReactiveStorageDeciderService.AllocationState createAllocationState(Map<String, Long> shardSize, ClusterState clusterState) {
406-
ClusterInfo info = new ClusterInfo(null, null, shardSize, null, null, null);
406+
ClusterInfo info = new ClusterInfo(Map.of(), Map.of(), shardSize, Map.of(), Map.of(), Map.of());
407407
ReactiveStorageDeciderService.AllocationState allocationState = new ReactiveStorageDeciderService.AllocationState(
408408
clusterState,
409409
null,
@@ -567,7 +567,7 @@ public void testUnmovableSize() {
567567
if (shardsWithSizes.isEmpty() == false) {
568568
shardSize.put(shardIdentifier(randomFrom(shardsWithSizes)), ByteSizeUnit.KB.toBytes(minShardSize));
569569
}
570-
ClusterInfo info = new ClusterInfo(diskUsages, diskUsages, shardSize, null, null, null);
570+
ClusterInfo info = new ClusterInfo(diskUsages, diskUsages, shardSize, Map.of(), Map.of(), Map.of());
571571

572572
ReactiveStorageDeciderService.AllocationState allocationState = new ReactiveStorageDeciderService.AllocationState(
573573
clusterState,

0 commit comments

Comments
 (0)