From 6e7d042708d27257f2ba9dbcfca178e95b2edfb4 Mon Sep 17 00:00:00 2001 From: haxiaolin Date: Wed, 22 May 2019 10:31:44 +0800 Subject: [PATCH] HBASE-22411 Refactor codes of moving reigons in RSGroup --- .../hadoop/hbase/rsgroup/RSGroupAdminServer.java | 194 ++++++++++----------- 1 file changed, 97 insertions(+), 97 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 20eb32700b..69510f3154 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 @@ -55,8 +55,8 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Maps; @InterfaceAudience.Private public class RSGroupAdminServer implements RSGroupAdmin { private static final Logger LOG = LoggerFactory.getLogger(RSGroupAdminServer.class); - public static final String KEEP_ONE_SERVER_IN_DEFAULT_ERROR_MESSAGE = "should keep at least " + - "one server in 'default' RSGroup."; + public static final String KEEP_ONE_SERVER_IN_DEFAULT_ERROR_MESSAGE = + "should keep at least " + "one server in 'default' RSGroup."; private MasterServices master; private final RSGroupInfoManager rsGroupInfoManager; @@ -77,17 +77,17 @@ public class RSGroupAdminServer implements RSGroupAdmin { // We are reading across two Maps in the below with out synchronizing across // them; should be safe most of the time. String groupName = rsGroupInfoManager.getRSGroupOfTable(tableName); - return groupName == null? null: rsGroupInfoManager.getRSGroup(groupName); + return groupName == null ? null : rsGroupInfoManager.getRSGroup(groupName); } private void checkOnlineServersOnly(Set
servers) throws ConstraintException { // This uglyness is because we only have Address, not ServerName. // Online servers are keyed by ServerName. Set
onlineServers = new HashSet<>(); - for(ServerName server: master.getServerManager().getOnlineServers().keySet()) { + for (ServerName server : master.getServerManager().getOnlineServers().keySet()) { onlineServers.add(server.getAddress()); } - for (Address address: servers) { + for (Address address : servers) { if (!onlineServers.contains(address)) { throw new ConstraintException( "Server " + address + " is not an online server in 'default' RSGroup."); @@ -99,8 +99,7 @@ public class RSGroupAdminServer implements RSGroupAdmin { * Check passed name. Fail if nulls or if corresponding RSGroupInfo not found. * @return The RSGroupInfo named name */ - private RSGroupInfo getAndCheckRSGroupInfo(String name) - throws IOException { + private RSGroupInfo getAndCheckRSGroupInfo(String name) throws IOException { if (StringUtils.isEmpty(name)) { throw new ConstraintException("RSGroup cannot be null."); } @@ -116,8 +115,8 @@ public class RSGroupAdminServer implements RSGroupAdmin { */ private List getRegions(final Address server) { LinkedList regions = new LinkedList<>(); - for (Map.Entry el : - master.getAssignmentManager().getRegionStates().getRegionAssignments().entrySet()) { + for (Map.Entry el : master.getAssignmentManager().getRegionStates() + .getRegionAssignments().entrySet()) { if (el.getValue() == null) { continue; } @@ -154,29 +153,31 @@ public class RSGroupAdminServer implements RSGroupAdmin { * @throws IOException if nulls or if servers and tables not belong to the same group */ private void checkServersAndTables(Set
servers, Set tables, - String targetGroupName) throws IOException { + String targetGroupName) throws IOException { // Presume first server's source group. Later ensure all servers are from this group. Address firstServer = servers.iterator().next(); RSGroupInfo tmpSrcGrp = rsGroupInfoManager.getRSGroupOfServer(firstServer); if (tmpSrcGrp == null) { // Be careful. This exception message is tested for in TestRSGroupsBase... - throw new ConstraintException("Source RSGroup for server " + firstServer - + " does not exist."); + throw new ConstraintException( + "Source RSGroup for server " + firstServer + " does not exist."); } RSGroupInfo srcGrp = new RSGroupInfo(tmpSrcGrp); if (srcGrp.getName().equals(targetGroupName)) { - throw new ConstraintException("Target RSGroup " + targetGroupName + - " is same as source " + srcGrp.getName() + " RSGroup."); + throw new ConstraintException( + "Target RSGroup " + targetGroupName + " is same as source " + srcGrp.getName() + + " RSGroup."); } // Only move online servers checkOnlineServersOnly(servers); // Ensure all servers are of same rsgroup. - for (Address server: servers) { + for (Address server : servers) { String tmpGroup = rsGroupInfoManager.getRSGroupOfServer(server).getName(); if (!tmpGroup.equals(srcGrp.getName())) { - throw new ConstraintException("Move server request should only come from one source " + - "RSGroup. Expecting only " + srcGrp.getName() + " but contains " + tmpGroup); + throw new ConstraintException( + "Move server request should only come from one source " + "RSGroup. Expecting only " + + srcGrp.getName() + " but contains " + tmpGroup); } } @@ -184,14 +185,15 @@ public class RSGroupAdminServer implements RSGroupAdmin { for (TableName table : tables) { String tmpGroup = rsGroupInfoManager.getRSGroupOfTable(table); if (!tmpGroup.equals(srcGrp.getName())) { - throw new ConstraintException("Move table request should only come from one source " + - "RSGroup. Expecting only " + srcGrp.getName() + " but contains " + tmpGroup); + throw new ConstraintException( + "Move table request should only come from one source " + "RSGroup. Expecting only " + + srcGrp.getName() + " but contains " + tmpGroup); } } if (srcGrp.getServers().size() <= servers.size() && srcGrp.getTables().size() > tables.size()) { - throw new ConstraintException("Cannot leave a RSGroup " + srcGrp.getName() + - " that contains tables without servers to host them."); + throw new ConstraintException("Cannot leave a RSGroup " + srcGrp.getName() + + " that contains tables without servers to host them."); } } @@ -210,7 +212,7 @@ public class RSGroupAdminServer implements RSGroupAdmin { Set
allSevers = new HashSet<>(servers); do { foundRegionsToMove = false; - for (Iterator
iter = allSevers.iterator(); iter.hasNext();) { + for (Iterator
iter = allSevers.iterator(); iter.hasNext(); ) { Address rs = iter.next(); // Get regions that are associated with this server and filter regions by tables. List regions = new ArrayList<>(); @@ -220,10 +222,10 @@ public class RSGroupAdminServer implements RSGroupAdmin { } } - LOG.info("Moving " + regions.size() + " region(s) from " + rs + - " for server move to " + targetGroupName); + LOG.info("Moving " + regions.size() + " region(s) from " + rs + " for server move to " + + targetGroupName); if (!regions.isEmpty()) { - for (RegionInfo region: regions) { + for (RegionInfo region : regions) { // Regions might get assigned from tables of target group so we need to filter if (!targetGrp.containsTable(region.getTable())) { this.master.getAssignmentManager().move(region); @@ -258,12 +260,12 @@ public class RSGroupAdminServer implements RSGroupAdmin { */ private void moveRegionsToServers(Set
servers, Set tables, String targetGroupName) throws IOException { - for (TableName table: tables) { + for (TableName table : tables) { LOG.info("Moving region(s) from " + table + " for table move to " + targetGroupName); for (RegionInfo region : master.getAssignmentManager().getRegionStates() - .getRegionsOfTable(table)) { - ServerName sn = master.getAssignmentManager().getRegionStates() - .getRegionServerOfRegion(region); + .getRegionsOfTable(table)) { + ServerName sn = + master.getAssignmentManager().getRegionStates().getRegionServerOfRegion(region); if (!servers.contains(sn.getAddress())) { master.getAssignmentManager().move(region); } @@ -272,11 +274,10 @@ public class RSGroupAdminServer implements RSGroupAdmin { } @edu.umd.cs.findbugs.annotations.SuppressWarnings( - value="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", - justification="Ignoring complaint because don't know what it is complaining about") + value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", + justification = "Ignoring complaint because don't know what it is complaining about") @Override - public void moveServers(Set
servers, String targetGroupName) - throws IOException { + public void moveServers(Set
servers, String targetGroupName) throws IOException { if (servers == null) { throw new ConstraintException("The list of servers to move cannot be null."); } @@ -295,12 +296,12 @@ public class RSGroupAdminServer implements RSGroupAdmin { RSGroupInfo srcGrp = rsGroupInfoManager.getRSGroupOfServer(firstServer); if (srcGrp == null) { // Be careful. This exception message is tested for in TestRSGroupsBase... - throw new ConstraintException("Source RSGroup for server " + firstServer - + " does not exist."); + throw new ConstraintException( + "Source RSGroup for server " + firstServer + " does not exist."); } if (srcGrp.getName().equals(targetGroupName)) { - throw new ConstraintException("Target RSGroup " + targetGroupName + - " is same as source " + srcGrp + " RSGroup."); + throw new ConstraintException( + "Target RSGroup " + targetGroupName + " is same as source " + srcGrp + " RSGroup."); } // Only move online servers (when moving from 'default') or servers from other // groups. This prevents bogus servers from entering groups @@ -311,34 +312,35 @@ public class RSGroupAdminServer implements RSGroupAdmin { checkOnlineServersOnly(servers); } // Ensure all servers are of same rsgroup. - for (Address server: servers) { + for (Address server : servers) { String tmpGroup = rsGroupInfoManager.getRSGroupOfServer(server).getName(); if (!tmpGroup.equals(srcGrp.getName())) { - throw new ConstraintException("Move server request should only come from one source " + - "RSGroup. Expecting only " + srcGrp.getName() + " but contains " + tmpGroup); + throw new ConstraintException( + "Move server request should only come from one source " + "RSGroup. Expecting only " + + srcGrp.getName() + " but contains " + tmpGroup); } } if (srcGrp.getServers().size() <= servers.size() && srcGrp.getTables().size() > 0) { - throw new ConstraintException("Cannot leave a RSGroup " + srcGrp.getName() + - " that contains tables without servers to host them."); + throw new ConstraintException("Cannot leave a RSGroup " + srcGrp.getName() + + " that contains tables without servers to host them."); } // MovedServers may be < passed in 'servers'. - Set
movedServers = rsGroupInfoManager.moveServers(servers, srcGrp.getName(), - targetGroupName); + Set
movedServers = + rsGroupInfoManager.moveServers(servers, srcGrp.getName(), targetGroupName); List
editableMovedServers = Lists.newArrayList(movedServers); boolean foundRegionsToMove; do { foundRegionsToMove = false; - for (Iterator
iter = editableMovedServers.iterator(); iter.hasNext();) { + for (Iterator
iter = editableMovedServers.iterator(); iter.hasNext(); ) { Address rs = iter.next(); // Get regions that are associated with this server. List regions = getRegions(rs); - LOG.info("Moving " + regions.size() + " region(s) from " + rs + - " for server move to " + targetGroupName); + LOG.info("Moving " + regions.size() + " region(s) from " + rs + " for server move to " + + targetGroupName); - for (RegionInfo region: regions) { + for (RegionInfo region : regions) { // Regions might get assigned from tables of target group so we need to filter if (targetGrp.containsTable(region.getTable())) { continue; @@ -383,22 +385,22 @@ public class RSGroupAdminServer implements RSGroupAdmin { // Hold a lock on the manager instance while moving servers to prevent // another writer changing our state while we are working. synchronized (rsGroupInfoManager) { - if(targetGroup != null) { + if (targetGroup != null) { RSGroupInfo destGroup = rsGroupInfoManager.getRSGroup(targetGroup); - if(destGroup == null) { + if (destGroup == null) { throw new ConstraintException("Target " + targetGroup + " RSGroup does not exist."); } - if(destGroup.getServers().size() < 1) { + if (destGroup.getServers().size() < 1) { throw new ConstraintException("Target RSGroup must have at least one server."); } } for (TableName table : tables) { String srcGroup = rsGroupInfoManager.getRSGroupOfTable(table); - if(srcGroup != null && srcGroup.equals(targetGroup)) { + if (srcGroup != null && srcGroup.equals(targetGroup)) { throw new ConstraintException( - "Source RSGroup " + srcGroup + " is same as target " + targetGroup + - " RSGroup for table " + table); + "Source RSGroup " + srcGroup + " is same as target " + targetGroup + + " RSGroup for table " + table); } LOG.info("Moving table " + table.getNameAsString() + " to RSGroup " + targetGroup); } @@ -407,15 +409,14 @@ public class RSGroupAdminServer implements RSGroupAdmin { // targetGroup is null when a table is being deleted. In this case no further // action is required. if (targetGroup != null) { - for (TableName table: tables) { + for (TableName table : tables) { if (master.getAssignmentManager().isTableDisabled(table)) { LOG.debug("Skipping move regions because the table" + table + " is disabled."); continue; } - for (RegionInfo region : - master.getAssignmentManager().getRegionStates().getRegionsOfTable(table)) { - LOG.info("Moving region " + region.getShortNameToLog() + - " to RSGroup " + targetGroup); + for (RegionInfo region : master.getAssignmentManager().getRegionStates() + .getRegionsOfTable(table)) { + LOG.info("Moving region " + region.getShortNameToLog() + " to RSGroup " + targetGroup); master.getAssignmentManager().move(region); } } @@ -439,21 +440,21 @@ public class RSGroupAdminServer implements RSGroupAdmin { } int tableCount = rsGroupInfo.getTables().size(); if (tableCount > 0) { - throw new ConstraintException("RSGroup " + name + " has " + tableCount + - " tables; you must remove these tables from the rsgroup before " + - "the rsgroup can be removed."); + throw new ConstraintException("RSGroup " + name + " has " + tableCount + + " tables; you must remove these tables from the rsgroup before " + + "the rsgroup can be removed."); } int serverCount = rsGroupInfo.getServers().size(); if (serverCount > 0) { - throw new ConstraintException("RSGroup " + name + " has " + serverCount + - " servers; you must remove these servers from the RSGroup before" + - "the RSGroup can be removed."); + throw new ConstraintException("RSGroup " + name + " has " + serverCount + + " servers; you must remove these servers from the RSGroup before" + + "the RSGroup can be removed."); } for (NamespaceDescriptor ns : master.getClusterSchema().getNamespaces()) { String nsGroup = ns.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP); if (nsGroup != null && nsGroup.equals(name)) { throw new ConstraintException( - "RSGroup " + name + " is referenced by namespace: " + ns.getName()); + "RSGroup " + name + " is referenced by namespace: " + ns.getName()); } } rsGroupInfoManager.removeRSGroup(name); @@ -473,29 +474,29 @@ public class RSGroupAdminServer implements RSGroupAdmin { } if (getRSGroupInfo(groupName) == null) { - throw new ConstraintException("RSGroup does not exist: "+groupName); + throw new ConstraintException("RSGroup does not exist: " + groupName); } // Only allow one balance run at at time. Map groupRIT = rsGroupGetRegionsInTransition(groupName); if (groupRIT.size() > 0) { - LOG.debug("Not running balancer because " + groupRIT.size() + " region(s) in transition: " + - StringUtils.abbreviate( - master.getAssignmentManager().getRegionStates().getRegionsInTransition().toString(), - 256)); + LOG.debug("Not running balancer because " + groupRIT.size() + " region(s) in transition: " + + StringUtils.abbreviate( + master.getAssignmentManager().getRegionStates().getRegionsInTransition().toString(), + 256)); return false; } if (serverManager.areDeadServersInProgress()) { - LOG.debug("Not running balancer because processing dead regionserver(s): " + - serverManager.getDeadServers()); + LOG.debug("Not running balancer because processing dead regionserver(s): " + serverManager + .getDeadServers()); return false; } //We balance per group instead of per table List plans = new ArrayList<>(); - for(Map.Entry>> tableMap: + for (Map.Entry>> tableMap : getRSGroupAssignmentsByTable(groupName).entrySet()) { - LOG.info("Creating partial plan for table " + tableMap.getKey() + ": " - + tableMap.getValue()); + LOG.info( + "Creating partial plan for table " + tableMap.getKey() + ": " + tableMap.getValue()); List partialPlans = balancer.balanceCluster(tableMap.getValue()); LOG.info("Partial plan for table " + tableMap.getKey() + ": " + partialPlans); if (partialPlans != null) { @@ -506,12 +507,12 @@ public class RSGroupAdminServer implements RSGroupAdmin { boolean balancerRan = !plans.isEmpty(); if (balancerRan) { LOG.info("RSGroup balance " + groupName + " starting with plan count: " + plans.size()); - for (RegionPlan plan: plans) { + for (RegionPlan plan : plans) { LOG.info("balance " + plan); assignmentManager.moveAsync(plan); } - LOG.info("RSGroup balance " + groupName + " completed after " + - (System.currentTimeMillis()-startTime) + " seconds"); + LOG.info("RSGroup balance " + groupName + " completed after " + (System.currentTimeMillis() + - startTime) + " seconds"); } return balancerRan; } @@ -555,8 +556,8 @@ public class RSGroupAdminServer implements RSGroupAdmin { //move regions which should belong to these servers moveRegionsToServers(servers, tables, targetGroup); } - LOG.info("Move servers and tables done. Severs :" - + servers + " , Tables : " + tables + " => " + targetGroup); + LOG.info("Move servers and tables done. Severs :" + servers + " , Tables : " + tables + " => " + + targetGroup); } @Override @@ -580,10 +581,10 @@ public class RSGroupAdminServer implements RSGroupAdmin { throws IOException { Map rit = Maps.newTreeMap(); AssignmentManager am = master.getAssignmentManager(); - for(TableName tableName : getRSGroupInfo(groupName).getTables()) { - for(RegionInfo regionInfo: am.getRegionStates().getRegionsOfTable(tableName)) { + for (TableName tableName : getRSGroupInfo(groupName).getTables()) { + for (RegionInfo regionInfo : am.getRegionStates().getRegionsOfTable(tableName)) { RegionState state = am.getRegionStates().getRegionTransitionState(regionInfo); - if(state != null) { + if (state != null) { rit.put(regionInfo.getEncodedName(), state); } } @@ -591,13 +592,13 @@ public class RSGroupAdminServer implements RSGroupAdmin { return rit; } - private Map>> - getRSGroupAssignmentsByTable(String groupName) throws IOException { + private Map>> getRSGroupAssignmentsByTable( + String groupName) throws IOException { Map>> result = Maps.newHashMap(); RSGroupInfo rsGroupInfo = getRSGroupInfo(groupName); Map>> assignments = Maps.newHashMap(); - for(Map.Entry entry: - master.getAssignmentManager().getRegionStates().getRegionAssignments().entrySet()) { + for (Map.Entry entry : master.getAssignmentManager().getRegionStates() + .getRegionAssignments().entrySet()) { TableName currTable = entry.getKey().getTable(); ServerName currServer = entry.getValue(); RegionInfo currRegion = entry.getKey(); @@ -609,15 +610,15 @@ public class RSGroupAdminServer implements RSGroupAdmin { } Map> serverMap = Maps.newHashMap(); - for(ServerName serverName: master.getServerManager().getOnlineServers().keySet()) { - if(rsGroupInfo.getServers().contains(serverName.getAddress())) { + for (ServerName serverName : master.getServerManager().getOnlineServers().keySet()) { + if (rsGroupInfo.getServers().contains(serverName.getAddress())) { serverMap.put(serverName, Collections.emptyList()); } } // add all tables that are members of the group - for(TableName tableName : rsGroupInfo.getTables()) { - if(assignments.containsKey(tableName)) { + for (TableName tableName : rsGroupInfo.getTables()) { + if (assignments.containsKey(tableName)) { result.put(tableName, new HashMap<>()); result.get(tableName).putAll(serverMap); result.get(tableName).putAll(assignments.get(tableName)); @@ -644,19 +645,18 @@ public class RSGroupAdminServer implements RSGroupAdmin { } Set
deadServers = new HashSet<>(); - for(ServerName server: master.getServerManager().getDeadServers().copyServerNames()) { + for (ServerName server : master.getServerManager().getDeadServers().copyServerNames()) { deadServers.add(server.getAddress()); } - for (Address address: servers) { + for (Address address : servers) { if (onlineServers.contains(address)) { throw new ConstraintException( "Server " + address + " is an online server, not allowed to remove."); } if (deadServers.contains(address)) { - throw new ConstraintException( - "Server " + address + " is on the dead servers list," - + " Maybe it will come back again, not allowed to remove."); + throw new ConstraintException("Server " + address + " is on the dead servers list," + + " Maybe it will come back again, not allowed to remove."); } } } -- 2.14.1