Avoid atomic overwrite tests on FS repositories (#70483)

Today we leniently permit overwrites of blobs in a repository not to be
atomic, since they are not in shared filesystem repositories. In fact
it's worse, on Windows overwrites do not even work if there is a
concurrent reader. In practice this isn't very important, we do almost
no overwrites and almost never read the file that's being overwritten,
but we do still test for atomic overwrites in the repository analyzer.

With this commit we suppress the atomic overwrite checks in the
repository analyzer for FS repositories, and remove the lenience since
all other repositories should implement atomic overwrites correctly.

Closes #70303

Author: DaveCTurner

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

@@ -2740,6 +2740,15 @@ public boolean supportURLRepo() {
         return supportURLRepo;
     }
 
+    /**
+     * @return whether this repository performs overwrites atomically. In practice we only overwrite the `index.latest` blob so this
+     * is not very important, but the repository analyzer does test that overwrites happen atomically. It will skip those tests if the
+     * repository overrides this method to indicate that it does not support atomic overwrites.
+     */
+    public boolean hasAtomicOverwrites() {
+        return true;
+    }
+
     /**
      * The result of removing a snapshot from a shard folder in the repository.
      */

server/src/main/java/org/elasticsearch/repositories/fs/FsRepository.java

@@ -103,4 +103,11 @@ protected BlobStore createBlobStore() throws Exception {
     protected ByteSizeValue chunkSize() {
         return chunkSize;
     }
+
+    @Override
+    public boolean hasAtomicOverwrites() {
+        // We overwrite a file by deleting the old file and then renaming the new file into place, which is not atomic.
+        // Also on Windows the overwrite may fail if the file is opened for reading at the time.
+        return false;
+    }
 }

x-pack/plugin/repository-encrypted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepository.java

@@ -379,6 +379,11 @@ private void validateLocalRepositorySecret(Map<String, Object> snapshotUserMetad
         }
     }
 
+    @Override
+    public boolean hasAtomicOverwrites() {
+        return delegatedRepository.hasAtomicOverwrites();
+    }
+
     // pkg-private for tests
     static final class EncryptedBlobStore implements BlobStore {
         private final BlobStore delegatedBlobStore;

x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/BlobAnalyzeAction.java

@@ -486,8 +486,6 @@ private void onReadsComplete(Collection<NodeResponse> responses, WriteDetails wr
                 if (response.isNotFound()) {
                     if (request.readEarly) {
                         nodeFailure = null; // "not found" is legitimate iff we tried to read it before the write completed
-                    } else if (request.writeAndOverwrite) {
-                        nodeFailure = null; // overwrites surprisingly not necessarily atomic, e.g. in a FsBlobContainer
                     } else {
                         nodeFailure = new RepositoryVerificationException(
                             request.getRepositoryName(),

x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/RepositoryAnalyzeAction.java

@@ -465,7 +465,10 @@ public void run() {
                         request.getReadNodeCount(),
                         request.getEarlyReadNodeCount(),
                         smallBlob && random.nextDouble() < request.getRareActionProbability(),
-                        repository.supportURLRepo() && smallBlob && random.nextDouble() < request.getRareActionProbability()
+                        repository.supportURLRepo()
+                            && repository.hasAtomicOverwrites()
+                            && smallBlob
+                            && random.nextDouble() < request.getRareActionProbability()
                     )
                 );
                 queue.add(verifyBlobTask);