From d889c7b44275290a797446d652d0eb6ee8eac27e Mon Sep 17 00:00:00 2001
From: Bharath Vissapragada <bharathv@apache.org>
Date: Mon, 1 Jun 2020 09:11:27 -0700
Subject: [PATCH] HBASE-24480: Deflake TestRSGroupsBasics#testClearDeadServers
 (#1821)

More details about the flakiness in the jira comments.

Signed-off-by: Reid Chan <reidchan@apache.org>
Signed-off-by: Viraj Jasani <vjasani@apache.org>
---
 .../hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java  |  9 ++++++---
 .../hadoop/hbase/rsgroup/TestRSGroupsBasics.java    | 13 +++++++++----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
index 4ae8e0fa7ef..91325755d2d 100644
--- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
+++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
@@ -1166,13 +1166,16 @@ public class RSGroupAdminEndpoint extends RSGroupAdminService
   public void postClearDeadServers(ObserverContext<MasterCoprocessorEnvironment> ctx,
       List<ServerName> servers, List<ServerName> notClearedServers)
       throws IOException {
-    Set<Address> clearedServer = Sets.newHashSet();
+    Set<Address> clearedServers = Sets.newHashSet();
     for (ServerName server: servers) {
       if (!notClearedServers.contains(server)) {
-        clearedServer.add(server.getAddress());
+        clearedServers.add(server.getAddress());
       }
     }
-    groupAdminServer.removeServers(clearedServer);
+    if (clearedServers.isEmpty()) {
+      return;
+    }
+    groupAdminServer.removeServers(clearedServers);
   }
 
   @Override
diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java
index f1ca55823f4..a8cf06af981 100644
--- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java
+++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java
@@ -26,7 +26,6 @@ import static org.junit.Assert.assertTrue;
 import com.google.common.collect.Lists;
 
 import java.io.IOException;
-import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.logging.Log;
@@ -212,9 +211,15 @@ public class TestRSGroupsBasics extends TestRSGroupsBase {
     assertTrue(newGroup.getServers().contains(serverToStop.getAddress()));
 
     // clear dead servers list
-    List<ServerName> notClearedServers = admin.clearDeadServers(Lists.newArrayList(serverToStop));
-    assertEquals(0, notClearedServers.size());
-
+    // We need to retry here because the clearDeadServers() RPC may race with currently processing
+    // dead servers in the ServerManager and might not succeed.
+    final ServerName finalServerToStop = serverToStop;
+    TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate<Exception>() {
+      @Override
+      public boolean evaluate() throws Exception {
+        return admin.clearDeadServers(Lists.newArrayList(finalServerToStop)).isEmpty();
+      }
+    });
     // verify if the stopped region server gets cleared and removed from the group
     Set<Address> newGroupServers = rsGroupAdmin.getRSGroupInfo(newGroup.getName()).getServers();
     assertFalse(newGroupServers.contains(serverToStop.getAddress()));