Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Clear ehcache disk cache files during initialization #14806

Merged
merged 1 commit into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -127,13 +124,18 @@
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: %s 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<>();

private EhcacheDiskCache(Builder<K, V> builder) {
@SuppressForbidden(reason = "Ehcache uses File.io")
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");
this.expireAfterAccess = Objects.requireNonNull(builder.getExpireAfterAcess(), "ExpireAfterAccess value shouldn't " + "be null");
Expand All @@ -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), e);

Check warning on line 165 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-L165

Added lines #L164 - L165 were not covered by tests
}
}
if (builder.threadPoolAlias == null || builder.threadPoolAlias.isBlank()) {
this.threadPoolAlias = THREAD_POOL_ALIAS_PREFIX + "DiskWrite#" + UNIQUE_ID;
} else {
Expand All @@ -175,6 +189,11 @@
}
}

// Package private for testing
PersistentCacheManager getCacheManager() {
return this.cacheManager;
}

@SuppressWarnings({ "rawtypes" })
private Cache<ICacheKey, ByteArrayWrapper> buildCache(Duration expireAfterAccess, Builder<K, V> builder) {
// Creating the cache requires permissions specified in plugin-security.policy
Expand Down Expand Up @@ -255,7 +274,7 @@
}

@SuppressForbidden(reason = "Ehcache uses File.io")
private PersistentCacheManager buildCacheManager() {
PersistentCacheManager buildCacheManager() {
// In case we use multiple ehCaches, we can define this cache manager at a global level.
// Creating the cache manager also requires permissions specified in plugin-security.policy
return AccessController.doPrivileged((PrivilegedAction<PersistentCacheManager>) () -> {
Expand Down Expand Up @@ -444,20 +463,21 @@
@Override
@SuppressForbidden(reason = "Ehcache uses File.io")
public void close() {
cacheManager.removeCache(this.diskCacheAlias);
cacheManager.close();
try {
cacheManager.destroyCache(this.diskCacheAlias);
// Delete all the disk cache related files/data
Path ehcacheDirectory = Paths.get(this.storagePath);
if (Files.exists(ehcacheDirectory)) {
cacheManager.close();
} catch (Exception e) {
logger.error(() -> new ParameterizedMessage("Exception occurred while trying to close ehcache manager"), 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));

Check warning on line 477 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#L476-L477

Added lines #L476 - L477 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
Loading
Loading