Optimise shared-blob-cache evictions (elastic#128539)

* Optimise shared-blob-cache evictions (elastic#126581)

Closes: ES-10744

Co-authored-by: Tanguy Leroux <[email protected]>

(cherry picked from commit 83fe2ed)

* Update docs/changelog/128539.yaml

* Fix changelogs

* Delete docs/changelog/128539.yaml

* Restore original changelog

Author: nicktindall

docs/changelog/126581.yaml

@@ -0,0 +1,10 @@
+pr: 126581
+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: []

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.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<IndexFoldersDeletionListener> 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 reason
+                ) {
                     deletedIndices.add(index);
                 }
 
                 @Override
-                public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths) {
+                public void beforeShardFoldersDeleted(
+                    ShardId shardId,
+                    IndexSettings indexSettings,
+                    Path[] shardPaths,
+                    IndexRemovalReason reason
+                ) {
                     deletedShards.computeIfAbsent(shardId.getIndex(), i -> Collections.synchronizedList(new ArrayList<>())).add(shardId);
                 }
             });

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java

@@ -47,7 +47,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

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

@@ -82,7 +82,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

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;

server/src/main/java/org/elasticsearch/index/IndexService.java

@@ -81,6 +81,7 @@
 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.fielddata.cache.IndicesFieldDataCache;
 import org.elasticsearch.indices.recovery.RecoveryState;
@@ -485,7 +486,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) {
@@ -693,11 +699,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);
             }
         }
@@ -1043,9 +1049,9 @@ public static Function<String, String> dateMathExpressionResolverAt(long instant
     }
 
     public interface ShardStoreDeleter {
-        void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings) throws IOException;
+        void deleteShardStore(String reasonText, ShardLock lock, IndexSettings indexSettings, IndexRemovalReason reason) throws IOException;
 
-        void addPendingDelete(ShardId shardId, IndexSettings indexSettings);
+        void addPendingDelete(ShardId shardId, IndexSettings indexSettings, IndexRemovalReason reason);
     }
 
     public final EngineFactory getEngineFactory() {

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

server/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -128,6 +128,7 @@
 import org.elasticsearch.index.shard.SearchOperationListener;
 import org.elasticsearch.index.shard.ShardId;
 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;
@@ -966,7 +967,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);
                 }
             }));
         });
@@ -1038,7 +1039,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);
             }
@@ -1051,7 +1052,7 @@ public void deleteUnassignedIndex(String reason, IndexMetadata oldIndexMetadata,
      *
      * Package private for testing
      */
-    void deleteIndexStore(String reason, IndexMetadata metadata) throws IOException {
+    void deleteIndexStore(String reasonText, IndexMetadata metadata, IndexRemovalReason reason) throws IOException {
         if (nodeEnv.hasNodeFile()) {
             synchronized (this) {
                 Index index = metadata.getIndex();
@@ -1069,33 +1070,35 @@ void deleteIndexStore(String reason, IndexMetadata metadata) throws IOException
                 }
             }
             final IndexSettings indexSettings = buildIndexSettings(metadata);
-            deleteIndexStore(reason, indexSettings.getIndex(), indexSettings);
+            deleteIndexStore(reasonText, indexSettings.getIndex(), indexSettings, reason);
         }
     }
 
-    private void deleteIndexStore(String reason, Index index, IndexSettings indexSettings) throws IOException {
-        deleteIndexStoreIfDeletionAllowed(reason, index, indexSettings, DEFAULT_INDEX_DELETION_PREDICATE);
+    private void deleteIndexStore(String reasonText, Index index, IndexSettings indexSettings, IndexRemovalReason reason)
+        throws IOException {
+        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 IndexDeletionAllowedPredicate predicate,
+        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)
+                    paths -> indexFoldersDeletionListeners.beforeIndexFoldersDeleted(index, indexSettings, paths, reason)
                 );
             }
             success = true;
@@ -1105,7 +1108,7 @@ private void deleteIndexStoreIfDeletionAllowed(
             logger.warn(() -> format("%s failed to delete index", index), ex);
         } finally {
             if (success == false) {
-                addPendingDelete(index, indexSettings);
+                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.
@@ -1115,19 +1118,21 @@ private void deleteIndexStoreIfDeletionAllowed(
 
     /**
      * Deletes the shard with an already acquired shard lock.
-     * @param reason 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.
+     * @param reason the reason for the deletion (as an enum)
      * @throws IOException if an IOException occurs
      */
     @Override
-    public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexSettings) throws IOException {
+    public void deleteShardStore(String reasonText, 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, reasonText);
         nodeEnv.deleteShardDirectoryUnderLock(
             lock,
             indexSettings,
-            paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths)
+            paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths, reason)
         );
     }
 
@@ -1139,13 +1144,14 @@ public void deleteShardStore(String reason, ShardLock lock, IndexSettings indexS
      * 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) throws IOException,
-        ShardLockObtainFailedException {
+    public void deleteShardStore(String reasonText, ShardId shardId, ClusterState clusterState, IndexRemovalReason reason)
+        throws IOException, ShardLockObtainFailedException {
         final IndexMetadata metadata = clusterState.getMetadata().indices().get(shardId.getIndexName());
 
         final IndexSettings indexSettings = buildIndexSettings(metadata);
@@ -1156,15 +1162,15 @@ 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, 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);
+                    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);
@@ -1220,7 +1226,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
@@ -1278,22 +1284,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);
     }
 
@@ -1309,25 +1315,28 @@ private static final class PendingDelete implements Comparable<PendingDelete> {
         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
@@ -1391,7 +1400,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) {
@@ -1403,7 +1417,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);