Skip to content

Commit

Permalink
Make ehcache size setting take ByteSizeValue
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
  • Loading branch information
Peter Alfonsi committed May 30, 2024
1 parent 0c0a2b3 commit 7ce5180
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 5 deletions.
3 changes: 2 additions & 1 deletion ...s/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions plugins/cache-ehcache/src/main/java/org/opensearch/cache/EhcacheDiskCacheSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -102,9 +103,9 @@ public class EhcacheDiskCacheSettings {
/**
* Disk cache max size setting.
*/
public static final Setting.AffixSetting<Long> DISK_CACHE_MAX_SIZE_IN_BYTES_SETTING = Setting.suffixKeySetting(
public static final Setting.AffixSetting<ByteSizeValue> 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)
);

/**
Expand Down
8 changes: 7 additions & 1 deletion plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -701,7 +707,7 @@ public <K, V> ICache<K, V> create(CacheConfig<K, V> 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();
}
Expand Down
105 changes: 104 additions & 1 deletion ...ns/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<String, String> removalListener = new MockRemovalListener<>();
try (NodeEnvironment env = newNodeEnvironment(Settings.EMPTY)) {
// First try various valid options for the cache size setting
List<Settings> validSettings = new ArrayList<>();
List<Long> 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<String> 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<String, String> ehcacheTest = ehcacheFactory.create(
new CacheConfig.Builder<String, String>().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<String, String>) ehcacheTest).getMaxWeightInBytes());
ehcacheTest.close();
}

// Next try an invalid one and show we can't construct the disk cache
assertThrows(IllegalArgumentException.class, () -> {
ICache<String, String> ehcacheTest = ehcacheFactory.create(
new CacheConfig.Builder<String, String>().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<String> getRandomDimensions(List<String> dimensionNames) {
Random rand = Randomness.get();
int bound = 3;
Expand Down

0 comments on commit 7ce5180

Please sign in to comment.