Retry synced-flush in FullClusterRestartIT#testRecovery

Today we examine the response of a synced-flush, then issue another request if the current one is not entirely successful. However, this approach is not correct as method #performRequest throws ResponseException for a partial result. Closes #31530
elastic · Jun 22, 2018 · bbdeb65 · bbdeb65
1 parent 95c073a
commit bbdeb65
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 10 deletions.
diff --git a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java
@@ -25,6 +25,7 @@
 import org.apache.http.util.EntityUtils;
 import org.elasticsearch.Version;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Booleans;
 import org.elasticsearch.common.CheckedFunction;
@@ -699,7 +700,6 @@ public void testEmptyShard() throws IOException {
      * Tests recovery of an index with or without a translog and the
      * statistics we gather about that.
      */
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/31530")
     public void testRecovery() throws Exception {
         int count;
         boolean shouldHaveTranslog;
@@ -719,11 +719,14 @@ public void testRecovery() throws Exception {
                 // We have to spin synced-flush requests here because we fire the global checkpoint sync for the last write operation.
                 // A synced-flush request considers the global checkpoint sync as an going operation because it acquires a shard permit.
                 assertBusy(() -> {
-                    Response resp = client().performRequest("POST", index + "/_flush/synced");
-                    assertOK(resp);
-                    Map<String, Object> result = ObjectPath.createFromResponse(resp).evaluate("_shards");
-                    assertThat(result.get("successful"), equalTo(result.get("total")));
-                    assertThat(result.get("failed"), equalTo(0));
+                    try {
+                        Response resp = client().performRequest("POST", index + "/_flush/synced");
+                        Map<String, Object> result = ObjectPath.createFromResponse(resp).evaluate("_shards");
+                        assertThat(result.get("successful"), equalTo(result.get("total")));
+                        assertThat(result.get("failed"), equalTo(0));
+                    } catch (ResponseException ex) {
+                        throw new AssertionError(ex); // cause assert busy to retry
+                    }
                 });
             } else {
                 // Explicitly flush so we're sure to have a bunch of documents in the Lucene index

diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java
@@ -23,6 +23,7 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.AbstractRunnable;
@@ -300,10 +301,14 @@ public void testRecoverSyncedFlushIndex() throws Exception {
             // We have to spin synced-flush requests here because we fire the global checkpoint sync for the last write operation.
             // A synced-flush request considers the global checkpoint sync as an going operation because it acquires a shard permit.
             assertBusy(() -> {
-                Response resp = client().performRequest("POST", index + "/_flush/synced");
-                assertOK(resp);
-                Map<String, Object> result = ObjectPath.createFromResponse(resp).evaluate("_shards");
-                assertThat(result.get("successful"), equalTo(2));
+                try {
+                    Response resp = client().performRequest("POST", index + "/_flush/synced");
+                    Map<String, Object> result = ObjectPath.createFromResponse(resp).evaluate("_shards");
+                    assertThat(result.get("successful"), equalTo(result.get("total")));
+                    assertThat(result.get("failed"), equalTo(0));
+                } catch (ResponseException ex) {
+                    throw new AssertionError(ex); // cause assert busy to retry
+                }
             });
         }
         ensureGreen(index);