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 85253b3538a7e..08729c54188a1 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 @@ -482,6 +482,7 @@ public void onEvent(CacheEvent, ? extends byte[]> event) this.removalListener.onRemoval(new RemovalNotification<>(event.getKey(), valueSerializer.deserialize(event.getOldValue()), RemovalReason.EVICTED)); stats.decrementEntriesByDimensions(event.getKey().dimensions); stats.incrementMemorySizeByDimensions(event.getKey().dimensions, -getOldValuePairSize(event)); + stats.incrementEvictionsByDimensions(event.getKey().dimensions); assert event.getNewValue() == null; break; case REMOVED: @@ -556,11 +557,30 @@ public EhcacheDiskCacheFactory() {} public ICache create(CacheConfig config, CacheType cacheType, Map cacheFactories) { Map> settingList = EhcacheDiskCacheSettings.getSettingListForCacheType(cacheType); Settings settings = config.getSettings(); + + Serializer keySerializer = null; + try { + keySerializer = (Serializer) config.getKeySerializer(); + } catch (ClassCastException e) { + throw new IllegalArgumentException("EhcacheDiskCache requires a key serializer of type Serializer"); + } + + Serializer valueSerializer = null; + try { + valueSerializer = (Serializer) config.getValueSerializer(); + } catch (ClassCastException e) { + throw new IllegalArgumentException("EhcacheDiskCache requires a value serializer of type Serializer"); + } + return new Builder().setStoragePath((String) settingList.get(DISK_STORAGE_PATH_KEY).get(settings)) .setDiskCacheAlias((String) settingList.get(DISK_CACHE_ALIAS_KEY).get(settings)) .setCacheType(cacheType) .setKeyType((config.getKeyType())) .setValueType(config.getValueType()) + .setKeySerializer(keySerializer) + .setValueSerializer(valueSerializer) + .setShardIdDimensionName(config.getDimensionNames().get(0)) // TODO: Rework this to pass in whole list, once stats is changed + .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)) 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 eb37db4a0bd3d..a43ae63a1cf49 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 @@ -110,6 +110,10 @@ public void testBasicGetAndPutUsingFactory() throws IOException { new CacheConfig.Builder().setValueType(String.class) .setKeyType(String.class) .setRemovalListener(removalListener) + .setKeySerializer(new StringSerializer()) + .setValueSerializer(new StringSerializer()) + .setDimensionNames(List.of(dimensionName)) + .setWeigher(getWeigher()) .setSettings( Settings.builder() .put( @@ -316,19 +320,11 @@ public void testEvictions() throws Exception { String value = generateRandomString(100); // Trying to generate more than 100kb to cause evictions. - long sizeOfAttemptedAdds = 0; - long sizeOfAttemptedAddsValue = 0; for (int i = 0; i < 1000; i++) { String key = "Key" + i; ICacheKey iCacheKey = getICacheKey((key)); - sizeOfAttemptedAdds += weigher.applyAsLong(iCacheKey, value); ehcacheTest.put(iCacheKey, value); - } - /*System.out.println("Total size of attempted adds = " + sizeOfAttemptedAdds); - System.out.println("Total size of attempted adds (value only) = " + sizeOfAttemptedAddsValue); - System.out.println("Total memory size = " + ehcacheTest.stats().getTotalMemorySize());*/ - // TODO: Figure out why ehcache is evicting at ~30-40% of its max size rather than 100% (see commented out prints above) assertTrue(mockRemovalListener.onRemovalCount.get() > 0); assertEquals(660, ehcacheTest.stats().getTotalEvictions()); ehcacheTest.close(); @@ -539,8 +535,9 @@ public String load(ICacheKey key) throws Exception { } public void testMemoryTracking() throws Exception { - // This test leaks threads because of an issue in Ehcache: + // TODO: This test leaks threads because of an issue in Ehcache: // https://github.com/ehcache/ehcache3/issues/3204 + // Test all cases for EhCacheEventListener.onEvent and check stats memory usage is updated correctly Settings settings = Settings.builder().build(); ToLongBiFunction, String> weigher = getWeigher(); @@ -685,7 +682,7 @@ public void onRemoval(RemovalNotification, V> notification) { } } - private static class StringSerializer implements Serializer { + static class StringSerializer implements Serializer { private final Charset charset = StandardCharsets.UTF_8; @Override public byte[] serialize(String object) { diff --git a/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java b/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java index a77ef4653f951..e0b06406d8f6d 100644 --- a/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java +++ b/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java @@ -142,9 +142,12 @@ public static class OpenSearchOnHeapCacheFactory implements Factory { public ICache create(CacheConfig config, CacheType cacheType, Map cacheFactories) { Map> settingList = OpenSearchOnHeapCacheSettings.getSettingListForCacheType(cacheType); Settings settings = config.getSettings(); - return new Builder().setMaximumWeightInBytes( - ((ByteSizeValue) settingList.get(MAXIMUM_SIZE_IN_BYTES_KEY).get(settings)).getBytes() - ).setWeigher(config.getWeigher()).setRemovalListener(config.getRemovalListener()).build(); + return new Builder() + .setShardIdDimensionName(config.getDimensionNames().get(0)) //TODO: Make it accept >1 dimension names + .setMaximumWeightInBytes(((ByteSizeValue) settingList.get(MAXIMUM_SIZE_IN_BYTES_KEY).get(settings)).getBytes()) + .setWeigher(config.getWeigher()) + .setRemovalListener(config.getRemovalListener()) + .build(); } @Override diff --git a/server/src/main/java/org/opensearch/common/cache/store/config/CacheConfig.java b/server/src/main/java/org/opensearch/common/cache/store/config/CacheConfig.java index c837ee899a283..0bf325cdd5a86 100644 --- a/server/src/main/java/org/opensearch/common/cache/store/config/CacheConfig.java +++ b/server/src/main/java/org/opensearch/common/cache/store/config/CacheConfig.java @@ -11,8 +11,10 @@ import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.cache.RemovalListener; import org.opensearch.common.cache.ICacheKey; +import org.opensearch.common.cache.serializer.Serializer; import org.opensearch.common.settings.Settings; +import java.util.List; import java.util.function.ToLongBiFunction; /** @@ -42,12 +44,20 @@ public class CacheConfig { private final RemovalListener, V> removalListener; + private final Serializer keySerializer; + private final Serializer valueSerializer; + + private final List dimensionNames; + private CacheConfig(Builder builder) { this.keyType = builder.keyType; this.valueType = builder.valueType; this.settings = builder.settings; this.removalListener = builder.removalListener; this.weigher = builder.weigher; + this.keySerializer = builder.keySerializer; + this.valueSerializer = builder.valueSerializer; + this.dimensionNames = builder.dimensionNames; } public RemovalListener, V> getRemovalListener() { @@ -70,6 +80,18 @@ public ToLongBiFunction, V> getWeigher() { return weigher; } + public Serializer getKeySerializer() { + return keySerializer; + } + + public Serializer getValueSerializer() { + return valueSerializer; + } + + public List getDimensionNames() { + return dimensionNames; + } + /** * Builder class to build Cache config related parameters. * @param Type of key. @@ -86,6 +108,11 @@ public static class Builder { private ToLongBiFunction, V> weigher; + private Serializer keySerializer; + private Serializer valueSerializer; + + private List dimensionNames; + public Builder() {} public Builder setSettings(Settings settings) { @@ -113,6 +140,21 @@ public Builder setWeigher(ToLongBiFunction, V> weigher) { return this; } + public Builder setKeySerializer(Serializer keySerializer) { + this.keySerializer = keySerializer; + return this; + } + + public Builder setValueSerializer(Serializer valueSerializer) { + this.valueSerializer = valueSerializer; + return this; + } + + public Builder setDimensionNames(List dimensionNames) { + this.dimensionNames = dimensionNames; + return this; + } + public CacheConfig build() { return new CacheConfig<>(this); }