Gracefully handle concurent repository modifications

elastic · Nov 12, 2019 · c540d39 · c540d39
1 parent 46ade8d
commit c540d39
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 12 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
@@ -919,18 +919,30 @@ public void endVerification(String seed) {
 
     @Override
     public RepositoryData getRepositoryData() {
-        final long generation;
-        try {
-            generation = latestIndexBlobId();
-        } catch (IOException ioe) {
-            throw new RepositoryException(metadata.name(), "Could not determine repository generation from root blobs", ioe);
-        }
-        final long genToLoad = latestKnownRepoGen.updateAndGet(known -> Math.max(known, generation));
-        if (genToLoad != generation) {
-            logger.warn("Determined repository generation [" + generation + "] from repository contents but correct generation must be " +
-                "at least [" + genToLoad + "]");
+        // Retry loading RepositoryData in a loop in case we run into concurrent modifications of the repository.
+        while (true) {
+            final long generation;
+            try {
+                generation = latestIndexBlobId();
+            } catch (IOException ioe) {
+                throw new RepositoryException(metadata.name(), "Could not determine repository generation from root blobs", ioe);
+            }
+            final long genToLoad = latestKnownRepoGen.updateAndGet(known -> Math.max(known, generation));
+            if (genToLoad != generation) {
+                logger.warn("Determined repository generation [" + generation
+                    + "] from repository contents but correct generation must be at least [" + genToLoad + "]");
+            }
+            try {
+                return getRepositoryData(genToLoad);
+            } catch (RepositoryException e) {
+                if (genToLoad != latestKnownRepoGen.get()) {
+                    logger.warn("Failed to load repository data generation [" + genToLoad +
+                        "] because a concurrent operation moved the current generation to [" + latestKnownRepoGen.get() + "]", e);
+                    continue;
+                }
+                throw e;
+            }
         }
-        return getRepositoryData(genToLoad);
     }
 
     private RepositoryData getRepositoryData(long indexGen) {
@@ -947,6 +959,13 @@ private RepositoryData getRepositoryData(long indexGen) {
                 return RepositoryData.snapshotsFromXContent(parser, indexGen);
             }
         } catch (IOException ioe) {
+            // If we fail to load the generation we tracked in latestKnownRepoGen we reset it.
+            // This is done as a fail-safe in case a user manually deletes the contents of the repository in which case subsequent
+            // operations must start from the EMPTY_REPO_GEN again
+            if (RepositoryData.EMPTY_REPO_GEN ==
+                latestKnownRepoGen.updateAndGet(known -> known == indexGen ? RepositoryData.EMPTY_REPO_GEN : known)) {
+                logger.warn("Resetting repository generation tracker because we failed to read generation [" + indexGen + "]");
+            }
             throw new RepositoryException(metadata.name(), "could not read repository data from index blob", ioe);
         }
     }

diff --git a/...rk/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java b/...rk/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java
@@ -76,7 +76,6 @@ public void setUp() throws Exception {
     @Override
     public void tearDown() throws Exception {
         deleteAndAssertEmpty(getRepository().basePath());
-        client().admin().cluster().prepareDeleteRepository("test-repo").get();
         super.tearDown();
     }
 
@@ -170,6 +169,8 @@ protected void assertBlobsByPrefix(BlobPath path, String prefix, Map<String, Blo
     }
 
     public void testCleanup() throws Exception {
+        createRepository("test-repo");
+
         createIndex("test-idx-1");
         createIndex("test-idx-2");
         createIndex("test-idx-3");