From 2fcef9efc8381bc5bf22e20fd1ab80a5fd987878 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 26 Jan 2021 11:26:38 +0100 Subject: [PATCH 1/2] Add ClusterUUID to RepositoryData Record the clusterUUID of the last cluster to write to a repository in the `RepositoryData` and use it for more meaningful logging when running into a concurrent modification issue. --- .../MultiVersionRepositoryAccessIT.java | 37 ++++-- .../CorruptedBlobStoreRepositoryIT.java | 5 +- .../snapshots/MultiClusterRepoAccessIT.java | 5 +- .../repositories/RepositoryData.java | 111 +++++++++++++++--- .../blobstore/BlobStoreRepository.java | 58 ++++++++- .../snapshots/SnapshotsService.java | 17 ++- .../repositories/RepositoryDataTests.java | 15 ++- .../index/shard/RestoreOnlyRepository.java | 3 +- .../blobstore/BlobStoreTestUtil.java | 4 +- .../AbstractSnapshotIntegTestCase.java | 2 + .../xpack/ccr/repository/CcrRepository.java | 3 +- 11 files changed, 214 insertions(+), 46 deletions(-) diff --git a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java index df1d04b52d2be..8f7593ad7d399 100644 --- a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java +++ b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.upgrades; import org.elasticsearch.ElasticsearchStatusException; +import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest; import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; @@ -116,7 +117,7 @@ public void testCreateAndRestoreSnapshot() throws IOException { final int shards = 3; final String index = "test-index"; createIndex(client, index, shards); - createRepository(client, repoName, false); + createRepository(client, repoName, false, true); createSnapshot(client, repoName, "snapshot-" + TEST_STEP, index); final String snapshotToDeleteName = "snapshot-to-delete"; // Create a snapshot and delete it right away again to test the impact of each version's cleanup functionality that is run @@ -161,7 +162,7 @@ public void testReadOnlyRepo() throws IOException { try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { final int shards = 3; final boolean readOnly = TEST_STEP.ordinal() > 1; // only restore from read-only repo in steps 3 and 4 - createRepository(client, repoName, readOnly); + createRepository(client, repoName, readOnly, true); final String index = "test-index"; if (readOnly == false) { createIndex(client, index, shards); @@ -193,13 +194,25 @@ public void testReadOnlyRepo() throws IOException { } } + private static final List> EXPECTED_BWC_EXCEPTIONS = + List.of(ResponseException.class, ElasticsearchStatusException.class); + public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { final String repoName = getTestName(); try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { final int shards = 3; final String index= "test-index"; createIndex(client, index, shards); - createRepository(client, repoName, false); + final Version minNodeVersion = minimumNodeVersion(); + // 7.11.1+ and newer will try to load RepositoryData during repo creation if verify is true, which is impossible in case + // of version incompatibility in the downgrade test step. We verify that it is impossible here and then create the repo using + // verify=false to check behavior on other operations below. + final boolean verify = TEST_STEP != TestStep.STEP3_OLD_CLUSTER || + SnapshotsService.includesClusterUUID(minNodeVersion) || minNodeVersion.onOrBefore(Version.V_7_11_0); + if (verify == false) { + expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> createRepository(client, repoName, false, true)); + } + createRepository(client, repoName, false, verify); // only create some snapshots in the first two steps if (TEST_STEP == TestStep.STEP1_OLD_CLUSTER || TEST_STEP == TestStep.STEP2_NEW_CLUSTER) { createSnapshot(client, repoName, "snapshot-" + TEST_STEP, index); @@ -220,14 +233,12 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { ensureSnapshotRestoreWorks(client, repoName, "snapshot-2", shards); } } else { - if (SnapshotsService.includesRepositoryUuid(minimumNodeVersion()) == false) { + if (SnapshotsService.includesClusterUUID(minNodeVersion) == false) { assertThat(TEST_STEP, is(TestStep.STEP3_OLD_CLUSTER)); - final List> expectedExceptions = - List.of(ResponseException.class, ElasticsearchStatusException.class); - expectThrowsAnyOf(expectedExceptions, () -> listSnapshots(repoName)); - expectThrowsAnyOf(expectedExceptions, () -> deleteSnapshot(client, repoName, "snapshot-1")); - expectThrowsAnyOf(expectedExceptions, () -> deleteSnapshot(client, repoName, "snapshot-2")); - expectThrowsAnyOf(expectedExceptions, () -> createSnapshot(client, repoName, "snapshot-impossible", index)); + expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> listSnapshots(repoName)); + expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> deleteSnapshot(client, repoName, "snapshot-1")); + expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> deleteSnapshot(client, repoName, "snapshot-2")); + expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> createSnapshot(client, repoName, "snapshot-impossible", index)); } else { assertThat(listSnapshots(repoName), hasSize(2)); if (TEST_STEP == TestStep.STEP4_NEW_CLUSTER) { @@ -280,9 +291,11 @@ private static void ensureSnapshotRestoreWorks(RestHighLevelClient client, Strin assertThat(restoreInfo.successfulShards(), equalTo(shards)); } - private static void createRepository(RestHighLevelClient client, String repoName, boolean readOnly) throws IOException { + private static void createRepository(RestHighLevelClient client, String repoName, boolean readOnly, + boolean verify) throws IOException { assertThat(client.snapshot().createRepository(new PutRepositoryRequest(repoName).type("fs").settings( - Settings.builder().put("location", "./" + repoName).put("readonly", readOnly)), RequestOptions.DEFAULT).isAcknowledged(), + Settings.builder().put("location", "./" + repoName).put("readonly", readOnly)).verify(verify), RequestOptions.DEFAULT) + .isAcknowledged(), is(true)); } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java index 1b308afaf63ae..35847a2e70499 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java @@ -214,7 +214,8 @@ public void testHandlingMissingRootLevelSnapshotMetadata() throws Exception { Collections.emptyMap(), Collections.emptyMap(), ShardGenerations.EMPTY, - IndexMetaDataGenerations.EMPTY); + IndexMetaDataGenerations.EMPTY, + RepositoryData.MISSING_UUID); // old-format repository has no cluster UUID Files.write(repo.resolve(BlobStoreRepository.INDEX_FILE_PREFIX + withoutVersions.getGenId()), BytesReference.toBytes(BytesReference.bytes( @@ -346,7 +347,7 @@ public void testRepairBrokenShardGenerations() throws Exception { snapshotIds.values().stream().collect(Collectors.toMap(SnapshotId::getUUID, repositoryData::getVersion)), repositoryData.getIndices().values().stream().collect(Collectors.toMap(Function.identity(), repositoryData::getSnapshots)), ShardGenerations.builder().putAll(repositoryData.shardGenerations()).put(indexId, 0, "0").build(), - repositoryData.indexMetaDataGenerations()); + repositoryData.indexMetaDataGenerations(), repositoryData.getClusterUUID()); Files.write(repoPath.resolve(BlobStoreRepository.INDEX_FILE_PREFIX + repositoryData.getGenId()), BytesReference.toBytes(BytesReference.bytes( brokenRepoData.snapshotsToXContent(XContentFactory.jsonBuilder(), Version.CURRENT))), diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/MultiClusterRepoAccessIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/MultiClusterRepoAccessIT.java index 7d26c05df8b97..1a31f4b958399 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/MultiClusterRepoAccessIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/MultiClusterRepoAccessIT.java @@ -104,7 +104,10 @@ public void testConcurrentDeleteFromOtherCluster() throws InterruptedException { assertThat(sne.getMessage(), containsString("failed to update snapshot in repository")); final RepositoryException cause = (RepositoryException) sne.getCause(); assertThat(cause.getMessage(), containsString("[" + repoNameOnFirstCluster + - "] concurrent modification of the index-N file, expected current generation [2] but it was not found in the repository")); + "] concurrent modification of the index-N file, expected current generation [2] but it was not found in the repository." + + " The last cluster to write to this repository was [" + + secondCluster.client().admin().cluster().prepareState().get().getState().metadata().clusterUUID() + + "] at generation [4].")); assertAcked(client().admin().cluster().prepareDeleteRepository(repoNameOnFirstCluster).get()); createRepository(repoNameOnFirstCluster, "fs", repoPath); createFullSnapshot(repoNameOnFirstCluster, "snap-5"); diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java index de5082ab0cd59..5e2e76d886c34 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java @@ -23,6 +23,7 @@ import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.util.CollectionUtils; @@ -85,13 +86,19 @@ public final class RepositoryData { Collections.emptyMap(), Collections.emptyMap(), ShardGenerations.EMPTY, - IndexMetaDataGenerations.EMPTY); + IndexMetaDataGenerations.EMPTY, + MISSING_UUID); /** * A UUID that identifies this repository. */ private final String uuid; + /** + * Cluster UUID as returned by {@link Metadata#clusterUUID()} of the cluster that wrote this instance to the repository. + */ + private final String clusterUUID; + /** * The generational id of the index file from which the repository data was read. */ @@ -133,7 +140,8 @@ public RepositoryData( Map snapshotVersions, Map> indexSnapshots, ShardGenerations shardGenerations, - IndexMetaDataGenerations indexMetaDataGenerations) { + IndexMetaDataGenerations indexMetaDataGenerations, + String clusterUUID) { this( uuid, genId, @@ -143,7 +151,8 @@ public RepositoryData( indexSnapshots.keySet().stream().collect(Collectors.toUnmodifiableMap(IndexId::getName, Function.identity())), Collections.unmodifiableMap(indexSnapshots), shardGenerations, - indexMetaDataGenerations); + indexMetaDataGenerations, + clusterUUID); } private RepositoryData( @@ -155,7 +164,8 @@ private RepositoryData( Map indices, Map> indexSnapshots, ShardGenerations shardGenerations, - IndexMetaDataGenerations indexMetaDataGenerations) { + IndexMetaDataGenerations indexMetaDataGenerations, + String clusterUUID) { this.uuid = Objects.requireNonNull(uuid); this.genId = genId; this.snapshotIds = snapshotIds; @@ -165,6 +175,7 @@ private RepositoryData( this.shardGenerations = shardGenerations; this.indexMetaDataGenerations = indexMetaDataGenerations; this.snapshotVersions = snapshotVersions; + this.clusterUUID = clusterUUID; assert indices.values().containsAll(shardGenerations.indices()) : "ShardGenerations contained indices " + shardGenerations.indices() + " but snapshots only reference indices " + indices.values(); assert indexSnapshots.values().stream().noneMatch(snapshotIdList -> Set.copyOf(snapshotIdList).size() != snapshotIdList.size()) : @@ -181,7 +192,8 @@ protected RepositoryData copy() { indices, indexSnapshots, shardGenerations, - indexMetaDataGenerations); + indexMetaDataGenerations, + clusterUUID); } /** @@ -204,7 +216,8 @@ public RepositoryData withVersions(Map versions) { indices, indexSnapshots, shardGenerations, - indexMetaDataGenerations); + indexMetaDataGenerations, + clusterUUID); } public ShardGenerations shardGenerations() { @@ -219,6 +232,14 @@ public String getUuid() { return uuid; } + /** + * @return the cluster UUID of the cluster that wrote this instance to the repository or {@link RepositoryData#MISSING_UUID} if this + * instance was written by a cluster older than {@link SnapshotsService#CLUSTER_UUID_IN_REPO_DATA_VERSION}. + */ + public String getClusterUUID() { + return clusterUUID; + } + /** * Gets the generational index file id from which this instance was read. */ @@ -366,7 +387,8 @@ public RepositoryData addSnapshot(final SnapshotId snapshotId, newSnapshotVersions, allIndexSnapshots, ShardGenerations.builder().putAll(this.shardGenerations).putAll(shardGenerations).build(), - newIndexMetaGenerations); + newIndexMetaGenerations, + clusterUUID); } /** @@ -388,7 +410,8 @@ public RepositoryData withGenId(long newGeneration) { indices, indexSnapshots, shardGenerations, - indexMetaDataGenerations); + indexMetaDataGenerations, + clusterUUID); } /** @@ -406,7 +429,8 @@ public RepositoryData withUuid(String uuid) { indices, indexSnapshots, shardGenerations, - indexMetaDataGenerations); + indexMetaDataGenerations, + clusterUUID); } /** @@ -422,7 +446,41 @@ public RepositoryData withoutUuid() { indices, indexSnapshots, shardGenerations, - indexMetaDataGenerations); + indexMetaDataGenerations, + MISSING_UUID); + } + + public RepositoryData withClusterUuid(String clusterUUID) { + assert clusterUUID.equals(MISSING_UUID) == false; + return new RepositoryData( + uuid, + genId, + snapshotIds, + snapshotStates, + snapshotVersions, + indices, + indexSnapshots, + shardGenerations, + indexMetaDataGenerations, + clusterUUID); + } + + /** + * For test purposes, make a copy of this instance with the cluster UUID removed and all other fields unchanged, + * as if from an older version. + */ + public RepositoryData withoutClusterUuid() { + return new RepositoryData( + uuid, + genId, + snapshotIds, + snapshotStates, + snapshotVersions, + indices, + indexSnapshots, + shardGenerations, + indexMetaDataGenerations, + MISSING_UUID); } /** @@ -471,7 +529,8 @@ public RepositoryData removeSnapshots(final Collection snapshots, fi indexSnapshots, ShardGenerations.builder().putAll(shardGenerations).putAll(updatedShardGenerations) .retainIndicesAndPruneDeletes(indexSnapshots.keySet()).build(), - indexMetaDataGenerations.withRemovedSnapshots(snapshots) + indexMetaDataGenerations.withRemovedSnapshots(snapshots), + clusterUUID ); } @@ -559,6 +618,7 @@ public List resolveNewIndices(List indicesToResolve, Map> indexMetaLookup = new HashMap<>(); Map indexMetaIdentifiers = null; String uuid = MISSING_UUID; + String clusterUUID = MISSING_UUID; while (parser.nextToken() == XContentParser.Token.FIELD_NAME) { final String field = parser.currentName(); switch (field) { @@ -722,6 +797,11 @@ public static RepositoryData snapshotsFromXContent(XContentParser parser, long g uuid = parser.text(); assert uuid.equals(MISSING_UUID) == false; break; + case CLUSTER_UUID: + XContentParserUtils.ensureExpectedToken(XContentParser.Token.VALUE_STRING, parser.nextToken(), parser); + clusterUUID = parser.text(); + assert clusterUUID.equals(MISSING_UUID) == false; + break; default: XContentParserUtils.throwUnknownField(field, parser.getTokenLocation()); } @@ -738,7 +818,8 @@ public static RepositoryData snapshotsFromXContent(XContentParser parser, long g snapshotVersions, indexSnapshots, shardGenerations.build(), - buildIndexMetaGenerations(indexMetaLookup, indexLookup, indexMetaIdentifiers)); + buildIndexMetaGenerations(indexMetaLookup, indexLookup, indexMetaIdentifiers), + clusterUUID); } /** 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 2d8f9dc88dc7d..51846153e6192 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1354,7 +1354,7 @@ public void getRepositoryData(ActionListener listener) { assert clusterService.localNode().isMasterNode() : "should only load repository data on master nodes"; if (latestKnownRepoGen.get() == RepositoryData.CORRUPTED_REPO_GEN) { - listener.onFailure(corruptedStateException(null)); + listener.onFailure(corruptedStateException(null, null)); return; } final CachedRepositoryData cached = latestKnownRepositoryData.get(); @@ -1530,8 +1530,15 @@ private void doGetRepositoryData(ActionListener listener) { if (bestEffortConsistency == false && ExceptionsHelper.unwrap(e, NoSuchFileException.class) != null) { // We did not find the expected index-N even though the cluster state continues to point at the missing value // of N so we mark this repository as corrupted. + Tuple previousWriterInformation = null; + try { + previousWriterInformation = readLastWriterInfo(); + } catch (Exception ex) { + e.addSuppressed(ex); + } + final Tuple finalLastInfo = previousWriterInformation; markRepoCorrupted(genToLoad, e, - ActionListener.wrap(v -> listener.onFailure(corruptedStateException(e)), listener::onFailure)); + ActionListener.wrap(v -> listener.onFailure(corruptedStateException(e, finalLastInfo)), listener::onFailure)); } else { listener.onFailure(e); } @@ -1596,14 +1603,21 @@ private BytesReference compressRepoDataForCache(BytesReference uncompressed) { } } - private RepositoryException corruptedStateException(@Nullable Exception cause) { + private RepositoryException corruptedStateException(@Nullable Exception cause, @Nullable Tuple previousWriterInfo) { return new RepositoryException(metadata.name(), "Could not read repository data because the contents of the repository do not match its " + "expected state. This is likely the result of either concurrently modifying the contents of the " + "repository by a process other than this cluster or an issue with the repository's underlying storage. " + "The repository has been disabled to prevent corrupting its contents. To re-enable it " + "and continue using it please remove the repository from the cluster and add it again to make " + - "the cluster recover the known state of the repository from its physical contents.", cause); + "the cluster recover the known state of the repository from its physical contents." + + previousWriterMessage(previousWriterInfo), + cause); + } + + private String previousWriterMessage(@Nullable Tuple previousWriterInfo) { + return previousWriterInfo == null ? "" : " The last cluster to write to this repository was [" + previousWriterInfo.v2() + + "] at generation [" + previousWriterInfo.v1() + "]."; } /** @@ -1874,6 +1888,12 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS } private RepositoryData updateRepositoryData(RepositoryData repositoryData, Version repositoryMetaversion, long newGen) { + if (SnapshotsService.includesClusterUUID(repositoryMetaversion)) { + final String clusterUUID = clusterService.state().metadata().clusterUUID(); + if (repositoryData.getClusterUUID().equals(clusterUUID) == false) { + repositoryData = repositoryData.withClusterUuid(clusterUUID); + } + } if (SnapshotsService.includesRepositoryUuid(repositoryMetaversion) && repositoryData.getUuid().equals(MISSING_UUID)) { return repositoryData.withGenId(newGen).withUuid(UUIDs.randomBase64UUID()); } else { @@ -1911,9 +1931,19 @@ private void maybeWriteIndexLatest(long newGen) { private boolean ensureSafeGenerationExists(long safeGeneration, Consumer onFailure) throws IOException { logger.debug("Ensure generation [{}] that is the basis for this write exists in [{}]", safeGeneration, metadata.name()); if (safeGeneration != RepositoryData.EMPTY_REPO_GEN && blobContainer().blobExists(INDEX_FILE_PREFIX + safeGeneration) == false) { + Tuple previousWriterInfo = null; + Exception readRepoDataEx = null; + try { + previousWriterInfo = readLastWriterInfo(); + } catch (Exception ex) { + readRepoDataEx = ex; + } final Exception exception = new RepositoryException(metadata.name(), - "concurrent modification of the index-N file, expected current generation [" + safeGeneration + - "] but it was not found in the repository"); + "concurrent modification of the index-N file, expected current generation [" + safeGeneration + + "] but it was not found in the repository." + previousWriterMessage(previousWriterInfo)); + if (readRepoDataEx != null) { + exception.addSuppressed(readRepoDataEx); + } markRepoCorrupted(safeGeneration, exception, new ActionListener<>() { @Override public void onResponse(Void aVoid) { @@ -1930,6 +1960,22 @@ public void onFailure(Exception e) { return true; } + /** + * Tries to find the latest cluster UUID that wrote to this repository on a best effort basis by listing out repository root contents + * to find the latest repository generation and then reading the cluster UUID of the last writer from the {@link RepositoryData} found + * at this generation. + * + * @return cluster UUID of the last cluster to write to this repository or {@code null} if none was recorded + */ + @Nullable + private Tuple readLastWriterInfo() throws IOException { + assert bestEffortConsistency == false : "This should only be used for adding information to errors in consistent mode"; + final long latestGeneration = latestIndexBlobId(); + final RepositoryData actualRepositoryData = getRepositoryData(latestGeneration); + return Tuple.tuple(latestGeneration, + actualRepositoryData.getClusterUUID().equals(MISSING_UUID) == false ? actualRepositoryData.getClusterUUID() : null); + } + /** * Updates the repository generation that running deletes and snapshot finalizations will be based on for this repository if any such * operations are found in the cluster state while setting the safe repository generation. diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index bdf12e929cb1d..0f5628e010813 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -134,6 +134,9 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus public static final Version REPOSITORY_UUID_IN_REPO_DATA_VERSION = Version.V_7_12_0; + // TODO: fold this into #REPOSITORY_UUID_IN_REPO_DATA_VERSION and remove separate handling of uuid and clusterUUID BwC where possible + public static final Version CLUSTER_UUID_IN_REPO_DATA_VERSION = Version.V_8_0_0; + public static final Version OLD_SNAPSHOT_FORMAT = Version.V_7_5_0; private static final Logger logger = LogManager.getLogger(SnapshotsService.class); @@ -1738,15 +1741,25 @@ public static boolean useIndexGenerations(Version repositoryMetaVersion) { } /** - * Checks whether the metadata version supports writing {@link ShardGenerations} to the repository. + * Checks whether the metadata version supports writing a repository uuid to the repository. * * @param repositoryMetaVersion version to check - * @return true if version supports {@link ShardGenerations} + * @return true if version supports writing repository uuid to the repository */ public static boolean includesRepositoryUuid(Version repositoryMetaVersion) { return repositoryMetaVersion.onOrAfter(REPOSITORY_UUID_IN_REPO_DATA_VERSION); } + /** + * Checks whether the metadata version supports writing the cluster uuid to the repository. + * + * @param repositoryMetaVersion version to check + * @return true if version supports {@link ShardGenerations} + */ + public static boolean includesClusterUUID(Version repositoryMetaVersion) { + return repositoryMetaVersion.onOrAfter(CLUSTER_UUID_IN_REPO_DATA_VERSION); + } + /** Deletes snapshot from repository * * @param deleteEntry delete entry in cluster state diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java index 1b649181ce752..33daa167da47b 100644 --- a/server/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java @@ -76,7 +76,8 @@ public void testIndicesToUpdateAfterRemovingSnapshot() { } public void testXContent() throws IOException { - RepositoryData repositoryData = generateRandomRepoData().withUuid(UUIDs.randomBase64UUID()); + RepositoryData repositoryData = + generateRandomRepoData().withUuid(UUIDs.randomBase64UUID(random())).withClusterUuid(UUIDs.randomBase64UUID(random())); XContentBuilder builder = JsonXContent.contentBuilder(); repositoryData.snapshotsToXContent(builder, Version.CURRENT); try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) { @@ -149,7 +150,8 @@ public void testInitIndices() { Collections.emptyMap(), Collections.emptyMap(), ShardGenerations.EMPTY, - IndexMetaDataGenerations.EMPTY); + IndexMetaDataGenerations.EMPTY, + MISSING_UUID); // test that initializing indices works Map> indices = randomIndices(snapshotIds); RepositoryData newRepoData = new RepositoryData( @@ -160,7 +162,8 @@ public void testInitIndices() { snapshotVersions, indices, ShardGenerations.EMPTY, - IndexMetaDataGenerations.EMPTY); + IndexMetaDataGenerations.EMPTY, + UUIDs.randomBase64UUID(random())); List expected = new ArrayList<>(repositoryData.getSnapshotIds()); Collections.sort(expected); List actual = new ArrayList<>(newRepoData.getSnapshotIds()); @@ -206,7 +209,8 @@ public void testGetSnapshotState() { public void testIndexThatReferencesAnUnknownSnapshot() throws IOException { final XContent xContent = randomFrom(XContentType.values()).xContent(); - final RepositoryData repositoryData = generateRandomRepoData().withUuid(UUIDs.randomBase64UUID()); + final RepositoryData repositoryData = + generateRandomRepoData().withUuid(UUIDs.randomBase64UUID()).withClusterUuid(UUIDs.randomBase64UUID(random())); XContentBuilder builder = XContentBuilder.builder(xContent); repositoryData.snapshotsToXContent(builder, Version.CURRENT); @@ -251,7 +255,8 @@ public void testIndexThatReferencesAnUnknownSnapshot() throws IOException { snapshotVersions, indexSnapshots, shardGenBuilder.build(), - IndexMetaDataGenerations.EMPTY); + IndexMetaDataGenerations.EMPTY, + UUIDs.randomBase64UUID(random())); final XContentBuilder corruptedBuilder = XContentBuilder.builder(xContent); corruptedRepositoryData.snapshotsToXContent(corruptedBuilder, Version.CURRENT); diff --git a/test/framework/src/main/java/org/elasticsearch/index/shard/RestoreOnlyRepository.java b/test/framework/src/main/java/org/elasticsearch/index/shard/RestoreOnlyRepository.java index e532968439d69..d41a1b59bbf2e 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/shard/RestoreOnlyRepository.java +++ b/test/framework/src/main/java/org/elasticsearch/index/shard/RestoreOnlyRepository.java @@ -101,7 +101,8 @@ public void getRepositoryData(ActionListener listener) { Collections.emptyMap(), Collections.singletonMap(indexId, emptyList()), ShardGenerations.EMPTY, - IndexMetaDataGenerations.EMPTY)); + IndexMetaDataGenerations.EMPTY, + MISSING_UUID)); } @Override diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java index da385bdae95ed..19523fd5c81f6 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java @@ -35,6 +35,7 @@ import org.elasticsearch.cluster.service.ClusterApplierService; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetadata; import org.elasticsearch.common.blobstore.BlobPath; @@ -69,6 +70,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; +import static org.apache.lucene.util.LuceneTestCase.random; import static org.elasticsearch.test.ESTestCase.buildNewFakeTransportAddress; import static org.elasticsearch.test.ESTestCase.randomIntBetween; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -290,7 +292,7 @@ public static ClusterService mockClusterService() { */ public static ClusterService mockClusterService(RepositoryMetadata metadata) { return mockClusterService(ClusterState.builder(ClusterState.EMPTY_STATE).metadata( - Metadata.builder().putCustom(RepositoriesMetadata.TYPE, + Metadata.builder().clusterUUID(UUIDs.randomBase64UUID(random())).putCustom(RepositoriesMetadata.TYPE, new RepositoriesMetadata(Collections.singletonList(metadata))).build()).build()); } diff --git a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java index fb234fcf3f5e6..25eb5a8c111d0 100644 --- a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -159,6 +159,8 @@ protected RepositoryData getRepositoryData(String repoName, Version version) { final RepositoryData repositoryData = getRepositoryData(repoName); if (SnapshotsService.includesRepositoryUuid(version) == false) { return repositoryData.withoutUuid(); + } else if (SnapshotsService.includesClusterUUID(version) == false) { + return repositoryData.withoutClusterUuid(); } else { return repositoryData; } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java index 001721baea9cf..054e0f74a9d1b 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java @@ -265,7 +265,8 @@ public void getRepositoryData(ActionListener listener) { snapshotVersions, indexSnapshots, ShardGenerations.EMPTY, - IndexMetaDataGenerations.EMPTY); + IndexMetaDataGenerations.EMPTY, + MISSING_UUID); }); } From c3bdf2b8381e59c5f2f6f1d6c1c3b3022f552882 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 28 Jan 2021 11:17:18 +0100 Subject: [PATCH 2/2] CR: nits --- .../s3/S3BlobStoreRepositoryTests.java | 2 +- .../MultiVersionRepositoryAccessIT.java | 10 +++++----- .../repositories/RepositoryData.java | 20 ++++++++++++++++++- .../blobstore/BlobStoreRepository.java | 6 ++---- .../AbstractSnapshotIntegTestCase.java | 2 +- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/plugins/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java b/plugins/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java index bb3222fa080c7..168c130c52c7b 100644 --- a/plugins/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java +++ b/plugins/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java @@ -158,7 +158,7 @@ public void testEnforcedCooldownPeriod() throws IOException { final BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(repoName); final RepositoryData repositoryData = getRepositoryData(repository); final RepositoryData modifiedRepositoryData = repositoryData.withVersions(Collections.singletonMap(fakeOldSnapshot, - SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.minimumCompatibilityVersion())).withoutUuid(); + SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.minimumCompatibilityVersion())).withoutRepositoryUUID().withoutClusterUUID(); final BytesReference serialized = BytesReference.bytes(modifiedRepositoryData.snapshotsToXContent(XContentFactory.jsonBuilder(), SnapshotsService.OLD_SNAPSHOT_FORMAT)); PlainActionFuture.get(f -> repository.threadPool().generic().execute(ActionRunnable.run(f, () -> diff --git a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java index 8f7593ad7d399..f2f45fb42f24b 100644 --- a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java +++ b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java @@ -204,11 +204,11 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { final String index= "test-index"; createIndex(client, index, shards); final Version minNodeVersion = minimumNodeVersion(); - // 7.11.1+ and newer will try to load RepositoryData during repo creation if verify is true, which is impossible in case - // of version incompatibility in the downgrade test step. We verify that it is impossible here and then create the repo using - // verify=false to check behavior on other operations below. - final boolean verify = TEST_STEP != TestStep.STEP3_OLD_CLUSTER || - SnapshotsService.includesClusterUUID(minNodeVersion) || minNodeVersion.onOrBefore(Version.V_7_11_0); + // 7.12.0+ will try to load RepositoryData during repo creation if verify is true, which is impossible in case of version + // incompatibility in the downgrade test step. We verify that it is impossible here and then create the repo using verify=false + // to check behavior on other operations below. + final boolean verify = TEST_STEP != TestStep.STEP3_OLD_CLUSTER || SnapshotsService.includesClusterUUID(minNodeVersion) + || minNodeVersion.before(Version.V_7_12_0); if (verify == false) { expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> createRepository(client, repoName, false, true)); } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java index 5e2e76d886c34..960b48791d0e4 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java @@ -436,7 +436,7 @@ public RepositoryData withUuid(String uuid) { /** * For test purposes, make a copy of this instance with the UUID removed and all other fields unchanged, as if from an older version. */ - public RepositoryData withoutUuid() { + public RepositoryData withoutRepositoryUUID() { return new RepositoryData( MISSING_UUID, genId, @@ -447,6 +447,24 @@ public RepositoryData withoutUuid() { indexSnapshots, shardGenerations, indexMetaDataGenerations, + clusterUUID); + } + + /** + * For test purposes, make a copy of this instance with the cluster UUID removed and all other fields unchanged, as if from an older + * version. + */ + public RepositoryData withoutClusterUUID() { + return new RepositoryData( + uuid, + genId, + snapshotIds, + snapshotStates, + snapshotVersions, + indices, + indexSnapshots, + shardGenerations, + indexMetaDataGenerations, MISSING_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 dd311ff8d1a07..56938133d6ef7 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1975,15 +1975,13 @@ public void onFailure(Exception e) { * to find the latest repository generation and then reading the cluster UUID of the last writer from the {@link RepositoryData} found * at this generation. * - * @return cluster UUID of the last cluster to write to this repository or {@code null} if none was recorded + * @return tuple of repository generation and cluster UUID of the last cluster to write to this repository */ - @Nullable private Tuple readLastWriterInfo() throws IOException { assert bestEffortConsistency == false : "This should only be used for adding information to errors in consistent mode"; final long latestGeneration = latestIndexBlobId(); final RepositoryData actualRepositoryData = getRepositoryData(latestGeneration); - return Tuple.tuple(latestGeneration, - actualRepositoryData.getClusterUUID().equals(MISSING_UUID) == false ? actualRepositoryData.getClusterUUID() : null); + return Tuple.tuple(latestGeneration, actualRepositoryData.getClusterUUID()); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java index 25eb5a8c111d0..0df38dc732e8b 100644 --- a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -158,7 +158,7 @@ protected void disableRepoConsistencyCheck(String reason) { protected RepositoryData getRepositoryData(String repoName, Version version) { final RepositoryData repositoryData = getRepositoryData(repoName); if (SnapshotsService.includesRepositoryUuid(version) == false) { - return repositoryData.withoutUuid(); + return repositoryData.withoutRepositoryUUID().withoutClusterUUID(); } else if (SnapshotsService.includesClusterUUID(version) == false) { return repositoryData.withoutClusterUuid(); } else {