Add min/max size methods for MemorySizeSetting

CHANGELOG.md

@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add new extensible method to DocRequest to specify type ([#19313](https://github.com/opensearch-project/OpenSearch/pull/19313))
 - [Rule based auto-tagging] Add Rule based auto-tagging IT ([#18550](https://github.com/opensearch-project/OpenSearch/pull/18550))
 - Add all-active ingestion as docrep equivalent in pull-based ingestion ([#19316](https://github.com/opensearch-project/OpenSearch/pull/19316))
+- Add convenience methods for making MemorySizeSetting with bounds ([#19230](https://github.com/opensearch-project/OpenSearch/pull/19230))
 
 ### Changed
 - Refactor `if-else` chains to use `Java 17 pattern matching switch expressions`(([#18965](https://github.com/opensearch-project/OpenSearch/pull/18965))

server/src/main/java/org/opensearch/common/settings/Setting.java

@@ -2243,43 +2243,103 @@ public static Setting<ByteSizeValue> memorySizeSetting(String key, String defaul
         return new Setting<>(key, (s) -> defaultPercentage, new MemorySizeValueParser(key), properties);
     }
 
+    /**
+     * Creates a setting which specifies a memory size. This can either be
+     * specified as an absolute bytes value or as a percentage of the heap
+     * memory.
+     *
+     * @param key the key for the setting
+     * @param defaultPercentage the default value of this setting as a percentage of the heap memory
+     * @param minPercentage the minimum allowable value of this setting as a percentage of the heap memory
+     * @param maxPercentage the maximum allowable value of this setting as a percentage of the heap memory
+     * @param properties properties for this setting like scope, filtering...
+     * @return the setting object
+     */
+    public static Setting<ByteSizeValue> memorySizeSetting(
+        String key,
+        String defaultPercentage,
+        String minPercentage,
+        String maxPercentage,
+        Property... properties
+    ) {
+        return new Setting<>(key, (s) -> defaultPercentage, new MemorySizeValueParser(key, minPercentage, maxPercentage), properties);
+    }
+
     /**
      * A writeable parser for memory size value
      */
     public static class MemorySizeValueParser implements Function<String, ByteSizeValue>, Writeable {
         private String key;
+        private final ByteSizeValue minValue;
+        private final ByteSizeValue maxValue;
 
         public MemorySizeValueParser(String key) {
             this.key = key;
+            this.minValue = new ByteSizeValue(-1);
+            this.maxValue = new ByteSizeValue(Long.MAX_VALUE);
+        }
+
+        public MemorySizeValueParser(String key, String minValue, String maxValue) {
+            this.key = key;
+            this.minValue = MemorySizeValue.parseBytesSizeValueOrHeapRatio(minValue, key);
+            this.maxValue = MemorySizeValue.parseBytesSizeValueOrHeapRatio(maxValue, key);
         }
 
         public MemorySizeValueParser(StreamInput in) throws IOException {
             key = in.readString();
+            if (in.getVersion().onOrAfter(Version.V_3_3_0)) {
+                this.minValue = new ByteSizeValue(in);
+                this.maxValue = new ByteSizeValue(in);
+            } else {
+                this.minValue = new ByteSizeValue(-1);
+                this.maxValue = new ByteSizeValue(Long.MAX_VALUE);
+            }
         }
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             out.writeString(key);
+            if (out.getVersion().onOrAfter(Version.V_3_3_0)) {
+                minValue.writeTo(out);
+                maxValue.writeTo(out);
+            }
         }
 
         public String getKey() {
             return key;
         }
 
+        public ByteSizeValue getMin() {
+            return minValue;
+        }
+
+        public ByteSizeValue getMax() {
+            return maxValue;
+        }
+
         @Override
         public ByteSizeValue apply(String s) {
-            return MemorySizeValue.parseBytesSizeValueOrHeapRatio(s, key);
+            ByteSizeValue parsedValue = MemorySizeValue.parseBytesSizeValueOrHeapRatio(s, key);
+            if (parsedValue.getBytes() < minValue.getBytes()) {
+                String err = "Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue.toString();
+                throw new IllegalArgumentException(err);
+            }
+            if (parsedValue.getBytes() > maxValue.getBytes()) {
+                String err = "Failed to parse value [" + s + "] for setting [" + key + "] must be <= " + maxValue.toString();
+                throw new IllegalArgumentException(err);
+            }
+            return parsedValue;
         }
 
         public boolean equals(Object obj) {
             if (this == obj) return true;
             if (obj == null || getClass() != obj.getClass()) return false;
             MemorySizeValueParser that = (MemorySizeValueParser) obj;
-            return Objects.equals(key, that.key);
+            return Objects.equals(key, that.key) && minValue.equals(that.minValue) && maxValue.equals(that.maxValue);
         }
 
         public int hashCode() {
-            return Objects.hash(key);
+            return Objects.hash(key, minValue, maxValue);
         }
     }
 

server/src/test/java/org/opensearch/common/settings/MemorySizeSettingsTests.java

@@ -45,6 +45,9 @@
 import org.opensearch.repositories.blobstore.BlobStoreRepository;
 import org.opensearch.test.OpenSearchTestCase;
 
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.notNullValue;
@@ -142,6 +145,73 @@ public void testSnapshotRepositoryDataCacheSizeSetting() {
         );
     }
 
+    public void testMinMaxSettingValues() {
+        String key = "setting_key";
+        double minPercentage = 0.5;
+        double maxPercentage = 10;
+        long minBytes = (long) (minPercentage * 0.01 * JvmInfo.jvmInfo().getMem().getHeapMax().getBytes());
+        long maxBytes = (long) (maxPercentage * 0.01 * JvmInfo.jvmInfo().getMem().getHeapMax().getBytes());
+        double defaultPercentage = 1;
+        long defaultBytes = (long) (defaultPercentage * 0.01 * JvmInfo.jvmInfo().getMem().getHeapMax().getBytes());
+
+        // Test inputs both as percents and byte strings
+        Setting<ByteSizeValue> dummySettingWithPercents = Setting.memorySizeSetting(
+            key,
+            defaultPercentage + "%",
+            minPercentage + "%",
+            maxPercentage + "%",
+            Property.Dynamic
+        );
+        Setting<ByteSizeValue> dummySettingWithBytes = Setting.memorySizeSetting(
+            key,
+            defaultBytes + "b",
+            minBytes + "b",
+            maxBytes + "b",
+            Property.Dynamic
+        );
+        for (Setting<ByteSizeValue> dummySetting : List.of(dummySettingWithBytes, dummySettingWithPercents)) {
+            assertThrows(IllegalArgumentException.class, () -> { dummySetting.get(Settings.builder().put(key, "0.49%").build()); });
+            assertThrows(IllegalArgumentException.class, () -> { dummySetting.get(Settings.builder().put(key, "11%").build()); });
+
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> { dummySetting.get(Settings.builder().put(key, minBytes - 1 + "b").build()); }
+            );
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> { dummySetting.get(Settings.builder().put(key, maxBytes + 1 + "b").build()); }
+            );
+
+            Settings settings = Settings.builder().put(key, "10%").build();
+            ByteSizeValue value = dummySetting.get(settings);
+            assertEquals(value.getBytes(), (long) (JvmInfo.jvmInfo().getMem().getHeapMax().getBytes() * 0.1));
+
+            settings = Settings.builder().put(key, "0.5%").build();
+            value = dummySetting.get(settings);
+            assertEquals(value.getBytes(), (long) (JvmInfo.jvmInfo().getMem().getHeapMax().getBytes() * 0.005));
+
+            assertEquals(dummySetting.get(Settings.EMPTY).getBytes(), defaultBytes);
+
+            // Show we can't dynamically update it out of the bounds
+            AtomicReference<ByteSizeValue> reference = new AtomicReference<>(null);
+            ClusterSettings.SettingUpdater<ByteSizeValue> settingUpdater = dummySetting.newUpdater(reference::set, logger);
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> { settingUpdater.apply(Settings.builder().put(key, "11%").build(), Settings.EMPTY); }
+            );
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> { settingUpdater.apply(Settings.builder().put(key, "0.49%").build(), Settings.EMPTY); }
+            );
+            assertThrows(IllegalArgumentException.class, () -> {
+                settingUpdater.apply(Settings.builder().put(key, minBytes - 1 + "b").build(), Settings.EMPTY);
+            });
+            assertThrows(IllegalArgumentException.class, () -> {
+                settingUpdater.apply(Settings.builder().put(key, maxBytes + 1 + "b").build(), Settings.EMPTY);
+            });
+        }
+    }
+
     private void assertMemorySizeSetting(Setting<ByteSizeValue> setting, String settingKey, ByteSizeValue defaultValue) {
         assertMemorySizeSetting(setting, settingKey, defaultValue, Settings.EMPTY);
     }

server/src/test/java/org/opensearch/common/settings/SettingTests.java

@@ -1383,16 +1383,21 @@ public void testByteSizeValueParser() throws Exception {
     // MemorySizeValue
     public void testMemorySizeValueParser() throws Exception {
         String expectedKey = "test key";
-        MemorySizeValueParser memorySizeValueParser = new MemorySizeValueParser(expectedKey);
-
-        assertEquals(expectedKey, memorySizeValueParser.getKey());
-
-        try (BytesStreamOutput out = new BytesStreamOutput()) {
-            memorySizeValueParser.writeTo(out);
-            out.flush();
-            try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) {
-                memorySizeValueParser = new MemorySizeValueParser(in);
-                assertEquals(expectedKey, memorySizeValueParser.getKey());
+        MemorySizeValueParser memorySizeValueParserWithoutBounds = new MemorySizeValueParser(expectedKey);
+        MemorySizeValueParser memorySizeValueParserWithBounds = new MemorySizeValueParser(expectedKey, "0%", "10%");
+        for (MemorySizeValueParser memorySizeValueParser : List.of(memorySizeValueParserWithBounds, memorySizeValueParserWithoutBounds)) {
+
+            assertEquals(expectedKey, memorySizeValueParser.getKey());
+
+            try (BytesStreamOutput out = new BytesStreamOutput()) {
+                memorySizeValueParser.writeTo(out);
+                out.flush();
+                try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) {
+                    MemorySizeValueParser deserialized = new MemorySizeValueParser(in);
+                    assertEquals(expectedKey, deserialized.getKey());
+                    assertEquals(memorySizeValueParser.getMax(), deserialized.getMax());
+                    assertEquals(memorySizeValueParser.getMin(), deserialized.getMin());
+                }
             }
         }
     }