Browse Source

Fix Test Failure in BlobStoreRepositoryCleanupIT#testMasterFailoverDuringCleanup (#73609)

This only really triggers with https://github.com/elastic/elasticsearch/pull/73190
merged but is technically possible in the current code already. The master failover
for cleanups works error free in almost all cases, except for the rare case
where the removal of the cleanup due to master failover did not yet happen and a new
cleanup is attempted.
I do not think it's worth fixing this corner case when we have similar retry noise
for other snapshot- and master-actions in general so I made the test survive this.
Armin Braun 4 years ago
parent
commit
842b74bd20

+ 10 - 1
server/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryCleanupIT.java

@@ -24,6 +24,7 @@ import java.io.IOException;
 import java.util.concurrent.ExecutionException;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFutureThrows;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 
@@ -43,7 +44,15 @@ public class BlobStoreRepositoryCleanupIT extends AbstractSnapshotIntegTestCase
         awaitClusterState(state ->
                 state.custom(RepositoryCleanupInProgress.TYPE, RepositoryCleanupInProgress.EMPTY).hasCleanupInProgress() == false);
 
-        cleanupFuture.get();
+        try {
+            cleanupFuture.get();
+        } catch (ExecutionException e) {
+            // rare case where the master failure triggers a client retry that executes quicker than the removal of the initial
+            // cleanup in progress
+            final Throwable ise = ExceptionsHelper.unwrap(e, IllegalStateException.class);
+            assertThat(ise, instanceOf(IllegalStateException.class));
+            assertThat(ise.getMessage(), containsString(" a repository cleanup is already in-progress in "));
+        }
     }
 
     public void testRepeatCleanupsDontRemove() throws Exception {