From bd75f94a903bbb7e4562c91928c26fd79389f8be Mon Sep 17 00:00:00 2001 From: haxiaolin Date: Mon, 27 May 2019 11:04:42 +0800 Subject: [PATCH] HBASE-22411 Refactor codes of moving reigons in RSGroup --- .../hadoop/hbase/rsgroup/RSGroupAdminServer.java | 177 +++++++-------------- 1 file changed, 59 insertions(+), 118 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..7e9d907b0a 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 @@ -46,7 +46,6 @@ import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.common.collect.Lists; import org.apache.hbase.thirdparty.com.google.common.collect.Maps; /** @@ -61,8 +60,7 @@ public class RSGroupAdminServer implements RSGroupAdmin { private MasterServices master; private final RSGroupInfoManager rsGroupInfoManager; - public RSGroupAdminServer(MasterServices master, RSGroupInfoManager rsGroupInfoManager) - throws IOException { + public RSGroupAdminServer(MasterServices master, RSGroupInfoManager rsGroupInfoManager) { this.master = master; this.rsGroupInfoManager = rsGroupInfoManager; } @@ -196,46 +194,38 @@ public class RSGroupAdminServer implements RSGroupAdmin { } /** - * Moves every region from servers which are currently located on these servers, + * Move every region from servers which are currently located on these servers, * but should not be located there. + * * @param servers the servers that will move to new group - * @param tables these tables will be kept on the servers, others will be moved * @param targetGroupName the target group name * @throws IOException if moving the server and tables fail */ - private void moveRegionsFromServers(Set
servers, Set tables, - String targetGroupName) throws IOException { - boolean foundRegionsToMove; + private void moveServerRegionsFromGroup(Set
servers, String targetGroupName) + throws IOException { + boolean hasRegionsToMove; RSGroupInfo targetGrp = getRSGroupInfo(targetGroupName); Set
allSevers = new HashSet<>(servers); do { - foundRegionsToMove = false; + hasRegionsToMove = false; 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<>(); + // Get regions that are associated with this server and filter regions by group tables. for (RegionInfo region : getRegions(rs)) { - if (!tables.contains(region.getTable())) { - regions.add(region); - } - } - - LOG.info("Moving " + regions.size() + " region(s) from " + rs + - " for server move to " + targetGroupName); - if (!regions.isEmpty()) { - 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); - if (master.getAssignmentManager().getRegionStates(). - getRegionState(region).isFailedOpen()) { - continue; - } - foundRegionsToMove = true; + if (!targetGrp.containsTable(region.getTable())) { + LOG.info("Moving server region {}, which do not belong to RSGroup {}", + region.getShortNameToLog(), targetGroupName); + this.master.getAssignmentManager().move(region); + if (master.getAssignmentManager().getRegionStates(). + getRegionState(region).isFailedOpen()) { + continue; } + hasRegionsToMove = true; } } - if (!foundRegionsToMove) { + + if (!hasRegionsToMove) { + LOG.info("Server {} has no more regions to move for RSGroup", rs.getHostname()); iter.remove(); } } @@ -245,26 +235,31 @@ public class RSGroupAdminServer implements RSGroupAdmin { LOG.warn("Sleep interrupted", e); Thread.currentThread().interrupt(); } - } while (foundRegionsToMove); + } while (hasRegionsToMove); } /** - * Moves every region of tables which should be kept on the servers, - * but currently they are located on other servers. - * @param servers the regions of these servers will be kept on the servers, others will be moved + * Moves regions of tables which are not on target group servers. + * * @param tables the tables that will move to new group * @param targetGroupName the target group name * @throws IOException if moving the region fails */ - private void moveRegionsToServers(Set
servers, Set tables, - String targetGroupName) throws IOException { - for (TableName table: tables) { - LOG.info("Moving region(s) from " + table + " for table move to " + targetGroupName); + private void moveTableRegionsToGroup(Set tables, String targetGroupName) + throws IOException { + RSGroupInfo targetGrp = getRSGroupInfo(targetGroupName); + for (TableName table : tables) { + if (master.getAssignmentManager().isTableDisabled(table)) { + LOG.debug("Skipping move regions because the table {} is disabled", table); + continue; + } + LOG.info("Moving region(s) for table {} to RSGroup {}", table, targetGroupName); for (RegionInfo region : master.getAssignmentManager().getRegionStates() - .getRegionsOfTable(table)) { - ServerName sn = master.getAssignmentManager().getRegionStates() - .getRegionServerOfRegion(region); - if (!servers.contains(sn.getAddress())) { + .getRegionsOfTable(table)) { + ServerName sn = + master.getAssignmentManager().getRegionStates().getRegionServerOfRegion(region); + if (!targetGrp.containsServer(sn.getAddress())) { + LOG.info("Moving region {} to RSGroup {}", region.getShortNameToLog(), targetGroupName); master.getAssignmentManager().move(region); } } @@ -285,7 +280,8 @@ public class RSGroupAdminServer implements RSGroupAdmin { // TODO. Why? Stuff breaks if I equate them. return; } - RSGroupInfo targetGrp = getAndCheckRSGroupInfo(targetGroupName); + //check target group + getAndCheckRSGroupInfo(targetGroupName); // Hold a lock on the manager instance while moving servers to prevent // another writer changing our state while we are working. @@ -326,47 +322,8 @@ public class RSGroupAdminServer implements RSGroupAdmin { // MovedServers may be < passed in 'servers'. 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();) { - 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); - - for (RegionInfo region: regions) { - // Regions might get assigned from tables of target group so we need to filter - if (targetGrp.containsTable(region.getTable())) { - continue; - } - LOG.info("Moving region " + region.getShortNameToLog()); - this.master.getAssignmentManager().move(region); - if (master.getAssignmentManager().getRegionStates(). - getRegionState(region).isFailedOpen()) { - // If region is in FAILED_OPEN state, it won't recover, not without - // operator intervention... in hbase-2.0.0 at least. Continue rather - // than mark region as 'foundRegionsToMove'. - continue; - } - foundRegionsToMove = true; - } - if (!foundRegionsToMove) { - iter.remove(); - } - } - try { - rsGroupInfoManager.wait(1000); - } catch (InterruptedException e) { - LOG.warn("Sleep interrupted", e); - Thread.currentThread().interrupt(); - } - } while (foundRegionsToMove); - - LOG.info("Move server done: " + srcGrp.getName() + "=>" + targetGroupName); + moveServerRegionsFromGroup(movedServers, targetGroupName); + LOG.info("Move servers done: {} => {}", srcGrp.getName(), targetGroupName); } } @@ -400,26 +357,11 @@ public class RSGroupAdminServer implements RSGroupAdmin { "Source RSGroup " + srcGroup + " is same as target " + targetGroup + " RSGroup for table " + table); } - LOG.info("Moving table " + table.getNameAsString() + " to RSGroup " + targetGroup); + LOG.info("Moving table {} to RSGroup {}", table.getNameAsString(), targetGroup); } rsGroupInfoManager.moveTables(tables, targetGroup); - // targetGroup is null when a table is being deleted. In this case no further - // action is required. - if (targetGroup != null) { - 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); - master.getAssignmentManager().move(region); - } - } - } + moveTableRegionsToGroup(tables, targetGroup); } } @@ -478,14 +420,14 @@ public class RSGroupAdminServer implements RSGroupAdmin { // 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( + LOG.debug("Not running balancer because {} region(s) in transition: {}", groupRIT.size(), + StringUtils.abbreviate( master.getAssignmentManager().getRegionStates().getRegionsInTransition().toString(), 256)); return false; } if (serverManager.areDeadServersInProgress()) { - LOG.debug("Not running balancer because processing dead regionserver(s): " + + LOG.debug("Not running balancer because processing dead regionserver(s): {}", serverManager.getDeadServers()); return false; } @@ -494,10 +436,9 @@ public class RSGroupAdminServer implements RSGroupAdmin { List plans = new ArrayList<>(); 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); + LOG.info("Partial plan for table {} : {}", tableMap.getKey(), partialPlans); if (partialPlans != null) { plans.addAll(partialPlans); } @@ -505,13 +446,13 @@ public class RSGroupAdminServer implements RSGroupAdmin { long startTime = System.currentTimeMillis(); boolean balancerRan = !plans.isEmpty(); if (balancerRan) { - LOG.info("RSGroup balance " + groupName + " starting with plan count: " + plans.size()); + LOG.info("RSGroup balance {} starting with plan count: {}", groupName, plans.size()); for (RegionPlan plan: plans) { - LOG.info("balance " + plan); + LOG.info("balance {}", plan); assignmentManager.moveAsync(plan); } - LOG.info("RSGroup balance " + groupName + " completed after " + - (System.currentTimeMillis()-startTime) + " seconds"); + LOG.info("RSGroup balance {} completed after {} seconds", groupName, + (System.currentTimeMillis() - startTime)); } return balancerRan; } @@ -550,13 +491,13 @@ public class RSGroupAdminServer implements RSGroupAdmin { String srcGroup = getRSGroupOfServer(servers.iterator().next()).getName(); rsGroupInfoManager.moveServersAndTables(servers, tables, srcGroup, targetGroup); - //move regions which should not belong to these tables - moveRegionsFromServers(servers, tables, targetGroup); - //move regions which should belong to these servers - moveRegionsToServers(servers, tables, targetGroup); + //move regions on these servers which do not belong to group tables + moveServerRegionsFromGroup(servers, targetGroup); + //move regions of these tables which are not on group servers + moveTableRegionsToGroup(tables, targetGroup); } - LOG.info("Move servers and tables done. Severs :" - + servers + " , Tables : " + tables + " => " + targetGroup); + LOG.info("Move servers and tables done. Severs: {}, Tables: {} => {}", servers, tables, + targetGroup); } @Override @@ -571,7 +512,7 @@ public class RSGroupAdminServer implements RSGroupAdmin { //check the set of servers checkForDeadOrOnlineServers(servers); rsGroupInfoManager.removeServers(servers); - LOG.info("Remove decommissioned servers " + servers + " from rsgroup done."); + LOG.info("Remove decommissioned servers {} from RSGroup done", servers); } } } @@ -621,7 +562,7 @@ public class RSGroupAdminServer implements RSGroupAdmin { result.put(tableName, new HashMap<>()); result.get(tableName).putAll(serverMap); result.get(tableName).putAll(assignments.get(tableName)); - LOG.debug("Adding assignments for " + tableName + ": " + assignments.get(tableName)); + LOG.debug("Adding assignments for {}: {}", tableName, assignments.get(tableName)); } } -- 2.14.1