Mark Deleted Snapshot Directories with Tombstones

* Closes elastic#39852
original-brownbear · Mar 19, 2019 · bd4685c · bd4685c
1 parent 37a0c76
commit bd4685c
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 5 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
@@ -429,6 +429,15 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) {
         try {
             // Delete snapshot from the index file, since it is the maintainer of truth of active snapshots
             final RepositoryData updatedRepositoryData = repositoryData.removeSnapshot(snapshotId);
+            // cleanup indices that are no longer part of the repository
+            final Collection<IndexId> indicesToCleanUp = Sets.newHashSet(repositoryData.getIndices().values());
+            indicesToCleanUp.removeAll(updatedRepositoryData.getIndices().values());
+            final BlobContainer indicesBlobContainer = blobStore().blobContainer(basePath().add("indices"));
+            final BytesReference tombstoneBytes = new BytesArray(" ");
+            for (final IndexId indexId : indicesToCleanUp) {
+                indicesBlobContainer.writeBlobAtomic(
+                    indexId.getId() + ".tombstone", tombstoneBytes.streamInput(), tombstoneBytes.length(), false);
+            }
             writeIndexGen(updatedRepositoryData, repositoryStateId);
 
             // delete the snapshot file
@@ -466,11 +475,8 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) {
                 }
             }
 
-            // cleanup indices that are no longer part of the repository
-            final Collection<IndexId> indicesToCleanUp = Sets.newHashSet(repositoryData.getIndices().values());
-            indicesToCleanUp.removeAll(updatedRepositoryData.getIndices().values());
-            final BlobContainer indicesBlobContainer = blobStore().blobContainer(basePath().add("indices"));
             for (final IndexId indexId : indicesToCleanUp) {
+                IOException deleteException = null;
                 try {
                     indicesBlobContainer.deleteBlob(indexId.getId());
                 } catch (DirectoryNotEmptyException dnee) {
@@ -480,10 +486,15 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) {
                     // snapshotting indices of the same name
                     logger.debug(() -> new ParameterizedMessage("[{}] index [{}] no longer part of any snapshots in the repository, " +
                         "but failed to clean up its index folder due to the directory not being empty.", metadata.name(), indexId), dnee);
+                    deleteException = dnee;
                 } catch (IOException ioe) {
                     // a different IOException occurred while trying to delete - will just log the issue for now
                     logger.debug(() -> new ParameterizedMessage("[{}] index [{}] no longer part of any snapshots in the repository, " +
                         "but failed to clean up its index folder.", metadata.name(), indexId), ioe);
+                    deleteException = ioe;
+                }
+                if (deleteException == null) {
+                    indicesBlobContainer.deleteBlob(indexId.getId() + ".tombstone");
                 }
             }
         } catch (IOException | ResourceNotFoundException ex) {

diff --git a/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java
@@ -39,8 +39,10 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.equalTo;
 
@@ -84,7 +86,26 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
                 return FileVisitResult.CONTINUE;
             }
         });
-        assertEquals("Unexpected file count, found: [" + found + "].", expectedCount, found.size());
+        // filtering out delete tombstones
+        final Set<Path> tombstones =
+            found.stream().filter(p -> p.getFileName().toString().endsWith(".tombstone")).collect(Collectors.toSet());
+        final Set<Path> marked = tombstones.stream().map(
+            p -> p.getParent().resolve(p.getFileName().toString().replaceAll("\\.tombstone$", ""))
+        ).collect(Collectors.toSet());
+        final List<Path> filtered = found.stream().filter(p -> {
+            if(tombstones.contains(p)) {
+                return false;
+            }
+            Path path = p;
+            while (path != null) {
+                if (marked.contains(path)) {
+                    return false;
+                }
+                path = path.getParent();
+            }
+            return true;
+        }).collect(Collectors.toList());
+        assertEquals("Unexpected file count, found: [" + found + "].", expectedCount, filtered.size());
     }
 
     public static int numberOfFiles(Path dir) throws IOException {