Skip unnecessary loading of IndexMetadata during snapshot deletion

server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java

@@ -23,6 +23,8 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.common.UUIDs.RANDOM_BASED_UUID_STRING_LENGTH;
+
 /**
  * Tracks the blob uuids of blobs containing {@link IndexMetadata} for snapshots as well an identifier for each of these blobs.
  * Before writing a new {@link IndexMetadata} blob during snapshot finalization in
@@ -193,4 +195,20 @@ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) {
             + "-"
             + indexMetaData.getAliasesVersion();
     }
+
+    /**
+     * Generates a map of blob id to Index UUID. This is a reverse lookup of {@link #identifiers}
+     * @return A map of blob id to index UUID
+     */
+    public Map<String, String> getBlobIdToIndexUuidMap() {
+        return identifiers.entrySet()
+            .stream()
+            .collect(
+                Collectors.toUnmodifiableMap(
+                    Map.Entry::getValue,
+                    // Parses the index UUID from the beginning of the unique index metadata identifier
+                    entry -> entry.getKey().substring(0, RANDOM_BASED_UUID_STRING_LENGTH)
+                )
+            );
+    }
 }

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

@@ -1125,6 +1125,11 @@ class SnapshotsDeletion {
          */
         private final ShardBlobsToDelete shardBlobsToDelete = new ShardBlobsToDelete();
 
+        /**
+         * A map of blob id to index UUID
+         */
+        private final Map<String, String> blobIdToIndexUuidMap;
+
         SnapshotsDeletion(
             Collection<SnapshotId> snapshotIds,
             long originalRepositoryDataGeneration,
@@ -1140,6 +1145,7 @@ class SnapshotsDeletion {
             this.originalRootBlobs = originalRootBlobs;
             this.originalIndexContainers = originalIndexContainers;
             this.originalRepositoryData = originalRepositoryData;
+            this.blobIdToIndexUuidMap = originalRepositoryData.indexMetaDataGenerations().getBlobIdToIndexUuidMap();
         }
 
         // ---------------------------------------------------------------------------------------------------------------------------------
@@ -1288,6 +1294,7 @@ private class IndexSnapshotsDeletion {
             private final IndexId indexId;
             private final Set<SnapshotId> snapshotsWithIndex;
             private final BlobContainer indexContainer;
+            private final Set<String> indexUUIDs = new HashSet<>();
 
             IndexSnapshotsDeletion(IndexId indexId) {
                 this.indexId = indexId;
@@ -1311,37 +1318,46 @@ void run(ActionListener<Void> listener) {
 
             private void determineShardCount(ActionListener<Void> listener) {
                 try (var listeners = new RefCountingListener(listener)) {
-                    for (final var indexMetaGeneration : snapshotIds.stream()
+                    for (final var blobId : snapshotIds.stream()
                         .filter(snapshotsWithIndex::contains)
                         .map(id -> originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(id, indexId))
                         .collect(Collectors.toSet())) {
                         // NB since 7.9.0 we deduplicate index metadata blobs, and one of the components of the deduplication key is the
                         // index UUID; the shard count is going to be the same for all metadata with the same index UUID, so it is
                         // unnecessary to read multiple metadata blobs corresponding to the same index UUID.
-                        // TODO Skip this unnecessary work? Maybe track the shard count in RepositoryData?
-                        snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> getOneShardCount(indexMetaGeneration)));
+                        // NB if the index metadata blob is in the pre-7.9.0 format then this will return null
+                        String indexUUID = blobIdToIndexUuidMap.get(blobId);
+
+                        // Without an index UUID, we don't know if we've encountered this index before and must read its IndexMetadata
+                        // from heap. If this is a new index UUID, it could have a higher shard count, so we also need to read
+                        // its IndexMetadata from heap
+                        if (indexUUID == null || indexUUIDs.add(indexUUID)) {
+                            snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> {
+                                try {
+                                    IndexMetadata indexMetadata = INDEX_METADATA_FORMAT.read(
+                                        getProjectRepo(),
+                                        indexContainer,
+                                        blobId,
+                                        namedXContentRegistry
+                                    );
+                                    updateShardCount(indexMetadata.getNumberOfShards());
+                                } catch (Exception ex) {
+                                    logger.warn(() -> format("[%s] [%s] failed to read metadata for index", blobId, indexId.getName()), ex);
+                                    // Definitely indicates something fairly badly wrong with the repo, but not immediately fatal here: we
+                                    // might get the shard count from another metadata blob, or we might just not process these shards.
+                                    // If we skip these shards then the repository will technically enter an invalid state
+                                    // (these shards' index-XXX blobs will refer to snapshots that no longer exist) and may contain dangling
+                                    // blobs too. A subsequent delete that hits this index may repair the state if the metadata read error
+                                    // is transient, but if not then the stale indices cleanup will eventually remove this index and all its
+                                    // extra data anyway.
+                                    // TODO: Should we fail the delete here? See https://github.com/elastic/elasticsearch/issues/100569.
+                                }
+                            }));
+                        }
                     }
                 }
             }
 
-            private void getOneShardCount(String indexMetaGeneration) {
-                try {
-                    updateShardCount(
-                        INDEX_METADATA_FORMAT.read(getProjectRepo(), indexContainer, indexMetaGeneration, namedXContentRegistry)
-                            .getNumberOfShards()
-                    );
-                } catch (Exception ex) {
-                    logger.warn(() -> format("[%s] [%s] failed to read metadata for index", indexMetaGeneration, indexId.getName()), ex);
-                    // Definitely indicates something fairly badly wrong with the repo, but not immediately fatal here: we might get the
-                    // shard count from another metadata blob, or we might just not process these shards. If we skip these shards then the
-                    // repository will technically enter an invalid state (these shards' index-XXX blobs will refer to snapshots that no
-                    // longer exist) and may contain dangling blobs too. A subsequent delete that hits this index may repair the state if
-                    // the metadata read error is transient, but if not then the stale indices cleanup will eventually remove this index
-                    // and all its extra data anyway.
-                    // TODO: Should we fail the delete here? See https://github.com/elastic/elasticsearch/issues/100569.
-                }
-            }
-
             private void processShards(ActionListener<Void> listener) {
                 final Set<SnapshotId> survivingSnapshots = snapshotsWithIndex.stream()
                     .filter(id -> snapshotIds.contains(id) == false)

server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java

@@ -0,0 +1,132 @@
+/*
+ * 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.repositories;
+
+import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.common.UUIDs;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.snapshots.SnapshotId;
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class IndexMetaDataGenerationsTests extends ESTestCase {
+
+    public void testIndexMetaDataGenerations() {
+        Map<String, String> identifiers = new HashMap<>();
+        Map<IndexId, String> lookupInternal = new HashMap<>();
+
+        int numberOfMetadataIdentifiers = randomIntBetween(5, 10);
+        for (int i = 0; i < numberOfMetadataIdentifiers; i++) {
+            String indexUUID = generateUUID();
+            String metaIdentifier = generateMetaIdentifier(indexUUID);
+            String blobUUID = randomAlphanumericOfLength(randomIntBetween(5, 10));
+            identifiers.put(metaIdentifier, blobUUID);
+
+            IndexId indexId = new IndexId(randomAlphanumericOfLength(10), indexUUID);
+            lookupInternal.put(indexId, metaIdentifier);
+        }
+
+        SnapshotId snapshotId = new SnapshotId(randomAlphanumericOfLength(10), randomUUID());
+        Map<SnapshotId, Map<IndexId, String>> lookup = Map.of(snapshotId, lookupInternal);
+
+        IndexMetaDataGenerations generations = new IndexMetaDataGenerations(lookup, identifiers);
+
+        assertEquals(lookup, generations.lookup);
+        assertEquals(identifiers, generations.identifiers);
+    }
+
+    public void testBuildUniqueIdentifierWithAllFieldsPresent() {
+        String indexUUID = generateUUID();
+        String historyUUID = randomAlphanumericOfLength(randomIntBetween(10, 64));
+        long settingsVersion = randomLong();
+        long mappingVersion = randomLong();
+        long aliasesVersion = randomLong();
+
+        IndexMetadata indexMetadata = createIndexMetadata(indexUUID, historyUUID, settingsVersion, mappingVersion, aliasesVersion);
+
+        String result = IndexMetaDataGenerations.buildUniqueIdentifier(indexMetadata);
+        assertEquals(indexUUID + "-" + historyUUID + "-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion, result);
+    }
+
+    public void testBuildUniqueIdentifierWithMissingHistoryUUID() {
+        String indexUUID = generateUUID();
+        long settingsVersion = randomLong();
+        long mappingVersion = randomLong();
+        long aliasesVersion = randomLong();
+
+        IndexMetadata indexMetadata = createIndexMetadata(indexUUID, null, settingsVersion, mappingVersion, aliasesVersion);
+
+        String result = IndexMetaDataGenerations.buildUniqueIdentifier(indexMetadata);
+        assertEquals(indexUUID + "-_na_-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion, result);
+    }
+
+    public void testGetBlobIdToIndexUuidMap() {
+        String indexUUID = generateUUID();
+        String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10));
+        long settingsVersion = randomNonNegativeLong();
+        long mappingsVersion = randomNonNegativeLong();
+        long aliasesVersion = randomNonNegativeLong();
+        String uniqueIdentifier = indexUUID + "-" + randomSetting + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion;
+        String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10));
+
+        // Creates the lookup map
+        SnapshotId snapshotId = new SnapshotId("snapshot", randomUUID());
+        IndexId indexId = new IndexId("index", indexUUID);
+        Map<SnapshotId, Map<IndexId, String>> lookup = Map.of(snapshotId, Map.of(indexId, uniqueIdentifier));
+
+        IndexMetaDataGenerations generations = new IndexMetaDataGenerations(lookup, Map.of(uniqueIdentifier, blobId));
+
+        Map<String, String> expectedBlobIdToindexUuidMap = Map.of(blobId, indexUUID);
+        assertEquals(expectedBlobIdToindexUuidMap, generations.getBlobIdToIndexUuidMap());
+    }
+
+    public void testGetBlobIdToIndexUuidMapWithNoIdentifierMap() {
+        IndexMetaDataGenerations generations = new IndexMetaDataGenerations(Map.of(), Map.of());
+        assertEquals(Collections.emptyMap(), generations.getBlobIdToIndexUuidMap());
+    }
+
+    private String generateUUID() {
+        return UUIDs.randomBase64UUID(random());
+    }
+
+    private String generateMetaIdentifier(String indexUUID) {
+        String historyUUID = generateUUID();
+        long settingsVersion = randomLong();
+        long mappingVersion = randomLong();
+        long aliasesVersion = randomLong();
+        return indexUUID + "-" + historyUUID + "-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion;
+    }
+
+    private IndexMetadata createIndexMetadata(
+        String indexUUID,
+        String historyUUID,
+        long settingsVersion,
+        long mappingVersion,
+        long aliasesVersion
+    ) {
+        IndexMetadata indexMetadata = mock(IndexMetadata.class);
+        Settings.Builder settingsBuilder = Settings.builder();
+        if (historyUUID != null) {
+            settingsBuilder.put(IndexMetadata.SETTING_HISTORY_UUID, historyUUID);
+        }
+        when(indexMetadata.getIndexUUID()).thenReturn(indexUUID);
+        when(indexMetadata.getSettings()).thenReturn(settingsBuilder.build());
+        when(indexMetadata.getSettingsVersion()).thenReturn(settingsVersion);
+        when(indexMetadata.getMappingVersion()).thenReturn(mappingVersion);
+        when(indexMetadata.getAliasesVersion()).thenReturn(aliasesVersion);
+        return indexMetadata;
+    }
+}