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 aa8c1601bd0cd..8875c0617d652 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 @@ -458,23 +458,30 @@ public void refresh() { @Override @SuppressForbidden(reason = "Ehcache uses File.io") public void close() { + // We are calling each of below function in its own try/catch block so that we still try to close the cache + // manager in case removeCache failed (for example) try { cacheManager.removeCache(this.diskCacheAlias); + } catch (Exception e) { + logger.error(() -> new ParameterizedMessage("Exception occurred while trying to remove cache from manager"), e); + } + try { cacheManager.close(); + } catch (Exception e) { + logger.error(() -> new ParameterizedMessage("Exception occurred while trying to close ehcache manager"), e); + } + try { cacheManager.destroyCache(this.diskCacheAlias); } 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)) { - try { - IOUtils.rm(ehcacheDirectory); - } catch (IOException e) { - logger.error( - () -> new ParameterizedMessage("Failed to delete ehcache disk cache data under path: {}", this.storagePath) - ); - } + logger.error(() -> new ParameterizedMessage("Exception occurred while trying to destroy cache and " + "associated data"), e); + } + // Delete all the disk cache related files/data in case it is present + Path ehcacheDirectory = Paths.get(this.storagePath); + if (Files.exists(ehcacheDirectory)) { + try { + IOUtils.rm(ehcacheDirectory); + } catch (IOException e) { + logger.error(() -> new ParameterizedMessage("Failed to delete ehcache disk cache data under path: {}", this.storagePath)); } } } 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 e8f4fc623198a..1bc30e295ab97 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 @@ -25,6 +25,7 @@ import org.opensearch.common.metrics.CounterMetric; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.bytes.CompositeBytesReference; @@ -942,7 +943,7 @@ public void testDiskCacheCloseCalledTwiceAndVerifyDiskDataIsCleanedUp() throws E ToLongBiFunction, String> weigher = getWeigher(); try (NodeEnvironment env = newNodeEnvironment(settings)) { String path = env.nodePaths()[0].path.toString() + "/request_cache"; - ICache ehcacheTest = new EhcacheDiskCache.Builder().setThreadPoolAlias("ehcacheTest") + ICache ehcacheTest = new EhcacheDiskCache.Builder().setThreadPoolAlias(null) .setStoragePath(path) .setIsEventListenerModeSync(true) .setKeyType(String.class) @@ -973,6 +974,41 @@ public void testDiskCacheCloseCalledTwiceAndVerifyDiskDataIsCleanedUp() throws E } } + public void testDiskCacheCloseAfterCleaningUpFilesManually() throws Exception { + Settings settings = Settings.builder().build(); + MockRemovalListener removalListener = new MockRemovalListener<>(); + ToLongBiFunction, String> weigher = getWeigher(); + try (NodeEnvironment env = newNodeEnvironment(settings)) { + String path = env.nodePaths()[0].path.toString() + "/request_cache"; + ICache ehcacheTest = new EhcacheDiskCache.Builder().setThreadPoolAlias(null) + .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 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()); + } + IOUtils.rm(Path.of(path)); + ehcacheTest.close(); + } + } + public void testEhcacheDiskCacheWithoutStoragePathDefined() throws Exception { Settings settings = Settings.builder().build(); MockRemovalListener removalListener = new MockRemovalListener<>(); @@ -1000,6 +1036,34 @@ public void testEhcacheDiskCacheWithoutStoragePathDefined() throws Exception { } } + public void testEhcacheDiskCacheWithoutStoragePathNull() throws Exception { + Settings settings = Settings.builder().build(); + MockRemovalListener removalListener = new MockRemovalListener<>(); + ToLongBiFunction, String> weigher = getWeigher(); + try (NodeEnvironment env = newNodeEnvironment(settings)) { + assertThrows( + IllegalArgumentException.class, + () -> new EhcacheDiskCache.Builder().setThreadPoolAlias("ehcacheTest") + .setStoragePath(null) + .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 removalListener = new MockRemovalListener<>(); @@ -1027,6 +1091,33 @@ public void testEhcacheWithStorageSizeLowerThanMinimumExpected() throws Exceptio } } + public void testEhcacheWithStorageSizeZero() throws Exception { + Settings settings = Settings.builder().build(); + MockRemovalListener removalListener = new MockRemovalListener<>(); + ToLongBiFunction, String> weigher = getWeigher(); + try (NodeEnvironment env = newNodeEnvironment(settings)) { + assertThrows( + IllegalArgumentException.class, + () -> new EhcacheDiskCache.Builder().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(0) + .setRemovalListener(removalListener) + .setWeigher(weigher) + .setStatsTrackingEnabled(false) + .build() + ); + } + } + private List getRandomDimensions(List dimensionNames) { Random rand = Randomness.get(); int bound = 3;