From c86859c9b4df9dfee55495c8373aaf39e39541aa Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Thu, 10 Apr 2025 16:30:56 +1000 Subject: [PATCH 01/25] Evict from the shared blob cache asynchronously --- .../blobcache/BlobCachePlugin.java | 3 +- .../shared/SharedBlobCacheService.java | 43 +++++++++++++++++++ .../shared/SharedBlobCacheServiceTests.java | 38 ++++++++++++++++ .../SearchableSnapshotIndexEventListener.java | 4 +- ...eSnapshotIndexFoldersDeletionListener.java | 10 +++-- 5 files changed, 93 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCachePlugin.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCachePlugin.java index 4f9ac3eb99348..64d4c8d4dc511 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCachePlugin.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCachePlugin.java @@ -27,7 +27,8 @@ public List> getSettings() { SharedBlobCacheService.SHARED_CACHE_DECAY_INTERVAL_SETTING, SharedBlobCacheService.SHARED_CACHE_MIN_TIME_DELTA_SETTING, SharedBlobCacheService.SHARED_CACHE_MMAP, - SharedBlobCacheService.SHARED_CACHE_COUNT_READS + SharedBlobCacheService.SHARED_CACHE_COUNT_READS, + SharedBlobCacheService.SHARED_CACHE_CONCURRENT_EVICTIONS_SETTING ); } } diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java index ed7965b85a36a..40ddeea394fe5 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.RelativeByteSizeValue; import org.elasticsearch.common.util.concurrent.AbstractRunnable; +import org.elasticsearch.common.util.concurrent.ThrottledTaskRunner; import org.elasticsearch.core.AbstractRefCounted; import org.elasticsearch.core.Assertions; import org.elasticsearch.core.Nullable; @@ -97,6 +98,13 @@ public class SharedBlobCacheService implements Releasable { Setting.Property.NodeScope ); + public static final Setting SHARED_CACHE_CONCURRENT_EVICTIONS_SETTING = Setting.intSetting( + SHARED_CACHE_SETTINGS_PREFIX + "concurrent_evictions", + 5, + 1, + Setting.Property.NodeScope + ); + private static Setting.Validator getPageSizeAlignedByteSizeValueValidator(String settingName) { return value -> { if (value.getBytes() == -1) { @@ -283,6 +291,8 @@ private interface Cache extends Releasable { CacheEntry get(K cacheKey, long fileLength, int region); int forceEvict(Predicate cacheKeyPredicate); + + void forceEvictAsync(Predicate cacheKey); } private abstract static class CacheEntry { @@ -328,6 +338,7 @@ private CacheEntry(T chunk) { private final Runnable evictIncrementer; private final LongSupplier relativeTimeInNanosSupplier; + private final ThrottledTaskRunner evictionsRunner; public SharedBlobCacheService( NodeEnvironment environment, @@ -388,6 +399,11 @@ public SharedBlobCacheService( this.blobCacheMetrics = blobCacheMetrics; this.evictIncrementer = blobCacheMetrics.getEvictedCountNonZeroFrequency()::increment; this.relativeTimeInNanosSupplier = relativeTimeInNanosSupplier; + this.evictionsRunner = new ThrottledTaskRunner( + "shared_blob_cache_evictions", + SHARED_CACHE_CONCURRENT_EVICTIONS_SETTING.get(settings), + threadPool.generic() + ); } public static long calculateCacheSize(Settings settings, long totalFsSize) { @@ -704,6 +720,15 @@ public int forceEvict(Predicate cacheKeyPredicate) { } + /** + * Evict entries from the cache that match the given predicate asynchronously + * + * @param cacheKeyPredicate + */ + public void forceEvictAsync(Predicate cacheKeyPredicate) { + cache.forceEvictAsync(cacheKeyPredicate); + } + // used by tests int getFreq(CacheFileRegion cacheFileRegion) { if (cache instanceof LFUCache lfuCache) { @@ -1606,6 +1631,24 @@ public int forceEvict(Predicate cacheKeyPredicate) { return evictedCount; } + @Override + public void forceEvictAsync(Predicate cacheKeyPredicate) { + evictionsRunner.enqueueTask(new ActionListener<>() { + @Override + public void onResponse(Releasable releasable) { + forceEvict(cacheKeyPredicate); + } + + @Override + public void onFailure(Exception e) { + // should be impossible, GENERIC pool doesn't reject anything + final String message = "unexpected failure evicting from shared blob cache"; + logger.error(message, e); + assert false : new AssertionError(message, e); + } + }); + } + private LFUCacheEntry initChunk(LFUCacheEntry entry) { assert Thread.holdsLock(entry.chunk); RegionKey regionKey = entry.chunk.regionKey; diff --git a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java index 8364cb3078466..04658606ce132 100644 --- a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java +++ b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java @@ -272,6 +272,44 @@ public void testForceEvictResponse() throws IOException { } } + public void testAsynchronousEviction() throws Exception { + Settings settings = Settings.builder() + .put(NODE_NAME_SETTING.getKey(), "node") + .put(SharedBlobCacheService.SHARED_CACHE_SIZE_SETTING.getKey(), ByteSizeValue.ofBytes(size(500)).getStringRep()) + .put(SharedBlobCacheService.SHARED_CACHE_REGION_SIZE_SETTING.getKey(), ByteSizeValue.ofBytes(size(100)).getStringRep()) + .put("path.home", createTempDir()) + .build(); + final DeterministicTaskQueue taskQueue = new DeterministicTaskQueue(); + try ( + NodeEnvironment environment = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings)); + var cacheService = new SharedBlobCacheService<>( + environment, + settings, + taskQueue.getThreadPool(), + taskQueue.getThreadPool().executor(ThreadPool.Names.GENERIC), + BlobCacheMetrics.NOOP + ) + ) { + final var cacheKey1 = generateCacheKey(); + final var cacheKey2 = generateCacheKey(); + assertEquals(5, cacheService.freeRegionCount()); + final var region0 = cacheService.get(cacheKey1, size(250), 0); + assertEquals(4, cacheService.freeRegionCount()); + final var region1 = cacheService.get(cacheKey2, size(250), 1); + assertEquals(3, cacheService.freeRegionCount()); + assertFalse(region0.isEvicted()); + assertFalse(region1.isEvicted()); + cacheService.forceEvictAsync(ck -> ck == cacheKey1); + assertFalse(region0.isEvicted()); + assertFalse(region1.isEvicted()); + // run the async task + taskQueue.runAllRunnableTasks(); + assertTrue(region0.isEvicted()); + assertFalse(region1.isEvicted()); + assertEquals(4, cacheService.freeRegionCount()); + } + } + public void testDecay() throws IOException { // we have 8 regions Settings settings = Settings.builder() diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java index 4caf932a99807..ccaff1edfb720 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java @@ -114,7 +114,9 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea ); } if (sharedBlobCacheService != null) { - sharedBlobCacheService.forceEvict(SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings())); + sharedBlobCacheService.forceEvictAsync( + SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) + ); } } } diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java index 4d7174e0f7ff4..e59ec43e1db94 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java @@ -72,8 +72,12 @@ private void markShardAsEvictedInCache(ShardId shardId, IndexSettings indexSetti shardId ); - final SharedBlobCacheService sharedBlobCacheService = this.frozenCacheServiceSupplier.get(); - assert sharedBlobCacheService != null : "frozen cache service not initialized"; - sharedBlobCacheService.forceEvict(SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings())); + // Only partial searchable snapshots use the SharedBlobCacheService + if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot()) { + final SharedBlobCacheService sharedBlobCacheService = + SearchableSnapshotIndexFoldersDeletionListener.this.frozenCacheServiceSupplier.get(); + assert sharedBlobCacheService != null : "frozen cache service not initialized"; + sharedBlobCacheService.forceEvictAsync(SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings())); + } } } From c143dba99394d190442297ff807ba856a4f5d637 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Thu, 10 Apr 2025 16:59:12 +1000 Subject: [PATCH 02/25] Only evict from shared cache when index is partial (SharedSnapshotIndexEventListener) --- .../allocation/SearchableSnapshotIndexEventListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java index ccaff1edfb720..e18a0ec9c6dc1 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java @@ -113,7 +113,7 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea shardId ); } - if (sharedBlobCacheService != null) { + if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot() && sharedBlobCacheService != null) { sharedBlobCacheService.forceEvictAsync( SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) ); From a62ac009812c0d9070dec6d2542a2136f410fef7 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Thu, 10 Apr 2025 17:12:50 +1000 Subject: [PATCH 03/25] Update docs/changelog/126581.yaml --- docs/changelog/126581.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/126581.yaml diff --git a/docs/changelog/126581.yaml b/docs/changelog/126581.yaml new file mode 100644 index 0000000000000..912d23bf30c77 --- /dev/null +++ b/docs/changelog/126581.yaml @@ -0,0 +1,5 @@ +pr: 126581 +summary: Evict from the shared blob cache asynchronously +area: Searchable Snapshots +type: enhancement +issues: [] From ff3a25d1979b3c94fcded4575fee5882a6c03015 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Thu, 10 Apr 2025 22:55:56 +1000 Subject: [PATCH 04/25] Fix changelog --- docs/changelog/126581.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/126581.yaml b/docs/changelog/126581.yaml index 912d23bf30c77..3f72f7ba5a147 100644 --- a/docs/changelog/126581.yaml +++ b/docs/changelog/126581.yaml @@ -1,5 +1,5 @@ pr: 126581 summary: Evict from the shared blob cache asynchronously -area: Searchable Snapshots +area: Snapshot/Restore type: enhancement issues: [] From 2ef16c925c5ae6b4f145fc10bf4e54f4f74d2322 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Mon, 14 Apr 2025 15:55:03 +1000 Subject: [PATCH 05/25] Update x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java Co-authored-by: Tanguy Leroux --- .../blobcache/shared/SharedBlobCacheService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java index 40ddeea394fe5..60499a5a9ccde 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java @@ -1636,7 +1636,9 @@ public void forceEvictAsync(Predicate cacheKeyPredicate) { evictionsRunner.enqueueTask(new ActionListener<>() { @Override public void onResponse(Releasable releasable) { - forceEvict(cacheKeyPredicate); + try (releasable) { + forceEvict(cacheKeyPredicate); + } } @Override From d3ce50612a3f7288474ab8066e4c85d66f9982b3 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Mon, 14 Apr 2025 15:55:46 +1000 Subject: [PATCH 06/25] Fix indenting --- .../blobcache/shared/SharedBlobCacheService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java index 60499a5a9ccde..90d34c6b7e549 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java @@ -1636,9 +1636,9 @@ public void forceEvictAsync(Predicate cacheKeyPredicate) { evictionsRunner.enqueueTask(new ActionListener<>() { @Override public void onResponse(Releasable releasable) { - try (releasable) { - forceEvict(cacheKeyPredicate); - } + try (releasable) { + forceEvict(cacheKeyPredicate); + } } @Override From bd35686ad3dadd6ca452271db00ca6e47241647f Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Mon, 14 Apr 2025 15:56:40 +1000 Subject: [PATCH 07/25] evictionsRunner -> asyncEvictionsRunner --- .../blobcache/shared/SharedBlobCacheService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java index 90d34c6b7e549..829b299253975 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java @@ -338,7 +338,7 @@ private CacheEntry(T chunk) { private final Runnable evictIncrementer; private final LongSupplier relativeTimeInNanosSupplier; - private final ThrottledTaskRunner evictionsRunner; + private final ThrottledTaskRunner asyncEvictionsRunner; public SharedBlobCacheService( NodeEnvironment environment, @@ -399,7 +399,7 @@ public SharedBlobCacheService( this.blobCacheMetrics = blobCacheMetrics; this.evictIncrementer = blobCacheMetrics.getEvictedCountNonZeroFrequency()::increment; this.relativeTimeInNanosSupplier = relativeTimeInNanosSupplier; - this.evictionsRunner = new ThrottledTaskRunner( + this.asyncEvictionsRunner = new ThrottledTaskRunner( "shared_blob_cache_evictions", SHARED_CACHE_CONCURRENT_EVICTIONS_SETTING.get(settings), threadPool.generic() @@ -1633,7 +1633,7 @@ public int forceEvict(Predicate cacheKeyPredicate) { @Override public void forceEvictAsync(Predicate cacheKeyPredicate) { - evictionsRunner.enqueueTask(new ActionListener<>() { + asyncEvictionsRunner.enqueueTask(new ActionListener<>() { @Override public void onResponse(Releasable releasable) { try (releasable) { From 632afbc356764b28fc8cbbe8951a87ebb7364d0b Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Mon, 14 Apr 2025 16:06:20 +1000 Subject: [PATCH 08/25] Only evict asynchronously for shards we know are not coming back --- .../allocation/SearchableSnapshotIndexEventListener.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java index e18a0ec9c6dc1..e5cd44961c84a 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java @@ -114,9 +114,12 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea ); } if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot() && sharedBlobCacheService != null) { - sharedBlobCacheService.forceEvictAsync( - SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) - ); + // Evict shards we know are not coming back asynchronously. Let any other shards expire. + switch (reason) { + case DELETED, FAILURE -> sharedBlobCacheService.forceEvictAsync( + SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) + ); + } } } } From 7ac3220665afa5acc5b1b0515e6facd965bfa004 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 13:40:21 +1000 Subject: [PATCH 09/25] Propagate IndexRemovalReason to deletion listeners --- .../IndexFoldersDeletionListenerIT.java | 15 ++++- .../org/elasticsearch/index/IndexService.java | 16 +++-- .../elasticsearch/indices/IndicesService.java | 59 +++++++++++-------- ...CompositeIndexFoldersDeletionListener.java | 19 ++++-- .../indices/store/IndicesStore.java | 9 +-- .../plugins/IndexStorePlugin.java | 7 ++- .../elasticsearch/index/IndexModuleTests.java | 18 ++++-- .../indices/IndicesServiceTests.java | 27 ++++++--- ...eSnapshotIndexFoldersDeletionListener.java | 25 +++++--- 9 files changed, 135 insertions(+), 60 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java index 21b3e2831f216..6eb6d51070817 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java @@ -22,6 +22,7 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.IndicesService; +import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.junit.annotations.TestLogging; @@ -345,12 +346,22 @@ public static class IndexFoldersDeletionListenerPlugin extends Plugin implements public List getIndexFoldersDeletionListeners() { return List.of(new IndexFoldersDeletionListener() { @Override - public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths) { + public void beforeIndexFoldersDeleted( + Index index, + IndexSettings indexSettings, + Path[] indexPaths, + IndexRemovalReason indexRemovalReason + ) { deletedIndices.add(index); } @Override - public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths) { + public void beforeShardFoldersDeleted( + ShardId shardId, + IndexSettings indexSettings, + Path[] shardPaths, + IndexRemovalReason indexRemovalReason + ) { deletedShards.computeIfAbsent(shardId.getIndex(), i -> Collections.synchronizedList(new ArrayList<>())).add(shardId); } }); diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index cdf1ad177f1e4..56dbd3fb1aca8 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -84,6 +84,7 @@ import org.elasticsearch.index.translog.Translog; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.cluster.IndicesClusterStateService; +import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache; import org.elasticsearch.indices.recovery.RecoveryState; import org.elasticsearch.plugins.IndexStorePlugin; @@ -494,7 +495,12 @@ public synchronized IndexShard createShard( nodeEnv, lock, this.indexSettings, - shardPaths -> indexFoldersDeletionListener.beforeShardFoldersDeleted(shardId, this.indexSettings, shardPaths) + shardPaths -> indexFoldersDeletionListener.beforeShardFoldersDeleted( + shardId, + this.indexSettings, + shardPaths, + IndexRemovalReason.FAILURE + ) ); path = ShardPath.loadShardPath(logger, nodeEnv, shardId, this.indexSettings.customDataPath()); } catch (Exception inner) { @@ -704,11 +710,11 @@ private void onShardClose(ShardLock lock) { try { eventListener.beforeIndexShardDeleted(lock.getShardId(), indexSettings.getSettings()); } finally { - shardStoreDeleter.deleteShardStore("delete index", lock, indexSettings); + shardStoreDeleter.deleteShardStore("delete index", lock, indexSettings, IndexRemovalReason.DELETED); eventListener.afterIndexShardDeleted(lock.getShardId(), indexSettings.getSettings()); } } catch (IOException e) { - shardStoreDeleter.addPendingDelete(lock.getShardId(), indexSettings); + shardStoreDeleter.addPendingDelete(lock.getShardId(), indexSettings, IndexRemovalReason.DELETED); logger.debug(() -> "[" + lock.getShardId().id() + "] failed to delete shard content - scheduled a retry", e); } } @@ -1062,9 +1068,9 @@ public static Function dateMathExpressionResolverAt(long instant } public interface ShardStoreDeleter { - void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings) throws IOException; + void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason permanent) throws IOException; - void addPendingDelete(ShardId shardId, IndexSettings indexSettings); + void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason permanent); } public final EngineFactory getEngineFactory() { diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index c3adcf1160e66..2b4ede7b9da0c 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1005,7 +1005,7 @@ public void removeIndex( listener.afterIndexRemoved(indexService.index(), indexSettings, reason); if (reason == IndexRemovalReason.DELETED) { // now we are done - try to wipe data on disk if possible - deleteIndexStore(extraInfo, indexService.index(), indexSettings); + deleteIndexStore(extraInfo, indexService.index(), indexSettings, reason); } })); }); @@ -1077,7 +1077,7 @@ public void deleteUnassignedIndex(String reason, IndexMetadata oldIndexMetadata, + "]" ); } - deleteIndexStore(reason, oldIndexMetadata); + deleteIndexStore(reason, oldIndexMetadata, IndexRemovalReason.DELETED); } catch (Exception e) { logger.warn(() -> format("[%s] failed to delete unassigned index (reason [%s])", oldIndexMetadata.getIndex(), reason), e); } @@ -1090,7 +1090,7 @@ public void deleteUnassignedIndex(String reason, IndexMetadata oldIndexMetadata, * * Package private for testing */ - void deleteIndexStore(String reason, IndexMetadata metadata) throws IOException { + void deleteIndexStore(String reason, IndexMetadata metadata, IndexRemovalReason indexRemovalReason) throws IOException { if (nodeEnv.hasNodeFile()) { synchronized (this) { Index index = metadata.getIndex(); @@ -1108,19 +1108,21 @@ void deleteIndexStore(String reason, IndexMetadata metadata) throws IOException } } final IndexSettings indexSettings = buildIndexSettings(metadata); - deleteIndexStore(reason, indexSettings.getIndex(), indexSettings); + deleteIndexStore(reason, indexSettings.getIndex(), indexSettings, indexRemovalReason); } } - private void deleteIndexStore(String reason, Index index, IndexSettings indexSettings) throws IOException { - deleteIndexStoreIfDeletionAllowed(reason, index, indexSettings, DEFAULT_INDEX_DELETION_PREDICATE); + private void deleteIndexStore(String reason, Index index, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) + throws IOException { + deleteIndexStoreIfDeletionAllowed(reason, index, indexSettings, DEFAULT_INDEX_DELETION_PREDICATE, indexRemovalReason); } private void deleteIndexStoreIfDeletionAllowed( final String reason, final Index index, final IndexSettings indexSettings, - final IndexDeletionAllowedPredicate predicate + final IndexDeletionAllowedPredicate predicate, + final IndexRemovalReason indexRemovalReason ) throws IOException { boolean success = false; try { @@ -1134,7 +1136,7 @@ private void deleteIndexStoreIfDeletionAllowed( index, 0, indexSettings, - paths -> indexFoldersDeletionListeners.beforeIndexFoldersDeleted(index, indexSettings, paths) + paths -> indexFoldersDeletionListeners.beforeIndexFoldersDeleted(index, indexSettings, paths, indexRemovalReason) ); } success = true; @@ -1144,7 +1146,7 @@ private void deleteIndexStoreIfDeletionAllowed( logger.warn(() -> format("%s failed to delete index", index), ex); } finally { if (success == false) { - addPendingDelete(index, indexSettings); + addPendingDelete(index, indexSettings, indexRemovalReason); } // this is a pure protection to make sure this index doesn't get re-imported as a dangling index. // we should in the future rather write a tombstone rather than wiping the metadata. @@ -1160,13 +1162,14 @@ private void deleteIndexStoreIfDeletionAllowed( * @throws IOException if an IOException occurs */ @Override - public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings) throws IOException { + public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) + throws IOException { ShardId shardId = lock.getShardId(); logger.trace("{} deleting shard reason [{}]", shardId, reason); nodeEnv.deleteShardDirectoryUnderLock( lock, indexSettings, - paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths) + paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths, indexRemovalReason) ); } @@ -1183,8 +1186,8 @@ public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexS * @param clusterState . This is required to access the indexes settings etc. * @throws IOException if an IOException occurs */ - public void deleteShardStore(String reason, ShardId shardId, ClusterState clusterState) throws IOException, - ShardLockObtainFailedException { + public void deleteShardStore(String reason, ShardId shardId, ClusterState clusterState, IndexRemovalReason indexRemovalReason) + throws IOException, ShardLockObtainFailedException { final IndexMetadata metadata = clusterState.getMetadata().getProject().indices().get(shardId.getIndexName()); final IndexSettings indexSettings = buildIndexSettings(metadata); @@ -1195,7 +1198,7 @@ public void deleteShardStore(String reason, ShardId shardId, ClusterState cluste nodeEnv.deleteShardDirectorySafe( shardId, indexSettings, - paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths) + paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths, indexRemovalReason) ); logger.debug("{} deleted shard reason [{}]", shardId, reason); @@ -1203,7 +1206,7 @@ public void deleteShardStore(String reason, ShardId shardId, ClusterState cluste if (nodeEnv.findAllShardIds(shardId.getIndex()).isEmpty()) { try { // note that deleteIndexStore have more safety checks and may throw an exception if index was concurrently created. - deleteIndexStore("no longer used", metadata); + deleteIndexStore("no longer used", metadata, indexRemovalReason); } catch (Exception e) { // wrap the exception to indicate we already deleted the shard throw new ElasticsearchException("failed to delete unused index after deleting its last shard (" + shardId + ")", e); @@ -1259,7 +1262,7 @@ public IndexMetadata verifyIndexIsDeleted(final Index index, final ClusterState } final IndexSettings indexSettings = buildIndexSettings(metadata); try { - deleteIndexStoreIfDeletionAllowed("stale deleted index", index, indexSettings, ALWAYS_TRUE); + deleteIndexStoreIfDeletionAllowed("stale deleted index", index, indexSettings, ALWAYS_TRUE, IndexRemovalReason.DELETED); } catch (Exception e) { // we just warn about the exception here because if deleteIndexStoreIfDeletionAllowed // throws an exception, it gets added to the list of pending deletes to be tried again @@ -1317,22 +1320,22 @@ private IndexSettings buildIndexSettings(IndexMetadata metadata) { * Adds a pending delete for the given index shard. */ @Override - public void addPendingDelete(ShardId shardId, IndexSettings settings) { + public void addPendingDelete(ShardId shardId, IndexSettings settings, IndexRemovalReason reason) { if (shardId == null) { throw new IllegalArgumentException("shardId must not be null"); } if (settings == null) { throw new IllegalArgumentException("settings must not be null"); } - PendingDelete pendingDelete = new PendingDelete(shardId, settings); + PendingDelete pendingDelete = new PendingDelete(shardId, settings, reason); addPendingDelete(shardId.getIndex(), pendingDelete); } /** * Adds a pending delete for the given index. */ - public void addPendingDelete(Index index, IndexSettings settings) { - PendingDelete pendingDelete = new PendingDelete(index, settings); + public void addPendingDelete(Index index, IndexSettings settings, IndexRemovalReason reason) { + PendingDelete pendingDelete = new PendingDelete(index, settings, reason); addPendingDelete(index, pendingDelete); } @@ -1348,25 +1351,28 @@ private static final class PendingDelete implements Comparable { final int shardId; final IndexSettings settings; final boolean deleteIndex; + final IndexRemovalReason reason; /** * Creates a new pending delete of an index */ - PendingDelete(ShardId shardId, IndexSettings settings) { + PendingDelete(ShardId shardId, IndexSettings settings, IndexRemovalReason reason) { this.index = shardId.getIndex(); this.shardId = shardId.getId(); this.settings = settings; this.deleteIndex = false; + this.reason = reason; } /** * Creates a new pending delete of a shard */ - PendingDelete(Index index, IndexSettings settings) { + PendingDelete(Index index, IndexSettings settings, IndexRemovalReason reason) { this.index = index; this.shardId = -1; this.settings = settings; this.deleteIndex = true; + this.reason = reason; } @Override @@ -1430,7 +1436,12 @@ public void processPendingDeletes(Index index, IndexSettings indexSettings, Time nodeEnv.deleteIndexDirectoryUnderLock( index, indexSettings, - paths -> indexFoldersDeletionListeners.beforeIndexFoldersDeleted(index, indexSettings, paths) + paths -> indexFoldersDeletionListeners.beforeIndexFoldersDeleted( + index, + indexSettings, + paths, + delete.reason + ) ); iterator.remove(); } catch (IOException ex) { @@ -1442,7 +1453,7 @@ public void processPendingDeletes(Index index, IndexSettings indexSettings, Time final ShardLock shardLock = locks.get(shardId); if (shardLock != null) { try { - deleteShardStore("pending delete", shardLock, delete.settings); + deleteShardStore("pending delete", shardLock, delete.settings, delete.reason); iterator.remove(); } catch (IOException ex) { logger.debug(() -> format("%s retry pending delete", shardLock.getShardId()), ex); diff --git a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java index 0fffef74df25f..02f6d8076c086 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java +++ b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java @@ -12,6 +12,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.plugins.IndexStorePlugin; import java.nio.file.Path; @@ -31,10 +32,15 @@ public CompositeIndexFoldersDeletionListener(List { if (clusterStateVersion != currentState.getVersion()) { @@ -349,7 +350,7 @@ private void deleteShardStoreOnApplierThread(ShardId shardId, long clusterStateV return; } try { - indicesService.deleteShardStore("no longer used", shardId, currentState); + indicesService.deleteShardStore("no longer used", shardId, currentState, indexRemovalReason); } catch (Exception ex) { logger.debug(() -> format("%s failed to delete unallocated shard, ignoring", shardId), ex); } diff --git a/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java b/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java index 4b7520afbe7b6..2566babbd2020 100644 --- a/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java @@ -19,6 +19,7 @@ import org.elasticsearch.index.engine.EngineException; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.shard.ShardPath; +import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.indices.recovery.RecoveryState; import java.io.IOException; @@ -102,8 +103,9 @@ interface IndexFoldersDeletionListener { * @param index the {@link Index} of the index whose folders are going to be deleted * @param indexSettings settings for the index whose folders are going to be deleted * @param indexPaths the paths of the folders that are going to be deleted + * @param reason the reason for the deletion */ - void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths); + void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths, IndexRemovalReason reason); /** * Invoked before the folders of a shard are deleted from disk. The list of folders contains {@link Path}s that may or may not @@ -112,8 +114,9 @@ interface IndexFoldersDeletionListener { * @param shardId the {@link ShardId} of the shard whose folders are going to be deleted * @param indexSettings index settings of the shard whose folders are going to be deleted * @param shardPaths the paths of the folders that are going to be deleted + * @param reason the reason for the deletion */ - void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths); + void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths, IndexRemovalReason reason); } /** diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 565037eba8369..303102e004b81 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -144,18 +144,28 @@ public class IndexModuleTests extends ESTestCase { private IndexService.ShardStoreDeleter deleter = new IndexService.ShardStoreDeleter() { @Override - public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings) throws IOException {} + public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) {} @Override - public void addPendingDelete(ShardId shardId, IndexSettings indexSettings) {} + public void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) {} }; private IndexStorePlugin.IndexFoldersDeletionListener indexDeletionListener = new IndexStorePlugin.IndexFoldersDeletionListener() { @Override - public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths) {} + public void beforeIndexFoldersDeleted( + Index index, + IndexSettings indexSettings, + Path[] indexPaths, + IndexRemovalReason indexRemovalReason + ) {} @Override - public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths) {} + public void beforeShardFoldersDeleted( + ShardId shardId, + IndexSettings indexSettings, + Path[] shardPaths, + IndexRemovalReason indexRemovalReason + ) {} }; private final IndexFieldDataCache.Listener listener = new IndexFieldDataCache.Listener() { diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index d1344fbdb2d80..fbac5a58d4b24 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -65,6 +65,7 @@ import org.elasticsearch.index.shard.ShardPath; import org.elasticsearch.index.similarity.NonNegativeScoresSimilarity; import org.elasticsearch.indices.IndicesService.ShardDeletionCheckResult; +import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.plugins.EnginePlugin; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.Plugin; @@ -323,7 +324,10 @@ public void testDeleteIndexStore() throws Exception { test.getIndexSettings().customDataPath() ); - expectThrows(IllegalStateException.class, () -> indicesService.deleteIndexStore("boom", firstMetadata)); + expectThrows( + IllegalStateException.class, + () -> indicesService.deleteIndexStore("boom", firstMetadata, randomReasonOtherThanDeleted()) + ); assertTrue(firstPath.exists()); GatewayMetaState gwMetaState = getInstanceFromNode(GatewayMetaState.class); @@ -353,7 +357,10 @@ public void testDeleteIndexStore() throws Exception { ); assertTrue(secondPath.exists()); - expectThrows(IllegalStateException.class, () -> indicesService.deleteIndexStore("boom", secondMetadata)); + expectThrows( + IllegalStateException.class, + () -> indicesService.deleteIndexStore("boom", secondMetadata, randomReasonOtherThanDeleted()) + ); assertTrue(secondPath.exists()); assertAcked(client().admin().indices().prepareOpen("test")); @@ -384,13 +391,13 @@ public void testPendingTasks() throws Exception { int numPending = 1; if (randomBoolean()) { - indicesService.addPendingDelete(indexShard.shardId(), indexSettings); + indicesService.addPendingDelete(indexShard.shardId(), indexSettings, randomReasonOtherThanDeleted()); } else { if (randomBoolean()) { numPending++; - indicesService.addPendingDelete(indexShard.shardId(), indexSettings); + indicesService.addPendingDelete(indexShard.shardId(), indexSettings, randomReasonOtherThanDeleted()); } - indicesService.addPendingDelete(index, indexSettings); + indicesService.addPendingDelete(index, indexSettings, randomReasonOtherThanDeleted()); } assertAcked(client().admin().indices().prepareClose("test")); @@ -410,9 +417,9 @@ public void testPendingTasks() throws Exception { final boolean hasBogus = randomBoolean(); if (hasBogus) { - indicesService.addPendingDelete(new ShardId(index, 0), indexSettings); - indicesService.addPendingDelete(new ShardId(index, 1), indexSettings); - indicesService.addPendingDelete(new ShardId("bogus", "_na_", 1), indexSettings); + indicesService.addPendingDelete(new ShardId(index, 0), indexSettings, randomReasonOtherThanDeleted()); + indicesService.addPendingDelete(new ShardId(index, 1), indexSettings, randomReasonOtherThanDeleted()); + indicesService.addPendingDelete(new ShardId("bogus", "_na_", 1), indexSettings, randomReasonOtherThanDeleted()); assertEquals(indicesService.numPendingDeletes(index), numPending + 2); assertTrue(indicesService.hasUncompletedPendingDeletes()); } @@ -884,4 +891,8 @@ public void testWithTempIndexServiceHandlesExistingIndex() throws Exception { private Set resolvedExpressions(String... expressions) { return Arrays.stream(expressions).map(ResolvedExpression::new).collect(Collectors.toSet()); } + + private IndexRemovalReason randomReasonOtherThanDeleted() { + return randomValueOtherThan(IndexRemovalReason.DELETED, () -> randomFrom(IndexRemovalReason.values())); + } } diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java index e59ec43e1db94..87d04845fbf48 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java @@ -13,6 +13,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.plugins.IndexStorePlugin; import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots; import org.elasticsearch.xpack.searchablesnapshots.cache.common.CacheKey; @@ -46,22 +47,32 @@ public SearchableSnapshotIndexFoldersDeletionListener( } @Override - public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths) { + public void beforeIndexFoldersDeleted( + Index index, + IndexSettings indexSettings, + Path[] indexPaths, + IndexRemovalReason indexRemovalReason + ) { if (indexSettings.getIndexMetadata().isSearchableSnapshot()) { for (int shard = 0; shard < indexSettings.getNumberOfShards(); shard++) { - markShardAsEvictedInCache(new ShardId(index, shard), indexSettings); + markShardAsEvictedInCache(new ShardId(index, shard), indexSettings, indexRemovalReason); } } } @Override - public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths) { + public void beforeShardFoldersDeleted( + ShardId shardId, + IndexSettings indexSettings, + Path[] shardPaths, + IndexRemovalReason indexRemovalReason + ) { if (indexSettings.getIndexMetadata().isSearchableSnapshot()) { - markShardAsEvictedInCache(shardId, indexSettings); + markShardAsEvictedInCache(shardId, indexSettings, indexRemovalReason); } } - private void markShardAsEvictedInCache(ShardId shardId, IndexSettings indexSettings) { + private void markShardAsEvictedInCache(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) { final CacheService cacheService = this.cacheServiceSupplier.get(); assert cacheService != null : "cache service not initialized"; @@ -72,8 +83,8 @@ private void markShardAsEvictedInCache(ShardId shardId, IndexSettings indexSetti shardId ); - // Only partial searchable snapshots use the SharedBlobCacheService - if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot()) { + // Only partial searchable snapshots use the shared blob cache. Only force-evict if we know the shard won't be coming back. + if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot() && indexRemovalReason == IndexRemovalReason.DELETED) { final SharedBlobCacheService sharedBlobCacheService = SearchableSnapshotIndexFoldersDeletionListener.this.frozenCacheServiceSupplier.get(); assert sharedBlobCacheService != null : "frozen cache service not initialized"; From 8e1864448fe655696d8cd2490b29d2fe7b7b6342 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 13:45:47 +1000 Subject: [PATCH 10/25] Fix naming (reasonMessage/reason) --- .../main/java/org/elasticsearch/index/IndexService.java | 5 +++-- .../java/org/elasticsearch/indices/IndicesService.java | 8 ++++---- .../java/org/elasticsearch/index/IndexModuleTests.java | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 56dbd3fb1aca8..65edfa3678aa8 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -1068,9 +1068,10 @@ public static Function dateMathExpressionResolverAt(long instant } public interface ShardStoreDeleter { - void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason permanent) throws IOException; + void deleteShardStore(String reasonMessage, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) + throws IOException; - void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason permanent); + void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason reason); } public final EngineFactory getEngineFactory() { diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index 2b4ede7b9da0c..2a7a8406cda4f 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1156,20 +1156,20 @@ private void deleteIndexStoreIfDeletionAllowed( /** * Deletes the shard with an already acquired shard lock. - * @param reason the reason for the shard deletion + * @param reasonMessage the reason for the shard deletion * @param lock the lock of the shard to delete * @param indexSettings the shards index settings. * @throws IOException if an IOException occurs */ @Override - public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) + public void deleteShardStore(String reasonMessage, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) throws IOException { ShardId shardId = lock.getShardId(); - logger.trace("{} deleting shard reason [{}]", shardId, reason); + logger.trace("{} deleting shard reason [{}]", shardId, reasonMessage); nodeEnv.deleteShardDirectoryUnderLock( lock, indexSettings, - paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths, indexRemovalReason) + paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths, reason) ); } diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 303102e004b81..923c5b66f4407 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -144,10 +144,10 @@ public class IndexModuleTests extends ESTestCase { private IndexService.ShardStoreDeleter deleter = new IndexService.ShardStoreDeleter() { @Override - public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) {} + public void deleteShardStore(String reasonMessage, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) {} @Override - public void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) {} + public void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason reason) {} }; private IndexStorePlugin.IndexFoldersDeletionListener indexDeletionListener = new IndexStorePlugin.IndexFoldersDeletionListener() { From 410fb351ba9f13b39287289497ad87b31093c473 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 13:48:13 +1000 Subject: [PATCH 11/25] Fix naming (reasonText/reason) --- .../org/elasticsearch/index/IndexService.java | 2 +- .../elasticsearch/indices/IndicesService.java | 24 +++++++++---------- .../elasticsearch/index/IndexModuleTests.java | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 65edfa3678aa8..6611c3e851194 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -1068,7 +1068,7 @@ public static Function dateMathExpressionResolverAt(long instant } public interface ShardStoreDeleter { - void deleteShardStore(String reasonMessage, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) + void deleteShardStore(String reasonText, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) throws IOException; void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason reason); diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index 2a7a8406cda4f..b2bd83ab3caec 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1090,7 +1090,7 @@ public void deleteUnassignedIndex(String reason, IndexMetadata oldIndexMetadata, * * Package private for testing */ - void deleteIndexStore(String reason, IndexMetadata metadata, IndexRemovalReason indexRemovalReason) throws IOException { + void deleteIndexStore(String reasonText, IndexMetadata metadata, IndexRemovalReason reason) throws IOException { if (nodeEnv.hasNodeFile()) { synchronized (this) { Index index = metadata.getIndex(); @@ -1108,35 +1108,35 @@ void deleteIndexStore(String reason, IndexMetadata metadata, IndexRemovalReason } } final IndexSettings indexSettings = buildIndexSettings(metadata); - deleteIndexStore(reason, indexSettings.getIndex(), indexSettings, indexRemovalReason); + deleteIndexStore(reasonText, indexSettings.getIndex(), indexSettings, reason); } } - private void deleteIndexStore(String reason, Index index, IndexSettings indexSettings, IndexRemovalReason indexRemovalReason) + private void deleteIndexStore(String reasonText, Index index, IndexSettings indexSettings, IndexRemovalReason reason) throws IOException { - deleteIndexStoreIfDeletionAllowed(reason, index, indexSettings, DEFAULT_INDEX_DELETION_PREDICATE, indexRemovalReason); + deleteIndexStoreIfDeletionAllowed(reasonText, index, indexSettings, DEFAULT_INDEX_DELETION_PREDICATE, reason); } private void deleteIndexStoreIfDeletionAllowed( - final String reason, + final String reasonText, final Index index, final IndexSettings indexSettings, final IndexDeletionAllowedPredicate predicate, - final IndexRemovalReason indexRemovalReason + final IndexRemovalReason reason ) throws IOException { boolean success = false; try { // we are trying to delete the index store here - not a big deal if the lock can't be obtained // the store metadata gets wiped anyway even without the lock this is just best effort since // every shards deletes its content under the shard lock it owns. - logger.debug("{} deleting index store reason [{}]", index, reason); + logger.debug("{} deleting index store reason [{}]", index, reasonText); if (predicate.apply(index, indexSettings)) { // its safe to delete all index metadata and shard data nodeEnv.deleteIndexDirectorySafe( index, 0, indexSettings, - paths -> indexFoldersDeletionListeners.beforeIndexFoldersDeleted(index, indexSettings, paths, indexRemovalReason) + paths -> indexFoldersDeletionListeners.beforeIndexFoldersDeleted(index, indexSettings, paths, reason) ); } success = true; @@ -1146,7 +1146,7 @@ private void deleteIndexStoreIfDeletionAllowed( logger.warn(() -> format("%s failed to delete index", index), ex); } finally { if (success == false) { - addPendingDelete(index, indexSettings, indexRemovalReason); + addPendingDelete(index, indexSettings, reason); } // this is a pure protection to make sure this index doesn't get re-imported as a dangling index. // we should in the future rather write a tombstone rather than wiping the metadata. @@ -1156,16 +1156,16 @@ private void deleteIndexStoreIfDeletionAllowed( /** * Deletes the shard with an already acquired shard lock. - * @param reasonMessage the reason for the shard deletion + * @param reasonText the reason for the shard deletion * @param lock the lock of the shard to delete * @param indexSettings the shards index settings. * @throws IOException if an IOException occurs */ @Override - public void deleteShardStore(String reasonMessage, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) + public void deleteShardStore(String reasonText, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) throws IOException { ShardId shardId = lock.getShardId(); - logger.trace("{} deleting shard reason [{}]", shardId, reasonMessage); + logger.trace("{} deleting shard reason [{}]", shardId, reasonText); nodeEnv.deleteShardDirectoryUnderLock( lock, indexSettings, diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 923c5b66f4407..f1b3207d13eeb 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -144,7 +144,7 @@ public class IndexModuleTests extends ESTestCase { private IndexService.ShardStoreDeleter deleter = new IndexService.ShardStoreDeleter() { @Override - public void deleteShardStore(String reasonMessage, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) {} + public void deleteShardStore(String reasonText, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) {} @Override public void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason reason) {} From 23720563a8efbcb9515febe29fbdb67c7edbda73 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 13:49:44 +1000 Subject: [PATCH 12/25] Naming --- .../IndexFoldersDeletionListenerIT.java | 4 ++-- .../CompositeIndexFoldersDeletionListener.java | 18 ++++-------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java index 6eb6d51070817..5139e383a6c5e 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java @@ -350,7 +350,7 @@ public void beforeIndexFoldersDeleted( Index index, IndexSettings indexSettings, Path[] indexPaths, - IndexRemovalReason indexRemovalReason + IndexRemovalReason reason ) { deletedIndices.add(index); } @@ -360,7 +360,7 @@ public void beforeShardFoldersDeleted( ShardId shardId, IndexSettings indexSettings, Path[] shardPaths, - IndexRemovalReason indexRemovalReason + IndexRemovalReason reason ) { deletedShards.computeIfAbsent(shardId.getIndex(), i -> Collections.synchronizedList(new ArrayList<>())).add(shardId); } diff --git a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java index 02f6d8076c086..e3df891462483 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java +++ b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java @@ -32,15 +32,10 @@ public CompositeIndexFoldersDeletionListener(List Date: Wed, 23 Apr 2025 03:56:07 +0000 Subject: [PATCH 13/25] [CI] Auto commit changes from spotless --- server/src/main/java/org/elasticsearch/index/IndexService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 6611c3e851194..408a38b96c9a2 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -1068,8 +1068,7 @@ public static Function dateMathExpressionResolverAt(long instant } public interface ShardStoreDeleter { - void deleteShardStore(String reasonText, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) - throws IOException; + void deleteShardStore(String reasonText, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) throws IOException; void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason reason); } From 87d1ba4a2919a96c762346c7e9d07334a1450524 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 13:56:06 +1000 Subject: [PATCH 14/25] Naming/javadoc --- .../org/elasticsearch/indices/IndicesService.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index b2bd83ab3caec..1d3de2ebf3d06 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1181,12 +1181,13 @@ public void deleteShardStore(String reasonText, ShardLock lock, IndexSettings in * On data nodes, if the deleted shard is the last shard folder in its index, the method will attempt to remove * the index folder as well. * - * @param reason the reason for the shard deletion + * @param reasonText the reason for the shard deletion * @param shardId the shards ID to delete * @param clusterState . This is required to access the indexes settings etc. + * @param reason The reason for the deletion (as an enum) * @throws IOException if an IOException occurs */ - public void deleteShardStore(String reason, ShardId shardId, ClusterState clusterState, IndexRemovalReason indexRemovalReason) + public void deleteShardStore(String reasonText, ShardId shardId, ClusterState clusterState, IndexRemovalReason reason) throws IOException, ShardLockObtainFailedException { final IndexMetadata metadata = clusterState.getMetadata().getProject().indices().get(shardId.getIndexName()); @@ -1198,15 +1199,15 @@ public void deleteShardStore(String reason, ShardId shardId, ClusterState cluste nodeEnv.deleteShardDirectorySafe( shardId, indexSettings, - paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths, indexRemovalReason) + paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths, reason) ); - logger.debug("{} deleted shard reason [{}]", shardId, reason); + logger.debug("{} deleted shard reason [{}]", shardId, reasonText); if (canDeleteIndexContents(shardId.getIndex())) { if (nodeEnv.findAllShardIds(shardId.getIndex()).isEmpty()) { try { // note that deleteIndexStore have more safety checks and may throw an exception if index was concurrently created. - deleteIndexStore("no longer used", metadata, indexRemovalReason); + deleteIndexStore("no longer used", metadata, reason); } catch (Exception e) { // wrap the exception to indicate we already deleted the shard throw new ElasticsearchException("failed to delete unused index after deleting its last shard (" + shardId + ")", e); From ea43b2dfac9c442bf202e714e83ad58310824772 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 14:02:18 +1000 Subject: [PATCH 15/25] randomReason() --- .../indices/IndicesServiceTests.java | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index fbac5a58d4b24..9112d4aec4492 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -324,10 +324,7 @@ public void testDeleteIndexStore() throws Exception { test.getIndexSettings().customDataPath() ); - expectThrows( - IllegalStateException.class, - () -> indicesService.deleteIndexStore("boom", firstMetadata, randomReasonOtherThanDeleted()) - ); + expectThrows(IllegalStateException.class, () -> indicesService.deleteIndexStore("boom", firstMetadata, randomReason())); assertTrue(firstPath.exists()); GatewayMetaState gwMetaState = getInstanceFromNode(GatewayMetaState.class); @@ -357,10 +354,7 @@ public void testDeleteIndexStore() throws Exception { ); assertTrue(secondPath.exists()); - expectThrows( - IllegalStateException.class, - () -> indicesService.deleteIndexStore("boom", secondMetadata, randomReasonOtherThanDeleted()) - ); + expectThrows(IllegalStateException.class, () -> indicesService.deleteIndexStore("boom", secondMetadata, randomReason())); assertTrue(secondPath.exists()); assertAcked(client().admin().indices().prepareOpen("test")); @@ -391,13 +385,13 @@ public void testPendingTasks() throws Exception { int numPending = 1; if (randomBoolean()) { - indicesService.addPendingDelete(indexShard.shardId(), indexSettings, randomReasonOtherThanDeleted()); + indicesService.addPendingDelete(indexShard.shardId(), indexSettings, randomReason()); } else { if (randomBoolean()) { numPending++; - indicesService.addPendingDelete(indexShard.shardId(), indexSettings, randomReasonOtherThanDeleted()); + indicesService.addPendingDelete(indexShard.shardId(), indexSettings, randomReason()); } - indicesService.addPendingDelete(index, indexSettings, randomReasonOtherThanDeleted()); + indicesService.addPendingDelete(index, indexSettings, randomReason()); } assertAcked(client().admin().indices().prepareClose("test")); @@ -417,9 +411,9 @@ public void testPendingTasks() throws Exception { final boolean hasBogus = randomBoolean(); if (hasBogus) { - indicesService.addPendingDelete(new ShardId(index, 0), indexSettings, randomReasonOtherThanDeleted()); - indicesService.addPendingDelete(new ShardId(index, 1), indexSettings, randomReasonOtherThanDeleted()); - indicesService.addPendingDelete(new ShardId("bogus", "_na_", 1), indexSettings, randomReasonOtherThanDeleted()); + indicesService.addPendingDelete(new ShardId(index, 0), indexSettings, randomReason()); + indicesService.addPendingDelete(new ShardId(index, 1), indexSettings, randomReason()); + indicesService.addPendingDelete(new ShardId("bogus", "_na_", 1), indexSettings, randomReason()); assertEquals(indicesService.numPendingDeletes(index), numPending + 2); assertTrue(indicesService.hasUncompletedPendingDeletes()); } @@ -892,7 +886,7 @@ private Set resolvedExpressions(String... expressions) { return Arrays.stream(expressions).map(ResolvedExpression::new).collect(Collectors.toSet()); } - private IndexRemovalReason randomReasonOtherThanDeleted() { - return randomValueOtherThan(IndexRemovalReason.DELETED, () -> randomFrom(IndexRemovalReason.values())); + private IndexRemovalReason randomReason() { + return randomFrom(IndexRemovalReason.values()); } } From c6e7a05817594e6e3d8c34b01d19172e4d48dcaa Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 16:16:08 +1000 Subject: [PATCH 16/25] Don't evict shards when IndexRemovalReason is FAILURE --- .../allocation/SearchableSnapshotIndexEventListener.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java index e5cd44961c84a..c4eb4a40a02d1 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java @@ -115,8 +115,8 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea } if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot() && sharedBlobCacheService != null) { // Evict shards we know are not coming back asynchronously. Let any other shards expire. - switch (reason) { - case DELETED, FAILURE -> sharedBlobCacheService.forceEvictAsync( + if (reason == IndexRemovalReason.DELETED) { + sharedBlobCacheService.forceEvictAsync( SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) ); } From 7eebc42815e49dcf053322232daf1d9dfd8c1591 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 23 Apr 2025 16:59:16 +1000 Subject: [PATCH 17/25] javadoc/naming --- .../org/elasticsearch/indices/IndicesService.java | 1 + .../org/elasticsearch/index/IndexModuleTests.java | 14 ++------------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index 1d3de2ebf3d06..06a06d704f03c 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1159,6 +1159,7 @@ private void deleteIndexStoreIfDeletionAllowed( * @param reasonText the reason for the shard deletion * @param lock the lock of the shard to delete * @param indexSettings the shards index settings. + * @param reason the reason for the deletion (as an enum) * @throws IOException if an IOException occurs */ @Override diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index f1b3207d13eeb..18d303d40bed2 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -152,20 +152,10 @@ public void addPendingDelete(ShardId shardId, IndexSettings indexSettings, Index private IndexStorePlugin.IndexFoldersDeletionListener indexDeletionListener = new IndexStorePlugin.IndexFoldersDeletionListener() { @Override - public void beforeIndexFoldersDeleted( - Index index, - IndexSettings indexSettings, - Path[] indexPaths, - IndexRemovalReason indexRemovalReason - ) {} + public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths, IndexRemovalReason reason) {} @Override - public void beforeShardFoldersDeleted( - ShardId shardId, - IndexSettings indexSettings, - Path[] shardPaths, - IndexRemovalReason indexRemovalReason - ) {} + public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths, IndexRemovalReason reason) {} }; private final IndexFieldDataCache.Listener listener = new IndexFieldDataCache.Listener() { From d3cd806abdc60ea93d3c76110c7e6cfb79b6a1cc Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 21 May 2025 15:05:24 +1000 Subject: [PATCH 18/25] Make IndexRemovalReason a top-level enum for sharing --- .../IndexFoldersDeletionListenerIT.java | 2 +- .../metadata/MetadataIndexAliasesService.java | 2 +- .../MetadataIndexTemplateService.java | 2 +- .../index/CompositeIndexEventListener.java | 2 +- .../org/elasticsearch/index/IndexService.java | 2 +- .../index/shard/IndexEventListener.java | 2 +- .../elasticsearch/indices/IndicesService.java | 1 + .../indices/cluster/IndexRemovalReason.java | 54 +++++++++++++++++++ .../cluster/IndicesClusterStateService.java | 53 +++--------------- ...CompositeIndexFoldersDeletionListener.java | 2 +- .../indices/store/IndicesStore.java | 2 +- .../plugins/IndexStorePlugin.java | 2 +- .../elasticsearch/search/SearchService.java | 2 +- .../elasticsearch/index/IndexModuleTests.java | 2 +- ...dicesLifecycleListenerSingleNodeTests.java | 4 +- .../indices/IndicesServiceTests.java | 2 +- .../search/SearchServiceSingleNodeTests.java | 2 +- .../test/MockIndexEventListener.java | 2 +- .../SearchableSnapshotIndexEventListener.java | 2 +- ...eSnapshotIndexFoldersDeletionListener.java | 2 +- 20 files changed, 79 insertions(+), 65 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/indices/cluster/IndexRemovalReason.java diff --git a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java index 5139e383a6c5e..ba5f250c91599 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java @@ -22,7 +22,7 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.IndicesService; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.junit.annotations.TestLogging; diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java index d523f233e876a..5bb0aa4e73434 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java @@ -48,7 +48,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.NO_LONGER_ASSIGNED; /** * Service responsible for submitting add and remove aliases requests diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java index 6c26a302a9304..689f3cc26d17d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java @@ -83,7 +83,7 @@ import static java.util.Collections.emptyMap; import static org.elasticsearch.cluster.metadata.MetadataCreateDataStreamService.validateTimestampFieldMapping; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.NO_LONGER_ASSIGNED; /** * Service responsible for submitting index templates updates diff --git a/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java b/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java index 03bbf77b66046..1947bcd6e148f 100644 --- a/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java +++ b/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java @@ -21,7 +21,7 @@ import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.IndexShardState; import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.threadpool.ThreadPool; import java.util.Collection; diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 408a38b96c9a2..d5c00294aa6b8 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -83,8 +83,8 @@ import org.elasticsearch.index.store.Store; import org.elasticsearch.index.translog.Translog; import org.elasticsearch.indices.breaker.CircuitBreakerService; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.indices.cluster.IndicesClusterStateService; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache; import org.elasticsearch.indices.recovery.RecoveryState; import org.elasticsearch.plugins.IndexStorePlugin; diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexEventListener.java b/server/src/main/java/org/elasticsearch/index/shard/IndexEventListener.java index 92af26228948c..d0113897432de 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexEventListener.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexEventListener.java @@ -15,7 +15,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.IndexSettings; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; /** * An index event listener is the primary extension point for plugins and build-in services diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index 94d5ed7e73512..3d2c64c814ddb 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -132,6 +132,7 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.translog.TranslogStats; import org.elasticsearch.indices.breaker.CircuitBreakerService; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.indices.cluster.IndicesClusterStateService; import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache; import org.elasticsearch.indices.recovery.PeerRecoveryTargetService; diff --git a/server/src/main/java/org/elasticsearch/indices/cluster/IndexRemovalReason.java b/server/src/main/java/org/elasticsearch/indices/cluster/IndexRemovalReason.java new file mode 100644 index 0000000000000..01fcd8c680bec --- /dev/null +++ b/server/src/main/java/org/elasticsearch/indices/cluster/IndexRemovalReason.java @@ -0,0 +1,54 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.indices.cluster; + +/** + * The reasons why an index or shard is being removed from a node. + */ +public enum IndexRemovalReason { + /** + * Shard of this index were previously assigned to this node but all shards have been relocated. + * The index should be removed and all associated resources released. Persistent parts of the index + * like the shards files, state and transaction logs are kept around in the case of a disaster recovery. + */ + NO_LONGER_ASSIGNED, + + /** + * The index is deleted. Persistent parts of the index like the shards files, state and transaction logs are removed once + * all resources are released. + */ + DELETED, + + /** + * The index has been closed. The index should be removed and all associated resources released. Persistent parts of the index + * like the shards files, state and transaction logs are kept around in the case of a disaster recovery. + */ + CLOSED, + + /** + * Something around index management has failed and the index should be removed. + * Persistent parts of the index like the shards files, state and transaction logs are kept around in the + * case of a disaster recovery. + */ + FAILURE, + + /** + * The index has been reopened. The index should be removed and all associated resources released. Persistent parts of the index + * like the shards files, state and transaction logs are kept around in the case of a disaster recovery. + */ + REOPENED, + + /** + * The index is closed as part of the node shutdown process. The index should be removed and all associated resources released. + * Persistent parts of the index like the shards files, state and transaction logs should be kept around in the case the node + * restarts. + */ + SHUTDOWN, +} diff --git a/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java b/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java index c0e897bc34319..ff1a83d26592f 100644 --- a/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java +++ b/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java @@ -97,11 +97,11 @@ import java.util.function.Consumer; import static org.elasticsearch.core.Strings.format; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.CLOSED; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.DELETED; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.FAILURE; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.REOPENED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.CLOSED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.DELETED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.FAILURE; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.NO_LONGER_ASSIGNED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.REOPENED; public class IndicesClusterStateService extends AbstractLifecycleComponent implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(IndicesClusterStateService.class); @@ -480,7 +480,7 @@ private void removeIndicesAndShards(final ClusterChangedEvent event) { final IndexMetadata indexMetadata = project.map(proj -> proj.index(index)).orElse(null); final IndexMetadata existingMetadata = indexService.getIndexSettings().getIndexMetadata(); - AllocatedIndices.IndexRemovalReason reason = null; + IndexRemovalReason reason = null; if (indexMetadata != null && indexMetadata.getState() != existingMetadata.getState()) { reason = indexMetadata.getState() == IndexMetadata.State.CLOSE ? CLOSED : REOPENED; } else if (localRoutingNode == null || localRoutingNode.hasIndex(index) == false) { @@ -1335,47 +1335,6 @@ default T getShardOrNull(ShardId shardId) { void processPendingDeletes(Index index, IndexSettings indexSettings, TimeValue timeValue) throws IOException, InterruptedException, ShardLockObtainFailedException; - - enum IndexRemovalReason { - /** - * Shard of this index were previously assigned to this node but all shards have been relocated. - * The index should be removed and all associated resources released. Persistent parts of the index - * like the shards files, state and transaction logs are kept around in the case of a disaster recovery. - */ - NO_LONGER_ASSIGNED, - - /** - * The index is deleted. Persistent parts of the index like the shards files, state and transaction logs are removed once - * all resources are released. - */ - DELETED, - - /** - * The index has been closed. The index should be removed and all associated resources released. Persistent parts of the index - * like the shards files, state and transaction logs are kept around in the case of a disaster recovery. - */ - CLOSED, - - /** - * Something around index management has failed and the index should be removed. - * Persistent parts of the index like the shards files, state and transaction logs are kept around in the - * case of a disaster recovery. - */ - FAILURE, - - /** - * The index has been reopened. The index should be removed and all associated resources released. Persistent parts of the index - * like the shards files, state and transaction logs are kept around in the case of a disaster recovery. - */ - REOPENED, - - /** - * The index is closed as part of the node shutdown process. The index should be removed and all associated resources released. - * Persistent parts of the index like the shards files, state and transaction logs should be kept around in the case the node - * restarts. - */ - SHUTDOWN, - } } static class ShardCloseExecutor implements Executor { diff --git a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java index e3df891462483..1cc57469bc955 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java +++ b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java @@ -12,7 +12,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.plugins.IndexStorePlugin; import java.nio.file.Path; diff --git a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java index bc26c55539101..427e60a4b7144 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java +++ b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java @@ -43,8 +43,8 @@ import org.elasticsearch.index.shard.IndexShardState; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.IndicesService; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.indices.cluster.IndicesClusterStateService; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; diff --git a/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java b/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java index 2566babbd2020..73be788079c4f 100644 --- a/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/IndexStorePlugin.java @@ -19,7 +19,7 @@ import org.elasticsearch.index.engine.EngineException; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.shard.ShardPath; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.indices.recovery.RecoveryState; import java.io.IOException; diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 2a814a1a36489..d485b53e7e409 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -78,7 +78,7 @@ import org.elasticsearch.indices.ExecutorSelector; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.indices.breaker.CircuitBreakerService; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.script.FieldScript; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.aggregations.AggregationInitializationException; diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 18d303d40bed2..043b982ad4344 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -86,7 +86,7 @@ import org.elasticsearch.indices.analysis.AnalysisModule; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache; import org.elasticsearch.indices.recovery.RecoveryState; import org.elasticsearch.plugins.IndexStorePlugin; diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java index 619714119a05e..ad69ad162190f 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java @@ -26,7 +26,7 @@ import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.IndexShardTestCase; import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.indices.recovery.RecoveryState; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -34,7 +34,7 @@ import java.util.concurrent.atomic.AtomicInteger; import static java.util.Collections.emptySet; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.DELETED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.DELETED; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; public class IndicesLifecycleListenerSingleNodeTests extends ESSingleNodeTestCase { diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index 9112d4aec4492..967dd3d7626b7 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -65,7 +65,7 @@ import org.elasticsearch.index.shard.ShardPath; import org.elasticsearch.index.similarity.NonNegativeScoresSimilarity; import org.elasticsearch.indices.IndicesService.ShardDeletionCheckResult; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.plugins.EnginePlugin; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.Plugin; diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceSingleNodeTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceSingleNodeTests.java index 66b9e69aea906..9ef888da81596 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceSingleNodeTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceSingleNodeTests.java @@ -159,7 +159,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; -import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.DELETED; +import static org.elasticsearch.indices.cluster.IndexRemovalReason.DELETED; import static org.elasticsearch.search.SearchService.DEFAULT_SIZE; import static org.elasticsearch.search.SearchService.QUERY_PHASE_PARALLEL_COLLECTION_ENABLED; import static org.elasticsearch.search.SearchService.SEARCH_WORKER_THREADS_ENABLED; diff --git a/test/framework/src/main/java/org/elasticsearch/test/MockIndexEventListener.java b/test/framework/src/main/java/org/elasticsearch/test/MockIndexEventListener.java index ba5ffa49c33a0..10a31cbf3fae0 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/MockIndexEventListener.java +++ b/test/framework/src/main/java/org/elasticsearch/test/MockIndexEventListener.java @@ -21,7 +21,7 @@ import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.IndexShardState; import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.plugins.Plugin; import java.util.Arrays; diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java index c4eb4a40a02d1..9436f543624e8 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java @@ -22,7 +22,7 @@ import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.store.ByteSizeCachingDirectory; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots; import org.elasticsearch.xpack.searchablesnapshots.cache.common.CacheKey; diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java index 87d04845fbf48..e1074e9e0e907 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java @@ -13,7 +13,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.indices.cluster.IndexRemovalReason; import org.elasticsearch.plugins.IndexStorePlugin; import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots; import org.elasticsearch.xpack.searchablesnapshots.cache.common.CacheKey; From 5eabc0f634320f5442f77b81532caeb0a842352b Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 21 May 2025 15:12:56 +1000 Subject: [PATCH 19/25] Fix eviction logic --- .../SearchableSnapshotIndexEventListener.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java index 9436f543624e8..5a38035945f72 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java @@ -114,11 +114,16 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea ); } if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot() && sharedBlobCacheService != null) { - // Evict shards we know are not coming back asynchronously. Let any other shards expire. - if (reason == IndexRemovalReason.DELETED) { - sharedBlobCacheService.forceEvictAsync( + switch (reason) { + // Shards we know are not coming back - we can evict asynchronously + case DELETED -> sharedBlobCacheService.forceEvictAsync( SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) ); + // A failure occurred - we should eagerly clear the state + case FAILURE -> sharedBlobCacheService.forceEvict( + SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) + ); + // Any other reason - we let the cache entries expire naturally } } } From 53ad8776e4d0ba9e4a8b294ce81695a1055853ed Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 21 May 2025 15:14:29 +1000 Subject: [PATCH 20/25] Comment --- .../allocation/SearchableSnapshotIndexEventListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java index 5a38035945f72..3ae1cc1444a2f 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexEventListener.java @@ -115,7 +115,7 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea } if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot() && sharedBlobCacheService != null) { switch (reason) { - // Shards we know are not coming back - we can evict asynchronously + // This index was deleted, it's not coming back - we can evict asynchronously case DELETED -> sharedBlobCacheService.forceEvictAsync( SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings()) ); From fa17c6687c731ee3202623aba55ac7491a4680eb Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 21 May 2025 15:17:18 +1000 Subject: [PATCH 21/25] Fix eviction logic --- ...eSnapshotIndexFoldersDeletionListener.java | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java index e1074e9e0e907..e6891ff859248 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java @@ -83,12 +83,25 @@ private void markShardAsEvictedInCache(ShardId shardId, IndexSettings indexSetti shardId ); - // Only partial searchable snapshots use the shared blob cache. Only force-evict if we know the shard won't be coming back. - if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot() && indexRemovalReason == IndexRemovalReason.DELETED) { - final SharedBlobCacheService sharedBlobCacheService = - SearchableSnapshotIndexFoldersDeletionListener.this.frozenCacheServiceSupplier.get(); - assert sharedBlobCacheService != null : "frozen cache service not initialized"; - sharedBlobCacheService.forceEvictAsync(SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings())); + // Only partial searchable snapshots use the shared blob cache. + if (indexSettings.getIndexMetadata().isPartialSearchableSnapshot()) { + switch (indexRemovalReason) { + // The index was deleted, it's not coming back - we can evict asynchronously + case DELETED -> { + final SharedBlobCacheService sharedBlobCacheService = + SearchableSnapshotIndexFoldersDeletionListener.this.frozenCacheServiceSupplier.get(); + assert sharedBlobCacheService != null : "frozen cache service not initialized"; + sharedBlobCacheService.forceEvictAsync(SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings())); + } + // An error occurred - we should eagerly clear the state + case FAILURE -> { + final SharedBlobCacheService sharedBlobCacheService = + SearchableSnapshotIndexFoldersDeletionListener.this.frozenCacheServiceSupplier.get(); + assert sharedBlobCacheService != null : "frozen cache service not initialized"; + sharedBlobCacheService.forceEvict(SearchableSnapshots.forceEvictPredicate(shardId, indexSettings.getSettings())); + } + // Any other reason - we let the cache entries expire naturally + } } } } From 69e748fa0f49d18ae7f0bfe3529a2e652c2bba0a Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Wed, 21 May 2025 15:27:37 +1000 Subject: [PATCH 22/25] Improve change summary --- docs/changelog/126581.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/changelog/126581.yaml b/docs/changelog/126581.yaml index 3f72f7ba5a147..53fcb8a6057b3 100644 --- a/docs/changelog/126581.yaml +++ b/docs/changelog/126581.yaml @@ -1,5 +1,10 @@ pr: 126581 -summary: Evict from the shared blob cache asynchronously +summary: "Optimize shared blob cache evictions on shard removal + Shared blob cache evictions occur on the cluster applier thread when shards are + removed from a node. These can be expensive if a large number of shards are + being removed. This change uses the context of the removal to avoid unnecessary + evictions that might hold up the applier thread. + " area: Snapshot/Restore type: enhancement issues: [] From c2f3b0fd35a6d2e6443406a1410913fcefe07186 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Fri, 23 May 2025 17:15:26 +1000 Subject: [PATCH 23/25] Add tests --- .../shared/SharedCacheEvictionTests.java | 240 ++++++++++++++++++ .../SearchableSnapshots.java | 24 +- 2 files changed, 260 insertions(+), 4 deletions(-) create mode 100644 x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java new file mode 100644 index 0000000000000..9f4c1a10771d0 --- /dev/null +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java @@ -0,0 +1,240 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.searchablesnapshots.cache.shared; + +import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse; +import org.elasticsearch.blobcache.BlobCacheMetrics; +import org.elasticsearch.blobcache.shared.SharedBlobCacheService; +import org.elasticsearch.cluster.health.ClusterHealthStatus; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.NodeEnvironment; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.index.shard.ShardStateMetadata; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.SystemIndexPlugin; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin; +import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest; +import org.elasticsearch.xpack.searchablesnapshots.BaseFrozenSearchableSnapshotsIntegTestCase; +import org.elasticsearch.xpack.searchablesnapshots.LocalStateSearchableSnapshots; +import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots; +import org.elasticsearch.xpack.searchablesnapshots.cache.common.CacheKey; +import org.junit.Before; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; + +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.Matchers.anEmptyMap; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +public class SharedCacheEvictionTests extends BaseFrozenSearchableSnapshotsIntegTestCase { + + private static final Map> sharedBlobCacheServices = new ConcurrentHashMap<>(); + + @Override + protected Collection> nodePlugins() { + Collection> classes = super.nodePlugins(); + return Stream.concat( + classes.stream().filter(plugin -> plugin != LocalStateSearchableSnapshots.class), + Stream.of(SpyableSharedCacheSearchableSnapshots.class) + ).toList(); + } + + @SuppressWarnings("unchecked") + @Before + public void clearEvictionStats() { + sharedBlobCacheServices.values().forEach(Mockito::clearInvocations); + } + + public void testPartialShardsAreEvictedAsynchronouslyOnDelete() throws Exception { + final String mountedSnapshotName = "mounted_" + randomIdentifier(); + snapshotAndMount(mountedSnapshotName, MountSearchableSnapshotRequest.Storage.SHARED_CACHE); + + final Map allocations = getShardCounts(mountedSnapshotName); + + assertAcked(indicesAdmin().prepareDelete(mountedSnapshotName)); + allocations.forEach((nodeId, shardCount) -> { + SharedBlobCacheService sharedBlobCacheService = sharedBlobCacheServices.get(nodeId); + verify(sharedBlobCacheService, Mockito.atLeast(shardCount)).forceEvictAsync(ArgumentMatchers.any()); + verify(sharedBlobCacheService, never()).forceEvict(ArgumentMatchers.any()); + }); + } + + /** + * Fully mounted snapshots don't use the shared blob cache, so we don't need to evict them from it + */ + public void testFullFullShardsAreNotEvictedOnDelete() throws Exception { + final String mountedSnapshotName = "mounted_" + randomIdentifier(); + snapshotAndMount(mountedSnapshotName, MountSearchableSnapshotRequest.Storage.FULL_COPY); + + final Map allocations = getShardCounts(mountedSnapshotName); + + assertAcked(indicesAdmin().prepareDelete(mountedSnapshotName)); + allocations.forEach((nodeId, shardCount) -> { + SharedBlobCacheService sharedBlobCacheService = sharedBlobCacheServices.get(nodeId); + verify(sharedBlobCacheService, never()).forceEvictAsync(ArgumentMatchers.any()); + verify(sharedBlobCacheService, never()).forceEvict(ArgumentMatchers.any()); + }); + } + + /** + * We let relocated shards age out of the cache, rather than evicting them + */ + public void testPartialShardsAreNotEvictedOnRelocate() throws Exception { + final String mountedSnapshotName = "mounted_" + randomIdentifier(); + snapshotAndMount(mountedSnapshotName, MountSearchableSnapshotRequest.Storage.SHARED_CACHE); + + final Map allocations = getShardCounts(mountedSnapshotName); + + // Create another node to relocate to + assertThat(internalCluster().numDataNodes(), equalTo(1)); + internalCluster().startNode(); + ensureStableCluster(2); + + final String nodeToVacateId = randomFrom(allocations.keySet()); + updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._id", nodeToVacateId)); + try { + waitForRelocation(ClusterHealthStatus.GREEN); + assertThat(getShardCounts(mountedSnapshotName).keySet(), not(contains(nodeToVacateId))); + + final SharedBlobCacheService sharedBlobCacheService = sharedBlobCacheServices.get(nodeToVacateId); + verify(sharedBlobCacheService, never()).forceEvictAsync(ArgumentMatchers.any()); + verify(sharedBlobCacheService, never()).forceEvict(ArgumentMatchers.any()); + } finally { + updateClusterSettings(Settings.builder().putNull("cluster.routing.allocation.exclude._id")); + } + } + + public void testPartialShardsAreEvictedSynchronouslyOnFailure() throws Exception { + final String mountedSnapshotName = "mounted_" + randomIdentifier(); + + snapshotAndMount(mountedSnapshotName, MountSearchableSnapshotRequest.Storage.SHARED_CACHE); + + final IndicesStatsResponse indicesStatsResponse = indicesAdmin().prepareStats(mountedSnapshotName).get(); + final Set allShards = Arrays.stream(indicesStatsResponse.getIndex(mountedSnapshotName).getShards()) + .map(sh -> sh.getShardRouting().shardId()) + .collect(Collectors.toSet()); + final String indexUUID = indicesStatsResponse.getIndex(mountedSnapshotName).getUuid(); + + // Start another node to relocate to + assertThat(internalCluster().numDataNodes(), equalTo(1)); + final String newNodeName = internalCluster().startNode(); + ensureStableCluster(2); + + // Put some conflicting shard state in the new node's shard paths to trigger a failure + final NodeEnvironment nodeEnvironment = internalCluster().getInstance(NodeEnvironment.class, newNodeName); + for (ShardId shardId : allShards) { + for (Path p : nodeEnvironment.availableShardPaths(shardId)) { + ShardStateMetadata.FORMAT.write( + new ShardStateMetadata(true, randomValueOtherThan(indexUUID, ESTestCase::randomIdentifier), null), + p + ); + } + } + + // Force relocation, it should fail + final Map allocations = getShardCounts(mountedSnapshotName); + final String nodeToVacateId = randomFrom(allocations.keySet()); + updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._id", nodeToVacateId)); + + try { + waitForRelocation(); + + final SharedBlobCacheService sharedBlobCacheService = sharedBlobCacheServices.get(getNodeId(newNodeName)); + verify(sharedBlobCacheService, never()).forceEvictAsync(ArgumentMatchers.any()); + verify(sharedBlobCacheService, Mockito.atLeast(allShards.size())).forceEvict(ArgumentMatchers.any()); + } finally { + updateClusterSettings(Settings.builder().putNull("cluster.routing.allocation.exclude._id")); + } + } + + private Map getShardCounts(String indexName) { + final IndicesStatsResponse indicesStatsResponse = indicesAdmin().prepareStats(indexName).get(); + final Map allocations = new HashMap<>(); + Arrays.stream(indicesStatsResponse.getShards()) + .forEach(shardStats -> allocations.compute(shardStats.getShardRouting().currentNodeId(), (s, v) -> v == null ? 1 : v + 1)); + assertThat(allocations, not(anEmptyMap())); + return allocations; + } + + private void snapshotAndMount(String mountedSnapshotName, MountSearchableSnapshotRequest.Storage storage) throws Exception { + final String repositoryName = randomIdentifier(); + final String indexName = randomValueOtherThan(mountedSnapshotName, ESTestCase::randomIdentifier); + final String snapshotName = randomIdentifier(); + + createRepository(repositoryName, "fs"); + createIndexWithRandomDocs(indexName, 100); + createSnapshot(repositoryName, snapshotName, List.of(indexName)); + mountSnapshot(repositoryName, snapshotName, indexName, mountedSnapshotName, Settings.EMPTY, storage); + ensureGreen(mountedSnapshotName); + } + + public static class SpyableSharedCacheSearchableSnapshots extends LocalStateCompositeXPackPlugin implements SystemIndexPlugin { + + private final SearchableSnapshots plugin; + + public SpyableSharedCacheSearchableSnapshots(final Settings settings, final Path configPath) { + super(settings, configPath); + this.plugin = new SearchableSnapshots(settings) { + + @Override + protected XPackLicenseState getLicenseState() { + return SpyableSharedCacheSearchableSnapshots.this.getLicenseState(); + } + + @Override + protected SharedBlobCacheService createSharedBlobCacheService( + Settings settings, + ThreadPool threadPool, + NodeEnvironment nodeEnvironment, + BlobCacheMetrics blobCacheMetrics + ) { + final SharedBlobCacheService spy = Mockito.spy( + super.createSharedBlobCacheService(settings, threadPool, nodeEnvironment, blobCacheMetrics) + ); + sharedBlobCacheServices.put(nodeEnvironment.nodeId(), spy); + return spy; + } + }; + plugins.add(plugin); + } + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return plugin.getSystemIndexDescriptors(settings); + } + + @Override + public String getFeatureName() { + return plugin.getFeatureName(); + } + + @Override + public String getFeatureDescription() { + return plugin.getFeatureDescription(); + } + } +} diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java index 69c1530453f7f..cb4d0f5eeda37 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java @@ -323,12 +323,12 @@ public Collection createComponents(PluginServices services) { if (DiscoveryNode.canContainData(settings)) { final CacheService cacheService = new CacheService(settings, clusterService, threadPool, new PersistentCache(nodeEnvironment)); this.cacheService.set(cacheService); - final SharedBlobCacheService sharedBlobCacheService = new SharedBlobCacheService<>( - nodeEnvironment, + final BlobCacheMetrics blobCacheMetrics = new BlobCacheMetrics(services.telemetryProvider().getMeterRegistry()); + final SharedBlobCacheService sharedBlobCacheService = createSharedBlobCacheService( settings, threadPool, - threadPool.executor(SearchableSnapshots.CACHE_FETCH_ASYNC_THREAD_POOL_NAME), - new BlobCacheMetrics(services.telemetryProvider().getMeterRegistry()) + nodeEnvironment, + blobCacheMetrics ); this.frozenCacheService.set(sharedBlobCacheService); components.add(cacheService); @@ -367,6 +367,22 @@ public Collection createComponents(PluginServices services) { return Collections.unmodifiableList(components); } + // overridable for testing + protected SharedBlobCacheService createSharedBlobCacheService( + final Settings settings, + final ThreadPool threadPool, + final NodeEnvironment nodeEnvironment, + final BlobCacheMetrics blobCacheMetrics + ) { + return new SharedBlobCacheService<>( + nodeEnvironment, + settings, + threadPool, + threadPool.executor(SearchableSnapshots.CACHE_FETCH_ASYNC_THREAD_POOL_NAME), + blobCacheMetrics + ); + } + @Override public void onIndexModule(IndexModule indexModule) { if (indexModule.indexSettings().getIndexMetadata().isSearchableSnapshot()) { From 63f4b1feaf90cd4cd2edd727197fe0b32b1aa4a3 Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Fri, 23 May 2025 17:26:26 +1000 Subject: [PATCH 24/25] Work with any number of nodes --- .../shared/SharedCacheEvictionTests.java | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java index 9f4c1a10771d0..0a5f94fe9ecf3 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java @@ -46,7 +46,6 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -109,10 +108,11 @@ public void testPartialShardsAreNotEvictedOnRelocate() throws Exception { final Map allocations = getShardCounts(mountedSnapshotName); - // Create another node to relocate to - assertThat(internalCluster().numDataNodes(), equalTo(1)); - internalCluster().startNode(); - ensureStableCluster(2); + // Create another node to relocate to if we need to + if (internalCluster().numDataNodes() == 1) { + internalCluster().startNode(); + ensureStableCluster(2); + } final String nodeToVacateId = randomFrom(allocations.keySet()); updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._id", nodeToVacateId)); @@ -139,13 +139,11 @@ public void testPartialShardsAreEvictedSynchronouslyOnFailure() throws Exception .collect(Collectors.toSet()); final String indexUUID = indicesStatsResponse.getIndex(mountedSnapshotName).getUuid(); - // Start another node to relocate to - assertThat(internalCluster().numDataNodes(), equalTo(1)); - final String newNodeName = internalCluster().startNode(); - ensureStableCluster(2); + // Add a node to the cluster, we'll force relocation to it + final String targetNode = internalCluster().startNode(); // Put some conflicting shard state in the new node's shard paths to trigger a failure - final NodeEnvironment nodeEnvironment = internalCluster().getInstance(NodeEnvironment.class, newNodeName); + final NodeEnvironment nodeEnvironment = internalCluster().getInstance(NodeEnvironment.class, targetNode); for (ShardId shardId : allShards) { for (Path p : nodeEnvironment.availableShardPaths(shardId)) { ShardStateMetadata.FORMAT.write( @@ -156,18 +154,16 @@ public void testPartialShardsAreEvictedSynchronouslyOnFailure() throws Exception } // Force relocation, it should fail - final Map allocations = getShardCounts(mountedSnapshotName); - final String nodeToVacateId = randomFrom(allocations.keySet()); - updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._id", nodeToVacateId)); + updateClusterSettings(Settings.builder().put("cluster.routing.allocation.require._name", targetNode)); try { waitForRelocation(); - final SharedBlobCacheService sharedBlobCacheService = sharedBlobCacheServices.get(getNodeId(newNodeName)); + final SharedBlobCacheService sharedBlobCacheService = sharedBlobCacheServices.get(getNodeId(targetNode)); verify(sharedBlobCacheService, never()).forceEvictAsync(ArgumentMatchers.any()); verify(sharedBlobCacheService, Mockito.atLeast(allShards.size())).forceEvict(ArgumentMatchers.any()); } finally { - updateClusterSettings(Settings.builder().putNull("cluster.routing.allocation.exclude._id")); + updateClusterSettings(Settings.builder().putNull("cluster.routing.allocation.require._name")); } } From cd09f7d7145d452cb43100b9fcd34b7952bb1a8e Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Fri, 23 May 2025 17:33:46 +1000 Subject: [PATCH 25/25] Randomise number of docs --- .../cache/shared/SharedCacheEvictionTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java index 0a5f94fe9ecf3..b0f386bd78a2f 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedCacheEvictionTests.java @@ -182,7 +182,7 @@ private void snapshotAndMount(String mountedSnapshotName, MountSearchableSnapsho final String snapshotName = randomIdentifier(); createRepository(repositoryName, "fs"); - createIndexWithRandomDocs(indexName, 100); + createIndexWithRandomDocs(indexName, randomIntBetween(10, 300)); createSnapshot(repositoryName, snapshotName, List.of(indexName)); mountSnapshot(repositoryName, snapshotName, indexName, mountedSnapshotName, Settings.EMPTY, storage); ensureGreen(mountedSnapshotName);