From a29b3caf4dbc7b8833474ef5da5438f7f6907e00 Mon Sep 17 00:00:00 2001 From: haxiaolin Date: Mon, 26 Feb 2018 14:25:01 +0800 Subject: [PATCH] HBASE-19974 Fix decommissioned servers cannot be removed by remove_servers_rsgroup methods Signed-off-by: tedyu --- .../hbase/rsgroup/RSGroupAdminServer.java | 8 +++- .../hadoop/hbase/rsgroup/TestRSGroups.java | 7 ++- .../hbase/rsgroup/TestRSGroupsBase.java | 46 ++++++++++++------- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java index aba57fe667b..094fc1d948b 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java @@ -675,8 +675,12 @@ public class RSGroupAdminServer implements RSGroupAdmin { private void checkForDeadOrOnlineServers(Set
servers) throws ConstraintException { // This uglyness is because we only have Address, not ServerName. Set
onlineServers = new HashSet<>(); - for(ServerName server: master.getServerManager().getOnlineServers().keySet()) { - onlineServers.add(server.getAddress()); + List drainingServers = master.getServerManager().getDrainingServersList(); + for (ServerName server : master.getServerManager().getOnlineServers().keySet()) { + // Only online but not decommissioned servers are really online + if (!drainingServers.contains(server)) { + onlineServers.add(server.getAddress()); + } } Set
deadServers = new HashSet<>(); diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java index 9116f3bdfac..610278ad141 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java @@ -36,7 +36,6 @@ import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.Waiter.Predicate; import org.apache.hadoop.hbase.client.ClusterConnection; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; -import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.ServerManager; import org.apache.hadoop.hbase.master.snapshot.SnapshotManager; import org.apache.hadoop.hbase.net.Address; @@ -66,7 +65,6 @@ public class TestRSGroups extends TestRSGroupsBase { HBaseClassTestRule.forClass(TestRSGroups.class); protected static final Logger LOG = LoggerFactory.getLogger(TestRSGroups.class); - private static HMaster master; private static boolean INIT = false; private static RSGroupAdminEndpoint rsGroupAdminEndpoint; @@ -126,6 +124,11 @@ public class TestRSGroups extends TestRSGroupsBase { deleteNamespaceIfNecessary(); deleteGroups(); + for(ServerName sn : admin.listDecommissionedRegionServers()){ + admin.recommissionRegionServer(sn, null); + } + assertTrue(admin.listDecommissionedRegionServers().isEmpty()); + int missing = NUM_SLAVES_BASE - getNumServers(); LOG.info("Restoring servers: "+missing); for(int i=0; i() { @Override public boolean evaluate() throws Exception { return !master.getServerManager().areDeadServersInProgress() - && cluster.getClusterMetrics().getDeadServerNames().size() > 0; + && cluster.getClusterMetrics().getDeadServerNames().size() == NUM_DEAD_SERVERS; } }); assertFalse(cluster.getClusterMetrics().getLiveServerMetrics().containsKey(targetServer)); @@ -925,8 +930,10 @@ public abstract class TestRSGroupsBase { public void testRemoveServers() throws Exception { LOG.info("testRemoveServers"); final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()), 3); - ServerName targetServer = ServerName.parseServerName( - newGroup.getServers().iterator().next().toString()); + Iterator
iterator = newGroup.getServers().iterator(); + ServerName targetServer = ServerName.parseServerName(iterator.next().toString()); + + // remove online servers try { rsGroupAdmin.removeServers(Sets.newHashSet(targetServer.getAddress())); fail("Online servers shouldn't have been successfully removed."); @@ -938,6 +945,8 @@ public abstract class TestRSGroupsBase { } assertTrue(newGroup.getServers().contains(targetServer.getAddress())); + // remove dead servers + NUM_DEAD_SERVERS = cluster.getClusterMetrics().getDeadServerNames().size(); AdminProtos.AdminService.BlockingInterface targetRS = ((ClusterConnection) admin.getConnection()).getAdmin(targetServer); try { @@ -945,18 +954,19 @@ public abstract class TestRSGroupsBase { GetServerInfoRequest.newBuilder().build()).getServerInfo().getServerName()); //stopping may cause an exception //due to the connection loss + LOG.info("stopping server " + targetServer.getHostAndPort()); targetRS.stopServer(null, AdminProtos.StopServerRequest.newBuilder().setReason("Die").build()); + NUM_DEAD_SERVERS ++; } catch(Exception e) { } - HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); //wait for stopped regionserver to dead server list TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate() { @Override public boolean evaluate() throws Exception { return !master.getServerManager().areDeadServersInProgress() - && cluster.getClusterMetrics().getDeadServerNames().size() > 0; + && cluster.getClusterMetrics().getDeadServerNames().size() == NUM_DEAD_SERVERS; } }); @@ -971,17 +981,19 @@ public abstract class TestRSGroupsBase { } assertTrue(newGroup.getServers().contains(targetServer.getAddress())); - ServerName sn = TEST_UTIL.getHBaseClusterInterface().getClusterMetrics().getMasterName(); - TEST_UTIL.getHBaseClusterInterface().stopMaster(sn); - TEST_UTIL.getHBaseClusterInterface().waitForMasterToStop(sn, 60000); - TEST_UTIL.getHBaseClusterInterface().startMaster(sn.getHostname(), 0); - TEST_UTIL.getHBaseClusterInterface().waitForActiveAndReadyMaster(60000); + // remove decommissioned servers + List serversToDecommission = new ArrayList<>(); + targetServer = ServerName.parseServerName(iterator.next().toString()); + targetRS = ((ClusterConnection) admin.getConnection()).getAdmin(targetServer); + targetServer = ProtobufUtil.toServerName(targetRS.getServerInfo(null, + GetServerInfoRequest.newBuilder().build()).getServerInfo().getServerName()); + assertTrue(master.getServerManager().getOnlineServers().containsKey(targetServer)); + serversToDecommission.add(targetServer); + + admin.decommissionRegionServers(serversToDecommission, true); + assertEquals(1, admin.listDecommissionedRegionServers().size()); - assertEquals(3, cluster.getClusterMetrics().getLiveServerMetrics().size()); - assertFalse(cluster.getClusterMetrics().getLiveServerMetrics().containsKey(targetServer)); - assertFalse(cluster.getClusterMetrics().getDeadServerNames().contains(targetServer)); assertTrue(newGroup.getServers().contains(targetServer.getAddress())); - rsGroupAdmin.removeServers(Sets.newHashSet(targetServer.getAddress())); Set
newGroupServers = rsGroupAdmin.getRSGroupInfo(newGroup.getName()).getServers(); assertFalse(newGroupServers.contains(targetServer.getAddress()));