Skip to content

Commit 80796fb

Browse files
Small cleanups to Allocation Performance (#89378)
Two fixes: 1. Looking up `Custom` values over and over for every shard incurs a measurable cost. This removes that cost for desired nodes and node shutdown metadata. 2. Node shutdown metadata logic wasn't inlining nicely because of the wrapped map. No need to be as complicated as we were in many spots, use a simple immutable map for all operations and remove a bunch of branching.
1 parent e7a84b1 commit 80796fb

File tree

4 files changed

+16
-26
lines changed

4 files changed

+16
-26
lines changed

server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,9 +1024,7 @@ public Map<String, DataStreamAlias> dataStreamAliases() {
10241024
}
10251025

10261026
public Map<String, SingleNodeShutdownMetadata> nodeShutdowns() {
1027-
return Optional.ofNullable((NodesShutdownMetadata) this.custom(NodesShutdownMetadata.TYPE))
1028-
.map(NodesShutdownMetadata::getAllNodeMetadataMap)
1029-
.orElse(Collections.emptyMap());
1027+
return this.custom(NodesShutdownMetadata.TYPE, NodesShutdownMetadata.EMPTY).getAllNodeMetadataMap();
10301028
}
10311029

10321030
public Map<String, Custom> customs() {

server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.elasticsearch.xcontent.XContentParser;
2323

2424
import java.io.IOException;
25-
import java.util.Collections;
2625
import java.util.EnumSet;
2726
import java.util.HashMap;
2827
import java.util.List;
@@ -93,11 +92,11 @@ public static NodesShutdownMetadata getShutdownsOrEmpty(final ClusterState state
9392
private final Map<String, SingleNodeShutdownMetadata> nodes;
9493

9594
public NodesShutdownMetadata(Map<String, SingleNodeShutdownMetadata> nodes) {
96-
this.nodes = Collections.unmodifiableMap(nodes);
95+
this.nodes = Map.copyOf(nodes);
9796
}
9897

9998
public NodesShutdownMetadata(StreamInput in) throws IOException {
100-
this(in.readMap(StreamInput::readString, SingleNodeShutdownMetadata::new));
99+
this(in.readImmutableMap(StreamInput::readString, SingleNodeShutdownMetadata::new));
101100
}
102101

103102
@Override

server/src/main/java/org/elasticsearch/cluster/routing/allocation/RoutingAllocation.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.elasticsearch.snapshots.RestoreService.RestoreInProgressUpdater;
2727
import org.elasticsearch.snapshots.SnapshotShardSizeInfo;
2828

29-
import java.util.Collections;
3029
import java.util.HashMap;
3130
import java.util.HashSet;
3231
import java.util.Map;
@@ -73,6 +72,11 @@ public class RoutingAllocation {
7372

7473
private final Map<String, SingleNodeShutdownMetadata> nodeReplacementTargets;
7574

75+
private final Map<String, SingleNodeShutdownMetadata> nodeShutdowns;
76+
77+
@Nullable
78+
private final DesiredNodes desiredNodes;
79+
7680
public RoutingAllocation(
7781
AllocationDeciders deciders,
7882
ClusterState clusterState,
@@ -106,13 +110,15 @@ public RoutingAllocation(
106110
this.clusterInfo = clusterInfo;
107111
this.shardSizeInfo = shardSizeInfo;
108112
this.currentNanoTime = currentNanoTime;
113+
this.nodeShutdowns = clusterState.metadata().nodeShutdowns();
109114
Map<String, SingleNodeShutdownMetadata> targetNameToShutdown = new HashMap<>();
110-
for (SingleNodeShutdownMetadata shutdown : clusterState.metadata().nodeShutdowns().values()) {
115+
for (SingleNodeShutdownMetadata shutdown : nodeShutdowns.values()) {
111116
if (shutdown.getType() == SingleNodeShutdownMetadata.Type.REPLACE) {
112117
targetNameToShutdown.put(shutdown.getTargetNodeName(), shutdown);
113118
}
114119
}
115-
this.nodeReplacementTargets = Collections.unmodifiableMap(targetNameToShutdown);
120+
this.nodeReplacementTargets = Map.copyOf(targetNameToShutdown);
121+
this.desiredNodes = DesiredNodes.latestFromClusterState(clusterState);
116122
}
117123

118124
/** returns the nano time captured at the beginning of the allocation. used to make sure all time based decisions are aligned */
@@ -173,14 +179,14 @@ public SnapshotShardSizeInfo snapshotShardSizeInfo() {
173179

174180
@Nullable
175181
public DesiredNodes desiredNodes() {
176-
return DesiredNodes.latestFromClusterState(clusterState);
182+
return desiredNodes;
177183
}
178184

179185
/**
180186
* Returns the map of node id to shutdown metadata currently in the cluster
181187
*/
182188
public Map<String, SingleNodeShutdownMetadata> nodeShutdowns() {
183-
return metadata().nodeShutdowns();
189+
return nodeShutdowns;
184190
}
185191

186192
/**

server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/NodeShutdownAllocationDecider.java

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,11 @@
99
package org.elasticsearch.cluster.routing.allocation.decider;
1010

1111
import org.elasticsearch.cluster.metadata.IndexMetadata;
12-
import org.elasticsearch.cluster.metadata.Metadata;
13-
import org.elasticsearch.cluster.metadata.NodesShutdownMetadata;
1412
import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata;
1513
import org.elasticsearch.cluster.node.DiscoveryNode;
1614
import org.elasticsearch.cluster.routing.RoutingNode;
1715
import org.elasticsearch.cluster.routing.ShardRouting;
1816
import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
19-
import org.elasticsearch.core.Nullable;
2017

2118
/**
2219
* An allocation decider that prevents shards from being allocated to a
@@ -35,7 +32,7 @@ public class NodeShutdownAllocationDecider extends AllocationDecider {
3532
*/
3633
@Override
3734
public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
38-
final SingleNodeShutdownMetadata thisNodeShutdownMetadata = getNodeShutdownMetadata(allocation.metadata(), node.nodeId());
35+
final SingleNodeShutdownMetadata thisNodeShutdownMetadata = allocation.nodeShutdowns().get(node.nodeId());
3936

4037
if (thisNodeShutdownMetadata == null) {
4138
// There's no shutdown metadata for this node, return yes.
@@ -73,7 +70,7 @@ public Decision canRemain(IndexMetadata indexMetadata, ShardRouting shardRouting
7370
*/
7471
@Override
7572
public Decision shouldAutoExpandToNode(IndexMetadata indexMetadata, DiscoveryNode node, RoutingAllocation allocation) {
76-
SingleNodeShutdownMetadata thisNodeShutdownMetadata = getNodeShutdownMetadata(allocation.metadata(), node.getId());
73+
SingleNodeShutdownMetadata thisNodeShutdownMetadata = allocation.nodeShutdowns().get(node.getId());
7774

7875
if (thisNodeShutdownMetadata == null) {
7976
return allocation.decision(Decision.YES, NAME, "node [%s] is not preparing for removal from the cluster", node.getId());
@@ -95,14 +92,4 @@ public Decision shouldAutoExpandToNode(IndexMetadata indexMetadata, DiscoveryNod
9592
};
9693
}
9794

98-
@Nullable
99-
private static SingleNodeShutdownMetadata getNodeShutdownMetadata(Metadata metadata, String nodeId) {
100-
NodesShutdownMetadata nodesShutdownMetadata = metadata.custom(NodesShutdownMetadata.TYPE);
101-
if (nodesShutdownMetadata == null || nodesShutdownMetadata.getAllNodeMetadataMap() == null) {
102-
// There are no nodes in the process of shutting down, return null.
103-
return null;
104-
}
105-
106-
return nodesShutdownMetadata.getAllNodeMetadataMap().get(nodeId);
107-
}
10895
}

0 commit comments

Comments
 (0)