Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not load global state when deleting a snapshot #29278

Merged
merged 2 commits into from
Mar 29, 2018
Merged

Do not load global state when deleting a snapshot #29278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a258158
Do not load global state when deleting a snapshot
tlrx Mar 27, 2018
0b859ba
Add test
tlrx Mar 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 29 additions & 62 deletions server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,27 +341,17 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) {
if (isReadOnly()) {
throw new RepositoryException(metadata.name(), "cannot delete snapshot from a readonly repository");
}

final RepositoryData repositoryData = getRepositoryData();
List<String> indices = Collections.emptyList();
SnapshotInfo snapshot = null;
try {
snapshot = getSnapshotInfo(snapshotId);
indices = snapshot.indices();
} catch (SnapshotMissingException ex) {
throw ex;
} catch (IllegalStateException | SnapshotException | ElasticsearchParseException ex) {
logger.warn(() -> new ParameterizedMessage("cannot read snapshot file [{}]", snapshotId), ex);
}
MetaData metaData = null;
try {
if (snapshot != null) {
metaData = readSnapshotMetaData(snapshotId, snapshot.version(), repositoryData.resolveIndices(indices), true);
} else {
metaData = readSnapshotMetaData(snapshotId, null, repositoryData.resolveIndices(indices), true);
}
} catch (IOException | SnapshotException ex) {
logger.warn(() -> new ParameterizedMessage("cannot read metadata for snapshot [{}]", snapshotId), ex);
}

try {
// Delete snapshot from the index file, since it is the maintainer of truth of active snapshots
final RepositoryData updatedRepositoryData = repositoryData.removeSnapshot(snapshotId);
Expand All @@ -373,24 +363,29 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) {
deleteGlobalMetaDataBlobIgnoringErrors(snapshot, snapshotId.getUUID());

// Now delete all indices
for (String index : indices) {
final IndexId indexId = repositoryData.resolveIndexId(index);
BlobPath indexPath = basePath().add("indices").add(indexId.getId());
BlobContainer indexMetaDataBlobContainer = blobStore().blobContainer(indexPath);
try {
indexMetaDataFormat.delete(indexMetaDataBlobContainer, snapshotId.getUUID());
} catch (IOException ex) {
logger.warn(() -> new ParameterizedMessage("[{}] failed to delete metadata for index [{}]", snapshotId, index), ex);
}
if (metaData != null) {
IndexMetaData indexMetaData = metaData.index(index);
if (snapshot != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before, it was possible to even do the deletion when snapshot was null here (i.e. the file could not be read)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have not correctly read the previous code but when the snapshot metadata cannot be read, snapshot was null and indices was an empty list. So the index files were not deleted. This behavior is kept in this pull request, the snapshot != null just make it more obvious (I think).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I misread the code.

final List<String> indices = snapshot.indices();
for (String index : indices) {
final IndexId indexId = repositoryData.resolveIndexId(index);

IndexMetaData indexMetaData = null;
try {
indexMetaData = getSnapshotIndexMetaData(snapshotId, indexId);
} catch (ElasticsearchParseException | IOException ex) {
logger.warn(() ->
new ParameterizedMessage("[{}] [{}] failed to read metadata for index", snapshotId, index), ex);
}

deleteIndexMetaDataBlobIgnoringErrors(snapshot, indexId);

if (indexMetaData != null) {
for (int shardId = 0; shardId < indexMetaData.getNumberOfShards(); shardId++) {
try {
delete(snapshotId, snapshot.version(), indexId, new ShardId(indexMetaData.getIndex(), shardId));
} catch (SnapshotException ex) {
final int finalShardId = shardId;
logger.warn(() -> new ParameterizedMessage("[{}] failed to delete shard data for shard [{}][{}]", snapshotId, index, finalShardId), ex);
logger.warn(() -> new ParameterizedMessage("[{}] failed to delete shard data for shard [{}][{}]",
snapshotId, index, finalShardId), ex);
}
}
}
Expand Down Expand Up @@ -448,6 +443,16 @@ private void deleteGlobalMetaDataBlobIgnoringErrors(final SnapshotInfo snapshotI
}
}

private void deleteIndexMetaDataBlobIgnoringErrors(final SnapshotInfo snapshotInfo, final IndexId indexId) {
final SnapshotId snapshotId = snapshotInfo.snapshotId();
BlobContainer indexMetaDataBlobContainer = blobStore().blobContainer(basePath().add("indices").add(indexId.getId()));
try {
indexMetaDataFormat.delete(indexMetaDataBlobContainer, snapshotId.getUUID());
} catch (IOException ex) {
logger.warn(() -> new ParameterizedMessage("[{}] failed to delete metadata for index [{}]", snapshotId, indexId.getName()), ex);
}
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -508,44 +513,6 @@ public IndexMetaData getSnapshotIndexMetaData(final SnapshotId snapshotId, final
return indexMetaDataFormat.read(blobStore().blobContainer(indexPath), snapshotId.getUUID());
}

/**
* Returns the global metadata associated with the snapshot.
* <p>
* The returned meta data contains global metadata as well as metadata
* for all indices listed in the indices parameter.
*/
private MetaData readSnapshotMetaData(final SnapshotId snapshotId,
final Version snapshotVersion,
final List<IndexId> indices,
final boolean ignoreErrors) throws IOException {
if (snapshotVersion == null) {
// When we delete corrupted snapshots we might not know which version we are dealing with
// We can try detecting the version based on the metadata file format
assert ignoreErrors;
if (globalMetaDataFormat.exists(snapshotsBlobContainer, snapshotId.getUUID()) == false) {
throw new SnapshotMissingException(metadata.name(), snapshotId);
}
}

final MetaData.Builder metaData = MetaData.builder(getSnapshotGlobalMetaData(snapshotId));
if (indices != null) {
for (IndexId index : indices) {
try {
metaData.put(getSnapshotIndexMetaData(snapshotId, index), false);
} catch (ElasticsearchParseException | IOException ex) {
if (ignoreErrors == false) {
throw new SnapshotException(metadata.name(), snapshotId,
"[" + index.getName() + "] failed to read metadata for index", ex);
} else {
logger.warn(() ->
new ParameterizedMessage("[{}] [{}] failed to read metadata for index", snapshotId, index.getName()), ex);
}
}
}
}
return metaData.build();
}

/**
* Configures RateLimiter based on repository and global settings
*
Expand Down
6 changes: 6 additions & 0 deletions server/src/test/java/org/elasticsearch/snapshots/MetadataLoadingDuringSnapshotRestoreIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ public void testWhenMetadataAreLoaded() throws Exception {
assertGlobalMetadataLoads("snap", 1);
assertIndexMetadataLoads("snap", "docs", 4);
assertIndexMetadataLoads("snap", "others", 3);

// Deleting a snapshot does not load the global metadata state but loads each index metadata
assertAcked(client().admin().cluster().prepareDeleteSnapshot("repository", "snap").get());
assertGlobalMetadataLoads("snap", 1);
assertIndexMetadataLoads("snap", "docs", 5);
assertIndexMetadataLoads("snap", "others", 4);
}

private void assertGlobalMetadataLoads(final String snapshot, final int times) {
Expand Down
64 changes: 62 additions & 2 deletions server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1291,7 +1291,7 @@ public void testDeleteSnapshotWithMissingMetadata() throws Exception {
assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(0));
assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), equalTo(createSnapshotResponse.getSnapshotInfo().totalShards()));

logger.info("--> delete index metadata and shard metadata");
logger.info("--> delete global state metadata");
Path metadata = repo.resolve("meta-" + createSnapshotResponse.getSnapshotInfo().snapshotId().getUUID() + ".dat");
Files.delete(metadata);

Expand Down Expand Up @@ -1341,6 +1341,67 @@ public void testDeleteSnapshotWithCorruptedSnapshotFile() throws Exception {
assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), equalTo(createSnapshotResponse.getSnapshotInfo().totalShards()));
}

/** Tests that a snapshot with a corrupted global state file can still be deleted */
public void testDeleteSnapshotWithCorruptedGlobalState() throws Exception {
final Path repo = randomRepoPath();

assertAcked(client().admin().cluster().preparePutRepository("test-repo")
.setType("fs")
.setSettings(Settings.builder()
.put("location", repo)
.put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));

createIndex("test-idx-1", "test-idx-2");
indexRandom(true,
client().prepareIndex("test-idx-1", "_doc").setSource("foo", "bar"),
client().prepareIndex("test-idx-2", "_doc").setSource("foo", "bar"),
client().prepareIndex("test-idx-2", "_doc").setSource("foo", "bar"));
flushAndRefresh("test-idx-1", "test-idx-2");

CreateSnapshotResponse createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap")
.setIncludeGlobalState(true)
.setWaitForCompletion(true)
.get();
SnapshotInfo snapshotInfo = createSnapshotResponse.getSnapshotInfo();
assertThat(snapshotInfo.successfulShards(), greaterThan(0));
assertThat(snapshotInfo.successfulShards(), equalTo(snapshotInfo.totalShards()));

final Path globalStatePath = repo.resolve("meta-" + snapshotInfo.snapshotId().getUUID() + ".dat");
if (randomBoolean()) {
// Delete the global state metadata file
IOUtils.deleteFilesIgnoringExceptions(globalStatePath);
} else {
// Truncate the global state metadata file
try (SeekableByteChannel outChan = Files.newByteChannel(globalStatePath, StandardOpenOption.WRITE)) {
outChan.truncate(randomInt(10));
}
}

List<SnapshotInfo> snapshotInfos = client().admin().cluster().prepareGetSnapshots("test-repo").get().getSnapshots();
assertThat(snapshotInfos.size(), equalTo(1));
assertThat(snapshotInfos.get(0).state(), equalTo(SnapshotState.SUCCESS));
assertThat(snapshotInfos.get(0).snapshotId().getName(), equalTo("test-snap"));

SnapshotsStatusResponse snapshotStatusResponse =
client().admin().cluster().prepareSnapshotStatus("test-repo").setSnapshots("test-snap").get();
assertThat(snapshotStatusResponse.getSnapshots(), hasSize(1));
assertThat(snapshotStatusResponse.getSnapshots().get(0).getSnapshot().getSnapshotId().getName(), equalTo("test-snap"));

assertAcked(client().admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap").get());
assertThrows(client().admin().cluster().prepareGetSnapshots("test-repo").addSnapshots("test-snap"),
SnapshotMissingException.class);
assertThrows(client().admin().cluster().prepareSnapshotStatus("test-repo").addSnapshots("test-snap"),
SnapshotMissingException.class);

createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap")
.setIncludeGlobalState(true)
.setWaitForCompletion(true)
.get();
snapshotInfo = createSnapshotResponse.getSnapshotInfo();
assertThat(snapshotInfo.successfulShards(), greaterThan(0));
assertThat(snapshotInfo.successfulShards(), equalTo(snapshotInfo.totalShards()));
}

public void testSnapshotWithMissingShardLevelIndexFile() throws Exception {
Path repo = randomRepoPath();
logger.info("--> creating repository at {}", repo.toAbsolutePath());
Expand Down Expand Up @@ -2623,7 +2684,6 @@ public void testRestoreSnapshotWithCorruptedGlobalState() throws Exception {
assertThat(snapshotInfo.successfulShards(), greaterThan(0));
assertThat(snapshotInfo.successfulShards(), equalTo(snapshotInfo.totalShards()));

// Truncate the global state metadata file
final Path globalStatePath = repo.resolve("meta-" + snapshotInfo.snapshotId().getUUID() + ".dat");
try(SeekableByteChannel outChan = Files.newByteChannel(globalStatePath, StandardOpenOption.WRITE)) {
outChan.truncate(randomInt(10));
Expand Down