Преглед на файлове

Fix Slow DiskThresholdSettings (#78071)

This is only really relevant to test execution but it takes many seconds
to parse these settings during a run of the server:internalClusterTests for example
due to constant throwing in the adjusted methods. Added some trivial short-circuits
and slightly optimized `ByteSizeValue` for some defaults and to make it so this
class effectively disappears from profiling.

Co-authored-by: David Turner <david.turner@elastic.co>
Armin Braun преди 4 години
родител
ревизия
bb966909eb

+ 26 - 6
server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java

@@ -182,11 +182,13 @@ public class DiskThresholdSettings {
     }
 
     private static void doValidate(String low, String high, String flood) {
-        try {
-            doValidateAsPercentage(low, high, flood);
-            return; // early return so that we do not try to parse as bytes
-        } catch (final ElasticsearchParseException e) {
-            // swallow as we are now going to try to parse as bytes
+        if (definitelyNotPercentage(low) == false) { // only try to validate as percentage if it isn't obviously a byte size value
+            try {
+                doValidateAsPercentage(low, high, flood);
+                return; // early return so that we do not try to parse as bytes
+            } catch (final ElasticsearchParseException e) {
+                // swallow as we are now going to try to parse as bytes
+            }
         }
         try {
             doValidateAsBytes(low, high, flood);
@@ -376,6 +378,10 @@ public class DiskThresholdSettings {
      * @return the parsed percentage
      */
     private static double thresholdPercentageFromWatermark(String watermark, boolean lenient) {
+        if (lenient && definitelyNotPercentage(watermark)) {
+            // obviously not a percentage so return lenient fallback value like we would below on a parse failure
+            return 100.0;
+        }
         try {
             return RatioValue.parseRatioValue(watermark).getAsPercent();
         } catch (ElasticsearchParseException ex) {
@@ -412,7 +418,7 @@ public class DiskThresholdSettings {
             // NOTE: this is not end-user leniency, since up above we check that it's a valid byte or percentage, and then store the two
             // cases separately
             if (lenient) {
-                return ByteSizeValue.parseBytesSizeValue("0b", settingName);
+                return ByteSizeValue.ZERO;
             }
             throw ex;
         }
@@ -423,6 +429,11 @@ public class DiskThresholdSettings {
      * @return the watermark value given
      */
     private static String validWatermarkSetting(String watermark, String settingName) {
+        if (definitelyNotPercentage(watermark)) {
+            // short circuit to save expensive exception on obvious byte size value below
+            ByteSizeValue.parseBytesSizeValue(watermark, settingName);
+            return watermark;
+        }
         try {
             RatioValue.parseRatioValue(watermark);
         } catch (ElasticsearchParseException e) {
@@ -435,4 +446,13 @@ public class DiskThresholdSettings {
         }
         return watermark;
     }
+
+    // Checks that a value is definitely not a percentage by testing if it ends on `b` which implies that it is probably a byte size value
+    // instead. This is used to make setting validation skip attempting to parse a value as a percentage/ration for the settings in this
+    // class that accept either a byte size value. The main motivation of this method is to make tests faster. Some tests call this method
+    // frequently when starting up internal cluster nodes and using exception throwing and catching when trying to parse as a ratio as a
+    // means of identifying that a string is not a ratio is quite slow.
+    private static boolean definitelyNotPercentage(String value) {
+        return value.endsWith("b");
+    }
 }

+ 16 - 6
server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java

@@ -35,6 +35,8 @@ public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue>, ToXC
     }
 
     public static final ByteSizeValue ZERO = new ByteSizeValue(0, ByteSizeUnit.BYTES);
+    public static final ByteSizeValue ONE = new ByteSizeValue(1, ByteSizeUnit.BYTES);
+    public static final ByteSizeValue MINUS_ONE = new ByteSizeValue(-1, ByteSizeUnit.BYTES);
 
     public static ByteSizeValue ofBytes(long size) {
         return new ByteSizeValue(size);
@@ -202,6 +204,20 @@ public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue>, ToXC
         if (sValue == null) {
             return defaultValue;
         }
+        switch (sValue) {
+            case "0":
+            case "0b":
+            case "0B":
+                return ZERO;
+            case "1b":
+            case "1B":
+                // "1" is deliberately omitted, the units are required for all values except "0" and "-1"
+                return ONE;
+            case "-1":
+            case "-1b":
+            case "-1B":
+                return MINUS_ONE;
+        }
         String lowerSValue = sValue.toLowerCase(Locale.ROOT).trim();
         if (lowerSValue.endsWith("k")) {
             return parse(sValue, lowerSValue, "k", ByteSizeUnit.KB, settingName);
@@ -225,12 +241,6 @@ public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue>, ToXC
             return parse(sValue, lowerSValue, "pb", ByteSizeUnit.PB, settingName);
         } else if (lowerSValue.endsWith("b")) {
             return parseBytes(lowerSValue, settingName, sValue);
-        } else if (lowerSValue.equals("-1")) {
-            // Allow this special value to be unit-less:
-            return new ByteSizeValue(-1, ByteSizeUnit.BYTES);
-        } else if (lowerSValue.equals("0")) {
-            // Allow this special value to be unit-less:
-            return new ByteSizeValue(0, ByteSizeUnit.BYTES);
         } else {
             // Missing units:
             throw new ElasticsearchParseException(