Browse Source

Speed up Routing Nodes Priority Comparator (#78609)

This one shows up very hot if sorting a long list of shards because of the
cost of looking up the settings over and over.
Armin Braun 4 years ago
parent
commit
6089669d87

+ 20 - 8
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

@@ -392,6 +392,10 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
 
     private final IndexLongFieldRange timestampRange;
 
+    private final int priority;
+
+    private final long creationDate;
+
     private IndexMetadata(
             final Index index,
             final long version,
@@ -417,7 +421,9 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
             final ActiveShardCount waitForActiveShards,
             final ImmutableOpenMap<String, RolloverInfo> rolloverInfos,
             final boolean isSystem,
-            final IndexLongFieldRange timestampRange) {
+            final IndexLongFieldRange timestampRange,
+            final int priority,
+            final long creationDate) {
 
         this.index = index;
         this.version = version;
@@ -450,6 +456,8 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
         this.rolloverInfos = rolloverInfos;
         this.isSystem = isSystem;
         this.timestampRange = timestampRange;
+        this.priority = priority;
+        this.creationDate = creationDate;
         assert numberOfShards * routingFactor == routingNumShards :  routingNumShards + " must be a multiple of " + numberOfShards;
     }
 
@@ -509,7 +517,7 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
     }
 
     public long getCreationDate() {
-        return settings.getAsLong(SETTING_CREATION_DATE, -1L);
+        return creationDate;
     }
 
     public State getState() {
@@ -930,6 +938,10 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
         return isSystem;
     }
 
+    public int priority() {
+        return priority;
+    }
+
     public static Builder builder(String index) {
         return new Builder(index);
     }
@@ -1206,9 +1218,6 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
         }
 
         public IndexMetadata build() {
-            ImmutableOpenMap.Builder<String, AliasMetadata> tmpAliases = aliases;
-            Settings tmpSettings = settings;
-
             /*
              * We expect that the metadata has been properly built to set the number of shards and the number of replicas, and do not rely
              * on the default values here. Those must have been set upstream.
@@ -1294,9 +1303,9 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
                     state,
                     numberOfShards,
                     numberOfReplicas,
-                    tmpSettings,
+                    settings,
                     mappings.build(),
-                    tmpAliases.build(),
+                    aliases.build(),
                     customMetadata.build(),
                     filledInSyncAllocationIds.build(),
                     requireFilters,
@@ -1309,7 +1318,10 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
                     waitForActiveShards,
                     rolloverInfos.build(),
                     isSystem,
-                timestampRange);
+                    timestampRange,
+                    IndexMetadata.INDEX_PRIORITY_SETTING.get(settings),
+                    settings.getAsLong(SETTING_CREATION_DATE, -1L)
+            );
         }
 
         @SuppressWarnings("unchecked")

+ 0 - 4
server/src/main/java/org/elasticsearch/cluster/routing/RoutingNodes.java

@@ -16,7 +16,6 @@ import org.elasticsearch.Assertions;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.Metadata;
-import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.routing.UnassignedInfo.AllocationStatus;
 import org.elasticsearch.cluster.routing.allocation.ExistingShardsAllocator;
@@ -67,8 +66,6 @@ public class RoutingNodes implements Iterable<RoutingNode> {
 
     private final Map<ShardId, List<ShardRouting>> assignedShards = new HashMap<>();
 
-    private final Map<String, SingleNodeShutdownMetadata> nodeShutdowns;
-
     private final boolean readOnly;
 
     private int inactivePrimaryCount = 0;
@@ -87,7 +84,6 @@ public class RoutingNodes implements Iterable<RoutingNode> {
     public RoutingNodes(ClusterState clusterState, boolean readOnly) {
         this.readOnly = readOnly;
         final RoutingTable routingTable = clusterState.routingTable();
-        nodeShutdowns = clusterState.metadata().nodeShutdowns();
 
         Map<String, LinkedHashMap<ShardId, ShardRouting>> nodesToShards = new HashMap<>();
         // fill in the nodeToShards with the "live" nodes

+ 7 - 18
server/src/main/java/org/elasticsearch/gateway/PriorityComparator.java

@@ -11,7 +11,6 @@ package org.elasticsearch.gateway;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.routing.ShardRouting;
 import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.Index;
 
 import java.util.Comparator;
@@ -32,23 +31,21 @@ public abstract class PriorityComparator implements Comparator<ShardRouting> {
 
     @Override
     public final int compare(ShardRouting o1, ShardRouting o2) {
-        final String o1Index = o1.getIndexName();
-        final String o2Index = o2.getIndexName();
+        final Index o1Index = o1.index();
+        final Index o2Index = o2.index();
         int cmp = 0;
         if (o1Index.equals(o2Index) == false) {
-            final IndexMetadata metadata01 = getMetadata(o1.index());
-            final IndexMetadata metadata02 = getMetadata(o2.index());
+            final IndexMetadata metadata01 = getMetadata(o1Index);
+            final IndexMetadata metadata02 = getMetadata(o2Index);
             cmp = Boolean.compare(metadata02.isSystem(), metadata01.isSystem());
 
             if (cmp == 0) {
-                final Settings settingsO1 = metadata01.getSettings();
-                final Settings settingsO2 = metadata02.getSettings();
-                cmp = Long.compare(priority(settingsO2), priority(settingsO1));
+                cmp = Long.compare(metadata02.priority(), metadata01.priority());
 
                 if (cmp == 0) {
-                    cmp = Long.compare(timeCreated(settingsO2), timeCreated(settingsO1));
+                    cmp = Long.compare(metadata02.getCreationDate(), metadata01.getCreationDate());
                     if (cmp == 0) {
-                        cmp = o2Index.compareTo(o1Index);
+                        cmp = o2Index.getName().compareTo(o1Index.getName());
                     }
                 }
             }
@@ -56,14 +53,6 @@ public abstract class PriorityComparator implements Comparator<ShardRouting> {
         return cmp;
     }
 
-    private static int priority(Settings settings) {
-        return IndexMetadata.INDEX_PRIORITY_SETTING.get(settings);
-    }
-
-    private static long timeCreated(Settings settings) {
-        return settings.getAsLong(IndexMetadata.SETTING_CREATION_DATE, -1L);
-    }
-
     protected abstract IndexMetadata getMetadata(Index index);
 
     /**