CR: Fix incorrect consistency and deleting of empty trees

Author: original-brownbear

server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java

@@ -171,8 +171,12 @@
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.file.DirectoryNotEmptyException;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -538,6 +542,7 @@ private void assertNoStaleRepositoryData() throws IOException {
             repos = reposDir.filter(s -> s.getFileName().toString().startsWith("extra") == false).collect(Collectors.toList());
         }
         for (Path repoRoot : repos) {
+            cleanupEmptyTrees(repoRoot);
             final Path latestIndexGenBlob = repoRoot.resolve("index.latest");
             assertTrue("Could not find index.latest blob for repo at [" + repoRoot + ']', Files.exists(latestIndexGenBlob));
             final long latestGen = ByteBuffer.wrap(Files.readAllBytes(latestIndexGenBlob)).getLong(0);
@@ -553,6 +558,35 @@ private void assertNoStaleRepositoryData() throws IOException {
         }
     }
 
+    // Lucene's mock file system randomly generates empty `extra0` files that break the deletion of blob-store directories.
+    // We clean those up here before checking a blob-store for stale files in this test.
+    private void cleanupEmptyTrees(Path repoPath) {
+        try {
+            Files.walkFileTree(repoPath, new SimpleFileVisitor<>() {
+
+                @Override
+                public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+                    if (file.getFileName().toString().startsWith("extra")) {
+                        Files.delete(file);
+                    }
+                    return FileVisitResult.CONTINUE;
+                }
+
+                @Override
+                public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
+                    try {
+                        Files.delete(dir);
+                    } catch (DirectoryNotEmptyException e) {
+                        // We're only interested in deleting empty trees here, just ignore directories with content
+                    }
+                    return FileVisitResult.CONTINUE;
+                }
+            });
+        } catch (IOException e) {
+            throw new AssertionError(e);
+        }
+    }
+
     private static void assertIndexGenerations(Path repoRoot, long latestGen) throws IOException {
         try (Stream<Path> repoRootBlobs = Files.list(repoRoot)) {
             final long[] indexGenerations = repoRootBlobs.filter(p -> p.getFileName().toString().startsWith("index-"))

server/src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

@@ -131,13 +131,17 @@ public InputStream readBlob(String name) throws IOException {
             }
 
             private void ensureReadAfterWrite(String blobName) {
-                if (cachedMisses.contains(blobName) == false && pendingWrites.containsKey(blobName)) {
+                // TODO: Make this even less consistent by keeping track of blobs that existed before instead of simply ensuring
+                //       read after first write again if the blob doesn't exist
+                if (cachedMisses.contains(blobName) == false && pendingWrites.containsKey(blobName)
+                        && super.blobExists(blobName) == false) {
                     pendingWrites.remove(blobName).run();
                 }
             }
 
             @Override
             public void deleteBlob(String blobName) {
+                ensureReadAfterWrite(blobName);
                 // TODO: simulate longer delays here once the S3 blob store implementation can handle them
                 deterministicTaskQueue.scheduleNow(() -> {
                     try {