From bbdeb654cc4135b7aa0da26ffa8afaa60eea43cf Mon Sep 17 00:00:00 2001
From: Nhat Nguyen <nhat.nguyen@elastic.co>
Date: Fri, 22 Jun 2018 11:25:30 -0400
Subject: [PATCH] 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
---
 .../upgrades/FullClusterRestartIT.java            | 15 +++++++++------
 .../org/elasticsearch/upgrades/RecoveryIT.java    | 13 +++++++++----
 2 files changed, 18 insertions(+), 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
index 0656202f62e57..47937617020d2 100644
--- 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
index faab64a85d357..1b5b7336c26ae 100644
--- 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);