From fa08233d2f37a98db0bfc8fa38e8755ca0decfe2 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 10 Sep 2025 10:34:49 +0100 Subject: [PATCH 01/17] Skip unnecessary loading of `IndexMetadata` during snapshot deletion During snapshot deletion we load the metadata for an index into heap purely to calculate the shard count. Since the number of primary shards does not change for an index, we can store this to avoid recomputation, and avoid repeatedly loading large metadata objects into heap which can cause small nodes to OOMe. Closes ES-12539 --- .../IndexMetaDataGenerations.java | 28 ++- .../blobstore/BlobStoreRepository.java | 79 ++++--- .../IndexMetaDataGenerationsTests.java | 71 +++++++ ...ryShardCountComputedOncePerIndexTests.java | 192 ++++++++++++++++++ 4 files changed, 333 insertions(+), 37 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java create mode 100644 server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 0d697dc789de3..f805b7b9bd0d6 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -174,6 +174,9 @@ public String toString() { return "IndexMetaDataGenerations{lookup:" + lookup + "}{identifier:" + identifiers + "}"; } + // Cannot use - as a delimiter because getIndexUUID() produces UUIDs with - in them + private static final String DELIMITER = "/"; + /** * Compute identifier for {@link IndexMetadata} from its index- and history-uuid as well as its settings-, mapping- and alias-version. * If an index did not see a change in its settings, mappings or aliases between two points in time then the identifier will not change @@ -183,14 +186,21 @@ public String toString() { * @return identifier string */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { - return indexMetaData.getIndexUUID() - + "-" - + indexMetaData.getSettings().get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) - + "-" - + indexMetaData.getSettingsVersion() - + "-" - + indexMetaData.getMappingVersion() - + "-" - + indexMetaData.getAliasesVersion(); + return indexMetaData.getIndexUUID() + DELIMITER + indexMetaData.getSettings() + .get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + DELIMITER + indexMetaData.getSettingsVersion() + + DELIMITER + indexMetaData.getMappingVersion() + DELIMITER + indexMetaData.getAliasesVersion(); + } + + /** + * Parses the unique IndexMetadata ID generated by the {@code buildUniqueIdentifier} function and returns the UUID prefix. + * If a null uniqueIdentifier is given, then an empty string is returned + * @param uniqueIdentifier - The ID to parse the UUID from + * @return The UUID of the IndexMetadata object. + */ + public static String parseUUIDFromUniqueIdentifier(String uniqueIdentifier) { + if (uniqueIdentifier == null) { + return ""; + } + return uniqueIdentifier.split(DELIMITER, 2)[0]; } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index f4275801fff1e..4e5d73f9030fc 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -164,6 +164,7 @@ import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executor; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -183,6 +184,7 @@ import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo; import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo.canonicalName; import static org.elasticsearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING; +import static org.elasticsearch.repositories.IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier; import static org.elasticsearch.repositories.ProjectRepo.projectRepoString; /** @@ -497,6 +499,11 @@ public static String getRepositoryDataBlobName(long repositoryGeneration) { private final ThrottledTaskRunner staleBlobDeleteRunner; + /** + * Maps the Index UUID to its shard count + */ + private final ConcurrentMap indexUUIDToShardCountMap = new ConcurrentHashMap<>(); + /** * Constructs new BlobStoreRepository * @param metadata The metadata for this repository including name and settings @@ -1027,7 +1034,7 @@ private void createSnapshotsDeletion( * blob must not change until it is updated by this deletion and the {@code repositoryDataUpdateListener} is completed. *

*/ - class SnapshotsDeletion { + protected class SnapshotsDeletion { /** * The IDs of the snapshots to delete. This collection is empty if the deletion is a repository cleanup. @@ -1284,37 +1291,51 @@ void run(ActionListener listener) { private void determineShardCount(ActionListener listener) { try (var listeners = new RefCountingListener(listener)) { - for (final var indexMetaGeneration : 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))); + for (SnapshotId snapshotId : snapshotIds.stream().filter(snapshotsWithIndex::contains).collect(Collectors.toSet())) { + snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> { + String blobId = originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(snapshotId, indexId); + // The unique IndexMetadata ID prefixed by Index UUID + String indexMetadataId = originalRepositoryData.indexMetaDataGenerations() + .snapshotIndexMetadataIdentifier(snapshotId, indexId); + + // Guarantees that the indexUUID will not be "" since this would map multiple indexMetaData objects to the + // same shard count + assert indexMetadataId != null; + String indexUUID = parseUUIDFromUniqueIdentifier(indexMetadataId); + + if (indexUUIDToShardCountMap.containsKey(indexUUID) == false) { + try { + IndexMetadata indexMetadata = INDEX_METADATA_FORMAT.read( + getProjectRepo(), + indexContainer, + blobId, + namedXContentRegistry + ); + int numberOfShards = indexMetadata.getNumberOfShards(); + indexUUIDToShardCountMap.put(indexUUID, numberOfShards); + updateShardCount(numberOfShards); + } 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. + } + } else { + // indexUUIDToShardCountMap is shared across all threads. Therefore, while there may be an entry for this + // UUID, there is no guarantee that we've encountered it in this thread, so we update using the precomputed + // value, thus removing the unnecessary INDEX_METADATA_FORMAT.read call. + updateShardCount(indexUUIDToShardCountMap.get(indexUUID)); + } + })); } } } - 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 listener) { final Set survivingSnapshots = snapshotsWithIndex.stream() .filter(id -> snapshotIds.contains(id) == false) @@ -1853,6 +1874,8 @@ record RootBlobUpdateResult(RepositoryData oldRepositoryData, RepositoryData new // We don't yet have this version of the metadata so we write it metaUUID = UUIDs.base64UUID(); INDEX_METADATA_FORMAT.write(indexMetaData, indexContainer(index), metaUUID, compress); + // TODO - Could store the shard counts here in a map, or in the repo data, rather than loading + // the index metadata to heap to calculate metadataWriteResult.indexMetaIdentifiers().put(identifiers, metaUUID); } // else this task was largely a no-op - TODO no need to fork in that case metadataWriteResult.indexMetas().put(index, identifiers); diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java new file mode 100644 index 0000000000000..453801f648da5 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -0,0 +1,71 @@ +/* + * 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.settings.Settings; +import org.elasticsearch.test.ESTestCase; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class IndexMetaDataGenerationsTests extends ESTestCase { + + public void testBuildUniqueIdentifierWithAllFieldsPresent() { + String indexUUID = randomAlphanumericOfLength(randomIntBetween(10, 64)); + String historyUUID = randomAlphanumericOfLength(randomIntBetween(10, 64)); + long settingsVersion = randomLong(); + long mappingVersion = randomLong(); + long aliasesVersion = randomLong(); + + IndexMetadata indexMetadata = mock(IndexMetadata.class); + when(indexMetadata.getIndexUUID()).thenReturn(indexUUID); + when(indexMetadata.getSettings()).thenReturn(Settings.builder().put(IndexMetadata.SETTING_HISTORY_UUID, historyUUID).build()); + when(indexMetadata.getSettingsVersion()).thenReturn(settingsVersion); + when(indexMetadata.getMappingVersion()).thenReturn(mappingVersion); + when(indexMetadata.getAliasesVersion()).thenReturn(aliasesVersion); + + String result = IndexMetaDataGenerations.buildUniqueIdentifier(indexMetadata); + assertEquals(indexUUID + "/" + historyUUID + "/" + settingsVersion + "/" + mappingVersion + "/" + aliasesVersion, result); + + // Then test parseUUIDFromUniqueIdentifier + assertEquals(indexUUID, IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier(result)); + } + + public void testBuildUniqueIdentifierWithMissingHistoryUUID() { + String indexUUID = randomAlphanumericOfLength(randomIntBetween(10, 64)); + long settingsVersion = randomLong(); + long mappingVersion = randomLong(); + long aliasesVersion = randomLong(); + + IndexMetadata indexMetadata = mock(IndexMetadata.class); + when(indexMetadata.getIndexUUID()).thenReturn(indexUUID); + when(indexMetadata.getSettings()).thenReturn(Settings.builder().build()); + when(indexMetadata.getSettingsVersion()).thenReturn(settingsVersion); + when(indexMetadata.getMappingVersion()).thenReturn(mappingVersion); + when(indexMetadata.getAliasesVersion()).thenReturn(aliasesVersion); + + String result = IndexMetaDataGenerations.buildUniqueIdentifier(indexMetadata); + assertEquals(indexUUID + "/_na_/" + settingsVersion + "/" + mappingVersion + "/" + aliasesVersion, result); + } + + public void testParseUUIDFromUniqueIdentifierWithNullInput() { + assertEquals("", IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier(null)); + } + + public void testParseUUIDFromUniqueIdentifierWithEmptyString() { + assertEquals("", IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier("")); + } + + public void testParseUUIDFromUniqueIdentifierWithoutDelimiter() { + String uuid = randomAlphanumericOfLength(randomIntBetween(10, 64)); + assertEquals(uuid, IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier(uuid)); + } +} diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java new file mode 100644 index 0000000000000..694a2682968a2 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -0,0 +1,192 @@ +/* + * 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.blobstore; + +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.blobstore.BlobContainer; +import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.OperationPurpose; +import org.elasticsearch.common.blobstore.support.FilterBlobContainer; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.env.Environment; +import org.elasticsearch.indices.recovery.RecoverySettings; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.RepositoryPlugin; +import org.elasticsearch.repositories.RepositoriesMetrics; +import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.SnapshotMetrics; +import org.elasticsearch.repositories.fs.FsRepository; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.ESSingleNodeTestCase; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.junit.Before; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; + +/** + * This test ensures that we only load each IndexMetaData object into memory once and then store the shard count result (ES-12539) + */ +public class BlobStoreRepositoryShardCountComputedOncePerIndexTests extends ESSingleNodeTestCase { + + private static final String TEST_REPO_TYPE = "shard-count-computed-once-fs"; + private static final String TEST_REPO_NAME = "test-repo"; + private static AtomicInteger INDEX_LOADED_COUNT; + + @Before + public void setUp() throws Exception { + super.setUp(); + INDEX_LOADED_COUNT = new AtomicInteger(); + } + + protected Collection> getPlugins() { + return List.of(ShardCountComputedOncePerIndexFsRepositoryPlugin.class); + } + + public static class ShardCountComputedOncePerIndexFsRepositoryPlugin extends Plugin implements RepositoryPlugin { + @Override + public Map getRepositories( + Environment env, + NamedXContentRegistry namedXContentRegistry, + ClusterService clusterService, + BigArrays bigArrays, + RecoverySettings recoverySettings, + RepositoriesMetrics repositoriesMetrics, + SnapshotMetrics snapshotMetrics + ) { + return Collections.singletonMap( + TEST_REPO_TYPE, + (projectId, metadata) -> new FsRepository( + projectId, + metadata, + env, + namedXContentRegistry, + clusterService, + bigArrays, + recoverySettings + ) { + @Override + protected BlobStore createBlobStore() throws Exception { + return new ShardCountComputedOncePerIndexBlobStore(super.createBlobStore()); + } + } + ); + } + } + + private static class ShardCountComputedOncePerIndexBlobStore implements BlobStore { + private final BlobStore delegate; + + private ShardCountComputedOncePerIndexBlobStore(BlobStore delegate) { + this.delegate = delegate; + } + + @Override + public BlobContainer blobContainer(BlobPath path) { + return new ShardCountComputedOncePerIndexBlobContainer(delegate.blobContainer(path)); + } + + @Override + public void close() throws IOException { + delegate.close(); + } + } + + private static class ShardCountComputedOncePerIndexBlobContainer extends FilterBlobContainer { + ShardCountComputedOncePerIndexBlobContainer(BlobContainer delegate) { + super(delegate); + } + + @Override + protected BlobContainer wrapChild(BlobContainer child) { + return new ShardCountComputedOncePerIndexBlobContainer(child); + } + + @Override + public InputStream readBlob(OperationPurpose purpose, String blobName) throws IOException { + final var pathParts = path().parts(); + // Increment the count only when an index metadata is loaded into heap + if (pathParts.size() == 2 + && pathParts.getFirst().equals("indices") + && blobName.startsWith(BlobStoreRepository.METADATA_PREFIX)) { + INDEX_LOADED_COUNT.incrementAndGet(); + } + + return super.readBlob(purpose, blobName); + } + } + + /* + This test generates N indices, and for each has M snapshots. + We're testing that for each of the N indices, it's metadata is only loaded into heap once + */ + public void testShardCountComputedOncePerIndex() { + final var repoPath = ESIntegTestCase.randomRepoPath(node().settings()); + + int numberOfIndices = randomIntBetween(3, 10); + for (int i = 0; i < numberOfIndices; i++) { + String indexName = "index-" + i; + createIndex(indexName, indexSettings(between(1, 3), 0).build()); + ensureGreen(indexName); + } + + // Set up the repository contents, including snapshots, using a regular 'fs' repo + + assertAcked( + client().admin() + .cluster() + .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) + .setType(FsRepository.TYPE) + .setSettings(Settings.builder().put("location", repoPath)) + ); + + int numberOfSnapshots = randomIntBetween(3, 10); + List snapshotNames = new ArrayList<>(); + for (int i = 0; i < numberOfSnapshots; i++) { + String snapshotName = "snapshot-" + i; + snapshotNames.add(snapshotName); + client().admin() + .cluster() + .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) + .setWaitForCompletion(true) + .get(); + } + + assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); + + // Now delete one of the snapshots using the test repo implementation which verifies the shard count behaviour + + assertAcked( + client().admin() + .cluster() + .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) + .setType(TEST_REPO_TYPE) + .setSettings(Settings.builder().put("location", repoPath)) + ); + + for (String snapshotName : snapshotNames) { + assertAcked(client().admin().cluster().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName).get()); + } + + // We've loaded N indices over M snapshots but we should have only loaded each index into heap memory once + assertEquals(numberOfIndices, INDEX_LOADED_COUNT.get()); + assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); + } +} From ecf8bbfb4add0369f7afaace60e5fb0175f07b83 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 10 Sep 2025 11:38:49 +0100 Subject: [PATCH 02/17] Skip unnecessary loading of `IndexMetadata` during snapshot deletion During snapshot deletion we load the metadata for an index into heap purely to calculate the shard count. Since the number of primary shards does not change for an index, we can store this when the snapshot is being created, so that upon deletion we avoid recomputing this value and loading large metadata objects into heap which can cause small nodes to OOMe. Closes ES-12539 Closes ES-12538 Closes #100569 --- .../IndexMetaDataGenerations.java | 12 ++++-- .../blobstore/BlobStoreRepository.java | 40 ++----------------- ...ryShardCountComputedOncePerIndexTests.java | 40 +++++-------------- 3 files changed, 24 insertions(+), 68 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index f805b7b9bd0d6..519523e357948 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -186,9 +186,15 @@ public String toString() { * @return identifier string */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { - return indexMetaData.getIndexUUID() + DELIMITER + indexMetaData.getSettings() - .get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + DELIMITER + indexMetaData.getSettingsVersion() - + DELIMITER + indexMetaData.getMappingVersion() + DELIMITER + indexMetaData.getAliasesVersion(); + return indexMetaData.getIndexUUID() + + DELIMITER + + indexMetaData.getSettings().get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + + DELIMITER + + indexMetaData.getSettingsVersion() + + DELIMITER + + indexMetaData.getMappingVersion() + + DELIMITER + + indexMetaData.getAliasesVersion(); } /** diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 4e5d73f9030fc..44550f480e954 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1034,7 +1034,7 @@ private void createSnapshotsDeletion( * blob must not change until it is updated by this deletion and the {@code repositoryDataUpdateListener} is completed. *

*/ - protected class SnapshotsDeletion { + class SnapshotsDeletion { /** * The IDs of the snapshots to delete. This collection is empty if the deletion is a repository cleanup. @@ -1267,11 +1267,9 @@ private void writeUpdatedShardMetadataAndComputeDeletes(ActionListener lis private class IndexSnapshotsDeletion { private final IndexId indexId; private final Set snapshotsWithIndex; - private final BlobContainer indexContainer; IndexSnapshotsDeletion(IndexId indexId) { this.indexId = indexId; - this.indexContainer = indexContainer(indexId); this.snapshotsWithIndex = Set.copyOf(originalRepositoryData.getSnapshots(indexId)); } @@ -1293,44 +1291,15 @@ private void determineShardCount(ActionListener listener) { try (var listeners = new RefCountingListener(listener)) { for (SnapshotId snapshotId : snapshotIds.stream().filter(snapshotsWithIndex::contains).collect(Collectors.toSet())) { snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> { - String blobId = originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(snapshotId, indexId); // The unique IndexMetadata ID prefixed by Index UUID String indexMetadataId = originalRepositoryData.indexMetaDataGenerations() .snapshotIndexMetadataIdentifier(snapshotId, indexId); - // Guarantees that the indexUUID will not be "" since this would map multiple indexMetaData objects to the - // same shard count + // Guarantees that the indexUUID will not be "" assert indexMetadataId != null; String indexUUID = parseUUIDFromUniqueIdentifier(indexMetadataId); - if (indexUUIDToShardCountMap.containsKey(indexUUID) == false) { - try { - IndexMetadata indexMetadata = INDEX_METADATA_FORMAT.read( - getProjectRepo(), - indexContainer, - blobId, - namedXContentRegistry - ); - int numberOfShards = indexMetadata.getNumberOfShards(); - indexUUIDToShardCountMap.put(indexUUID, numberOfShards); - updateShardCount(numberOfShards); - } 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. - } - } else { - // indexUUIDToShardCountMap is shared across all threads. Therefore, while there may be an entry for this - // UUID, there is no guarantee that we've encountered it in this thread, so we update using the precomputed - // value, thus removing the unnecessary INDEX_METADATA_FORMAT.read call. - updateShardCount(indexUUIDToShardCountMap.get(indexUUID)); - } + updateShardCount(indexUUIDToShardCountMap.get(indexUUID)); })); } } @@ -1874,8 +1843,7 @@ record RootBlobUpdateResult(RepositoryData oldRepositoryData, RepositoryData new // We don't yet have this version of the metadata so we write it metaUUID = UUIDs.base64UUID(); INDEX_METADATA_FORMAT.write(indexMetaData, indexContainer(index), metaUUID, compress); - // TODO - Could store the shard counts here in a map, or in the repo data, rather than loading - // the index metadata to heap to calculate + indexUUIDToShardCountMap.put(indexMetaData.getIndexUUID(), indexMetaData.getNumberOfShards()); metadataWriteResult.indexMetaIdentifiers().put(identifiers, metaUUID); } // else this task was largely a no-op - TODO no need to fork in that case metadataWriteResult.indexMetas().put(index, identifiers); diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java index 694a2682968a2..81de35e55a98a 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -133,10 +133,6 @@ public InputStream readBlob(OperationPurpose purpose, String blobName) throws IO } } - /* - This test generates N indices, and for each has M snapshots. - We're testing that for each of the N indices, it's metadata is only loaded into heap once - */ public void testShardCountComputedOncePerIndex() { final var repoPath = ESIntegTestCase.randomRepoPath(node().settings()); @@ -147,14 +143,12 @@ public void testShardCountComputedOncePerIndex() { ensureGreen(indexName); } - // Set up the repository contents, including snapshots, using a regular 'fs' repo - assertAcked( - client().admin() - .cluster() - .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) - .setType(FsRepository.TYPE) - .setSettings(Settings.builder().put("location", repoPath)) + client().admin() + .cluster() + .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) + .setType(TEST_REPO_TYPE) + .setSettings(Settings.builder().put("location", repoPath)) ); int numberOfSnapshots = randomIntBetween(3, 10); @@ -163,30 +157,18 @@ public void testShardCountComputedOncePerIndex() { String snapshotName = "snapshot-" + i; snapshotNames.add(snapshotName); client().admin() - .cluster() - .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) - .setWaitForCompletion(true) - .get(); + .cluster() + .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) + .setWaitForCompletion(true) + .get(); } - assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); - - // Now delete one of the snapshots using the test repo implementation which verifies the shard count behaviour - - assertAcked( - client().admin() - .cluster() - .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) - .setType(TEST_REPO_TYPE) - .setSettings(Settings.builder().put("location", repoPath)) - ); - for (String snapshotName : snapshotNames) { assertAcked(client().admin().cluster().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName).get()); } - // We've loaded N indices over M snapshots but we should have only loaded each index into heap memory once - assertEquals(numberOfIndices, INDEX_LOADED_COUNT.get()); + // All metadata should have been cached upon writing the snapshots, so no Index MetaData should have been written to memory + assertEquals(0, INDEX_LOADED_COUNT.get()); assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); } } From 83114dd3190c24095826bf264aefa306b396b14d Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 10 Sep 2025 10:47:45 +0000 Subject: [PATCH 03/17] [CI] Auto commit changes from spotless --- .../repositories/IndexMetaDataGenerations.java | 12 +++--------- .../blobstore/BlobStoreRepository.java | 2 +- ...oryShardCountComputedOncePerIndexTests.java | 18 +++++++++--------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 519523e357948..f805b7b9bd0d6 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -186,15 +186,9 @@ public String toString() { * @return identifier string */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { - return indexMetaData.getIndexUUID() - + DELIMITER - + indexMetaData.getSettings().get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) - + DELIMITER - + indexMetaData.getSettingsVersion() - + DELIMITER - + indexMetaData.getMappingVersion() - + DELIMITER - + indexMetaData.getAliasesVersion(); + return indexMetaData.getIndexUUID() + DELIMITER + indexMetaData.getSettings() + .get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + DELIMITER + indexMetaData.getSettingsVersion() + + DELIMITER + indexMetaData.getMappingVersion() + DELIMITER + indexMetaData.getAliasesVersion(); } /** diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 44550f480e954..54ff67e0f894e 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1034,7 +1034,7 @@ private void createSnapshotsDeletion( * blob must not change until it is updated by this deletion and the {@code repositoryDataUpdateListener} is completed. *

*/ - class SnapshotsDeletion { + class SnapshotsDeletion { /** * The IDs of the snapshots to delete. This collection is empty if the deletion is a repository cleanup. diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java index 81de35e55a98a..3dcdacd82ed9c 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -144,11 +144,11 @@ public void testShardCountComputedOncePerIndex() { } assertAcked( - client().admin() - .cluster() - .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) - .setType(TEST_REPO_TYPE) - .setSettings(Settings.builder().put("location", repoPath)) + client().admin() + .cluster() + .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) + .setType(TEST_REPO_TYPE) + .setSettings(Settings.builder().put("location", repoPath)) ); int numberOfSnapshots = randomIntBetween(3, 10); @@ -157,10 +157,10 @@ public void testShardCountComputedOncePerIndex() { String snapshotName = "snapshot-" + i; snapshotNames.add(snapshotName); client().admin() - .cluster() - .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) - .setWaitForCompletion(true) - .get(); + .cluster() + .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) + .setWaitForCompletion(true) + .get(); } for (String snapshotName : snapshotNames) { From 1c8ac70a9aac6aabca82fda19956ebbebe97f269 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 10 Sep 2025 13:37:36 +0100 Subject: [PATCH 04/17] Skip unnecessary loading of `IndexMetadata` during snapshot deletion During snapshot deletion we load the metadata for an index into heap purely to calculate the shard count. On nodes with small heaps, loading multiple objects concurrently causes them to OOMe. Rather than recomputing the shard count value for the same index across multiple snapshots, this change stores the shard count in a map. This stops loading multiple IndexMetaData objects for the same index into memory. Closes ES-12539 --- .../IndexMetaDataGenerations.java | 12 +--- .../blobstore/BlobStoreRepository.java | 48 ++++++++++--- ...ryShardCountComputedOncePerIndexTests.java | 68 ++++++++++++++++++- 3 files changed, 107 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 519523e357948..f805b7b9bd0d6 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -186,15 +186,9 @@ public String toString() { * @return identifier string */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { - return indexMetaData.getIndexUUID() - + DELIMITER - + indexMetaData.getSettings().get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) - + DELIMITER - + indexMetaData.getSettingsVersion() - + DELIMITER - + indexMetaData.getMappingVersion() - + DELIMITER - + indexMetaData.getAliasesVersion(); + return indexMetaData.getIndexUUID() + DELIMITER + indexMetaData.getSettings() + .get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + DELIMITER + indexMetaData.getSettingsVersion() + + DELIMITER + indexMetaData.getMappingVersion() + DELIMITER + indexMetaData.getAliasesVersion(); } /** diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 44550f480e954..1b7b3c95e8555 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -499,11 +499,6 @@ public static String getRepositoryDataBlobName(long repositoryGeneration) { private final ThrottledTaskRunner staleBlobDeleteRunner; - /** - * Maps the Index UUID to its shard count - */ - private final ConcurrentMap indexUUIDToShardCountMap = new ConcurrentHashMap<>(); - /** * Constructs new BlobStoreRepository * @param metadata The metadata for this repository including name and settings @@ -1034,7 +1029,7 @@ private void createSnapshotsDeletion( * blob must not change until it is updated by this deletion and the {@code repositoryDataUpdateListener} is completed. *

*/ - class SnapshotsDeletion { + class SnapshotsDeletion { /** * The IDs of the snapshots to delete. This collection is empty if the deletion is a repository cleanup. @@ -1105,6 +1100,11 @@ class SnapshotsDeletion { */ private final ShardBlobsToDelete shardBlobsToDelete = new ShardBlobsToDelete(); + /** + * Maps the Index UUID to its shard count + */ + private final ConcurrentMap indexUUIDToShardCountMap = new ConcurrentHashMap<>(); + SnapshotsDeletion( Collection snapshotIds, long originalRepositoryDataGeneration, @@ -1267,9 +1267,11 @@ private void writeUpdatedShardMetadataAndComputeDeletes(ActionListener lis private class IndexSnapshotsDeletion { private final IndexId indexId; private final Set snapshotsWithIndex; + private final BlobContainer indexContainer; IndexSnapshotsDeletion(IndexId indexId) { this.indexId = indexId; + this.indexContainer = indexContainer(indexId); this.snapshotsWithIndex = Set.copyOf(originalRepositoryData.getSnapshots(indexId)); } @@ -1291,15 +1293,44 @@ private void determineShardCount(ActionListener listener) { try (var listeners = new RefCountingListener(listener)) { for (SnapshotId snapshotId : snapshotIds.stream().filter(snapshotsWithIndex::contains).collect(Collectors.toSet())) { snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> { + String blobId = originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(snapshotId, indexId); // The unique IndexMetadata ID prefixed by Index UUID String indexMetadataId = originalRepositoryData.indexMetaDataGenerations() .snapshotIndexMetadataIdentifier(snapshotId, indexId); - // Guarantees that the indexUUID will not be "" + // Guarantees that the indexUUID will not be "" since this would map multiple indexMetaData objects to the + // same shard count assert indexMetadataId != null; String indexUUID = parseUUIDFromUniqueIdentifier(indexMetadataId); - updateShardCount(indexUUIDToShardCountMap.get(indexUUID)); + if (indexUUIDToShardCountMap.containsKey(indexUUID) == false) { + try { + IndexMetadata indexMetadata = INDEX_METADATA_FORMAT.read( + getProjectRepo(), + indexContainer, + blobId, + namedXContentRegistry + ); + int numberOfShards = indexMetadata.getNumberOfShards(); + indexUUIDToShardCountMap.put(indexUUID, numberOfShards); + updateShardCount(numberOfShards); + } 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. + } + } else { + // indexUUIDToShardCountMap is shared across all threads. Therefore, while there may be an entry for this + // UUID, there is no guarantee that we've encountered it in this thread, so we update using the precomputed + // value, thus removing the unnecessary INDEX_METADATA_FORMAT.read call. + updateShardCount(indexUUIDToShardCountMap.get(indexUUID)); + } })); } } @@ -1843,7 +1874,6 @@ record RootBlobUpdateResult(RepositoryData oldRepositoryData, RepositoryData new // We don't yet have this version of the metadata so we write it metaUUID = UUIDs.base64UUID(); INDEX_METADATA_FORMAT.write(indexMetaData, indexContainer(index), metaUUID, compress); - indexUUIDToShardCountMap.put(indexMetaData.getIndexUUID(), indexMetaData.getNumberOfShards()); metadataWriteResult.indexMetaIdentifiers().put(identifiers, metaUUID); } // else this task was largely a no-op - TODO no need to fork in that case metadataWriteResult.indexMetas().put(index, identifiers); diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java index 81de35e55a98a..bca03f6f59eb3 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -133,7 +133,12 @@ public InputStream readBlob(OperationPurpose purpose, String blobName) throws IO } } - public void testShardCountComputedOncePerIndex() { + /* + This test generates N indices, and each index has M snapshots. + When deleting multiple snapshots within one request, each including the same index, + we expect each indices metadata to only be loaded once + */ + public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsConcurrently() { final var repoPath = ESIntegTestCase.randomRepoPath(node().settings()); int numberOfIndices = randomIntBetween(3, 10); @@ -143,6 +148,59 @@ public void testShardCountComputedOncePerIndex() { ensureGreen(indexName); } + // Set up our test repo + assertAcked( + client().admin() + .cluster() + .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) + .setType(TEST_REPO_TYPE) + .setSettings(Settings.builder().put("location", repoPath)) + ); + + int numberOfSnapshots = randomIntBetween(3, 10); + List snapshotNames = new ArrayList<>(); + for (int i = 0; i < numberOfSnapshots; i++) { + String snapshotName = "snapshot-" + i; + snapshotNames.add(snapshotName); + client().admin() + .cluster() + .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) + .setWaitForCompletion(true) + .get(); + } + + // We want to avoid deleting all snapshots since this would invoke cleanup code and bulk snapshot deletion + // which is out of scope of this test + List snapshotsToDelete = randomSubsetOf(randomIntBetween(1, numberOfSnapshots - 1), snapshotNames); + + assertAcked( + client().admin() + .cluster() + .prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotsToDelete.toArray(new String[0])) + .get() + ); + + // Each index metadata should only be loaded into heap memory once + assertEquals(numberOfIndices, INDEX_LOADED_COUNT.get()); + assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); + } + + /* + This test generates N indices, and each index has M snapshots. + When deleting multiple snapshots sequentially, even if they include the same index, + we expect each indices metadata to be loaded each time + */ + public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsSequentially() { + final var repoPath = ESIntegTestCase.randomRepoPath(node().settings()); + + int numberOfIndices = randomIntBetween(3, 10); + for (int i = 0; i < numberOfIndices; i++) { + String indexName = "index-" + i; + createIndex(indexName, indexSettings(between(1, 3), 0).build()); + ensureGreen(indexName); + } + + // Set up our test repo assertAcked( client().admin() .cluster() @@ -163,12 +221,16 @@ public void testShardCountComputedOncePerIndex() { .get(); } + // We want to avoid deleting all snapshots since this would invoke cleanup code and bulk snapshot deletion + // which is out of scope of this test + snapshotNames.removeLast(); + for (String snapshotName : snapshotNames) { assertAcked(client().admin().cluster().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName).get()); } - // All metadata should have been cached upon writing the snapshots, so no Index MetaData should have been written to memory - assertEquals(0, INDEX_LOADED_COUNT.get()); + // Each index metadata is loaded into heap for each snapshot deletion request + assertEquals(numberOfIndices * (numberOfSnapshots - 1), INDEX_LOADED_COUNT.get()); assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); } } From ba5bf13ee980c024e77daa3cc7aec9130a0854dc Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Thu, 11 Sep 2025 10:40:20 +0100 Subject: [PATCH 05/17] Moves indexUUIDToShardCountMap into IndexSnapshotsDeletion --- .../repositories/blobstore/BlobStoreRepository.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 1b7b3c95e8555..aefee737a0808 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1100,11 +1100,6 @@ class SnapshotsDeletion { */ private final ShardBlobsToDelete shardBlobsToDelete = new ShardBlobsToDelete(); - /** - * Maps the Index UUID to its shard count - */ - private final ConcurrentMap indexUUIDToShardCountMap = new ConcurrentHashMap<>(); - SnapshotsDeletion( Collection snapshotIds, long originalRepositoryDataGeneration, @@ -1269,6 +1264,11 @@ private class IndexSnapshotsDeletion { private final Set snapshotsWithIndex; private final BlobContainer indexContainer; + /** + * Maps the Index UUID to its shard count + */ + private final ConcurrentMap indexUUIDToShardCountMap = new ConcurrentHashMap<>(); + IndexSnapshotsDeletion(IndexId indexId) { this.indexId = indexId; this.indexContainer = indexContainer(indexId); From 2f6953907ebb2893af4cb94f4d978825a3d6ab8a Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Mon, 20 Oct 2025 15:40:20 +0100 Subject: [PATCH 06/17] Refactors determineShardCount Adds extra test capability to handle deleted indices --- .../IndexMetaDataGenerations.java | 42 +++++-- .../blobstore/BlobStoreRepository.java | 23 ++-- .../IndexMetaDataGenerationsTests.java | 114 ++++++++++++++++-- ...ryShardCountComputedOncePerIndexTests.java | 98 ++++++++------- .../test/ESSingleNodeTestCase.java | 12 ++ 5 files changed, 207 insertions(+), 82 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index f805b7b9bd0d6..df44bbc3dc700 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -21,6 +21,8 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; /** @@ -174,9 +176,6 @@ public String toString() { return "IndexMetaDataGenerations{lookup:" + lookup + "}{identifier:" + identifiers + "}"; } - // Cannot use - as a delimiter because getIndexUUID() produces UUIDs with - in them - private static final String DELIMITER = "/"; - /** * Compute identifier for {@link IndexMetadata} from its index- and history-uuid as well as its settings-, mapping- and alias-version. * If an index did not see a change in its settings, mappings or aliases between two points in time then the identifier will not change @@ -186,21 +185,38 @@ public String toString() { * @return identifier string */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { - return indexMetaData.getIndexUUID() + DELIMITER + indexMetaData.getSettings() - .get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + DELIMITER + indexMetaData.getSettingsVersion() - + DELIMITER + indexMetaData.getMappingVersion() + DELIMITER + indexMetaData.getAliasesVersion(); + // If modifying this identifier, then also extend the convertBlobIdToIndexUUID function below + return indexMetaData.getIndexUUID() + "-" + indexMetaData.getSettings() + .get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + "-" + indexMetaData.getSettingsVersion() + + "-" + indexMetaData.getMappingVersion() + "-" + indexMetaData.getAliasesVersion(); } /** - * Parses the unique IndexMetadata ID generated by the {@code buildUniqueIdentifier} function and returns the UUID prefix. - * If a null uniqueIdentifier is given, then an empty string is returned - * @param uniqueIdentifier - The ID to parse the UUID from - * @return The UUID of the IndexMetadata object. + * Given a blobId, returns the index UUID associated with it. + * If the blobId is not found, returns null. + * @param blobId The blob ID */ - public static String parseUUIDFromUniqueIdentifier(String uniqueIdentifier) { + @Nullable + public String convertBlobIdToIndexUUID(String blobId) { + // Find the unique identifier for this blobId + String uniqueIdentifier = null; + for (Map.Entry entry : this.identifiers.entrySet()) { + if (Objects.equals(entry.getValue(), blobId)) { + uniqueIdentifier = entry.getKey(); + break; + } + } if (uniqueIdentifier == null) { - return ""; + return null; + } + + // The uniqueIdentifier was built in buildUniqueIdentifier, and is of the format IndexUUID-String-long-long-long + // The regex accounts for the fact that the IndexUUID can also contain the DELIMITER value + Pattern pattern = Pattern.compile("^(.*?)-[^-]+-\\d+-\\d+-\\d+$"); + Matcher matcher = pattern.matcher(uniqueIdentifier); + if (matcher.matches()) { + return matcher.group(1); } - return uniqueIdentifier.split(DELIMITER, 2)[0]; + throw new IllegalArgumentException("Error parsing the IndexMetadata identifier"); } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index aefee737a0808..f7f41e3c5371c 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -184,7 +184,6 @@ import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo; import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo.canonicalName; import static org.elasticsearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING; -import static org.elasticsearch.repositories.IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier; import static org.elasticsearch.repositories.ProjectRepo.projectRepoString; /** @@ -1291,18 +1290,15 @@ void run(ActionListener listener) { private void determineShardCount(ActionListener listener) { try (var listeners = new RefCountingListener(listener)) { - for (SnapshotId snapshotId : snapshotIds.stream().filter(snapshotsWithIndex::contains).collect(Collectors.toSet())) { + for (final var blobId : snapshotIds.stream() + .filter(snapshotsWithIndex::contains) + .map(id -> originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(id, indexId)) + .collect(Collectors.toSet())) { snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> { - String blobId = originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(snapshotId, indexId); - // The unique IndexMetadata ID prefixed by Index UUID - String indexMetadataId = originalRepositoryData.indexMetaDataGenerations() - .snapshotIndexMetadataIdentifier(snapshotId, indexId); - - // Guarantees that the indexUUID will not be "" since this would map multiple indexMetaData objects to the - // same shard count - assert indexMetadataId != null; - String indexUUID = parseUUIDFromUniqueIdentifier(indexMetadataId); - + // 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. + String indexUUID = originalRepositoryData.indexMetaDataGenerations().convertBlobIdToIndexUUID(blobId); if (indexUUIDToShardCountMap.containsKey(indexUUID) == false) { try { IndexMetadata indexMetadata = INDEX_METADATA_FORMAT.read( @@ -1327,8 +1323,7 @@ private void determineShardCount(ActionListener listener) { } } else { // indexUUIDToShardCountMap is shared across all threads. Therefore, while there may be an entry for this - // UUID, there is no guarantee that we've encountered it in this thread, so we update using the precomputed - // value, thus removing the unnecessary INDEX_METADATA_FORMAT.read call. + // UUID in the map, there is no guarantee that we've encountered it in this thread. updateShardCount(indexUUIDToShardCountMap.get(indexUUID)); } })); diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index 453801f648da5..932b51b90d30f 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -11,8 +11,11 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.test.ESTestCase; +import java.util.Map; + import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -33,10 +36,7 @@ public void testBuildUniqueIdentifierWithAllFieldsPresent() { when(indexMetadata.getAliasesVersion()).thenReturn(aliasesVersion); String result = IndexMetaDataGenerations.buildUniqueIdentifier(indexMetadata); - assertEquals(indexUUID + "/" + historyUUID + "/" + settingsVersion + "/" + mappingVersion + "/" + aliasesVersion, result); - - // Then test parseUUIDFromUniqueIdentifier - assertEquals(indexUUID, IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier(result)); + assertEquals(indexUUID + "-" + historyUUID + "-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion, result); } public void testBuildUniqueIdentifierWithMissingHistoryUUID() { @@ -53,19 +53,109 @@ public void testBuildUniqueIdentifierWithMissingHistoryUUID() { when(indexMetadata.getAliasesVersion()).thenReturn(aliasesVersion); String result = IndexMetaDataGenerations.buildUniqueIdentifier(indexMetadata); - assertEquals(indexUUID + "/_na_/" + settingsVersion + "/" + mappingVersion + "/" + aliasesVersion, result); + assertEquals(indexUUID + "-_na_-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion, result); + } + + public void testConvertBlobIdToIndexUUIDReturnsIndexUUID() { + // May include the - symbol, which as of 9.3.0 is the same symbol as the delimiter + String indexUUID = randomUUID(); + 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> lookup = Map.of( + snapshotId, Map.of(indexId, uniqueIdentifier) + ); + + IndexMetaDataGenerations generations = new IndexMetaDataGenerations( + lookup, + Map.of(uniqueIdentifier, blobId) + ); + assertEquals(indexUUID, generations.convertBlobIdToIndexUUID(blobId)); + } + + public void testConvertBlobIdToIndexUUIDReturnsNullWhenBlobIdIsNotFound() { + IndexMetaDataGenerations generations = new IndexMetaDataGenerations( + Map.of(), + Map.of() + ); + assertNull(generations.convertBlobIdToIndexUUID(randomAlphanumericOfLength(randomIntBetween(5, 10)))); + } + + /** + * A helper function that tests whether an IAE is thrown when a malformed IndexMetadata Identifier is passed into + * the {@code convertBlobIdToIndexUUID} function + * @param indexUUID The indexUUID + * @param uniqueIdentifier The malformed identifier + * @param blobId The blobId + */ + private void testMalformedIndexMetadataIdentifierInternalThrowsIAE(String indexUUID, String uniqueIdentifier, String blobId) { + // Creates the lookup map + SnapshotId snapshotId = new SnapshotId("snapshot", randomUUID()); + IndexId indexId = new IndexId("index", indexUUID); + Map> lookup = Map.of( + snapshotId, Map.of(indexId, uniqueIdentifier) + ); + + IndexMetaDataGenerations malformedGenerations = new IndexMetaDataGenerations( + lookup, + Map.of(uniqueIdentifier, blobId) + ); + IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> malformedGenerations.convertBlobIdToIndexUUID(blobId) + ); + assertTrue(ex.getMessage().contains("Error parsing the IndexMetadata identifier")); + } + + public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingIndexUUID() { + String indexUUID = randomUUID(); + String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); + long settingsVersion = randomNonNegativeLong(); + long mappingsVersion = randomNonNegativeLong(); + long aliasesVersion = randomNonNegativeLong(); + // Build the unique identifier without including the index uuid + String uniqueIdentifier = randomSetting + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; + String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); + testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); } - public void testParseUUIDFromUniqueIdentifierWithNullInput() { - assertEquals("", IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier(null)); + public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingSettings() { + String indexUUID = randomUUID(); + long settingsVersion = randomNonNegativeLong(); + long mappingsVersion = randomNonNegativeLong(); + long aliasesVersion = randomNonNegativeLong(); + // Build the unique identifier without including the settings + String uniqueIdentifier = indexUUID + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; + String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); + testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); } - public void testParseUUIDFromUniqueIdentifierWithEmptyString() { - assertEquals("", IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier("")); + public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingSettingsVersion() { + String indexUUID = randomUUID(); + String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); + long mappingsVersion = randomNonNegativeLong(); + long aliasesVersion = randomNonNegativeLong(); + // Build the unique identifier without including the settings version + String uniqueIdentifier = indexUUID + "-" + randomSetting + "-" + mappingsVersion + "-" + aliasesVersion; + String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); + testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); } - public void testParseUUIDFromUniqueIdentifierWithoutDelimiter() { - String uuid = randomAlphanumericOfLength(randomIntBetween(10, 64)); - assertEquals(uuid, IndexMetaDataGenerations.parseUUIDFromUniqueIdentifier(uuid)); + public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierHasIncorrectTypes() { + String indexUUID = randomUUID(); + String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); + long settingsVersion = randomNonNegativeLong(); + long mappingsVersion = randomNonNegativeLong(); + String aliasesVersion = randomAlphaOfLength(randomIntBetween(5, 10)); + String uniqueIdentifier = indexUUID + "-" + randomSetting + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; + String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); + testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); } } diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java index 5d78753fd127d..c02293333205f 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -139,40 +139,11 @@ public InputStream readBlob(OperationPurpose purpose, String blobName) throws IO we expect each indices metadata to only be loaded once */ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsConcurrently() { - final var repoPath = ESIntegTestCase.randomRepoPath(node().settings()); - int numberOfIndices = randomIntBetween(3, 10); - for (int i = 0; i < numberOfIndices; i++) { - String indexName = "index-" + i; - createIndex(indexName, indexSettings(between(1, 3), 0).build()); - ensureGreen(indexName); - } - - // Set up our test repo - assertAcked( - client().admin() - .cluster() - .preparePutRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME) - .setType(TEST_REPO_TYPE) - .setSettings(Settings.builder().put("location", repoPath)) - ); - - int numberOfSnapshots = randomIntBetween(3, 10); - List snapshotNames = new ArrayList<>(); - for (int i = 0; i < numberOfSnapshots; i++) { - String snapshotName = "snapshot-" + i; - snapshotNames.add(snapshotName); - client().admin() - .cluster() - .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) - .setWaitForCompletion(true) - .get(); - } - - // We want to avoid deleting all snapshots since this would invoke cleanup code and bulk snapshot deletion - // which is out of scope of this test - List snapshotsToDelete = randomSubsetOf(randomIntBetween(1, numberOfSnapshots - 1), snapshotNames); + int numberOfIndicesRecreated = randomIntBetween(0, numberOfIndices); + List snapshotsToDelete = createIndicesAndSnapshots(numberOfIndices, numberOfIndicesRecreated, randomIntBetween(3, 10)); + // Delete all snapshots in one request assertAcked( client().admin() .cluster() @@ -180,8 +151,9 @@ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsConcu .get() ); - // Each index metadata should only be loaded into heap memory once - assertEquals(numberOfIndices, INDEX_LOADED_COUNT.get()); + // Each index metadata should only be loaded into heap memory once, plus numberOfIndicesRecreated + // indices were deleted and recreated, and have their own UUID + assertEquals(numberOfIndices + numberOfIndicesRecreated, INDEX_LOADED_COUNT.get()); assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); } @@ -191,11 +163,38 @@ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsConcu we expect each indices metadata to be loaded each time */ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsSequentially() { + int numberOfIndices = randomIntBetween(3, 10); + int numberOfIndicesRecreated = randomIntBetween(0, numberOfIndices); + int secondNumberOfSnapshots = randomIntBetween(3, 10); + List snapshotsToDelete = createIndicesAndSnapshots(numberOfIndices, numberOfIndicesRecreated, secondNumberOfSnapshots); + + for (String snapshotName : snapshotsToDelete) { + assertAcked(client().admin().cluster().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName).get()); + } + + // Work out how many snapshots included recreated indices + int snapshotsOnRecreatedIndices = 0; + for (String snapshotName : snapshotsToDelete) { + if (snapshotName.startsWith("second-snapshots")) { + snapshotsOnRecreatedIndices+=1; + } + } + + // Each index metadata is loaded into heap for each snapshot deletion request + int expectedNumberOfIndexMetaDataLoads = + numberOfIndices * snapshotsToDelete.size() + + numberOfIndicesRecreated * snapshotsOnRecreatedIndices; + assertEquals(expectedNumberOfIndexMetaDataLoads, INDEX_LOADED_COUNT.get()); + assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); + } + + private List createIndicesAndSnapshots(int numberOfIndices, int numberOfIndicesRecreated, int secondNumberOfSnapshots) { final var repoPath = ESIntegTestCase.randomRepoPath(node().settings()); - int numberOfIndices = randomIntBetween(3, 10); + List indexNames = new ArrayList<>(); for (int i = 0; i < numberOfIndices; i++) { String indexName = "index-" + i; + indexNames.add(indexName); createIndex(indexName, indexSettings(between(1, 3), 0).build()); ensureGreen(indexName); } @@ -221,16 +220,29 @@ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsSeque .get(); } - // We want to avoid deleting all snapshots since this would invoke cleanup code and bulk snapshot deletion - // which is out of scope of this test - snapshotNames.removeLast(); + // Now delete a random subset of indices, and then recreate them with the same name but a different shard count + // This will force the new indices to have the same indexId but a different UUID + List indicesToDelete = randomSubsetOf(numberOfIndicesRecreated, indexNames); + for (String indexName : indicesToDelete) { + deleteIndex(indexName); + // Creates a new index with the same name but a different number of shards + createIndex(indexName, indexSettings(between(4, 6), 0).build()); + ensureGreen(indexName); + } - for (String snapshotName : snapshotNames) { - assertAcked(client().admin().cluster().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName).get()); + // Do some more snapshots now + for (int i = 0; i < secondNumberOfSnapshots; i++) { + String snapshotName = "second-snapshots-" + i; + snapshotNames.add(snapshotName); + client().admin() + .cluster() + .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) + .setWaitForCompletion(true) + .get(); } - // Each index metadata is loaded into heap for each snapshot deletion request - assertEquals(numberOfIndices * (numberOfSnapshots - 1), INDEX_LOADED_COUNT.get()); - assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); + // We want to avoid deleting all snapshots since this would invoke cleanup code and bulk snapshot deletion + // which is out of scope of this test + return randomSubsetOf(randomIntBetween(1, numberOfSnapshots - 1), snapshotNames); } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java index 53214590e4a60..6613764676064 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java @@ -403,6 +403,18 @@ protected IndexService createIndex(String index, CreateIndexRequestBuilder creat return instanceFromNode.indexServiceSafe(resolveIndex(index)); } + /** + * Deletes the given index from the singleton node. + * Waits for the operation to complete and asserts it was acknowledged. + * + * @param index The name of the index to delete + */ + protected void deleteIndex(String index) { + assertAcked(indicesAdmin().prepareDelete(index).get()); + // Optionally, wait for the cluster to be green after deletion + ensureGreen(); + } + public Index resolveIndex(String index) { GetIndexResponse getIndexResponse = indicesAdmin().prepareGetIndex(TEST_REQUEST_TIMEOUT).setIndices(index).get(); assertTrue("index " + index + " not found", getIndexResponse.getSettings().containsKey(index)); From 00e264b6678c3cfa728b15bbfb8d2d0f1b305239 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Tue, 21 Oct 2025 14:03:43 +0100 Subject: [PATCH 07/17] Extend tests to delete and recreate indices --- .../IndexMetaDataGenerations.java | 12 +- .../IndexMetaDataGenerationsTests.java | 35 ++---- ...ryShardCountComputedOncePerIndexTests.java | 115 ++++++++++-------- 3 files changed, 87 insertions(+), 75 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index df44bbc3dc700..733f8c189a694 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -186,9 +186,15 @@ public String toString() { */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { // If modifying this identifier, then also extend the convertBlobIdToIndexUUID function below - return indexMetaData.getIndexUUID() + "-" + indexMetaData.getSettings() - .get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + "-" + indexMetaData.getSettingsVersion() - + "-" + indexMetaData.getMappingVersion() + "-" + indexMetaData.getAliasesVersion(); + return indexMetaData.getIndexUUID() + + "-" + + indexMetaData.getSettings().get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + + "-" + + indexMetaData.getSettingsVersion() + + "-" + + indexMetaData.getMappingVersion() + + "-" + + indexMetaData.getAliasesVersion(); } /** diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index 932b51b90d30f..9ade115ab6029 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -69,22 +69,14 @@ public void testConvertBlobIdToIndexUUIDReturnsIndexUUID() { // Creates the lookup map SnapshotId snapshotId = new SnapshotId("snapshot", randomUUID()); IndexId indexId = new IndexId("index", indexUUID); - Map> lookup = Map.of( - snapshotId, Map.of(indexId, uniqueIdentifier) - ); + Map> lookup = Map.of(snapshotId, Map.of(indexId, uniqueIdentifier)); - IndexMetaDataGenerations generations = new IndexMetaDataGenerations( - lookup, - Map.of(uniqueIdentifier, blobId) - ); + IndexMetaDataGenerations generations = new IndexMetaDataGenerations(lookup, Map.of(uniqueIdentifier, blobId)); assertEquals(indexUUID, generations.convertBlobIdToIndexUUID(blobId)); } public void testConvertBlobIdToIndexUUIDReturnsNullWhenBlobIdIsNotFound() { - IndexMetaDataGenerations generations = new IndexMetaDataGenerations( - Map.of(), - Map.of() - ); + IndexMetaDataGenerations generations = new IndexMetaDataGenerations(Map.of(), Map.of()); assertNull(generations.convertBlobIdToIndexUUID(randomAlphanumericOfLength(randomIntBetween(5, 10)))); } @@ -95,18 +87,13 @@ public void testConvertBlobIdToIndexUUIDReturnsNullWhenBlobIdIsNotFound() { * @param uniqueIdentifier The malformed identifier * @param blobId The blobId */ - private void testMalformedIndexMetadataIdentifierInternalThrowsIAE(String indexUUID, String uniqueIdentifier, String blobId) { + private void internalMalformedIndexMetadataIdentifierThrowsIAE(String indexUUID, String uniqueIdentifier, String blobId) { // Creates the lookup map SnapshotId snapshotId = new SnapshotId("snapshot", randomUUID()); IndexId indexId = new IndexId("index", indexUUID); - Map> lookup = Map.of( - snapshotId, Map.of(indexId, uniqueIdentifier) - ); + Map> lookup = Map.of(snapshotId, Map.of(indexId, uniqueIdentifier)); - IndexMetaDataGenerations malformedGenerations = new IndexMetaDataGenerations( - lookup, - Map.of(uniqueIdentifier, blobId) - ); + IndexMetaDataGenerations malformedGenerations = new IndexMetaDataGenerations(lookup, Map.of(uniqueIdentifier, blobId)); IllegalArgumentException ex = assertThrows( IllegalArgumentException.class, () -> malformedGenerations.convertBlobIdToIndexUUID(blobId) @@ -123,18 +110,18 @@ public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalfor // Build the unique identifier without including the index uuid String uniqueIdentifier = randomSetting + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); + internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); } public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingSettings() { - String indexUUID = randomUUID(); + String indexUUID = randomAlphanumericOfLength(randomIntBetween(5, 10)); long settingsVersion = randomNonNegativeLong(); long mappingsVersion = randomNonNegativeLong(); long aliasesVersion = randomNonNegativeLong(); // Build the unique identifier without including the settings String uniqueIdentifier = indexUUID + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); + internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); } public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingSettingsVersion() { @@ -145,7 +132,7 @@ public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalfor // Build the unique identifier without including the settings version String uniqueIdentifier = indexUUID + "-" + randomSetting + "-" + mappingsVersion + "-" + aliasesVersion; String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); + internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); } public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierHasIncorrectTypes() { @@ -156,6 +143,6 @@ public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalfor String aliasesVersion = randomAlphaOfLength(randomIntBetween(5, 10)); String uniqueIdentifier = indexUUID + "-" + randomSetting + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - testMalformedIndexMetadataIdentifierInternalThrowsIAE(indexUUID, uniqueIdentifier, blobId); + internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); } } diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java index c02293333205f..12f8fb781234c 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -134,14 +134,21 @@ public InputStream readBlob(OperationPurpose purpose, String blobName) throws IO } /* - This test generates N indices, and each index has M snapshots. - When deleting multiple snapshots within one request, each including the same index, - we expect each indices metadata to only be loaded once + This test: + - Generates A indices + - Generates M snapshots including these indices + - Deletes a subset B of indices + - Recreates the B indices with the same name + - Generates N subsequent snapshots + - Deletes a random subset of snapshots within one request + + When deleting multiple snapshots within one request, we expect the metadata to be loaded once for each index, + and then it's shard count cached */ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsConcurrently() { int numberOfIndices = randomIntBetween(3, 10); int numberOfIndicesRecreated = randomIntBetween(0, numberOfIndices); - List snapshotsToDelete = createIndicesAndSnapshots(numberOfIndices, numberOfIndicesRecreated, randomIntBetween(3, 10)); + List snapshotsToDelete = createIndicesAndSnapshots(numberOfIndices, numberOfIndicesRecreated); // Delete all snapshots in one request assertAcked( @@ -151,46 +158,43 @@ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsConcu .get() ); - // Each index metadata should only be loaded into heap memory once, plus numberOfIndicesRecreated - // indices were deleted and recreated, and have their own UUID + // Each index metadata should only be loaded into heap memory once, plus those indices that were recreated assertEquals(numberOfIndices + numberOfIndicesRecreated, INDEX_LOADED_COUNT.get()); assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); } /* - This test generates N indices, and each index has M snapshots. + This test: + - Generates A indices + - Generates M snapshots including these indices + - Deletes a subset B of indices + - Recreates the B indices with the same name + - Generates N subsequent snapshots + - Deletes a random subset of snapshots within one request + When deleting multiple snapshots sequentially, even if they include the same index, we expect each indices metadata to be loaded each time */ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsSequentially() { int numberOfIndices = randomIntBetween(3, 10); int numberOfIndicesRecreated = randomIntBetween(0, numberOfIndices); - int secondNumberOfSnapshots = randomIntBetween(3, 10); - List snapshotsToDelete = createIndicesAndSnapshots(numberOfIndices, numberOfIndicesRecreated, secondNumberOfSnapshots); + List snapshotsToDelete = createIndicesAndSnapshots(numberOfIndices, numberOfIndicesRecreated); for (String snapshotName : snapshotsToDelete) { assertAcked(client().admin().cluster().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName).get()); } - // Work out how many snapshots included recreated indices - int snapshotsOnRecreatedIndices = 0; - for (String snapshotName : snapshotsToDelete) { - if (snapshotName.startsWith("second-snapshots")) { - snapshotsOnRecreatedIndices+=1; - } - } - // Each index metadata is loaded into heap for each snapshot deletion request - int expectedNumberOfIndexMetaDataLoads = - numberOfIndices * snapshotsToDelete.size() + - numberOfIndicesRecreated * snapshotsOnRecreatedIndices; - assertEquals(expectedNumberOfIndexMetaDataLoads, INDEX_LOADED_COUNT.get()); + // For every index deleted and then recreated, there is still only one index of it present at any time + // Therefore, for every request, there are numberOfIndices reads to memory + assertEquals(numberOfIndices * snapshotsToDelete.size(), INDEX_LOADED_COUNT.get()); assertAcked(client().admin().cluster().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, TEST_REPO_NAME)); } - private List createIndicesAndSnapshots(int numberOfIndices, int numberOfIndicesRecreated, int secondNumberOfSnapshots) { + private List createIndicesAndSnapshots(int numberOfIndices, int numberOfIndicesRecreated) { final var repoPath = ESIntegTestCase.randomRepoPath(node().settings()); + // Create indices List indexNames = new ArrayList<>(); for (int i = 0; i < numberOfIndices; i++) { String indexName = "index-" + i; @@ -208,41 +212,56 @@ private List createIndicesAndSnapshots(int numberOfIndices, int numberOf .setSettings(Settings.builder().put("location", repoPath)) ); - int numberOfSnapshots = randomIntBetween(3, 10); - List snapshotNames = new ArrayList<>(); - for (int i = 0; i < numberOfSnapshots; i++) { - String snapshotName = "snapshot-" + i; - snapshotNames.add(snapshotName); + // Do the first batch of snapshots + int numberOfSnapshotsInFirstBatch = randomIntBetween(3, 10); + List firstBatchOfSnapshotNames = new ArrayList<>(); + for (int i = 0; i < numberOfSnapshotsInFirstBatch; i++) { + String snapshotName = "first-snapshot-" + i; + firstBatchOfSnapshotNames.add(snapshotName); client().admin() .cluster() .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) .setWaitForCompletion(true) .get(); } + // We want to avoid deleting all snapshots since this would invoke cleanup code and bulk snapshot deletion + // which is out of scope of this test + List firstBatchOfSnapshotNamesToDelete = randomSubsetOf( + randomIntBetween(1, numberOfSnapshotsInFirstBatch - 1), + firstBatchOfSnapshotNames + ); - // Now delete a random subset of indices, and then recreate them with the same name but a different shard count - // This will force the new indices to have the same indexId but a different UUID - List indicesToDelete = randomSubsetOf(numberOfIndicesRecreated, indexNames); - for (String indexName : indicesToDelete) { - deleteIndex(indexName); - // Creates a new index with the same name but a different number of shards - createIndex(indexName, indexSettings(between(4, 6), 0).build()); - ensureGreen(indexName); - } + List secondBatchOfSnapshotNamesToDelete = new ArrayList<>(); + if (numberOfIndicesRecreated > 0) { + // Now delete a random subset of indices, and then recreate them with the same name but a different shard count + // This will force the new indices to have the same indexId but a different UUID + List indicesToDelete = randomSubsetOf(numberOfIndicesRecreated, indexNames); + for (String indexName : indicesToDelete) { + deleteIndex(indexName); + // Creates a new index with the same name but a different number of shards + createIndex(indexName, indexSettings(between(4, 6), 0).build()); + ensureGreen(indexName); + } - // Do some more snapshots now - for (int i = 0; i < secondNumberOfSnapshots; i++) { - String snapshotName = "second-snapshots-" + i; - snapshotNames.add(snapshotName); - client().admin() - .cluster() - .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) - .setWaitForCompletion(true) - .get(); + // Do the second batch of snapshots + int numberOfSnapshotsInSecondBatch = randomIntBetween(3, 10); + List secondBatchOfSnapshotNames = new ArrayList<>(); + for (int i = 0; i < numberOfSnapshotsInSecondBatch; i++) { + String snapshotName = "second-snapshot-" + i; + secondBatchOfSnapshotNames.add(snapshotName); + client().admin() + .cluster() + .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) + .setWaitForCompletion(true) + .get(); + } + secondBatchOfSnapshotNamesToDelete = randomSubsetOf( + randomIntBetween(1, numberOfSnapshotsInSecondBatch - 1), + secondBatchOfSnapshotNames + ); } - // We want to avoid deleting all snapshots since this would invoke cleanup code and bulk snapshot deletion - // which is out of scope of this test - return randomSubsetOf(randomIntBetween(1, numberOfSnapshots - 1), snapshotNames); + firstBatchOfSnapshotNamesToDelete.addAll(secondBatchOfSnapshotNamesToDelete); + return firstBatchOfSnapshotNamesToDelete; } } From f76ad0034cf035d47b4a5e1baa291e60f33b6a8d Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Tue, 21 Oct 2025 14:13:44 +0100 Subject: [PATCH 08/17] Fix comments --- .../elasticsearch/repositories/IndexMetaDataGenerations.java | 5 +++-- .../java/org/elasticsearch/test/ESSingleNodeTestCase.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 733f8c189a694..bdb94130265a6 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -216,8 +216,9 @@ public String convertBlobIdToIndexUUID(String blobId) { return null; } - // The uniqueIdentifier was built in buildUniqueIdentifier, and is of the format IndexUUID-String-long-long-long - // The regex accounts for the fact that the IndexUUID can also contain the DELIMITER value + // The uniqueIdentifier was built in buildUniqueIdentifier, is of the format IndexUUID-String-long-long-long, + // and uses '-' as a delimiter. + // The below regex accounts for the fact that the IndexUUID can also contain the '-' character Pattern pattern = Pattern.compile("^(.*?)-[^-]+-\\d+-\\d+-\\d+$"); Matcher matcher = pattern.matcher(uniqueIdentifier); if (matcher.matches()) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java index 6613764676064..a71f2b9a2c0a1 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java @@ -411,7 +411,7 @@ protected IndexService createIndex(String index, CreateIndexRequestBuilder creat */ protected void deleteIndex(String index) { assertAcked(indicesAdmin().prepareDelete(index).get()); - // Optionally, wait for the cluster to be green after deletion + // Wait for the cluster to be green after deletion ensureGreen(); } From c181d778b6a75f2a61060ba740d9b287e488761e Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Thu, 23 Oct 2025 10:55:44 +0100 Subject: [PATCH 09/17] David comments --- .../IndexMetaDataGenerations.java | 56 +++++--- .../blobstore/BlobStoreRepository.java | 35 ++--- .../IndexMetaDataGenerationsTests.java | 133 ++++++++---------- 3 files changed, 110 insertions(+), 114 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index bdb94130265a6..820d0e624a3a8 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -9,6 +9,7 @@ package org.elasticsearch.repositories; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.set.Sets; @@ -47,12 +48,20 @@ public final class IndexMetaDataGenerations { */ final Map identifiers; + /** + * Map of blob uuid to index metadata identifier. This is a reverse lookup of the identifiers map. + */ + final Map blobUuidToIndexMetadataMap; + IndexMetaDataGenerations(Map> lookup, Map identifiers) { assert identifiers.keySet().equals(lookup.values().stream().flatMap(m -> m.values().stream()).collect(Collectors.toSet())) : "identifier mappings " + identifiers + " don't track the same blob ids as the lookup map " + lookup; assert lookup.values().stream().noneMatch(Map::isEmpty) : "Lookup contained empty map [" + lookup + "]"; this.lookup = Map.copyOf(lookup); this.identifiers = Map.copyOf(identifiers); + this.blobUuidToIndexMetadataMap = identifiers.entrySet() + .stream() + .collect(Collectors.toUnmodifiableMap(Map.Entry::getValue, Map.Entry::getKey)); } public boolean isEmpty() { @@ -71,6 +80,21 @@ public String getIndexMetaBlobId(String metaIdentifier) { return identifiers.get(metaIdentifier); } + /** + * Returns the index metadata identifier associated with the given blob UUID. + *

+ * This method provides a reverse lookup from a blob UUID to its corresponding index metadata identifier. + * If the specified blob UUID is not present, this method returns {@code null}. + *

+ * + * @param blobUuid the UUID of the blob whose index metadata identifier is to be retrieved + * @return the index metadata identifier associated with the given blob UUID, or {@code null} if not found + */ + @Nullable + public String getIndexMetadataIdentifierByBlobUuid(String blobUuid) { + return blobUuidToIndexMetadataMap.get(blobUuid); + } + /** * Get the blob id by {@link SnapshotId} and {@link IndexId} and fall back to the value of {@link SnapshotId#getUUID()} if none is * known to enable backwards compatibility with versions older than @@ -156,7 +180,7 @@ public IndexMetaDataGenerations withRemovedSnapshots(Collection snap @Override public int hashCode() { - return Objects.hash(identifiers, lookup); + return Objects.hash(identifiers, lookup, blobUuidToIndexMetadataMap); } @Override @@ -168,12 +192,12 @@ public boolean equals(Object that) { return false; } final IndexMetaDataGenerations other = (IndexMetaDataGenerations) that; - return lookup.equals(other.lookup) && identifiers.equals(other.identifiers); + return lookup.equals(other.lookup) && identifiers.equals(other.identifiers) && blobUuidToIndexMetadataMap.equals(other.blobUuidToIndexMetadataMap); } @Override public String toString() { - return "IndexMetaDataGenerations{lookup:" + lookup + "}{identifier:" + identifiers + "}"; + return "IndexMetaDataGenerations{lookup:" + lookup + "}{identifier:" + identifiers + "}{blobUuidToIndexMetadataMap:" + blobUuidToIndexMetadataMap + "}"; } /** @@ -199,31 +223,27 @@ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { /** * Given a blobId, returns the index UUID associated with it. + *

* If the blobId is not found, returns null. + *

+ * If the index UUID is _na_ {@code ClusterState.UNKNOWN_UUID} * @param blobId The blob ID */ @Nullable public String convertBlobIdToIndexUUID(String blobId) { // Find the unique identifier for this blobId - String uniqueIdentifier = null; - for (Map.Entry entry : this.identifiers.entrySet()) { - if (Objects.equals(entry.getValue(), blobId)) { - uniqueIdentifier = entry.getKey(); - break; - } - } + String uniqueIdentifier = blobUuidToIndexMetadataMap.get(blobId); if (uniqueIdentifier == null) { return null; } - // The uniqueIdentifier was built in buildUniqueIdentifier, is of the format IndexUUID-String-long-long-long, - // and uses '-' as a delimiter. - // The below regex accounts for the fact that the IndexUUID can also contain the '-' character - Pattern pattern = Pattern.compile("^(.*?)-[^-]+-\\d+-\\d+-\\d+$"); - Matcher matcher = pattern.matcher(uniqueIdentifier); - if (matcher.matches()) { - return matcher.group(1); + // The uniqueIdentifier is built in {@code buildUniqueIdentifier}, and is prefixed with indexUUID + // The indexUUID is either a random UUID of length 22, or _na_ + boolean na = uniqueIdentifier.startsWith(ClusterState.UNKNOWN_UUID + "-"); + if (na) { + return ClusterState.UNKNOWN_UUID; } - throw new IllegalArgumentException("Error parsing the IndexMetadata identifier"); + assert uniqueIdentifier.length() >=22; + return uniqueIdentifier.substring(0, 22); } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index f7f41e3c5371c..abb02d4fcaf81 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1262,11 +1262,7 @@ private class IndexSnapshotsDeletion { private final IndexId indexId; private final Set snapshotsWithIndex; private final BlobContainer indexContainer; - - /** - * Maps the Index UUID to its shard count - */ - private final ConcurrentMap indexUUIDToShardCountMap = new ConcurrentHashMap<>(); + private final Set indexUUIDs = new HashSet<>(); IndexSnapshotsDeletion(IndexId indexId) { this.indexId = indexId; @@ -1294,12 +1290,17 @@ private void determineShardCount(ActionListener listener) { .filter(snapshotsWithIndex::contains) .map(id -> originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(id, indexId)) .collect(Collectors.toSet())) { - snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> { - // 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. - String indexUUID = originalRepositoryData.indexMetaDataGenerations().convertBlobIdToIndexUUID(blobId); - if (indexUUIDToShardCountMap.containsKey(indexUUID) == false) { + // 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. + // NB if the index metadata blob is in the pre-7.9.0 format then this will return null + String indexUUID = originalRepositoryData.indexMetaDataGenerations().convertBlobIdToIndexUUID(blobId); + + // Without an index UUID, we don't know if we've encountered this index before and must read it's IndexMetadata + // from heap. If this is a new index UUID, with a possibly higher shard count, then we also need to read + // it's IndexMetadata from heap + if (indexUUID == null || indexUUIDs.add(indexUUID)) { + snapshotExecutor.execute(ActionRunnable.run(listeners.acquire(), () -> { try { IndexMetadata indexMetadata = INDEX_METADATA_FORMAT.read( getProjectRepo(), @@ -1307,9 +1308,7 @@ private void determineShardCount(ActionListener listener) { blobId, namedXContentRegistry ); - int numberOfShards = indexMetadata.getNumberOfShards(); - indexUUIDToShardCountMap.put(indexUUID, numberOfShards); - updateShardCount(numberOfShards); + 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 @@ -1321,12 +1320,8 @@ private void determineShardCount(ActionListener listener) { // extra data anyway. // TODO: Should we fail the delete here? See https://github.com/elastic/elasticsearch/issues/100569. } - } else { - // indexUUIDToShardCountMap is shared across all threads. Therefore, while there may be an entry for this - // UUID in the map, there is no guarantee that we've encountered it in this thread. - updateShardCount(indexUUIDToShardCountMap.get(indexUUID)); - } - })); + })); + } } } } diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index 9ade115ab6029..ff4154e3f21cc 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -9,11 +9,14 @@ package org.elasticsearch.repositories; +import org.elasticsearch.cluster.ClusterState; 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.HashMap; import java.util.Map; import static org.mockito.Mockito.mock; @@ -21,44 +24,62 @@ public class IndexMetaDataGenerationsTests extends ESTestCase { + public void testIndexMetaDataGenerations() { + Map identifiers = new HashMap<>(); + Map lookupInternal = new HashMap<>(); + Map blobUuidToIndexMetadataMap = 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); + blobUuidToIndexMetadataMap.put(blobUUID, metaIdentifier); + + IndexId indexId = new IndexId(randomAlphanumericOfLength(10), indexUUID); + lookupInternal.put(indexId, metaIdentifier); + } + + SnapshotId snapshotId = new SnapshotId(randomAlphanumericOfLength(10), randomUUID()); + Map> lookup = Map.of( + snapshotId, lookupInternal + ); + + IndexMetaDataGenerations generations = new IndexMetaDataGenerations(lookup, identifiers); + + assertEquals(lookup, generations.lookup); + assertEquals(identifiers, generations.identifiers); + assertEquals(blobUuidToIndexMetadataMap, generations.blobUuidToIndexMetadataMap); + } + public void testBuildUniqueIdentifierWithAllFieldsPresent() { - String indexUUID = randomAlphanumericOfLength(randomIntBetween(10, 64)); + String indexUUID = generateUUID(); String historyUUID = randomAlphanumericOfLength(randomIntBetween(10, 64)); long settingsVersion = randomLong(); long mappingVersion = randomLong(); long aliasesVersion = randomLong(); - IndexMetadata indexMetadata = mock(IndexMetadata.class); - when(indexMetadata.getIndexUUID()).thenReturn(indexUUID); - when(indexMetadata.getSettings()).thenReturn(Settings.builder().put(IndexMetadata.SETTING_HISTORY_UUID, historyUUID).build()); - when(indexMetadata.getSettingsVersion()).thenReturn(settingsVersion); - when(indexMetadata.getMappingVersion()).thenReturn(mappingVersion); - when(indexMetadata.getAliasesVersion()).thenReturn(aliasesVersion); + 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 = randomAlphanumericOfLength(randomIntBetween(10, 64)); + String indexUUID = generateUUID(); long settingsVersion = randomLong(); long mappingVersion = randomLong(); long aliasesVersion = randomLong(); - IndexMetadata indexMetadata = mock(IndexMetadata.class); - when(indexMetadata.getIndexUUID()).thenReturn(indexUUID); - when(indexMetadata.getSettings()).thenReturn(Settings.builder().build()); - when(indexMetadata.getSettingsVersion()).thenReturn(settingsVersion); - when(indexMetadata.getMappingVersion()).thenReturn(mappingVersion); - when(indexMetadata.getAliasesVersion()).thenReturn(aliasesVersion); + IndexMetadata indexMetadata = createIndexMetadata(indexUUID, null, settingsVersion, mappingVersion, aliasesVersion); String result = IndexMetaDataGenerations.buildUniqueIdentifier(indexMetadata); assertEquals(indexUUID + "-_na_-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion, result); } public void testConvertBlobIdToIndexUUIDReturnsIndexUUID() { - // May include the - symbol, which as of 9.3.0 is the same symbol as the delimiter - String indexUUID = randomUUID(); + String indexUUID = generateUUID(); String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); long settingsVersion = randomNonNegativeLong(); long mappingsVersion = randomNonNegativeLong(); @@ -80,69 +101,29 @@ public void testConvertBlobIdToIndexUUIDReturnsNullWhenBlobIdIsNotFound() { assertNull(generations.convertBlobIdToIndexUUID(randomAlphanumericOfLength(randomIntBetween(5, 10)))); } - /** - * A helper function that tests whether an IAE is thrown when a malformed IndexMetadata Identifier is passed into - * the {@code convertBlobIdToIndexUUID} function - * @param indexUUID The indexUUID - * @param uniqueIdentifier The malformed identifier - * @param blobId The blobId - */ - private void internalMalformedIndexMetadataIdentifierThrowsIAE(String indexUUID, String uniqueIdentifier, String blobId) { - // Creates the lookup map - SnapshotId snapshotId = new SnapshotId("snapshot", randomUUID()); - IndexId indexId = new IndexId("index", indexUUID); - Map> lookup = Map.of(snapshotId, Map.of(indexId, uniqueIdentifier)); - - IndexMetaDataGenerations malformedGenerations = new IndexMetaDataGenerations(lookup, Map.of(uniqueIdentifier, blobId)); - IllegalArgumentException ex = assertThrows( - IllegalArgumentException.class, - () -> malformedGenerations.convertBlobIdToIndexUUID(blobId) - ); - assertTrue(ex.getMessage().contains("Error parsing the IndexMetadata identifier")); + private String generateUUID() { + return usually() ? UUIDs.randomBase64UUID(random()) : ClusterState.UNKNOWN_UUID; } - public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingIndexUUID() { - String indexUUID = randomUUID(); - String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); - long settingsVersion = randomNonNegativeLong(); - long mappingsVersion = randomNonNegativeLong(); - long aliasesVersion = randomNonNegativeLong(); - // Build the unique identifier without including the index uuid - String uniqueIdentifier = randomSetting + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; - String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); - } - - public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingSettings() { - String indexUUID = randomAlphanumericOfLength(randomIntBetween(5, 10)); - long settingsVersion = randomNonNegativeLong(); - long mappingsVersion = randomNonNegativeLong(); - long aliasesVersion = randomNonNegativeLong(); - // Build the unique identifier without including the settings - String uniqueIdentifier = indexUUID + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; - String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); - } - - public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierIsMissingSettingsVersion() { - String indexUUID = randomUUID(); - String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); - long mappingsVersion = randomNonNegativeLong(); - long aliasesVersion = randomNonNegativeLong(); - // Build the unique identifier without including the settings version - String uniqueIdentifier = indexUUID + "-" + randomSetting + "-" + mappingsVersion + "-" + aliasesVersion; - String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); + private String generateMetaIdentifier(String indexUUID) { + String historyUUID = generateUUID(); + long settingsVersion = randomLong(); + long mappingVersion = randomLong(); + long aliasesVersion = randomLong(); + return indexUUID + "-" + historyUUID + "-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion; } - public void testConvertBlobIdToIndexUUIDThrowsIllegalArgumentExceptionWhenMalformedIndexMetadataIdentifierHasIncorrectTypes() { - String indexUUID = randomUUID(); - String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); - long settingsVersion = randomNonNegativeLong(); - long mappingsVersion = randomNonNegativeLong(); - String aliasesVersion = randomAlphaOfLength(randomIntBetween(5, 10)); - String uniqueIdentifier = indexUUID + "-" + randomSetting + "-" + settingsVersion + "-" + mappingsVersion + "-" + aliasesVersion; - String blobId = randomAlphanumericOfLength(randomIntBetween(5, 10)); - internalMalformedIndexMetadataIdentifierThrowsIAE(indexUUID, uniqueIdentifier, blobId); + 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; } } From ab17c35bfed39ff8f49495a2cd4142693e478378 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Thu, 23 Oct 2025 11:22:51 +0100 Subject: [PATCH 10/17] Clean up --- .../repositories/IndexMetaDataGenerations.java | 16 +++++++++++----- .../blobstore/BlobStoreRepository.java | 1 - .../IndexMetaDataGenerationsTests.java | 12 ++++++++---- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 820d0e624a3a8..49f134fdcc5c8 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -22,8 +22,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; /** @@ -192,12 +190,20 @@ public boolean equals(Object that) { return false; } final IndexMetaDataGenerations other = (IndexMetaDataGenerations) that; - return lookup.equals(other.lookup) && identifiers.equals(other.identifiers) && blobUuidToIndexMetadataMap.equals(other.blobUuidToIndexMetadataMap); + return lookup.equals(other.lookup) + && identifiers.equals(other.identifiers) + && blobUuidToIndexMetadataMap.equals(other.blobUuidToIndexMetadataMap); } @Override public String toString() { - return "IndexMetaDataGenerations{lookup:" + lookup + "}{identifier:" + identifiers + "}{blobUuidToIndexMetadataMap:" + blobUuidToIndexMetadataMap + "}"; + return "IndexMetaDataGenerations{lookup:" + + lookup + + "}{identifier:" + + identifiers + + "}{blobUuidToIndexMetadataMap:" + + blobUuidToIndexMetadataMap + + "}"; } /** @@ -243,7 +249,7 @@ public String convertBlobIdToIndexUUID(String blobId) { if (na) { return ClusterState.UNKNOWN_UUID; } - assert uniqueIdentifier.length() >=22; + assert uniqueIdentifier.length() >= 22; return uniqueIdentifier.substring(0, 22); } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index abb02d4fcaf81..0b8e24ac509e2 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -164,7 +164,6 @@ import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executor; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index ff4154e3f21cc..1331ea7bb858c 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -42,9 +42,7 @@ public void testIndexMetaDataGenerations() { } SnapshotId snapshotId = new SnapshotId(randomAlphanumericOfLength(10), randomUUID()); - Map> lookup = Map.of( - snapshotId, lookupInternal - ); + Map> lookup = Map.of(snapshotId, lookupInternal); IndexMetaDataGenerations generations = new IndexMetaDataGenerations(lookup, identifiers); @@ -113,7 +111,13 @@ private String generateMetaIdentifier(String indexUUID) { return indexUUID + "-" + historyUUID + "-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion; } - private IndexMetadata createIndexMetadata(String indexUUID, String historyUUID, long settingsVersion, long mappingVersion, long 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) { From 10f710df0d96168c12fadd06bc2193f1c01507ac Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Thu, 23 Oct 2025 11:30:51 +0100 Subject: [PATCH 11/17] Further clea up: - Remove unreferenced method - Rename convertBlobIdToIndexUUID to getIndexUUIDFromBlobId - Tweak comments --- .../IndexMetaDataGenerations.java | 21 +++---------------- .../blobstore/BlobStoreRepository.java | 2 +- .../IndexMetaDataGenerationsTests.java | 4 ++-- ...ryShardCountComputedOncePerIndexTests.java | 4 ++-- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 49f134fdcc5c8..7ffe3ff3bf8e6 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -78,21 +78,6 @@ public String getIndexMetaBlobId(String metaIdentifier) { return identifiers.get(metaIdentifier); } - /** - * Returns the index metadata identifier associated with the given blob UUID. - *

- * This method provides a reverse lookup from a blob UUID to its corresponding index metadata identifier. - * If the specified blob UUID is not present, this method returns {@code null}. - *

- * - * @param blobUuid the UUID of the blob whose index metadata identifier is to be retrieved - * @return the index metadata identifier associated with the given blob UUID, or {@code null} if not found - */ - @Nullable - public String getIndexMetadataIdentifierByBlobUuid(String blobUuid) { - return blobUuidToIndexMetadataMap.get(blobUuid); - } - /** * Get the blob id by {@link SnapshotId} and {@link IndexId} and fall back to the value of {@link SnapshotId#getUUID()} if none is * known to enable backwards compatibility with versions older than @@ -215,7 +200,7 @@ public String toString() { * @return identifier string */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { - // If modifying this identifier, then also extend the convertBlobIdToIndexUUID function below + // If modifying this identifier, then also extend the getIndexUUIDFromBlobId function below return indexMetaData.getIndexUUID() + "-" + indexMetaData.getSettings().get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) @@ -236,7 +221,7 @@ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { * @param blobId The blob ID */ @Nullable - public String convertBlobIdToIndexUUID(String blobId) { + public String getIndexUUIDFromBlobId(String blobId) { // Find the unique identifier for this blobId String uniqueIdentifier = blobUuidToIndexMetadataMap.get(blobId); if (uniqueIdentifier == null) { @@ -244,7 +229,7 @@ public String convertBlobIdToIndexUUID(String blobId) { } // The uniqueIdentifier is built in {@code buildUniqueIdentifier}, and is prefixed with indexUUID - // The indexUUID is either a random UUID of length 22, or _na_ + // The indexUUID is either a UUID of length 22, or _na_ boolean na = uniqueIdentifier.startsWith(ClusterState.UNKNOWN_UUID + "-"); if (na) { return ClusterState.UNKNOWN_UUID; diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 0b8e24ac509e2..ef59f66a4c630 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1293,7 +1293,7 @@ private void determineShardCount(ActionListener listener) { // 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. // NB if the index metadata blob is in the pre-7.9.0 format then this will return null - String indexUUID = originalRepositoryData.indexMetaDataGenerations().convertBlobIdToIndexUUID(blobId); + String indexUUID = originalRepositoryData.indexMetaDataGenerations().getIndexUUIDFromBlobId(blobId); // Without an index UUID, we don't know if we've encountered this index before and must read it's IndexMetadata // from heap. If this is a new index UUID, with a possibly higher shard count, then we also need to read diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index 1331ea7bb858c..f1325e0d3161b 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -91,12 +91,12 @@ public void testConvertBlobIdToIndexUUIDReturnsIndexUUID() { Map> lookup = Map.of(snapshotId, Map.of(indexId, uniqueIdentifier)); IndexMetaDataGenerations generations = new IndexMetaDataGenerations(lookup, Map.of(uniqueIdentifier, blobId)); - assertEquals(indexUUID, generations.convertBlobIdToIndexUUID(blobId)); + assertEquals(indexUUID, generations.getIndexUUIDFromBlobId(blobId)); } public void testConvertBlobIdToIndexUUIDReturnsNullWhenBlobIdIsNotFound() { IndexMetaDataGenerations generations = new IndexMetaDataGenerations(Map.of(), Map.of()); - assertNull(generations.convertBlobIdToIndexUUID(randomAlphanumericOfLength(randomIntBetween(5, 10)))); + assertNull(generations.getIndexUUIDFromBlobId(randomAlphanumericOfLength(randomIntBetween(5, 10)))); } private String generateUUID() { diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java index 12f8fb781234c..8d234672a71d7 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -138,7 +138,7 @@ public InputStream readBlob(OperationPurpose purpose, String blobName) throws IO - Generates A indices - Generates M snapshots including these indices - Deletes a subset B of indices - - Recreates the B indices with the same name + - Recreates the B indices with the same name but a different shard count - Generates N subsequent snapshots - Deletes a random subset of snapshots within one request @@ -168,7 +168,7 @@ public void testShardCountComputedOncePerIndexWhenDeletingMultipleSnapshotsConcu - Generates A indices - Generates M snapshots including these indices - Deletes a subset B of indices - - Recreates the B indices with the same name + - Recreates the B indices with the same name but a different shard count - Generates N subsequent snapshots - Deletes a random subset of snapshots within one request From 31de1cb2fcabf02c70b538e84de2b61b7ef19f3d Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Thu, 23 Oct 2025 11:50:09 +0100 Subject: [PATCH 12/17] Refactor Tests --- ...ryShardCountComputedOncePerIndexTests.java | 51 +++++++++---------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java index 8d234672a71d7..bb15c9bb08c93 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryShardCountComputedOncePerIndexTests.java @@ -231,35 +231,32 @@ private List createIndicesAndSnapshots(int numberOfIndices, int numberOf firstBatchOfSnapshotNames ); - List secondBatchOfSnapshotNamesToDelete = new ArrayList<>(); - if (numberOfIndicesRecreated > 0) { - // Now delete a random subset of indices, and then recreate them with the same name but a different shard count - // This will force the new indices to have the same indexId but a different UUID - List indicesToDelete = randomSubsetOf(numberOfIndicesRecreated, indexNames); - for (String indexName : indicesToDelete) { - deleteIndex(indexName); - // Creates a new index with the same name but a different number of shards - createIndex(indexName, indexSettings(between(4, 6), 0).build()); - ensureGreen(indexName); - } + // Now delete a random subset of indices (this can be 0) and then recreate them with the same name but a different shard count + // This will force the new indices to have the same indexId but a different UUID + List indicesToDelete = randomSubsetOf(numberOfIndicesRecreated, indexNames); + for (String indexName : indicesToDelete) { + deleteIndex(indexName); + // Creates a new index with the same name but a different number of shards + createIndex(indexName, indexSettings(between(4, 6), 0).build()); + ensureGreen(indexName); + } - // Do the second batch of snapshots - int numberOfSnapshotsInSecondBatch = randomIntBetween(3, 10); - List secondBatchOfSnapshotNames = new ArrayList<>(); - for (int i = 0; i < numberOfSnapshotsInSecondBatch; i++) { - String snapshotName = "second-snapshot-" + i; - secondBatchOfSnapshotNames.add(snapshotName); - client().admin() - .cluster() - .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) - .setWaitForCompletion(true) - .get(); - } - secondBatchOfSnapshotNamesToDelete = randomSubsetOf( - randomIntBetween(1, numberOfSnapshotsInSecondBatch - 1), - secondBatchOfSnapshotNames - ); + // Do the second batch of snapshots, whether we've modified any indices or not + int numberOfSnapshotsInSecondBatch = randomIntBetween(3, 10); + List secondBatchOfSnapshotNames = new ArrayList<>(); + for (int i = 0; i < numberOfSnapshotsInSecondBatch; i++) { + String snapshotName = "second-snapshot-" + i; + secondBatchOfSnapshotNames.add(snapshotName); + client().admin() + .cluster() + .prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, TEST_REPO_NAME, snapshotName) + .setWaitForCompletion(true) + .get(); } + List secondBatchOfSnapshotNamesToDelete = randomSubsetOf( + randomIntBetween(1, numberOfSnapshotsInSecondBatch - 1), + secondBatchOfSnapshotNames + ); firstBatchOfSnapshotNamesToDelete.addAll(secondBatchOfSnapshotNamesToDelete); return firstBatchOfSnapshotNamesToDelete; From 19104111db2d5d81a0092cc461d176327959d9bd Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Fri, 31 Oct 2025 13:43:00 +0000 Subject: [PATCH 13/17] Comments for IndexMetaDataGenerations --- .../IndexMetaDataGenerations.java | 55 ++++++------------- .../IndexMetaDataGenerationsTests.java | 7 +-- 2 files changed, 20 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 7ffe3ff3bf8e6..7da9bdda72aa0 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -9,7 +9,6 @@ package org.elasticsearch.repositories; -import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.set.Sets; @@ -24,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 @@ -46,20 +47,12 @@ public final class IndexMetaDataGenerations { */ final Map identifiers; - /** - * Map of blob uuid to index metadata identifier. This is a reverse lookup of the identifiers map. - */ - final Map blobUuidToIndexMetadataMap; - IndexMetaDataGenerations(Map> lookup, Map identifiers) { assert identifiers.keySet().equals(lookup.values().stream().flatMap(m -> m.values().stream()).collect(Collectors.toSet())) : "identifier mappings " + identifiers + " don't track the same blob ids as the lookup map " + lookup; assert lookup.values().stream().noneMatch(Map::isEmpty) : "Lookup contained empty map [" + lookup + "]"; this.lookup = Map.copyOf(lookup); this.identifiers = Map.copyOf(identifiers); - this.blobUuidToIndexMetadataMap = identifiers.entrySet() - .stream() - .collect(Collectors.toUnmodifiableMap(Map.Entry::getValue, Map.Entry::getKey)); } public boolean isEmpty() { @@ -163,7 +156,7 @@ public IndexMetaDataGenerations withRemovedSnapshots(Collection snap @Override public int hashCode() { - return Objects.hash(identifiers, lookup, blobUuidToIndexMetadataMap); + return Objects.hash(identifiers, lookup); } @Override @@ -175,20 +168,12 @@ public boolean equals(Object that) { return false; } final IndexMetaDataGenerations other = (IndexMetaDataGenerations) that; - return lookup.equals(other.lookup) - && identifiers.equals(other.identifiers) - && blobUuidToIndexMetadataMap.equals(other.blobUuidToIndexMetadataMap); + return lookup.equals(other.lookup) && identifiers.equals(other.identifiers); } @Override public String toString() { - return "IndexMetaDataGenerations{lookup:" - + lookup - + "}{identifier:" - + identifiers - + "}{blobUuidToIndexMetadataMap:" - + blobUuidToIndexMetadataMap - + "}"; + return "IndexMetaDataGenerations{lookup:" + lookup + "}{identifier:" + identifiers + "}"; } /** @@ -214,27 +199,23 @@ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { /** * Given a blobId, returns the index UUID associated with it. - *

- * If the blobId is not found, returns null. - *

- * If the index UUID is _na_ {@code ClusterState.UNKNOWN_UUID} * @param blobId The blob ID + * @return the index UUID associated with the blobId, or null if the blobId is not found */ @Nullable public String getIndexUUIDFromBlobId(String blobId) { - // Find the unique identifier for this blobId - String uniqueIdentifier = blobUuidToIndexMetadataMap.get(blobId); - if (uniqueIdentifier == null) { - return null; - } + // Map of blob id to index uuid. This is a reverse lookup of the identifiers map. + final Map blobUuidToIndexUUIDMap = 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) + ) + ); - // The uniqueIdentifier is built in {@code buildUniqueIdentifier}, and is prefixed with indexUUID - // The indexUUID is either a UUID of length 22, or _na_ - boolean na = uniqueIdentifier.startsWith(ClusterState.UNKNOWN_UUID + "-"); - if (na) { - return ClusterState.UNKNOWN_UUID; - } - assert uniqueIdentifier.length() >= 22; - return uniqueIdentifier.substring(0, 22); + // Find the unique identifier for this blobId + return blobUuidToIndexUUIDMap.get(blobId); } } diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index f1325e0d3161b..aa0169f0fb4f3 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -27,7 +27,6 @@ public class IndexMetaDataGenerationsTests extends ESTestCase { public void testIndexMetaDataGenerations() { Map identifiers = new HashMap<>(); Map lookupInternal = new HashMap<>(); - Map blobUuidToIndexMetadataMap = new HashMap<>(); int numberOfMetadataIdentifiers = randomIntBetween(5, 10); for (int i = 0; i < numberOfMetadataIdentifiers; i++) { @@ -35,7 +34,6 @@ public void testIndexMetaDataGenerations() { String metaIdentifier = generateMetaIdentifier(indexUUID); String blobUUID = randomAlphanumericOfLength(randomIntBetween(5, 10)); identifiers.put(metaIdentifier, blobUUID); - blobUuidToIndexMetadataMap.put(blobUUID, metaIdentifier); IndexId indexId = new IndexId(randomAlphanumericOfLength(10), indexUUID); lookupInternal.put(indexId, metaIdentifier); @@ -48,7 +46,6 @@ public void testIndexMetaDataGenerations() { assertEquals(lookup, generations.lookup); assertEquals(identifiers, generations.identifiers); - assertEquals(blobUuidToIndexMetadataMap, generations.blobUuidToIndexMetadataMap); } public void testBuildUniqueIdentifierWithAllFieldsPresent() { @@ -76,7 +73,7 @@ public void testBuildUniqueIdentifierWithMissingHistoryUUID() { assertEquals(indexUUID + "-_na_-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion, result); } - public void testConvertBlobIdToIndexUUIDReturnsIndexUUID() { + public void testGetIndexUUIDFromBlobIdReturnsIndexUUID() { String indexUUID = generateUUID(); String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); long settingsVersion = randomNonNegativeLong(); @@ -94,7 +91,7 @@ public void testConvertBlobIdToIndexUUIDReturnsIndexUUID() { assertEquals(indexUUID, generations.getIndexUUIDFromBlobId(blobId)); } - public void testConvertBlobIdToIndexUUIDReturnsNullWhenBlobIdIsNotFound() { + public void testGetIndexUUIDFromBlobIdReturnsNullWhenBlobIdIsNotFound() { IndexMetaDataGenerations generations = new IndexMetaDataGenerations(Map.of(), Map.of()); assertNull(generations.getIndexUUIDFromBlobId(randomAlphanumericOfLength(randomIntBetween(5, 10)))); } From 434b88f8d61a86c0bba59e36a53adf3813c5404c Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 5 Nov 2025 15:03:15 +0000 Subject: [PATCH 14/17] David Comments --- .../repositories/IndexMetaDataGenerations.java | 14 ++++---------- .../blobstore/BlobStoreRepository.java | 9 +++++---- .../IndexMetaDataGenerationsTests.java | 13 ++++++++----- .../elasticsearch/test/ESSingleNodeTestCase.java | 2 -- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 7da9bdda72aa0..c92551425d0c4 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -198,14 +198,11 @@ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { } /** - * Given a blobId, returns the index UUID associated with it. - * @param blobId The blob ID - * @return the index UUID associated with the blobId, or null if the blobId is not found + * Generates a map of blob id to Index UUID. This is a reverse lookup of {@code identifiers} + * @return A map of blob id to index UUID */ - @Nullable - public String getIndexUUIDFromBlobId(String blobId) { - // Map of blob id to index uuid. This is a reverse lookup of the identifiers map. - final Map blobUuidToIndexUUIDMap = identifiers.entrySet() + public Map getBlobIdToIndexUuidMap() { + return identifiers.entrySet() .stream() .collect( Collectors.toUnmodifiableMap( @@ -214,8 +211,5 @@ public String getIndexUUIDFromBlobId(String blobId) { entry -> entry.getKey().substring(0, RANDOM_BASED_UUID_STRING_LENGTH) ) ); - - // Find the unique identifier for this blobId - return blobUuidToIndexUUIDMap.get(blobId); } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 9058e6496e9b3..63750b59eef18 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1312,6 +1312,7 @@ void run(ActionListener listener) { private void determineShardCount(ActionListener listener) { try (var listeners = new RefCountingListener(listener)) { + Map blobIdToindexUuidMap = originalRepositoryData.indexMetaDataGenerations().getBlobIdToIndexUuidMap(); for (final var blobId : snapshotIds.stream() .filter(snapshotsWithIndex::contains) .map(id -> originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(id, indexId)) @@ -1320,11 +1321,11 @@ private void determineShardCount(ActionListener listener) { // 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. // NB if the index metadata blob is in the pre-7.9.0 format then this will return null - String indexUUID = originalRepositoryData.indexMetaDataGenerations().getIndexUUIDFromBlobId(blobId); + String indexUUID = blobIdToindexUuidMap.get(blobId); - // Without an index UUID, we don't know if we've encountered this index before and must read it's IndexMetadata - // from heap. If this is a new index UUID, with a possibly higher shard count, then we also need to read - // it's IndexMetadata from heap + // 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 { diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index aa0169f0fb4f3..1e6f74dcb1786 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.test.ESTestCase; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -73,7 +74,7 @@ public void testBuildUniqueIdentifierWithMissingHistoryUUID() { assertEquals(indexUUID + "-_na_-" + settingsVersion + "-" + mappingVersion + "-" + aliasesVersion, result); } - public void testGetIndexUUIDFromBlobIdReturnsIndexUUID() { + public void testGetBlobIdToIndexUuidMap() { String indexUUID = generateUUID(); String randomSetting = randomAlphaOfLength(randomIntBetween(5, 10)); long settingsVersion = randomNonNegativeLong(); @@ -88,16 +89,18 @@ public void testGetIndexUUIDFromBlobIdReturnsIndexUUID() { Map> lookup = Map.of(snapshotId, Map.of(indexId, uniqueIdentifier)); IndexMetaDataGenerations generations = new IndexMetaDataGenerations(lookup, Map.of(uniqueIdentifier, blobId)); - assertEquals(indexUUID, generations.getIndexUUIDFromBlobId(blobId)); + + Map expectedBlobIdToindexUuidMap = Map.of(blobId, indexUUID); + assertEquals(expectedBlobIdToindexUuidMap, generations.getBlobIdToIndexUuidMap()); } - public void testGetIndexUUIDFromBlobIdReturnsNullWhenBlobIdIsNotFound() { + public void testGetBlobIdToIndexUuidMapWithNoIdentifierMap() { IndexMetaDataGenerations generations = new IndexMetaDataGenerations(Map.of(), Map.of()); - assertNull(generations.getIndexUUIDFromBlobId(randomAlphanumericOfLength(randomIntBetween(5, 10)))); + assertEquals(Collections.emptyMap(), generations.getBlobIdToIndexUuidMap()); } private String generateUUID() { - return usually() ? UUIDs.randomBase64UUID(random()) : ClusterState.UNKNOWN_UUID; + return UUIDs.randomBase64UUID(random()); } private String generateMetaIdentifier(String indexUUID) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java index a71f2b9a2c0a1..b947dfc13ecfe 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java @@ -411,8 +411,6 @@ protected IndexService createIndex(String index, CreateIndexRequestBuilder creat */ protected void deleteIndex(String index) { assertAcked(indicesAdmin().prepareDelete(index).get()); - // Wait for the cluster to be green after deletion - ensureGreen(); } public Index resolveIndex(String index) { From 830d64b710b57dfc6956c71162e6bc131e18c892 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 5 Nov 2025 15:07:57 +0000 Subject: [PATCH 15/17] Fixing comment and variable name --- .../elasticsearch/repositories/IndexMetaDataGenerations.java | 1 - .../repositories/blobstore/BlobStoreRepository.java | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index c92551425d0c4..39f5f4c03088d 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -185,7 +185,6 @@ public String toString() { * @return identifier string */ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { - // If modifying this identifier, then also extend the getIndexUUIDFromBlobId function below return indexMetaData.getIndexUUID() + "-" + indexMetaData.getSettings().get(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 63750b59eef18..18ba1a6662e26 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1312,7 +1312,7 @@ void run(ActionListener listener) { private void determineShardCount(ActionListener listener) { try (var listeners = new RefCountingListener(listener)) { - Map blobIdToindexUuidMap = originalRepositoryData.indexMetaDataGenerations().getBlobIdToIndexUuidMap(); + Map blobIdToIndexUuidMap = originalRepositoryData.indexMetaDataGenerations().getBlobIdToIndexUuidMap(); for (final var blobId : snapshotIds.stream() .filter(snapshotsWithIndex::contains) .map(id -> originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(id, indexId)) @@ -1321,7 +1321,7 @@ private void determineShardCount(ActionListener listener) { // 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. // NB if the index metadata blob is in the pre-7.9.0 format then this will return null - String indexUUID = blobIdToindexUuidMap.get(blobId); + 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 From b8bd53c7800bbcd0133878d2a381ce97f7f37681 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 5 Nov 2025 15:15:37 +0000 Subject: [PATCH 16/17] [CI] Auto commit changes from spotless --- .../repositories/IndexMetaDataGenerationsTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index 1e6f74dcb1786..676de332c7c70 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -9,7 +9,6 @@ package org.elasticsearch.repositories; -import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; From 42a2830283046dfdf6ce9ecb5da292b2ab325225 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 12 Nov 2025 11:17:42 +0000 Subject: [PATCH 17/17] Move blobIdToIndexUuidMap int snapshots deletion --- .../repositories/IndexMetaDataGenerations.java | 2 +- .../repositories/blobstore/BlobStoreRepository.java | 7 ++++++- .../repositories/IndexMetaDataGenerationsTests.java | 1 - 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java index 39f5f4c03088d..c1b3ca100b492 100644 --- a/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java +++ b/server/src/main/java/org/elasticsearch/repositories/IndexMetaDataGenerations.java @@ -197,7 +197,7 @@ public static String buildUniqueIdentifier(IndexMetadata indexMetaData) { } /** - * Generates a map of blob id to Index UUID. This is a reverse lookup of {@code identifiers} + * 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 getBlobIdToIndexUuidMap() { diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 18ba1a6662e26..1de597bbc38d2 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/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 blobIdToIndexUuidMap; + SnapshotsDeletion( Collection snapshotIds, long originalRepositoryDataGeneration, @@ -1140,6 +1145,7 @@ class SnapshotsDeletion { this.originalRootBlobs = originalRootBlobs; this.originalIndexContainers = originalIndexContainers; this.originalRepositoryData = originalRepositoryData; + this.blobIdToIndexUuidMap = originalRepositoryData.indexMetaDataGenerations().getBlobIdToIndexUuidMap(); } // --------------------------------------------------------------------------------------------------------------------------------- @@ -1312,7 +1318,6 @@ void run(ActionListener listener) { private void determineShardCount(ActionListener listener) { try (var listeners = new RefCountingListener(listener)) { - Map blobIdToIndexUuidMap = originalRepositoryData.indexMetaDataGenerations().getBlobIdToIndexUuidMap(); for (final var blobId : snapshotIds.stream() .filter(snapshotsWithIndex::contains) .map(id -> originalRepositoryData.indexMetaDataGenerations().indexMetaBlobId(id, indexId)) diff --git a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java index 1e6f74dcb1786..676de332c7c70 100644 --- a/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/IndexMetaDataGenerationsTests.java @@ -9,7 +9,6 @@ package org.elasticsearch.repositories; -import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings;