From e18b5ea7e60e9dbf6a952f97b9a7dd1dc387682b Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Wed, 22 Sep 2021 11:06:07 +0200
Subject: [PATCH] Speed up DataTierAllocationDecider (#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 <joegallo@gmail.com>
---
 .../allocation/DataTierAllocationDecider.java | 38 ++++++++++++-------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java
index e5744a09f5abb..7d3e2c97dc66f 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java
+++ b/x-pack/plugin/core/src/main/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;
     }
 }