From e4ab1a79141023d6e90a813b386dda3f41ad59a5 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 29 Mar 2018 09:16:53 +0200 Subject: [PATCH] Do not load global state when deleting a snapshot (#29278) When deleting a snapshot, it is not necessary to load and to parse the global metadata of the snapshot to delete. Now indices are stored in the snapshot metadata file, we have all the information to resolve the shards files to delete. This commit removes the readSnapshotMetaData() method that was used to load both global and index metadata files. Test coverage should be enough as SharedClusterSnapshotRestoreIT already contains several deletion tests. Related to #28934 --- .../blobstore/BlobStoreRepository.java | 91 ++++++------------- ...etadataLoadingDuringSnapshotRestoreIT.java | 6 ++ .../SharedClusterSnapshotRestoreIT.java | 64 ++++++++++++- 3 files changed, 97 insertions(+), 64 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 d112129f46fef..ff4361dd7697e 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -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 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); @@ -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) { + final List 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); } } } @@ -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} */ @@ -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. - *

- * 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 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 * diff --git a/server/src/test/java/org/elasticsearch/snapshots/MetadataLoadingDuringSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/MetadataLoadingDuringSnapshotRestoreIT.java index bbc2a54b41baf..13b74df4e3d2b 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/MetadataLoadingDuringSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/MetadataLoadingDuringSnapshotRestoreIT.java @@ -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) { diff --git a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index d7bcd9b4bd143..9901fa4c8b220 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -1306,7 +1306,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); @@ -1356,6 +1356,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 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()); @@ -2642,7 +2703,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));