Remove IncompatibleSnapshots Logic from Codebase (#44096) (#44183)

* The incompatible snapshots logic was created to track 1.x snapshots that
became incompatible with 2.x
   * It serves no purpose at this point
   * It adds an additional GET request to every loading of
RepositoryData (from loading the incompatible snapshots blob)
This commit is contained in:
Armin Braun 2019-07-11 07:15:51 +02:00 committed by GitHub
parent 4e6cbc2890
commit 8a554f9737
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 25 additions and 190 deletions

View File

@ -130,10 +130,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
final List<SnapshotInfo> snapshotInfos;
if (request.verbose()) {
final Set<SnapshotId> incompatibleSnapshots = repositoryData != null ?
new HashSet<>(repositoryData.getIncompatibleSnapshotIds()) : Collections.emptySet();
snapshotInfos = snapshotsService.snapshots(repository, new ArrayList<>(toResolve),
incompatibleSnapshots, request.ignoreUnavailable());
snapshotInfos = snapshotsService.snapshots(repository, new ArrayList<>(toResolve), request.ignoreUnavailable());
} else {
if (repositoryData != null) {
// want non-current snapshots as well, which are found in the repository data

View File

@ -37,7 +37,6 @@ import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus;
import org.elasticsearch.repositories.RepositoryData;
import org.elasticsearch.snapshots.Snapshot;
import org.elasticsearch.snapshots.SnapshotException;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.snapshots.SnapshotInfo;
import org.elasticsearch.snapshots.SnapshotMissingException;
@ -213,8 +212,6 @@ public class TransportSnapshotsStatusAction extends TransportMasterNodeAction<Sn
} else {
throw new SnapshotMissingException(repositoryName, snapshotName);
}
} else if (repositoryData.getIncompatibleSnapshotIds().contains(snapshotId)) {
throw new SnapshotException(repositoryName, snapshotName, "cannot get the status for an incompatible snapshot");
}
SnapshotInfo snapshotInfo = snapshotsService.snapshot(repositoryName, snapshotId);
List<SnapshotIndexShardStatus> shardStatusBuilder = new ArrayList<>();

View File

@ -23,7 +23,6 @@ import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.snapshots.SnapshotId;
@ -56,7 +55,7 @@ public final class RepositoryData {
* An instance initialized for an empty repository.
*/
public static final RepositoryData EMPTY = new RepositoryData(EMPTY_REPO_GEN,
Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), Collections.emptyList());
Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap());
/**
* The generational id of the index file from which the repository data was read.
@ -78,27 +77,20 @@ public final class RepositoryData {
* The snapshots that each index belongs to.
*/
private final Map<IndexId, Set<SnapshotId>> indexSnapshots;
/**
* The snapshots that are no longer compatible with the current cluster ES version.
*/
private final List<SnapshotId> incompatibleSnapshotIds;
public RepositoryData(long genId,
Map<String, SnapshotId> snapshotIds,
Map<String, SnapshotState> snapshotStates,
Map<IndexId, Set<SnapshotId>> indexSnapshots,
List<SnapshotId> incompatibleSnapshotIds) {
public RepositoryData(long genId, Map<String, SnapshotId> snapshotIds, Map<String, SnapshotState> snapshotStates,
Map<IndexId, Set<SnapshotId>> indexSnapshots) {
this.genId = genId;
this.snapshotIds = Collections.unmodifiableMap(snapshotIds);
this.snapshotStates = Collections.unmodifiableMap(snapshotStates);
this.indices = Collections.unmodifiableMap(indexSnapshots.keySet().stream()
.collect(Collectors.toMap(IndexId::getName, Function.identity())));
this.indexSnapshots = Collections.unmodifiableMap(indexSnapshots);
this.incompatibleSnapshotIds = Collections.unmodifiableList(incompatibleSnapshotIds);
}
protected RepositoryData copy() {
return new RepositoryData(genId, snapshotIds, snapshotStates, indexSnapshots, incompatibleSnapshotIds);
return new RepositoryData(genId, snapshotIds, snapshotStates, indexSnapshots);
}
/**
@ -116,22 +108,10 @@ public final class RepositoryData {
}
/**
* Returns an immutable collection of the snapshot ids in the repository that are incompatible with the
* current ES version.
*/
public Collection<SnapshotId> getIncompatibleSnapshotIds() {
return incompatibleSnapshotIds;
}
/**
* Returns an immutable collection of all the snapshot ids in the repository, both active and
* incompatible snapshots.
* Returns an immutable collection of all the snapshot ids in the repository.
*/
public Collection<SnapshotId> getAllSnapshotIds() {
List<SnapshotId> allSnapshotIds = new ArrayList<>(snapshotIds.size() + incompatibleSnapshotIds.size());
allSnapshotIds.addAll(snapshotIds.values());
allSnapshotIds.addAll(incompatibleSnapshotIds);
return Collections.unmodifiableList(allSnapshotIds);
return new ArrayList<>(snapshotIds.values());
}
/**
@ -182,7 +162,7 @@ public final class RepositoryData {
allIndexSnapshots.put(indexId, ids);
}
}
return new RepositoryData(genId, snapshots, newSnapshotStates, allIndexSnapshots, incompatibleSnapshotIds);
return new RepositoryData(genId, snapshots, newSnapshotStates, allIndexSnapshots);
}
/**
@ -216,7 +196,7 @@ public final class RepositoryData {
indexSnapshots.put(indexId, set);
}
return new RepositoryData(genId, newSnapshotIds, newSnapshotStates, indexSnapshots, incompatibleSnapshotIds);
return new RepositoryData(genId, newSnapshotIds, newSnapshotStates, indexSnapshots);
}
/**
@ -242,13 +222,12 @@ public final class RepositoryData {
return snapshotIds.equals(that.snapshotIds)
&& snapshotStates.equals(that.snapshotStates)
&& indices.equals(that.indices)
&& indexSnapshots.equals(that.indexSnapshots)
&& incompatibleSnapshotIds.equals(that.incompatibleSnapshotIds);
&& indexSnapshots.equals(that.indexSnapshots);
}
@Override
public int hashCode() {
return Objects.hash(snapshotIds, snapshotStates, indices, indexSnapshots, incompatibleSnapshotIds);
return Objects.hash(snapshotIds, snapshotStates, indices, indexSnapshots);
}
/**
@ -297,7 +276,6 @@ public final class RepositoryData {
}
private static final String SNAPSHOTS = "snapshots";
private static final String INCOMPATIBLE_SNAPSHOTS = "incompatible-snapshots";
private static final String INDICES = "indices";
private static final String INDEX_ID = "id";
private static final String NAME = "name";
@ -305,8 +283,7 @@ public final class RepositoryData {
private static final String STATE = "state";
/**
* Writes the snapshots metadata and the related indices metadata to x-content, omitting the
* incompatible snapshots.
* Writes the snapshots metadata and the related indices metadata to x-content.
*/
public XContentBuilder snapshotsToXContent(final XContentBuilder builder) throws IOException {
builder.startObject();
@ -447,49 +424,7 @@ public final class RepositoryData {
} else {
throw new ElasticsearchParseException("start object expected");
}
return new RepositoryData(genId, snapshots, snapshotStates, indexSnapshots, Collections.emptyList());
}
/**
* Writes the incompatible snapshot ids to x-content.
*/
public XContentBuilder incompatibleSnapshotsToXContent(XContentBuilder builder) throws IOException {
builder.startObject();
// write the incompatible snapshots list
builder.startArray(INCOMPATIBLE_SNAPSHOTS);
for (final SnapshotId snapshot : getIncompatibleSnapshotIds()) {
snapshot.toXContent(builder, ToXContent.EMPTY_PARAMS);
}
builder.endArray();
builder.endObject();
return builder;
}
/**
* Reads the incompatible snapshot ids from x-content, loading them into a new instance of {@link RepositoryData}
* that is created from the invoking instance, plus the incompatible snapshots that are read from x-content.
*/
public RepositoryData incompatibleSnapshotsFromXContent(final XContentParser parser) throws IOException {
List<SnapshotId> incompatibleSnapshotIds = new ArrayList<>();
if (parser.nextToken() == XContentParser.Token.START_OBJECT) {
while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
String currentFieldName = parser.currentName();
if (INCOMPATIBLE_SNAPSHOTS.equals(currentFieldName)) {
if (parser.nextToken() == XContentParser.Token.START_ARRAY) {
while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
incompatibleSnapshotIds.add(SnapshotId.fromXContent(parser));
}
} else {
throw new ElasticsearchParseException("expected array for [" + currentFieldName + "]");
}
} else {
throw new ElasticsearchParseException("unknown field name [" + currentFieldName + "]");
}
}
} else {
throw new ElasticsearchParseException("start object expected");
}
return new RepositoryData(this.genId, this.snapshotIds, this.snapshotStates, this.indexSnapshots, incompatibleSnapshotIds);
return new RepositoryData(genId, snapshots, snapshotStates, indexSnapshots);
}
}

View File

@ -172,8 +172,6 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
private static final String INDEX_LATEST_BLOB = "index.latest";
private static final String INCOMPATIBLE_SNAPSHOTS_BLOB = "incompatible-snapshots";
private static final String TESTS_FILE = "tests-";
private static final String METADATA_NAME_FORMAT = "meta-%s.dat";
@ -676,25 +674,6 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
LoggingDeprecationHandler.INSTANCE, blob)) {
repositoryData = RepositoryData.snapshotsFromXContent(parser, indexGen);
}
// now load the incompatible snapshot ids, if they exist
try (InputStream blob = blobContainer().readBlob(INCOMPATIBLE_SNAPSHOTS_BLOB);
XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, blob)) {
repositoryData = repositoryData.incompatibleSnapshotsFromXContent(parser);
} catch (NoSuchFileException e) {
if (isReadOnly()) {
logger.debug("[{}] Incompatible snapshots blob [{}] does not exist, the likely " +
"reason is that there are no incompatible snapshots in the repository",
metadata.name(), INCOMPATIBLE_SNAPSHOTS_BLOB);
} else {
// write an empty incompatible-snapshots blob - we do this so that there
// is a blob present, which helps speed up some cloud-based repositories
// (e.g. S3), which retry if a blob is missing with exponential backoff,
// delaying the read of repository data and sometimes causing a timeout
writeIncompatibleSnapshots(RepositoryData.EMPTY);
}
}
return repositoryData;
} catch (NoSuchFileException ex) {
// repository doesn't have an index blob, its a new blank repo
@ -747,18 +726,6 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
}
}
/**
* Writes the incompatible snapshot ids list to the `incompatible-snapshots` blob in the repository.
*
* Package private for testing.
*/
void writeIncompatibleSnapshots(RepositoryData repositoryData) throws IOException {
assert isReadOnly() == false; // can not write to a read only repository
// write the incompatible snapshots blob
writeAtomic(INCOMPATIBLE_SNAPSHOTS_BLOB,
BytesReference.bytes(repositoryData.incompatibleSnapshotsToXContent(XContentFactory.jsonBuilder())), false);
}
/**
* Get the latest snapshot index blob id. Snapshot index blobs are named index-N, where N is
* the next version number from when the index blob was written. Each individual index-N blob is

View File

@ -178,11 +178,6 @@ public class RestoreService implements ClusterStateApplier {
Repository repository = repositoriesService.repository(repositoryName);
final RepositoryData repositoryData = repository.getRepositoryData();
final String snapshotName = request.snapshot();
final Optional<SnapshotId> incompatibleSnapshotId =
repositoryData.getIncompatibleSnapshotIds().stream().filter(s -> snapshotName.equals(s.getName())).findFirst();
if (incompatibleSnapshotId.isPresent()) {
throw new SnapshotRestoreException(repositoryName, snapshotName, "cannot restore incompatible snapshot");
}
final Optional<SnapshotId> matchingSnapshotId = repositoryData.getSnapshotIds().stream()
.filter(s -> snapshotName.equals(s.getName())).findFirst();
if (matchingSnapshotId.isPresent() == false) {

View File

@ -177,15 +177,11 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
*
* @param repositoryName repository name
* @param snapshotIds snapshots for which to fetch snapshot information
* @param incompatibleSnapshotIds snapshots for which not to fetch snapshot information
* @param ignoreUnavailable if true, snapshots that could not be read will only be logged with a warning,
* if false, they will throw an error
* @return list of snapshots
*/
public List<SnapshotInfo> snapshots(final String repositoryName,
final List<SnapshotId> snapshotIds,
final Set<SnapshotId> incompatibleSnapshotIds,
final boolean ignoreUnavailable) {
public List<SnapshotInfo> snapshots(final String repositoryName, final List<SnapshotId> snapshotIds, final boolean ignoreUnavailable) {
final Set<SnapshotInfo> snapshotSet = new HashSet<>();
final Set<SnapshotId> snapshotIdsToIterate = new HashSet<>(snapshotIds);
// first, look at the snapshots in progress
@ -199,13 +195,7 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
final Repository repository = repositoriesService.repository(repositoryName);
for (SnapshotId snapshotId : snapshotIdsToIterate) {
try {
if (incompatibleSnapshotIds.contains(snapshotId)) {
// an incompatible snapshot - cannot read its snapshot metadata file, just return
// a SnapshotInfo indicating its incompatible
snapshotSet.add(SnapshotInfo.incompatible(snapshotId));
} else {
snapshotSet.add(repository.getSnapshotInfo(snapshotId));
}
snapshotSet.add(repository.getSnapshotInfo(snapshotId));
} catch (Exception ex) {
if (ignoreUnavailable) {
logger.warn(() -> new ParameterizedMessage("failed to get snapshot [{}]", snapshotId), ex);
@ -1099,11 +1089,6 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
// First, look for the snapshot in the repository
final Repository repository = repositoriesService.repository(repositoryName);
final RepositoryData repositoryData = repository.getRepositoryData();
final Optional<SnapshotId> incompatibleSnapshotId =
repositoryData.getIncompatibleSnapshotIds().stream().filter(s -> snapshotName.equals(s.getName())).findFirst();
if (incompatibleSnapshotId.isPresent()) {
throw new SnapshotException(repositoryName, snapshotName, "cannot delete incompatible snapshot");
}
Optional<SnapshotId> matchedEntry = repositoryData.getSnapshotIds()
.stream()
.filter(s -> s.getName().equals(snapshotName))

View File

@ -112,11 +112,10 @@ public class RepositoryDataTests extends ESTestCase {
snapshotStates.put(snapshotId.getUUID(), randomFrom(SnapshotState.values()));
}
RepositoryData repositoryData = new RepositoryData(EMPTY_REPO_GEN, snapshotIds,
Collections.emptyMap(), Collections.emptyMap(), Collections.emptyList());
Collections.emptyMap(), Collections.emptyMap());
// test that initializing indices works
Map<IndexId, Set<SnapshotId>> indices = randomIndices(snapshotIds);
RepositoryData newRepoData = new RepositoryData(repositoryData.getGenId(), snapshotIds, snapshotStates, indices,
new ArrayList<>(repositoryData.getIncompatibleSnapshotIds()));
RepositoryData newRepoData = new RepositoryData(repositoryData.getGenId(), snapshotIds, snapshotStates, indices);
List<SnapshotId> expected = new ArrayList<>(repositoryData.getSnapshotIds());
Collections.sort(expected);
List<SnapshotId> actual = new ArrayList<>(newRepoData.getSnapshotIds());
@ -193,7 +192,7 @@ public class RepositoryDataTests extends ESTestCase {
assertNotNull(corruptedIndexId);
RepositoryData corruptedRepositoryData = new RepositoryData(parsedRepositoryData.getGenId(), snapshotIds, snapshotStates,
indexSnapshots, new ArrayList<>(parsedRepositoryData.getIncompatibleSnapshotIds()));
indexSnapshots);
final XContentBuilder corruptedBuilder = XContentBuilder.builder(xContent);
corruptedRepositoryData.snapshotsToXContent(corruptedBuilder);

View File

@ -197,45 +197,6 @@ public class BlobStoreRepositoryTests extends ESSingleNodeTestCase {
expectThrows(RepositoryException.class, () -> repository.writeIndexGen(repositoryData, repositoryData.getGenId()));
}
public void testReadAndWriteIncompatibleSnapshots() throws Exception {
final BlobStoreRepository repository = setupRepo();
// write to and read from incompatible snapshots file with no entries
assertEquals(0, repository.getRepositoryData().getIncompatibleSnapshotIds().size());
RepositoryData emptyData = RepositoryData.EMPTY;
repository.writeIndexGen(emptyData, emptyData.getGenId());
repository.writeIncompatibleSnapshots(emptyData);
RepositoryData readData = repository.getRepositoryData();
assertEquals(emptyData, readData);
assertEquals(0, readData.getIndices().size());
assertEquals(0, readData.getSnapshotIds().size());
// write to and read from incompatible snapshots with some number of entries
final int numSnapshots = randomIntBetween(1, 20);
final List<SnapshotId> snapshotIds = new ArrayList<>(numSnapshots);
for (int i = 0; i < numSnapshots; i++) {
snapshotIds.add(new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID()));
}
RepositoryData repositoryData = new RepositoryData(readData.getGenId(),
Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), snapshotIds);
repository.blobContainer().deleteBlob("incompatible-snapshots");
repository.writeIncompatibleSnapshots(repositoryData);
readData = repository.getRepositoryData();
assertEquals(repositoryData.getIncompatibleSnapshotIds(), readData.getIncompatibleSnapshotIds());
}
public void testIncompatibleSnapshotsBlobExists() throws Exception {
final BlobStoreRepository repository = setupRepo();
RepositoryData emptyData = RepositoryData.EMPTY;
repository.writeIndexGen(emptyData, emptyData.getGenId());
RepositoryData repoData = repository.getRepositoryData();
assertEquals(emptyData, repoData);
assertTrue(repository.blobContainer().blobExists("incompatible-snapshots"));
repoData = addRandomSnapshotsToRepoData(repository.getRepositoryData(), true);
repository.writeIndexGen(repoData, repoData.getGenId());
assertEquals(0, repository.getRepositoryData().getIncompatibleSnapshotIds().size());
}
public void testBadChunksize() throws Exception {
final Client client = client();
final Path location = ESIntegTestCase.randomRepoPath(node().settings());

View File

@ -499,8 +499,7 @@ public class DedicatedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTest
// (1) index-(N+1)
// (2) index-N (because we keep the previous version) and
// (3) index-latest
// (4) incompatible-snapshots
assertFileCount(repo, 4);
assertFileCount(repo, 3);
logger.info("--> done");
}

View File

@ -1322,8 +1322,8 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas
logger.info("--> delete the last snapshot");
client.admin().cluster().prepareDeleteSnapshot("test-repo", lastSnapshot).get();
logger.info("--> make sure that number of files is back to what it was when the first snapshot was made, " +
"plus two because one backup index-N file should remain and incompatible-snapshots");
assertThat(numberOfFiles(repo), equalTo(numberOfFiles[0] + 2));
"plus one because one backup index-N file should remain");
assertThat(numberOfFiles(repo), equalTo(numberOfFiles[0] + 1));
}
public void testDeleteSnapshotWithMissingIndexAndShardMetadata() throws Exception {
@ -1571,7 +1571,7 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas
createIndex("test-idx", "test-idx-closed");
ensureGreen();
logger.info("--> closing index test-idx-closed");
assertAcked(client.admin().indices().prepareClose("test-idx-closed"));
assertAcked(client.admin().indices().prepareClose("test-idx-closed").setWaitForActiveShards(ActiveShardCount.ALL));
ClusterStateResponse stateResponse = client.admin().cluster().prepareState().get();
assertThat(stateResponse.getState().metaData().index("test-idx-closed").getState(), equalTo(IndexMetaData.State.CLOSE));
assertThat(stateResponse.getState().routingTable().index("test-idx-closed"), notNullValue());

View File

@ -90,7 +90,7 @@ public abstract class RestoreOnlyRepository extends AbstractLifecycleComponent i
public RepositoryData getRepositoryData() {
Map<IndexId, Set<SnapshotId>> map = new HashMap<>();
map.put(new IndexId(indexName, "blah"), emptySet());
return new RepositoryData(EMPTY_REPO_GEN, Collections.emptyMap(), Collections.emptyMap(), map, Collections.emptyList());
return new RepositoryData(EMPTY_REPO_GEN, Collections.emptyMap(), Collections.emptyMap(), map);
}
@Override

View File

@ -245,7 +245,7 @@ public class CcrRepository extends AbstractLifecycleComponent implements Reposit
indexSnapshots.put(new IndexId(indexName, index.getUUID()), Collections.singleton(snapshotId));
}
return new RepositoryData(1, copiedSnapshotIds, snapshotStates, indexSnapshots, Collections.emptyList());
return new RepositoryData(1, copiedSnapshotIds, snapshotStates, indexSnapshots);
}
@Override