Skip to content

Commit

Permalink
Clear ehcache disk cache files during initialization
Browse files Browse the repository at this point in the history
Signed-off-by: Sagar Upadhyaya <[email protected]>
  • Loading branch information
sgup432 committed Jul 11, 2024
1 parent 6b8b3ef commit 80ef056
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import org.ehcache.Cache;
import org.ehcache.CachePersistenceException;
import org.ehcache.PersistentCacheManager;
import org.ehcache.StateTransitionException;
import org.ehcache.config.builders.CacheConfigurationBuilder;
import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder;
import org.ehcache.config.builders.CacheManagerBuilder;
Expand Down Expand Up @@ -133,6 +134,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");
Expand All @@ -151,6 +153,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(

Check warning on line 166 in plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L164 - L166 were not covered by tests
"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 {
Expand Down Expand Up @@ -444,19 +463,28 @@ 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 (CachePersistenceException e) {

Check warning on line 470 in plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L470 was not covered by tests
// When something goes wrong while destroying the persistence data
logger.error(() -> new ParameterizedMessage("Exception occurred while destroying ehcache and associated " + "data"), e);
throw new OpenSearchException("Exception occurred while destroying ehcache and associated data", e);
} catch (StateTransitionException e) {
logger.error(() -> new ParameterizedMessage("Exception occurred while trying to close/remove ehcache"), e);

Check warning on line 475 in plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L472 - L475 were not covered by tests
} 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)

Check warning on line 484 in plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L482 - L484 were not covered by tests
);
}
}
} 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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -882,6 +884,56 @@ 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()
}
}

private List<String> getRandomDimensions(List<String> dimensionNames) {
Random rand = Randomness.get();
int bound = 3;
Expand Down

0 comments on commit 80ef056

Please sign in to comment.