Skip to content

Commit

Permalink
Addressed Sagar's comment
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Alfonsi <[email protected]>
  • Loading branch information
Peter Alfonsi committed Mar 8, 2024
1 parent af35c6d commit 1f7f460
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 20 deletions.
19 changes: 10 additions & 9 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 @@ -8,6 +8,7 @@

package org.opensearch.cache;

import org.opensearch.cache.keystore.RBMIntKeyLookupStore;
import org.opensearch.cache.store.disk.EhcacheDiskCache;
import org.opensearch.common.cache.CacheType;
import org.opensearch.common.settings.Setting;
Expand Down Expand Up @@ -114,16 +115,16 @@ public class EhcacheDiskCacheSettings {
/**
* Defines whether to use an in-memory keystore to check for probable presence of keys before having to go to disk.
*/
public static final Setting.AffixSetting<Boolean> USE_RBM_KEYSTORE_SETTING = Setting.suffixKeySetting(
EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME + "use_keystore",
(key) -> Setting.boolSetting(key, true, NodeScope)
public static final Setting.AffixSetting<String> USE_KEYSTORE_SETTING = Setting.suffixKeySetting(
EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME + ".keystore",
(key) -> Setting.simpleString(key, RBMIntKeyLookupStore.KEYSTORE_NAME, NodeScope)
);

/**
* Defines the max size of the RBM keystore if used (as a percentage of heap memory)
*/
public static final Setting.AffixSetting<ByteSizeValue> RBM_KEYSTORE_SIZE_SETTING = Setting.suffixKeySetting(
EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME + "keystore_size",
public static final Setting.AffixSetting<ByteSizeValue> KEYSTORE_SIZE_SETTING = Setting.suffixKeySetting(
EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME + ".keystore_size",
(key) -> Setting.memorySizeSetting(key, "0.05%", NodeScope)
);

Expand Down Expand Up @@ -170,11 +171,11 @@ public class EhcacheDiskCacheSettings {
/**
* Key for whether to use RBM keystore
*/
public static final String USE_RBM_KEYSTORE_KEY = "use_keystore";
public static final String USE_KEYSTORE_KEY = "use_keystore";
/**
* Key for the keystore size in bytes
*/
public static final String RBM_KEYSTORE_SIZE_KEY = "keystore_size";
public static final String KEYSTORE_SIZE_KEY = "keystore_size";

/**
* Map of key to setting.
Expand All @@ -189,8 +190,8 @@ public class EhcacheDiskCacheSettings {
Map.entry(DISK_STORAGE_PATH_KEY, DISK_STORAGE_PATH_SETTING),
Map.entry(DISK_MAX_SIZE_IN_BYTES_KEY, DISK_CACHE_MAX_SIZE_IN_BYTES_SETTING),
Map.entry(DISK_LISTENER_MODE_SYNC_KEY, DISK_CACHE_LISTENER_MODE_SYNC_SETTING),
Map.entry(USE_RBM_KEYSTORE_KEY, USE_RBM_KEYSTORE_SETTING),
Map.entry(RBM_KEYSTORE_SIZE_KEY, RBM_KEYSTORE_SIZE_SETTING)
Map.entry(USE_KEYSTORE_KEY, USE_KEYSTORE_SETTING),
Map.entry(KEYSTORE_SIZE_KEY, KEYSTORE_SIZE_SETTING)
);

/**
Expand Down
57 changes: 57 additions & 0 deletions plugins/cache-ehcache/src/main/java/org/opensearch/cache/keystore/DummyKeystore.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.cache.keystore;

import org.opensearch.common.cache.keystore.KeyLookupStore;

/**
* A dummy keystore, which will always report that a key is contained in it.
*/
public class DummyKeystore implements KeyLookupStore<Integer> {
@Override
public boolean add(Integer value) {
return true;
}

@Override
public boolean contains(Integer value) {
return true;
}

@Override
public boolean remove(Integer value) {
return true;
}

@Override
public int getSize() {
return 0;
}

@Override
public long getMemorySizeInBytes() {
return 0;
}

@Override
public long getMemorySizeCapInBytes() {
return 0;
}

@Override
public boolean isFull() {
return false;
}

@Override
public void regenerateStore(Integer[] newValues) {}

@Override
public void clear() {}
}
3 changes: 3 additions & 0 deletions plugins/cache-ehcache/src/main/java/org/opensearch/cache/keystore/RBMIntKeyLookupStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
* The store estimates its memory footprint and will stop adding more values once it reaches its memory cap.
*/
public class RBMIntKeyLookupStore implements KeyLookupStore<Integer> {
/** Used in settings to distinguish between keystore types. */
public static final String KEYSTORE_NAME = "rbm";

/**
* An enum representing modulo values for use in the keystore
*/
Expand Down
29 changes: 18 additions & 11 deletions 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 @@ -12,6 +12,7 @@
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchException;
import org.opensearch.cache.EhcacheDiskCacheSettings;
import org.opensearch.cache.keystore.DummyKeystore;
import org.opensearch.cache.keystore.RBMIntKeyLookupStore;
import org.opensearch.common.SuppressForbidden;
import org.opensearch.common.annotation.ExperimentalApi;
Expand Down Expand Up @@ -70,8 +71,8 @@
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_WRITE_CONCURRENCY_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_WRITE_MAXIMUM_THREADS_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_WRITE_MIN_THREADS_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.RBM_KEYSTORE_SIZE_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.USE_RBM_KEYSTORE_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.KEYSTORE_SIZE_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.USE_KEYSTORE_KEY;

/**
* This variant of disk cache uses Ehcache underneath.
Expand Down Expand Up @@ -147,14 +148,14 @@ private EhcacheDiskCache(Builder<K, V> builder) {
this.removalListener = builder.getRemovalListener();
this.ehCacheEventListener = new EhCacheEventListener<K, V>(builder.getRemovalListener());
this.cache = buildCache(Duration.ofMillis(expireAfterAccess.getMillis()), builder);
boolean useRBMKeystore = (Boolean) EhcacheDiskCacheSettings.getSettingListForCacheType(cacheType)
.get(USE_RBM_KEYSTORE_KEY)
.get(settings);
if (useRBMKeystore) {
String keystoreType = (String) EhcacheDiskCacheSettings.getSettingListForCacheType(cacheType).get(USE_KEYSTORE_KEY).get(settings);
if (keystoreType.equals(RBMIntKeyLookupStore.KEYSTORE_NAME)) {
long keystoreSize = ((ByteSizeValue) EhcacheDiskCacheSettings.getSettingListForCacheType(cacheType)
.get(RBM_KEYSTORE_SIZE_KEY)
.get(KEYSTORE_SIZE_KEY)
.get(settings)).getBytes();
this.keystore = new RBMIntKeyLookupStore(keystoreSize);
} else {
this.keystore = new DummyKeystore();
}
}

Expand Down Expand Up @@ -253,7 +254,7 @@ public V get(K key) {
throw new IllegalArgumentException("Key passed to ehcache disk cache was null.");
}
V value = null;
if (keystore == null || keystore.contains(key.hashCode()) || keystore.isFull()) {
if (keystore.contains(key.hashCode()) || keystore.isFull()) {
try {
value = cache.get(key);
} catch (CacheLoadingException ex) {
Expand All @@ -272,9 +273,7 @@ public V get(K key) {
public void put(K key, V value) {
try {
cache.put(key, value);
if (keystore != null) {
keystore.add(key.hashCode());
}
keystore.add(key.hashCode());
} catch (CacheWritingException ex) {
throw new OpenSearchException("Exception occurred while put item to ehcache disk cache");
}
Expand Down Expand Up @@ -404,6 +403,14 @@ public void close() {
}
}

/**
* Returns keystore, for testing.
* @return keystore
*/
KeyLookupStore<Integer> getKeystore() {
return keystore;
}

/**
* This iterator wraps ehCache iterator and only iterates over its keys.
* @param <K> Type of key
Expand Down
94 changes: 94 additions & 0 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 @@ -9,6 +9,8 @@
package org.opensearch.cache.store.disk;

import org.opensearch.cache.EhcacheDiskCacheSettings;
import org.opensearch.cache.keystore.DummyKeystore;
import org.opensearch.cache.keystore.RBMIntKeyLookupStore;
import org.opensearch.common.cache.CacheType;
import org.opensearch.common.cache.ICache;
import org.opensearch.common.cache.LoadAwareCacheLoader;
Expand All @@ -35,6 +37,7 @@
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_STORAGE_PATH_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.USE_KEYSTORE_KEY;
import static org.hamcrest.CoreMatchers.instanceOf;

public class EhCacheDiskCacheTests extends OpenSearchSingleNodeTestCase {
Expand Down Expand Up @@ -481,6 +484,97 @@ public String load(String key) throws Exception {
}
}

public void testKeystoreSettings() throws Exception {
Settings useRBMsettings = Settings.builder()
.put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE).get(USE_KEYSTORE_KEY).getKey(),
RBMIntKeyLookupStore.KEYSTORE_NAME
)
.build();

MockRemovalListener<String, String> removalListener = new MockRemovalListener<>();
try (NodeEnvironment env = newNodeEnvironment(Settings.EMPTY)) {
EhcacheDiskCache<String, String> ehcacheTest = (EhcacheDiskCache<String, String>) new EhcacheDiskCache.Builder<String, String>()
.setDiskCacheAlias("test1")
.setThreadPoolAlias("ehcacheTest")
.setIsEventListenerModeSync(true)
.setStoragePath(env.nodePaths()[0].indicesPath.toString() + "/request_cache")
.setKeyType(String.class)
.setValueType(String.class)
.setCacheType(CacheType.INDICES_REQUEST_CACHE)
.setSettings(useRBMsettings)
.setExpireAfterAccess(TimeValue.MAX_VALUE)
.setMaximumWeightInBytes(CACHE_SIZE_IN_BYTES)
.setRemovalListener(removalListener)
.build();
assertEquals(RBMIntKeyLookupStore.class, ehcacheTest.getKeystore().getClass());
ehcacheTest.close();
}

Settings useDummySettings = Settings.builder()
.put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE).get(USE_KEYSTORE_KEY).getKey(),
"unrecognized_name"
)
.build();

try (NodeEnvironment env = newNodeEnvironment(Settings.EMPTY)) {
EhcacheDiskCache<String, String> ehcacheTest = (EhcacheDiskCache<String, String>) new EhcacheDiskCache.Builder<String, String>()
.setDiskCacheAlias("test1")
.setThreadPoolAlias("ehcacheTest")
.setIsEventListenerModeSync(true)
.setStoragePath(env.nodePaths()[0].indicesPath.toString() + "/request_cache")
.setKeyType(String.class)
.setValueType(String.class)
.setCacheType(CacheType.INDICES_REQUEST_CACHE)
.setSettings(useDummySettings)
.setExpireAfterAccess(TimeValue.MAX_VALUE)
.setMaximumWeightInBytes(CACHE_SIZE_IN_BYTES)
.setRemovalListener(removalListener)
.build();
assertEquals(DummyKeystore.class, ehcacheTest.getKeystore().getClass());

// Also test the dummy keystore doesn't incorrectly block any gets
int numKeys = 50;
Map<String, String> keyValueMap = new HashMap<>();
for (int i = 0; i < numKeys; i++) {
String key = generateRandomString(50);
String value = generateRandomString(50);
keyValueMap.put(key, value);
ehcacheTest.put(key, value);
}
for (String key : keyValueMap.keySet()) {
assertEquals(keyValueMap.get(key), ehcacheTest.get(key));
}
ehcacheTest.close();
}

// Check the factory correctly gives RBM keystore
try (NodeEnvironment env = newNodeEnvironment(Settings.EMPTY)) {
ICache.Factory ehcacheFactory = new EhcacheDiskCache.EhcacheDiskCacheFactory();
EhcacheDiskCache<String, String> ehcacheTest = (EhcacheDiskCache<String, String>) ehcacheFactory.create(
new CacheConfig.Builder<String, String>().setValueType(String.class)
.setKeyType(String.class)
.setRemovalListener(removalListener)
.setSettings(
Settings.builder()
.put(
EhcacheDiskCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(DISK_STORAGE_PATH_KEY)
.getKey(),
env.nodePaths()[0].indicesPath.toString() + "/request_cache"
)
.build()
)
.build(),
CacheType.INDICES_REQUEST_CACHE,
Map.of()
);
assertEquals(RBMIntKeyLookupStore.class, ehcacheTest.getKeystore().getClass());
ehcacheTest.close();
}
}

private static String generateRandomString(int length) {
String characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
StringBuilder randomString = new StringBuilder(length);
Expand Down

0 comments on commit 1f7f460

Please sign in to comment.