Skip to content

Commit

Permalink
Simplify BwC for UUIDs in RepositoryData (elastic#69335)
Browse files Browse the repository at this point in the history
We don't need to separately handle BwC for these two fields since they
were both introduced in `7.12`.
  • Loading branch information
original-brownbear committed Feb 22, 2021
1 parent cdeda10 commit 25ad020
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,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())).withoutRepositoryUUID().withoutClusterUUID();
SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.minimumCompatibilityVersion())).withoutUUIDs();
final BytesReference serialized = BytesReference.bytes(modifiedRepositoryData.snapshotsToXContent(XContentFactory.jsonBuilder(),
SnapshotsService.OLD_SNAPSHOT_FORMAT));
PlainActionFuture.get(f -> repository.threadPool().generic().execute(ActionRunnable.run(f, () ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException {
// 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)
final boolean verify = TEST_STEP != TestStep.STEP3_OLD_CLUSTER || SnapshotsService.includesUUIDs(minNodeVersion)
|| minNodeVersion.before(Version.V_7_12_0);
if (verify == false) {
expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> createRepository(client, repoName, false, true));
Expand All @@ -221,7 +221,7 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException {
ensureSnapshotRestoreWorks(repoName, "snapshot-2", shards);
}
} else {
if (SnapshotsService.includesClusterUUID(minNodeVersion) == false) {
if (SnapshotsService.includesUUIDs(minNodeVersion) == false) {
assertThat(TEST_STEP, is(TestStep.STEP3_OLD_CLUSTER));
expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> listSnapshots(repoName));
expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> deleteSnapshot(client, repoName, "snapshot-1"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public String type() {
/**
* Return the repository UUID, if set and known. The repository UUID is stored in the repository and typically populated here when the
* repository is registered or when we write to it. It may not be set if the repository is maintaining support for versions before
* {@link SnapshotsService#REPOSITORY_UUID_IN_REPO_DATA_VERSION}. It may not be known if the repository was registered with {@code
* {@link SnapshotsService#UUIDS_IN_REPO_DATA_VERSION}. It may not be known if the repository was registered with {@code
* ?verify=false} and has had no subsequent writes. Consumers may, if desired, try and fill in a missing value themselves by retrieving
* the {@link RepositoryData} and calling {@link org.elasticsearch.repositories.RepositoriesService#updateRepositoryUuidInMetadata}.
*
Expand Down Expand Up @@ -132,7 +132,7 @@ public long pendingGeneration() {

public RepositoryMetadata(StreamInput in) throws IOException {
name = in.readString();
if (in.getVersion().onOrAfter(SnapshotsService.REPOSITORY_UUID_IN_REPO_DATA_VERSION)) {
if (in.getVersion().onOrAfter(SnapshotsService.UUIDS_IN_REPO_DATA_VERSION)) {
uuid = in.readString();
} else {
uuid = RepositoryData.MISSING_UUID;
Expand All @@ -156,7 +156,7 @@ public RepositoryMetadata(StreamInput in) throws IOException {
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(name);
if (out.getVersion().onOrAfter(SnapshotsService.REPOSITORY_UUID_IN_REPO_DATA_VERSION)) {
if (out.getVersion().onOrAfter(SnapshotsService.UUIDS_IN_REPO_DATA_VERSION)) {
out.writeString(uuid);
}
out.writeString(type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ private RepositoryData(
this.shardGenerations = shardGenerations;
this.indexMetaDataGenerations = indexMetaDataGenerations;
this.snapshotVersions = snapshotVersions;
this.clusterUUID = clusterUUID;
this.clusterUUID = Objects.requireNonNull(clusterUUID);
assert uuid.equals(MISSING_UUID) == clusterUUID.equals(MISSING_UUID) : "Either repository- and cluster UUID must both be missing" +
" or neither of them must be missing but saw [" + uuid + "][" + 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 -> new HashSet<>(snapshotIdList).size() != snapshotIdList.size()) :
Expand Down Expand Up @@ -215,15 +217,15 @@ public ShardGenerations shardGenerations() {

/**
* @return The UUID of this repository, or {@link RepositoryData#MISSING_UUID} if this repository has no UUID because it still
* supports access from versions earlier than {@link SnapshotsService#REPOSITORY_UUID_IN_REPO_DATA_VERSION}.
* supports access from versions earlier than {@link SnapshotsService#UUIDS_IN_REPO_DATA_VERSION}.
*/
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}.
* instance was written by a cluster older than {@link SnapshotsService#UUIDS_IN_REPO_DATA_VERSION}.
*/
public String getClusterUUID() {
return clusterUUID;
Expand Down Expand Up @@ -404,28 +406,10 @@ public RepositoryData withGenId(long newGeneration) {
}

/**
* Make a copy of this instance with the given UUID and all other fields unchanged.
*/
public RepositoryData withUuid(String uuid) {
assert this.uuid.equals(MISSING_UUID) : this.uuid;
assert uuid.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 UUID removed and all other fields unchanged, as if from an older version.
* For test purposes, make a copy of this instance with the cluster- and repository UUIDs removed and all other fields unchanged,
* as if from an older version.
*/
public RepositoryData withoutRepositoryUUID() {
public RepositoryData withoutUUIDs() {
return new RepositoryData(
MISSING_UUID,
genId,
Expand All @@ -436,31 +420,13 @@ public RepositoryData withoutRepositoryUUID() {
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);
}

public RepositoryData withClusterUuid(String clusterUUID) {
assert clusterUUID.equals(MISSING_UUID) == false;
return new RepositoryData(
uuid,
uuid.equals(MISSING_UUID) ? UUIDs.randomBase64UUID() : uuid,
genId,
snapshotIds,
snapshotStates,
Expand All @@ -472,24 +438,6 @@ public RepositoryData withClusterUuid(String clusterUUID) {
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);
}

/**
* Remove snapshots and remove any indices that no longer exist in the repository due to the deletion of the snapshots.
*
Expand Down Expand Up @@ -647,24 +595,20 @@ public XContentBuilder snapshotsToXContent(
final Version repoMetaVersion,
boolean permitMissingUuid) throws IOException {

final boolean shouldWriteClusterId = SnapshotsService.includesClusterUUID(repoMetaVersion);
final boolean shouldWriteRepoUuid = SnapshotsService.includesRepositoryUuid(repoMetaVersion);
final boolean shouldWriteUUIDS = SnapshotsService.includesUUIDs(repoMetaVersion);
final boolean shouldWriteIndexGens = SnapshotsService.useIndexGenerations(repoMetaVersion);
final boolean shouldWriteShardGens = SnapshotsService.useShardGenerations(repoMetaVersion);

assert Boolean.compare(shouldWriteClusterId, shouldWriteRepoUuid) <= 0;
assert Boolean.compare(shouldWriteRepoUuid, shouldWriteIndexGens) <= 0;
assert Boolean.compare(shouldWriteUUIDS, shouldWriteIndexGens) <= 0;
assert Boolean.compare(shouldWriteIndexGens, shouldWriteShardGens) <= 0;

builder.startObject();

if (shouldWriteShardGens) {
// Add min version field to make it impossible for older ES versions to deserialize this object
final Version minVersion;
if (shouldWriteClusterId) {
minVersion = SnapshotsService.CLUSTER_UUID_IN_REPO_DATA_VERSION;
} else if (shouldWriteRepoUuid) {
minVersion = SnapshotsService.REPOSITORY_UUID_IN_REPO_DATA_VERSION;
if (shouldWriteUUIDS) {
minVersion = SnapshotsService.UUIDS_IN_REPO_DATA_VERSION;
} else if (shouldWriteIndexGens) {
minVersion = SnapshotsService.INDEX_GEN_IN_REPO_DATA_VERSION;
} else {
Expand All @@ -673,23 +617,19 @@ public XContentBuilder snapshotsToXContent(
builder.field(MIN_VERSION, minVersion.toString());
}

if (shouldWriteRepoUuid) {
if (shouldWriteUUIDS) {
if (uuid.equals(MISSING_UUID)) {
assert permitMissingUuid : "missing uuid";
} else {
builder.field(UUID, uuid);
}
} else {
assert uuid.equals(MISSING_UUID) : "lost uuid " + uuid;
}

if (shouldWriteClusterId) {
if (clusterUUID.equals(MISSING_UUID)) {
assert permitMissingUuid : "missing clusterUUID";
} else {
builder.field(CLUSTER_UUID, clusterUUID);
}
} else {
assert uuid.equals(MISSING_UUID) : "lost uuid " + uuid;
assert clusterUUID.equals(MISSING_UUID) : "lost clusterUUID " + clusterUUID;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@
import java.util.stream.Stream;

import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo.canonicalName;
import static org.elasticsearch.repositories.RepositoryData.MISSING_UUID;

/**
* BlobStore - based implementation of Snapshot Repository
Expand Down Expand Up @@ -1923,17 +1922,13 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
}

private RepositoryData updateRepositoryData(RepositoryData repositoryData, Version repositoryMetaversion, long newGen) {
if (SnapshotsService.includesClusterUUID(repositoryMetaversion)) {
if (SnapshotsService.includesUUIDs(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 {
return repositoryData.withGenId(newGen);
}
return repositoryData.withGenId(newGen);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,7 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus

public static final Version INDEX_GEN_IN_REPO_DATA_VERSION = Version.V_7_9_0;

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_7_12_0;
public static final Version UUIDS_IN_REPO_DATA_VERSION = Version.V_7_12_0;

public static final Version OLD_SNAPSHOT_FORMAT = Version.V_7_5_0;

Expand Down Expand Up @@ -2443,23 +2440,13 @@ public static boolean useIndexGenerations(Version repositoryMetaVersion) {
}

/**
* Checks whether the metadata version supports writing a repository uuid to the repository.
*
* @param repositoryMetaVersion version to check
* @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.
* Checks whether the metadata version supports writing the cluster- and repository-uuid to the repository.
*
* @param repositoryMetaVersion version to check
* @return true if version supports {@link ShardGenerations}
* @return true if version supports writing cluster- and repository-uuid to the repository
*/
public static boolean includesClusterUUID(Version repositoryMetaVersion) {
return repositoryMetaVersion.onOrAfter(CLUSTER_UUID_IN_REPO_DATA_VERSION);
public static boolean includesUUIDs(Version repositoryMetaVersion) {
return repositoryMetaVersion.onOrAfter(UUIDS_IN_REPO_DATA_VERSION);
}

/** Deletes snapshot from repository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ public void testIndicesToUpdateAfterRemovingSnapshot() {
}

public void testXContent() throws IOException {
RepositoryData repositoryData =
generateRandomRepoData().withUuid(UUIDs.randomBase64UUID(random())).withClusterUuid(UUIDs.randomBase64UUID(random()));
RepositoryData repositoryData = generateRandomRepoData().withClusterUuid(UUIDs.randomBase64UUID(random()));
XContentBuilder builder = JsonXContent.contentBuilder();
repositoryData.snapshotsToXContent(builder, Version.CURRENT);
try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
Expand Down Expand Up @@ -144,7 +143,7 @@ public void testInitIndices() {
// test that initializing indices works
Map<IndexId, List<SnapshotId>> indices = randomIndices(snapshotIds);
RepositoryData newRepoData = new RepositoryData(
repositoryData.getUuid(),
UUIDs.randomBase64UUID(random()),
repositoryData.getGenId(),
snapshotIds,
snapshotStates,
Expand Down Expand Up @@ -198,8 +197,7 @@ public void testGetSnapshotState() {

public void testIndexThatReferencesAnUnknownSnapshot() throws IOException {
final XContent xContent = randomFrom(XContentType.values()).xContent();
final RepositoryData repositoryData =
generateRandomRepoData().withUuid(UUIDs.randomBase64UUID()).withClusterUuid(UUIDs.randomBase64UUID(random()));
final RepositoryData repositoryData = generateRandomRepoData().withClusterUuid(UUIDs.randomBase64UUID(random()));

XContentBuilder builder = XContentBuilder.builder(xContent);
repositoryData.snapshotsToXContent(builder, Version.CURRENT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,8 @@ protected void disableRepoConsistencyCheck(String reason) {

protected RepositoryData getRepositoryData(String repoName, Version version) {
final RepositoryData repositoryData = getRepositoryData(repoName);
if (SnapshotsService.includesRepositoryUuid(version) == false) {
return repositoryData.withoutRepositoryUUID().withoutClusterUUID();
} else if (SnapshotsService.includesClusterUUID(version) == false) {
return repositoryData.withoutClusterUuid();
if (SnapshotsService.includesUUIDs(version) == false) {
return repositoryData.withoutUUIDs();
} else {
return repositoryData;
}
Expand Down

0 comments on commit 25ad020

Please sign in to comment.