Skip to content

Commit b3b743d

Browse files
authored
Clear ehcache disk cache files during initialization (opensearch-project#14738)
* Clear ehcache disk cache files during initialization Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Adding UT to fix line coverage Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Addressing comment Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Adding more Uts for better line coverage Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Throwing exception in case we fail to clear cache files during startup Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Adding more UTs Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Adding a UT for more coverage Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Fixing gradle build Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> * Update ehcache disk cache close() logic Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com> --------- Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com>
1 parent fca520f commit b3b743d

File tree

2 files changed

+328
-15
lines changed

2 files changed

+328
-15
lines changed

plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java

+35-15
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
import java.util.function.ToLongBiFunction;
6161

6262
import org.ehcache.Cache;
63-
import org.ehcache.CachePersistenceException;
6463
import org.ehcache.PersistentCacheManager;
6564
import org.ehcache.config.builders.CacheConfigurationBuilder;
6665
import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder;
@@ -104,8 +103,6 @@ public class EhcacheDiskCache<K, V> implements ICache<K, V> {
104103
// Unique id associated with this cache.
105104
private final static String UNIQUE_ID = UUID.randomUUID().toString();
106105
private final static String THREAD_POOL_ALIAS_PREFIX = "ehcachePool";
107-
private final static int MINIMUM_MAX_SIZE_IN_BYTES = 1024 * 100; // 100KB
108-
109106
// A Cache manager can create many caches.
110107
private final PersistentCacheManager cacheManager;
111108

@@ -127,13 +124,18 @@ public class EhcacheDiskCache<K, V> implements ICache<K, V> {
127124
private final Serializer<K, byte[]> keySerializer;
128125
private final Serializer<V, byte[]> valueSerializer;
129126

127+
final static int MINIMUM_MAX_SIZE_IN_BYTES = 1024 * 100; // 100KB
128+
final static String CACHE_DATA_CLEANUP_DURING_INITIALIZATION_EXCEPTION = "Failed to delete ehcache disk cache under "
129+
+ "path: %s during initialization. Please clean this up manually and restart the process";
130+
130131
/**
131132
* Used in computeIfAbsent to synchronize loading of a given key. This is needed as ehcache doesn't provide a
132133
* computeIfAbsent method.
133134
*/
134135
Map<ICacheKey<K>, CompletableFuture<Tuple<ICacheKey<K>, V>>> completableFutureMap = new ConcurrentHashMap<>();
135136

136-
private EhcacheDiskCache(Builder<K, V> builder) {
137+
@SuppressForbidden(reason = "Ehcache uses File.io")
138+
EhcacheDiskCache(Builder<K, V> builder) {
137139
this.keyType = Objects.requireNonNull(builder.keyType, "Key type shouldn't be null");
138140
this.valueType = Objects.requireNonNull(builder.valueType, "Value type shouldn't be null");
139141
this.expireAfterAccess = Objects.requireNonNull(builder.getExpireAfterAcess(), "ExpireAfterAccess value shouldn't " + "be null");
@@ -151,6 +153,18 @@ private EhcacheDiskCache(Builder<K, V> builder) {
151153
if (this.storagePath == null || this.storagePath.isBlank()) {
152154
throw new IllegalArgumentException("Storage path shouldn't be null or empty");
153155
}
156+
// Delete all the previous disk cache related files/data. We don't persist data between process restart for
157+
// now which is why need to do this. Clean up in case there was a non graceful restart and we had older disk
158+
// cache data still lying around.
159+
Path ehcacheDirectory = Paths.get(this.storagePath);
160+
if (Files.exists(ehcacheDirectory)) {
161+
try {
162+
logger.info("Found older disk cache data lying around during initialization under path: {}", this.storagePath);
163+
IOUtils.rm(ehcacheDirectory);
164+
} catch (IOException e) {
165+
throw new OpenSearchException(String.format(CACHE_DATA_CLEANUP_DURING_INITIALIZATION_EXCEPTION, this.storagePath), e);
166+
}
167+
}
154168
if (builder.threadPoolAlias == null || builder.threadPoolAlias.isBlank()) {
155169
this.threadPoolAlias = THREAD_POOL_ALIAS_PREFIX + "DiskWrite#" + UNIQUE_ID;
156170
} else {
@@ -175,6 +189,11 @@ private EhcacheDiskCache(Builder<K, V> builder) {
175189
}
176190
}
177191

192+
// Package private for testing
193+
PersistentCacheManager getCacheManager() {
194+
return this.cacheManager;
195+
}
196+
178197
@SuppressWarnings({ "rawtypes" })
179198
private Cache<ICacheKey, ByteArrayWrapper> buildCache(Duration expireAfterAccess, Builder<K, V> builder) {
180199
// Creating the cache requires permissions specified in plugin-security.policy
@@ -255,7 +274,7 @@ Map<ICacheKey<K>, CompletableFuture<Tuple<ICacheKey<K>, V>>> getCompletableFutur
255274
}
256275

257276
@SuppressForbidden(reason = "Ehcache uses File.io")
258-
private PersistentCacheManager buildCacheManager() {
277+
PersistentCacheManager buildCacheManager() {
259278
// In case we use multiple ehCaches, we can define this cache manager at a global level.
260279
// Creating the cache manager also requires permissions specified in plugin-security.policy
261280
return AccessController.doPrivileged((PrivilegedAction<PersistentCacheManager>) () -> {
@@ -444,20 +463,21 @@ public void refresh() {
444463
@Override
445464
@SuppressForbidden(reason = "Ehcache uses File.io")
446465
public void close() {
447-
cacheManager.removeCache(this.diskCacheAlias);
448-
cacheManager.close();
449466
try {
450-
cacheManager.destroyCache(this.diskCacheAlias);
451-
// Delete all the disk cache related files/data
452-
Path ehcacheDirectory = Paths.get(this.storagePath);
453-
if (Files.exists(ehcacheDirectory)) {
467+
cacheManager.close();
468+
} catch (Exception e) {
469+
logger.error(() -> new ParameterizedMessage("Exception occurred while trying to close ehcache manager"), e);
470+
}
471+
// Delete all the disk cache related files/data in case it is present
472+
Path ehcacheDirectory = Paths.get(this.storagePath);
473+
if (Files.exists(ehcacheDirectory)) {
474+
try {
454475
IOUtils.rm(ehcacheDirectory);
476+
} catch (IOException e) {
477+
logger.error(() -> new ParameterizedMessage("Failed to delete ehcache disk cache data under path: {}", this.storagePath));
455478
}
456-
} catch (CachePersistenceException e) {
457-
throw new OpenSearchException("Exception occurred while destroying ehcache and associated data", e);
458-
} catch (IOException e) {
459-
logger.error(() -> new ParameterizedMessage("Failed to delete ehcache disk cache data under path: {}", this.storagePath));
460479
}
480+
461481
}
462482

463483
/**

0 commit comments

Comments
 (0)