HBASE-24195 : Admin.getRegionServers() should return live servers exc… (#1523)

Signed-off-by: Mingliang Liu <liuml07@apache.org>
Signed-off-by: Jan Hentschel <jan.hentschel@ultratendency.com>

hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java

@@ -66,6 +66,8 @@ import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableList;
+
 /**
  * The administrative API for HBase. Obtain an instance from {@link Connection#getAdmin()} and
  * call {@link #close()} when done.
@@ -1064,7 +1066,28 @@ public interface Admin extends Abortable, Closeable {
    * @throws IOException if a remote or network exception occurs
    */
   default Collection<ServerName> getRegionServers() throws IOException {
-    return getClusterMetrics(EnumSet.of(Option.SERVERS_NAME)).getServersName();
+    return getRegionServers(false);
+  }
+
+  /**
+   * Retrieve all current live region servers including decommissioned
+   * if excludeDecommissionedRS is false, else non-decommissioned ones only
+   *
+   * @param excludeDecommissionedRS should we exclude decommissioned RS nodes
+   * @return all current live region servers including/excluding decommissioned hosts
+   * @throws IOException if a remote or network exception occurs
+   */
+  default Collection<ServerName> getRegionServers(boolean excludeDecommissionedRS)
+      throws IOException {
+    List<ServerName> allServers =
+      getClusterMetrics(EnumSet.of(Option.SERVERS_NAME)).getServersName();
+    if (!excludeDecommissionedRS) {
+      return allServers;
+    }
+    List<ServerName> decommissionedRegionServers = listDecommissionedRegionServers();
+    return allServers.stream()
+      .filter(s -> !decommissionedRegionServers.contains(s))
+      .collect(ImmutableList.toImmutableList());
   }
 
   /**

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java

@@ -25,6 +25,7 @@ import static org.junit.Assert.fail;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -864,4 +865,44 @@ public class TestAdmin2 extends TestAdminBase {
     Assert.assertEquals(onlineLogRecords.size(), 0);
   }
 
+  @Test
+  public void testGetRegionServers() throws Exception {
+    // get all live server names
+    List<ServerName> serverNames = new ArrayList<>(ADMIN.getRegionServers(true));
+    Assert.assertEquals(3, serverNames.size());
+
+    List<ServerName> serversToDecom = new ArrayList<>();
+    ServerName serverToDecommission = serverNames.get(0);
+
+    serversToDecom.add(serverToDecommission);
+    ADMIN.decommissionRegionServers(serversToDecom, false);
+    waitForServerCommissioned(serverToDecommission, true);
+
+    Assert.assertEquals(2, ADMIN.getRegionServers(true).size());
+    Assert.assertEquals(3, ADMIN.getRegionServers(false).size());
+
+    ADMIN.recommissionRegionServer(serverToDecommission, Collections.emptyList());
+    waitForServerCommissioned(null, false);
+
+    Assert.assertEquals(3, ADMIN.getRegionServers(true).size());
+    Assert.assertEquals(3, ADMIN.getRegionServers(false).size());
+  }
+
+  private static void waitForServerCommissioned(ServerName excludeServer,
+      boolean anyServerDecommissioned) {
+    TEST_UTIL.waitFor(3000, () -> {
+      try {
+        List<ServerName> decomServers = TEST_UTIL.getAdmin().listDecommissionedRegionServers();
+        if (anyServerDecommissioned) {
+          return decomServers.size() == 1
+            && decomServers.get(0).equals(excludeServer);
+        } else {
+          return decomServers.size() == 0;
+        }
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+    });
+  }
+
 }