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;
@@ -104,8 +103,6 @@
     // Unique id associated with this cache.
     private final static String UNIQUE_ID = UUID.randomUUID().toString();
     private final static String THREAD_POOL_ALIAS_PREFIX = "ehcachePool";
-    private final static int MINIMUM_MAX_SIZE_IN_BYTES = 1024 * 100; // 100KB
-
     // A Cache manager can create many caches.
     private final PersistentCacheManager cacheManager;
 
@@ -127,12 +124,17 @@
     private final Serializer<K, byte[]> keySerializer;
     private final Serializer<V, byte[]> valueSerializer;
 
+    final static int MINIMUM_MAX_SIZE_IN_BYTES = 1024 * 100; // 100KB
+    final static String CACHE_DATA_CLEANUP_DURING_INITIALIZATION_EXCEPTION = "Failed to delete ehcache disk cache under "
+        + "path: % during initialization. Please clean this up manually and restart the process";
+
     /**
      * Used in computeIfAbsent to synchronize loading of a given key. This is needed as ehcache doesn't provide a
      * computeIfAbsent method.
      */
     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 +153,18 @@
         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) {
+                throw new OpenSearchException(String.format(CACHE_DATA_CLEANUP_DURING_INITIALIZATION_EXCEPTION, this.storagePath));
+            }
+        }
         if (builder.threadPoolAlias == null || builder.threadPoolAlias.isBlank()) {
             this.threadPoolAlias = THREAD_POOL_ALIAS_PREFIX + "DiskWrite#" + UNIQUE_ID;
         } else {
@@ -444,19 +458,24 @@
     @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;
@@ -50,6 +52,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.store.disk.EhcacheDiskCache.MINIMUM_MAX_SIZE_IN_BYTES;
 import static org.hamcrest.CoreMatchers.instanceOf;
 
 @ThreadLeakFilters(filters = { EhcacheThreadLeakFilter.class })
@@ -882,6 +885,148 @@ 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)
+                .setThreadPoolAlias("")
+                .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();
+            assertFalse(Files.exists(Path.of(path))); // Verify everything is cleared up now after close()
+            // Call it again. This will throw an exception.
+            ehcacheTest.close();
+        }
+    }
+
+    public void testEhcacheDiskCacheWithoutStoragePathDefined() throws Exception {
+        Settings settings = Settings.builder().build();
+        MockRemovalListener<String, String> removalListener = new MockRemovalListener<>();
+        ToLongBiFunction<ICacheKey<String>, String> weigher = getWeigher();
+        try (NodeEnvironment env = newNodeEnvironment(settings)) {
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> new EhcacheDiskCache.Builder<String, String>().setThreadPoolAlias("ehcacheTest")
+                    .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()
+            );
+        }
+    }
+
+    public void testEhcacheWithStorageSizeLowerThanMinimumExpected() throws Exception {
+        Settings settings = Settings.builder().build();
+        MockRemovalListener<String, String> removalListener = new MockRemovalListener<>();
+        ToLongBiFunction<ICacheKey<String>, String> weigher = getWeigher();
+        try (NodeEnvironment env = newNodeEnvironment(settings)) {
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> new EhcacheDiskCache.Builder<String, String>().setThreadPoolAlias("ehcacheTest")
+                    .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(MINIMUM_MAX_SIZE_IN_BYTES)
+                    .setRemovalListener(removalListener)
+                    .setWeigher(weigher)
+                    .setStatsTrackingEnabled(false)
+                    .build()
+            );
+        }
+    }
+
     private List<String> getRandomDimensions(List<String> dimensionNames) {
         Random rand = Randomness.get();
         int bound = 3;