From b92d48be8fe384d1971dc0058d1811577f84c87f Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 2 Nov 2021 12:02:53 +0100 Subject: [PATCH 1/2] Cache `DiscoveryNode#trimTier` Result No need to recompute this over and over. This is the most expensive part of `FilterAllocationDecider` in benchmarking. Since we already cache the filter itself on the `IndexMetadata`, we can just cache the trimmed version also and `FilterAllocationDecider` effectively disappears from profiling. Also, made the filter immutable (which should as a side-effect also make it faster when iterating over it in the match method). --- .../cluster/node/DiscoveryNodeFilters.java | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java index 2eab251c23c6c..0d4e927de4e7b 100644 --- a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java +++ b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java @@ -20,7 +20,6 @@ import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; -import java.util.stream.Collectors; public class DiscoveryNodeFilters { @@ -66,9 +65,13 @@ public static DiscoveryNodeFilters buildFromKeyValue(OpType opType, Map filters) { this.opType = opType; - this.filters = filters; + this.filters = Map.copyOf(filters); + this.trimmed = doTrim(this); } private boolean matchByIP(String[] values, @Nullable String hostIp, @Nullable String publishIp) { @@ -89,18 +92,24 @@ private boolean matchByIP(String[] values, @Nullable String hostIp, @Nullable St */ @Nullable public static DiscoveryNodeFilters trimTier(@Nullable DiscoveryNodeFilters original) { - if (original == null) { - return null; - } + return original == null ? null : original.trimmed; + } - Map newFilters = original.filters.entrySet() - .stream() - // Remove all entries that use "_tier_preference", as these will be handled elsewhere - .filter(entry -> { - String attr = entry.getKey(); - return attr != null && attr.equals("_tier_preference") == false; - }) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + private static DiscoveryNodeFilters doTrim(DiscoveryNodeFilters original) { + boolean filtered = false; + final Map newFilters = new HashMap<>(original.filters.size()); + // Remove all entries that use "_tier_preference", as these will be handled elsewhere + for (Map.Entry entry : original.filters.entrySet()) { + String attr = entry.getKey(); + if (attr != null && attr.equals("_tier_preference") == false) { + newFilters.put(entry.getKey(), entry.getValue()); + } else { + filtered = true; + } + } + if (filtered == false) { + return original; + } if (newFilters.size() == 0) { return null; From cc60c40c0006ab4dc6660cde4089121f893a4e46 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 2 Nov 2021 13:18:40 +0100 Subject: [PATCH 2/2] faster --- .../cluster/node/DiscoveryNodeFilters.java | 35 ++++++------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java index 0d4e927de4e7b..e1723728d2230 100644 --- a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java +++ b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java @@ -51,7 +51,7 @@ public static DiscoveryNodeFilters buildFromKeyValue(OpType opType, Map bFilters = new HashMap<>(); for (Map.Entry entry : filters.entrySet()) { String[] values = Strings.tokenizeToStringArray(entry.getValue(), ","); - if (values.length > 0) { + if (values.length > 0 && entry.getKey() != null) { bFilters.put(entry.getKey(), values); } } @@ -66,12 +66,12 @@ public static DiscoveryNodeFilters buildFromKeyValue(OpType opType, Map filters) { + private DiscoveryNodeFilters(OpType opType, Map filters) { this.opType = opType; this.filters = Map.copyOf(filters); - this.trimmed = doTrim(this); + this.withoutTierPreferences = doTrimTier(this); } private boolean matchByIP(String[] values, @Nullable String hostIp, @Nullable String publishIp) { @@ -92,30 +92,17 @@ private boolean matchByIP(String[] values, @Nullable String hostIp, @Nullable St */ @Nullable public static DiscoveryNodeFilters trimTier(@Nullable DiscoveryNodeFilters original) { - return original == null ? null : original.trimmed; + return original == null ? null : original.withoutTierPreferences; } - private static DiscoveryNodeFilters doTrim(DiscoveryNodeFilters original) { - boolean filtered = false; - final Map newFilters = new HashMap<>(original.filters.size()); - // Remove all entries that use "_tier_preference", as these will be handled elsewhere - for (Map.Entry entry : original.filters.entrySet()) { - String attr = entry.getKey(); - if (attr != null && attr.equals("_tier_preference") == false) { - newFilters.put(entry.getKey(), entry.getValue()); - } else { - filtered = true; - } - } - if (filtered == false) { + private static DiscoveryNodeFilters doTrimTier(DiscoveryNodeFilters original) { + if (original.filters.containsKey("_tier_preference") == false) { return original; } - - if (newFilters.size() == 0) { - return null; - } else { - return new DiscoveryNodeFilters(original.opType, newFilters); - } + final Map newFilters = new HashMap<>(original.filters); + final String[] removed = newFilters.remove("_tier_preference"); + assert removed != null; + return newFilters.isEmpty() ? null : new DiscoveryNodeFilters(original.opType, newFilters); } public boolean match(DiscoveryNode node) {