Skip to content

Commit

Permalink
Changed logic to use new setting and deprecate old one
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Alfonsi <[email protected]>
  • Loading branch information
Peter Alfonsi committed Jul 26, 2024
1 parent 4510966 commit 588c00e
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 12 deletions.
21 changes: 18 additions & 3 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 @@ -101,10 +101,18 @@ public class EhcacheDiskCacheSettings {
);

/**
* Disk cache max size setting.
* Deprecated: Disk cache max size setting, which takes a long.
*/
public static final Setting.AffixSetting<ByteSizeValue> DISK_CACHE_MAX_SIZE_IN_BYTES_SETTING = Setting.suffixKeySetting(
public static final Setting.AffixSetting<Long> 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, Setting.Property.Deprecated)
);

/**
* Disk cache max size setting, which takes a ByteSizeValue.
*/
public static final Setting.AffixSetting<ByteSizeValue> DISK_CACHE_MAX_SIZE_SETTING = Setting.suffixKeySetting(
EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME + ".max_size",
(key) -> Setting.memorySizeSetting(key, new ByteSizeValue(DEFAULT_CACHE_SIZE_IN_BYTES), NodeScope)
);

Expand All @@ -121,9 +129,14 @@ public class EhcacheDiskCacheSettings {
*/
public static final String DISK_SEGMENT_KEY = "disk_segment";
/**
* Key for max size.
* Deprecated: Key for max size.
*/
public static final String DISK_MAX_SIZE_IN_BYTES_KEY = "max_size_in_bytes";

/**
* Key for max size.
*/
public static final String DISK_MAX_SIZE_KEY = "max_size";
/**
* Key for expire after access.
*/
Expand Down Expand Up @@ -177,6 +190,8 @@ public class EhcacheDiskCacheSettings {
DISK_STORAGE_PATH_SETTING,
DISK_MAX_SIZE_IN_BYTES_KEY,
DISK_CACHE_MAX_SIZE_IN_BYTES_SETTING,
DISK_MAX_SIZE_KEY,
DISK_CACHE_MAX_SIZE_SETTING,
DISK_LISTENER_MODE_SYNC_KEY,
DISK_CACHE_LISTENER_MODE_SYNC_SETTING
);
Expand Down
19 changes: 18 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 @@ -83,6 +83,7 @@
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_CACHE_EXPIRE_AFTER_ACCESS_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_LISTENER_MODE_SYNC_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_MAX_SIZE_IN_BYTES_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_MAX_SIZE_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_SEGMENT_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_STORAGE_PATH_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_WRITE_CONCURRENCY_KEY;
Expand Down Expand Up @@ -675,6 +676,22 @@ public static class EhcacheDiskCacheFactory implements ICache.Factory {
*/
public EhcacheDiskCacheFactory() {}

private long getMaxSizeInBytes(Map<String, Setting<?>> settingList, Settings settings) {
/*
Use DISK_CACHE_MAX_SIZE_SETTING. If absent, check for the deprecated DISK_CACHE_MAX_SIZE_IN_BYTES_SETTING
to use as a fallback. If that's also absent, use the default value of DISK_CACHE_MAX_SIZE_SETTING.
*/
if (settingList.get(DISK_MAX_SIZE_KEY).exists(settings)) {
return ((ByteSizeValue) settingList.get(DISK_MAX_SIZE_KEY).get(settings)).getBytes();
} else {
if (settingList.get(DISK_MAX_SIZE_IN_BYTES_KEY).exists(settings)) {
return (long) settingList.get(DISK_MAX_SIZE_IN_BYTES_KEY).get(settings);
} else {
return ((ByteSizeValue) settingList.get(DISK_MAX_SIZE_KEY).get(settings)).getBytes();
}
}
}

@Override
@SuppressWarnings({ "unchecked" }) // Required to ensure the serializers output byte[]
public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType, Map<String, Factory> cacheFactories) {
Expand Down Expand Up @@ -707,7 +724,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(((ByteSizeValue) settingList.get(DISK_MAX_SIZE_IN_BYTES_KEY).get(settings)).getBytes())
.setMaximumWeightInBytes(getMaxSizeInBytes(settingList, settings))
.setSettings(settings)
.build();
}
Expand Down
50 changes: 42 additions & 8 deletions ...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 @@ -49,8 +49,12 @@
import java.util.concurrent.Phaser;
import java.util.function.ToLongBiFunction;

import org.ehcache.impl.serialization.StringSerializer;

import static org.opensearch.cache.EhcacheDiskCacheSettings.DEFAULT_CACHE_SIZE_IN_BYTES;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_LISTENER_MODE_SYNC_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_MAX_SIZE_IN_BYTES_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_MAX_SIZE_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_STORAGE_PATH_KEY;
import static org.hamcrest.CoreMatchers.instanceOf;

Expand Down Expand Up @@ -895,15 +899,15 @@ public void testCacheSizeSetting() throws Exception {
new ByteSizeValue(10, ByteSizeUnit.GB).getBytes(),
new ByteSizeValue(1, ByteSizeUnit.GB).getBytes(),
new ByteSizeValue(50000000, ByteSizeUnit.BYTES).getBytes(),
EhcacheDiskCacheSettings.DEFAULT_CACHE_SIZE_IN_BYTES
DEFAULT_CACHE_SIZE_IN_BYTES,
1_000_000L,
2_000_000L
); // 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(),
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE).get(DISK_MAX_SIZE_KEY).getKey(),
new ByteSizeValue(CACHE_SIZE_IN_BYTES)
).build()
);
Expand All @@ -914,17 +918,44 @@ public void testCacheSizeSetting() throws Exception {
validSettings.add(
getSettingsExceptSize(env).put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(DISK_MAX_SIZE_IN_BYTES_KEY)
.get(DISK_MAX_SIZE_KEY)
.getKey(),
validString
).build()
);
}

// Passing in settings missing a size value should give us the default
// Passing in settings missing either size setting should give us the default
validSettings.add(getSettingsExceptSize(env).build());
assertEquals(validSettings.size(), expectedCacheSizes.size());

// Deprecated setting present, correct setting absent
validSettings.add(
getSettingsExceptSize(env).put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(DISK_MAX_SIZE_IN_BYTES_KEY)
.getKey(),
1_000_000L
).build()
);

// Both settings present, the correct one should be used
validSettings.add(
getSettingsExceptSize(env).put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(DISK_MAX_SIZE_IN_BYTES_KEY)
.getKey(),
1_000_000L
)
.put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(DISK_MAX_SIZE_KEY)
.getKey(),
new ByteSizeValue(2_000_000L)
)
.build()
);

assertEquals(validSettings.size(), expectedCacheSizes.size());
ICache.Factory ehcacheFactory = new EhcacheDiskCache.EhcacheDiskCacheFactory();

for (int i = 0; i < validSettings.size(); i++) {
Expand Down Expand Up @@ -958,7 +989,7 @@ public void testCacheSizeSetting() throws Exception {
.setSettings(
getSettingsExceptSize(env).put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(DISK_MAX_SIZE_IN_BYTES_KEY)
.get(DISK_MAX_SIZE_KEY)
.getKey(),
"1000000"
).build()
Expand All @@ -968,6 +999,9 @@ public void testCacheSizeSetting() throws Exception {
Map.of()
);
});
assertWarnings(
"[indices.requests.cache.ehcache_disk.max_size_in_bytes] setting was deprecated in OpenSearch and will be removed in a future release! See the breaking changes documentation for the next major version."
);
}
}

Expand Down

0 comments on commit 588c00e

Please sign in to comment.