From 68ce328b650f0a49575cb6233ec740aab12bf7e5 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Fri, 11 May 2018 14:34:11 +0200 Subject: [PATCH] Delete temporary blobs before creating index file (#30528) Fixes an (un-released) bug introduced in #30332. Closes #30507 --- .../blobstore/BlobStoreRepository.java | 16 +++++++++++++++- .../SharedClusterSnapshotRestoreIT.java | 1 - 2 files changed, 15 insertions(+), 2 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 539c5a2ad3000..c35fc95197b15 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -967,6 +967,20 @@ protected void finalize(final List snapshots, final BlobStoreIndexShardSnapshots updatedSnapshots = new BlobStoreIndexShardSnapshots(snapshots); try { + // Delete temporary index files first, as we might otherwise fail in the next step creating the new index file if an earlier + // attempt to write an index file with this generation failed mid-way after creating the temporary file. + for (final String blobName : blobs.keySet()) { + if (indexShardSnapshotsFormat.isTempBlobName(blobName)) { + try { + blobContainer.deleteBlobIgnoringIfNotExists(blobName); + } catch (IOException e) { + logger.warn(() -> new ParameterizedMessage("[{}][{}] failed to delete index blob [{}] during finalization", + snapshotId, shardId, blobName), e); + throw e; + } + } + } + // If we deleted all snapshots, we don't need to create a new index file if (snapshots.size() > 0) { indexShardSnapshotsFormat.writeAtomic(updatedSnapshots, blobContainer, indexGeneration); @@ -974,7 +988,7 @@ protected void finalize(final List snapshots, // Delete old index files for (final String blobName : blobs.keySet()) { - if (indexShardSnapshotsFormat.isTempBlobName(blobName) || blobName.startsWith(SNAPSHOT_INDEX_PREFIX)) { + if (blobName.startsWith(SNAPSHOT_INDEX_PREFIX)) { try { blobContainer.deleteBlobIgnoringIfNotExists(blobName); } catch (IOException e) { diff --git a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index a5909bdfb60bd..9cc44e4ae05c1 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -2870,7 +2870,6 @@ public void testSnapshotCanceledOnRemovedShard() throws Exception { assertEquals("IndexShardSnapshotFailedException[Aborted]", snapshotInfo.shardFailures().get(0).reason()); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30507") public void testSnapshotSucceedsAfterSnapshotFailure() throws Exception { logger.info("--> creating repository"); final Path repoPath = randomRepoPath();