Clear ehcache disk cache files during initialization

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

@@ -60,7 +60,6 @@
 import java.util.function.ToLongBiFunction;
 
 import org.ehcache.Cache;
-import org.ehcache.CachePersistenceException;
 import org.ehcache.PersistentCacheManager;
 import org.ehcache.config.builders.CacheConfigurationBuilder;
 import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder;
@@ -133,6 +132,7 @@ public class EhcacheDiskCache<K, V> implements ICache<K, V> {
  */
  Map<ICacheKey<K>, CompletableFuture<Tuple<ICacheKey<K>, V>>> completableFutureMap = new ConcurrentHashMap<>();
 
+ @SuppressForbidden(reason = "Ehcache uses File.io")
  private EhcacheDiskCache(Builder<K, V> builder) {
  this.keyType = Objects.requireNonNull(builder.keyType, "Key type shouldn't be null");
  this.valueType = Objects.requireNonNull(builder.valueType, "Value type shouldn't be null");
@@ -151,6 +151,23 @@ private EhcacheDiskCache(Builder<K, V> builder) {
  if (this.storagePath == null || this.storagePath.isBlank()) {
  throw new IllegalArgumentException("Storage path shouldn't be null or empty");
  }
+ // Delete all the previous disk cache related files/data. We don't persist data between process restart for
+ // now which is why need to do this. Clean up in case there was a non graceful restart and we had older disk
+ // cache data still lying around.
+ Path ehcacheDirectory = Paths.get(this.storagePath);
+ if (Files.exists(ehcacheDirectory)) {
+ try {
+ logger.info("Found older disk cache data lying around during initialization under path: {}", this.storagePath);
+ IOUtils.rm(ehcacheDirectory);
+ } catch (IOException e) {
+ logger.error(
+ () -> new ParameterizedMessage(
+ "Failed to delete ehcache disk cache data under path: {} " + "during initial",
+ this.storagePath
+ )
+ );
+ }
+ }
  if (builder.threadPoolAlias == null || builder.threadPoolAlias.isBlank()) {
  this.threadPoolAlias = THREAD_POOL_ALIAS_PREFIX + "DiskWrite#" + UNIQUE_ID;
  } else {
@@ -444,19 +461,24 @@ public void refresh() {
  @Override
  @SuppressForbidden(reason = "Ehcache uses File.io")
  public void close() {
- cacheManager.removeCache(this.diskCacheAlias);
- cacheManager.close();
  try {
+ cacheManager.removeCache(this.diskCacheAlias);
+ cacheManager.close();
  cacheManager.destroyCache(this.diskCacheAlias);
- // Delete all the disk cache related files/data
+ } catch (Exception e) {
+ logger.error(() -> new ParameterizedMessage("Exception occurred while trying to close/remove ehcache"), e);
+ } finally {
+ // Delete all the disk cache related files/data in case it is present
  Path ehcacheDirectory = Paths.get(this.storagePath);
  if (Files.exists(ehcacheDirectory)) {
- IOUtils.rm(ehcacheDirectory);
+ try {
+ IOUtils.rm(ehcacheDirectory);
+ } catch (IOException e) {
+ logger.error(
+ () -> new ParameterizedMessage("Failed to delete ehcache disk cache data under path: {}", this.storagePath)
+ );
+ }
  }
- } catch (CachePersistenceException e) {
- throw new OpenSearchException("Exception occurred while destroying ehcache and associated data", e);
- } catch (IOException e) {
- logger.error(() -> new ParameterizedMessage("Failed to delete ehcache disk cache data under path: {}", this.storagePath));
  }
  }
 

plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java

@@ -34,6 +34,8 @@
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -882,6 +884,93 @@ public void testStatsTrackingDisabled() throws Exception {
  }
  }
 
+ public void testDiskCacheFilesAreClearedUpDuringCloseAndInitialization() throws Exception {
+ Settings settings = Settings.builder().build();
+ MockRemovalListener<String, String> removalListener = new MockRemovalListener<>();
+ ToLongBiFunction<ICacheKey<String>, String> weigher = getWeigher();
+ try (NodeEnvironment env = newNodeEnvironment(settings)) {
+ String path = env.nodePaths()[0].path.toString() + "/request_cache";
+ // Create a dummy file to simulate a scenario where the data is already in the disk cache storage path
+ // beforehand.
+ Files.createDirectory(Path.of(path));
+ Path dummyFilePath = Files.createFile(Path.of(path + "/testing.txt"));
+ assertTrue(Files.exists(dummyFilePath));
+ ICache<String, String> ehcacheTest = new EhcacheDiskCache.Builder<String, String>().setThreadPoolAlias("ehcacheTest")
+ .setStoragePath(path)
+ .setIsEventListenerModeSync(true)
+ .setKeyType(String.class)
+ .setValueType(String.class)
+ .setKeySerializer(new StringSerializer())
+ .setDiskCacheAlias("test1")
+ .setValueSerializer(new StringSerializer())
+ .setDimensionNames(List.of(dimensionName))
+ .setCacheType(CacheType.INDICES_REQUEST_CACHE)
+ .setSettings(settings)
+ .setExpireAfterAccess(TimeValue.MAX_VALUE)
+ .setMaximumWeightInBytes(CACHE_SIZE_IN_BYTES)
+ .setRemovalListener(removalListener)
+ .setWeigher(weigher)
+ .setStatsTrackingEnabled(false)
+ .build();
+ int randomKeys = randomIntBetween(10, 100);
+ for (int i = 0; i < randomKeys; i++) {
+ ICacheKey<String> iCacheKey = getICacheKey(UUID.randomUUID().toString());
+ ehcacheTest.put(iCacheKey, UUID.randomUUID().toString());
+ assertEquals(0, ehcacheTest.count()); // Expect count of 0 if NoopCacheStatsHolder is used
+ assertEquals(new ImmutableCacheStats(0, 0, 0, 0, 0), ehcacheTest.stats().getTotalStats());
+ }
+ // Verify that older data was wiped out after initialization
+ assertFalse(Files.exists(dummyFilePath));
+
+ // Verify that there is data present under desired path by explicitly verifying the folder name by prefix
+ // (used from disk cache alias)
+ assertTrue(Files.exists(Path.of(path)));
+ boolean folderExists = Files.walk(Path.of(path))
+ .filter(Files::isDirectory)
+ .anyMatch(path1 -> path1.getFileName().toString().startsWith("test1"));
+ assertTrue(folderExists);
+ ehcacheTest.close();
+ assertFalse(Files.exists(Path.of(path))); // Verify everything is cleared up now after close()
+ }
+ }
+
+ public void testDiskCacheCloseCalledTwiceAndVerifyDiskDataIsCleanedUp() throws Exception {
+ Settings settings = Settings.builder().build();
+ MockRemovalListener<String, String> removalListener = new MockRemovalListener<>();
+ ToLongBiFunction<ICacheKey<String>, String> weigher = getWeigher();
+ try (NodeEnvironment env = newNodeEnvironment(settings)) {
+ String path = env.nodePaths()[0].path.toString() + "/request_cache";
+ ICache<String, String> ehcacheTest = new EhcacheDiskCache.Builder<String, String>().setThreadPoolAlias("ehcacheTest")
+ .setStoragePath(path)
+ .setIsEventListenerModeSync(true)
+ .setKeyType(String.class)
+ .setValueType(String.class)
+ .setKeySerializer(new StringSerializer())
+ .setDiskCacheAlias("test1")
+ .setValueSerializer(new StringSerializer())
+ .setDimensionNames(List.of(dimensionName))
+ .setCacheType(CacheType.INDICES_REQUEST_CACHE)
+ .setSettings(settings)
+ .setExpireAfterAccess(TimeValue.MAX_VALUE)
+ .setMaximumWeightInBytes(CACHE_SIZE_IN_BYTES)
+ .setRemovalListener(removalListener)
+ .setWeigher(weigher)
+ .setStatsTrackingEnabled(false)
+ .build();
+ int randomKeys = randomIntBetween(10, 100);
+ for (int i = 0; i < randomKeys; i++) {
+ ICacheKey<String> iCacheKey = getICacheKey(UUID.randomUUID().toString());
+ ehcacheTest.put(iCacheKey, UUID.randomUUID().toString());
+ assertEquals(0, ehcacheTest.count()); // Expect count storagePath 0 if NoopCacheStatsHolder is used
+ assertEquals(new ImmutableCacheStats(0, 0, 0, 0, 0), ehcacheTest.stats().getTotalStats());
+ }
+ ehcacheTest.close();
+ // Call it again. This will throw an exception.
+ ehcacheTest.close();
+ assertFalse(Files.exists(Path.of(path))); // Verify everything is cleared up now after close()
+ }
+ }
+
  private List<String> getRandomDimensions(List<String> dimensionNames) {
  Random rand = Randomness.get();
  int bound = 3;