Speed up DataTierAllocationDecider (elastic#78075)

This decider is quite slow in `allocationAllowed` and shows up in profiling. We should be able to get a much tighter loop with this change that avoids building the list of role names over and over and removes some dead code as well. Co-authored-by: Joe Gallo <[email protected]>
original-brownbear · Sep 22, 2021 · e18b5ea · e18b5ea
1 parent 67fb3b8
commit e18b5ea
Showing 1 changed file with 24 additions and 14 deletions.
diff --git a/...in/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java b/...in/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java
@@ -294,7 +294,12 @@ public static Optional<String> preferredAvailableTier(String prioritizedTiers, D
     }
 
     public static String[] parseTierList(String tiers) {
-        return Strings.tokenizeToStringArray(tiers, ",");
+        if (Strings.hasText(tiers) == false) {
+            // avoid parsing overhead in the null/empty string case
+            return Strings.EMPTY_ARRAY;
+        } else {
+            return Strings.tokenizeToStringArray(tiers, ",");
+        }
     }
 
     static boolean tierNodesPresent(String singleTier, DiscoveryNodes nodes) {
@@ -312,24 +317,29 @@ static boolean tierNodesPresent(String singleTier, DiscoveryNodes nodes) {
 
 
     private static boolean allocationAllowed(OpType opType, String tierSetting, Set<DiscoveryNodeRole> roles) {
-        String[] values = parseTierList(tierSetting);
-        for (String value : values) {
+        assert Strings.hasText(tierSetting) : "tierName must be not null and non-empty, but was [" + tierSetting + "]";
+
+        if (roles.contains(DiscoveryNodeRole.DATA_ROLE)) {
             // generic "data" roles are considered to have all tiers
-            if (roles.contains(DiscoveryNodeRole.DATA_ROLE) ||
-                roles.stream().map(DiscoveryNodeRole::roleName).collect(Collectors.toSet()).contains(value)) {
+            return true;
+        }
+        String[] values = parseTierList(tierSetting);
+        for (String tierName : values) {
+            boolean containsName = false;
+            for (DiscoveryNodeRole role : roles) {
+                if (tierName.equals(role.roleName())) {
+                    containsName = true;
+                    break;
+                }
+            }
+            if (containsName) {
                 if (opType == OpType.OR) {
                     return true;
                 }
-            } else {
-                if (opType == OpType.AND) {
-                    return false;
-                }
+            } else if (opType == OpType.AND) {
+                return false;
             }
         }
-        if (opType == OpType.OR) {
-            return false;
-        } else {
-            return true;
-        }
+        return opType == OpType.AND;
     }
 }