From ea2cc7172b2be905b10641cec7450558c88b003e Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 15 Apr 2020 18:47:59 +0530 Subject: [PATCH 1/2] HBASE-24195 : Admin.getRegionServers() should return live servers excluding decom RS optionally --- .../org/apache/hadoop/hbase/client/Admin.java | 29 +++++++++++++- .../hadoop/hbase/client/TestAdmin2.java | 39 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java index bd4e3051bcde..4a74d37c9f26 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java @@ -21,7 +21,9 @@ import java.io.Closeable; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Map; @@ -64,6 +66,7 @@ import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; +import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils; import org.apache.yetus.audience.InterfaceAudience; /** @@ -1064,7 +1067,31 @@ default Collection getBackupMasters() throws IOException { * @throws IOException if a remote or network exception occurs */ default Collection 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 getRegionServers(boolean excludeDecommissionedRS) + throws IOException { + List allServers = + getClusterMetrics(EnumSet.of(Option.SERVERS_NAME)).getServersName(); + if (!excludeDecommissionedRS) { + return allServers; + } + List decommissionedRegionServers = listDecommissionedRegionServers(); + if (CollectionUtils.isNotEmpty(decommissionedRegionServers)) { + allServers = new ArrayList<>(allServers); + allServers.removeIf(decommissionedRegionServers::contains); + return Collections.unmodifiableList(allServers); + } + return allServers; } /** diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java index 9c3792775524..839f6a8a3520 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java @@ -25,6 +25,7 @@ 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,42 @@ public void testSlowLogResponses() throws Exception { Assert.assertEquals(onlineLogRecords.size(), 0); } + @Test + public void testGetRegionServers() throws Exception { + // get all live server names + List serverNames = new ArrayList<>(ADMIN.getRegionServers(true)); + Assert.assertEquals(3, serverNames.size()); + + List serversToDecom = new ArrayList<>(); + ServerName serverToDecommission = serverNames.get(0); + + serversToDecom.add(serverToDecommission); + ADMIN.decommissionRegionServers(serversToDecom, false); + waitForServerDecom(serverToDecommission, true); + + Assert.assertEquals(2, ADMIN.getRegionServers(true).size()); + + ADMIN.recommissionRegionServer(serverToDecommission, Collections.emptyList()); + + Assert.assertEquals(3, ADMIN.getRegionServers(true).size()); + waitForServerDecom(serverToDecommission, false); + } + + private static void waitForServerDecom(ServerName excludeServer, + boolean anyServerDecommissioned) { + TEST_UTIL.waitFor(3000, () -> { + try { + List 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); + } + }); + } + } From d52a72040825d16517976a41eb9e49619e88a1e5 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Thu, 16 Apr 2020 15:19:56 +0530 Subject: [PATCH 2/2] addressing review comments --- .../java/org/apache/hadoop/hbase/client/Admin.java | 14 +++++--------- .../org/apache/hadoop/hbase/client/TestAdmin2.java | 8 +++++--- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java index 4a74d37c9f26..c82b1015951e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java @@ -21,9 +21,7 @@ import java.io.Closeable; import java.io.IOException; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Map; @@ -66,9 +64,10 @@ import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; -import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils; 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. @@ -1086,12 +1085,9 @@ default Collection getRegionServers(boolean excludeDecommissionedRS) return allServers; } List decommissionedRegionServers = listDecommissionedRegionServers(); - if (CollectionUtils.isNotEmpty(decommissionedRegionServers)) { - allServers = new ArrayList<>(allServers); - allServers.removeIf(decommissionedRegionServers::contains); - return Collections.unmodifiableList(allServers); - } - return allServers; + return allServers.stream() + .filter(s -> !decommissionedRegionServers.contains(s)) + .collect(ImmutableList.toImmutableList()); } /** diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java index 839f6a8a3520..09c9ab292070 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java @@ -876,17 +876,19 @@ public void testGetRegionServers() throws Exception { serversToDecom.add(serverToDecommission); ADMIN.decommissionRegionServers(serversToDecom, false); - waitForServerDecom(serverToDecommission, true); + 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()); - waitForServerDecom(serverToDecommission, false); + Assert.assertEquals(3, ADMIN.getRegionServers(false).size()); } - private static void waitForServerDecom(ServerName excludeServer, + private static void waitForServerCommissioned(ServerName excludeServer, boolean anyServerDecommissioned) { TEST_UTIL.waitFor(3000, () -> { try {