diff --git a/plugins/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java b/plugins/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java index 909a493c0734f..18f5ee9ef83ae 100644 --- a/plugins/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java +++ b/plugins/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java @@ -28,6 +28,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.env.NodeEnvironment; import org.opensearch.index.cache.request.RequestCacheStats; import org.opensearch.index.query.QueryBuilders; @@ -102,7 +103,7 @@ private Settings defaultSettings(long sizeInBytes, TimeValue expirationTime) { EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) .get(DISK_MAX_SIZE_IN_BYTES_KEY) .getKey(), - sizeInBytes + new ByteSizeValue(sizeInBytes) ) .put( EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) diff --git a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/EhcacheDiskCacheSettings.java b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/EhcacheDiskCacheSettings.java index d173155080f6a..7850bade98d73 100644 --- a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/EhcacheDiskCacheSettings.java +++ b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/EhcacheDiskCacheSettings.java @@ -12,6 +12,7 @@ import org.opensearch.common.cache.CacheType; import org.opensearch.common.settings.Setting; import org.opensearch.common.unit.TimeValue; +import org.opensearch.core.common.unit.ByteSizeValue; import java.util.HashMap; import java.util.Map; @@ -102,9 +103,9 @@ public class EhcacheDiskCacheSettings { /** * Disk cache max size setting. */ - public static final Setting.AffixSetting DISK_CACHE_MAX_SIZE_IN_BYTES_SETTING = Setting.suffixKeySetting( + public static final Setting.AffixSetting DISK_CACHE_MAX_SIZE_IN_BYTES_SETTING = Setting.suffixKeySetting( EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME + ".max_size_in_bytes", - (key) -> Setting.longSetting(key, DEFAULT_CACHE_SIZE_IN_BYTES, NodeScope) + (key) -> Setting.memorySizeSetting(key, new ByteSizeValue(DEFAULT_CACHE_SIZE_IN_BYTES), NodeScope) ); /** diff --git a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java index b4c62fbf85cb8..523bde96d4c38 100644 --- a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java +++ b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java @@ -35,6 +35,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.io.IOUtils; +import org.opensearch.core.common.unit.ByteSizeValue; import java.io.File; import java.io.IOException; @@ -654,6 +655,11 @@ private V deserializeValue(ByteArrayWrapper binary) { return valueSerializer.deserialize(binary.value); } + // For testing + long getMaxWeightInBytes() { + return maxWeightInBytes; + } + /** * Factory to create an ehcache disk cache. */ @@ -701,7 +707,7 @@ public ICache create(CacheConfig config, CacheType cacheType, .setWeigher(config.getWeigher()) .setRemovalListener(config.getRemovalListener()) .setExpireAfterAccess((TimeValue) settingList.get(DISK_CACHE_EXPIRE_AFTER_ACCESS_KEY).get(settings)) - .setMaximumWeightInBytes((Long) settingList.get(DISK_MAX_SIZE_IN_BYTES_KEY).get(settings)) + .setMaximumWeightInBytes(((ByteSizeValue) settingList.get(DISK_MAX_SIZE_IN_BYTES_KEY).get(settings)).getBytes()) .setSettings(settings) .build(); } diff --git a/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java b/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java index 29551befd3e9f..00135379a99f4 100644 --- a/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java +++ b/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java @@ -28,6 +28,8 @@ import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.bytes.CompositeBytesReference; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.env.NodeEnvironment; import org.opensearch.test.OpenSearchSingleNodeTestCase; @@ -128,7 +130,7 @@ public void testBasicGetAndPutUsingFactory() throws IOException { EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) .get(DISK_MAX_SIZE_IN_BYTES_KEY) .getKey(), - CACHE_SIZE_IN_BYTES + new ByteSizeValue(CACHE_SIZE_IN_BYTES) ) .put( EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) @@ -882,6 +884,107 @@ public void testStatsTrackingDisabled() throws Exception { } } + public void testCacheSizeSetting() throws Exception { + // Cache size setting should be a ByteSizeValue or parseable as bytes, not a long + MockRemovalListener removalListener = new MockRemovalListener<>(); + try (NodeEnvironment env = newNodeEnvironment(Settings.EMPTY)) { + // First try various valid options for the cache size setting + List validSettings = new ArrayList<>(); + List expectedCacheSizes = List.of( + (long) CACHE_SIZE_IN_BYTES, + new ByteSizeValue(10, ByteSizeUnit.GB).getBytes(), + new ByteSizeValue(1, ByteSizeUnit.GB).getBytes(), + new ByteSizeValue(50000000, ByteSizeUnit.BYTES).getBytes(), + EhcacheDiskCacheSettings.DEFAULT_CACHE_SIZE_IN_BYTES + ); // The expected size of the cache produced by each of the settings in validSettings + + // Should be able to pass a ByteSizeValue directly + validSettings.add( + getSettingsExceptSize(env).put( + EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) + .get(DISK_MAX_SIZE_IN_BYTES_KEY) + .getKey(), + new ByteSizeValue(CACHE_SIZE_IN_BYTES) + ).build() + ); + + // Should also be able to pass strings which can be parsed as bytes + List validSettingStrings = List.of("10GB", "1G", "50000000B"); + for (String validString : validSettingStrings) { + validSettings.add( + getSettingsExceptSize(env).put( + EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) + .get(DISK_MAX_SIZE_IN_BYTES_KEY) + .getKey(), + validString + ).build() + ); + } + + // Passing in settings missing a size value should give us the default + validSettings.add(getSettingsExceptSize(env).build()); + assertEquals(validSettings.size(), expectedCacheSizes.size()); + + ICache.Factory ehcacheFactory = new EhcacheDiskCache.EhcacheDiskCacheFactory(); + + for (int i = 0; i < validSettings.size(); i++) { + ICache ehcacheTest = ehcacheFactory.create( + new CacheConfig.Builder().setValueType(String.class) + .setKeyType(String.class) + .setRemovalListener(removalListener) + .setKeySerializer(new StringSerializer()) + .setValueSerializer(new StringSerializer()) + .setDimensionNames(List.of(dimensionName)) + .setWeigher(getWeigher()) + .setSettings(validSettings.get(i)) + .build(), + CacheType.INDICES_REQUEST_CACHE, + Map.of() + ); + assertEquals((long) expectedCacheSizes.get(i), ((EhcacheDiskCache) ehcacheTest).getMaxWeightInBytes()); + ehcacheTest.close(); + } + + // Next try an invalid one and show we can't construct the disk cache + assertThrows(IllegalArgumentException.class, () -> { + ICache ehcacheTest = ehcacheFactory.create( + new CacheConfig.Builder().setValueType(String.class) + .setKeyType(String.class) + .setRemovalListener(removalListener) + .setKeySerializer(new StringSerializer()) + .setValueSerializer(new StringSerializer()) + .setDimensionNames(List.of(dimensionName)) + .setWeigher(getWeigher()) + .setSettings( + getSettingsExceptSize(env).put( + EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) + .get(DISK_MAX_SIZE_IN_BYTES_KEY) + .getKey(), + "1000000" + ).build() + ) + .build(), + CacheType.INDICES_REQUEST_CACHE, + Map.of() + ); + }); + } + } + + private Settings.Builder getSettingsExceptSize(NodeEnvironment env) { + return Settings.builder() + .put( + EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE).get(DISK_STORAGE_PATH_KEY).getKey(), + env.nodePaths()[0].indicesPath.toString() + "/request_cache" + ) + .put( + EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE) + .get(DISK_LISTENER_MODE_SYNC_KEY) + .getKey(), + true + ); + } + private List getRandomDimensions(List dimensionNames) { Random rand = Randomness.get(); int bound = 3;