sunhelly commented on a change in pull request #323: HBASE-22414 Interruption
of moving regions in RSGroup will cause regi…
URL: https://github.com/apache/hbase/pull/323#discussion_r297952752
##########
File path:
hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
##########
@@ -193,76 +198,104 @@ private void checkServersAndTables(Set<Address>
servers, Set<TableName> tables,
}
}
+ private enum MoveType {
+ TO, FROM
+ }
+
/**
- * Move every region from servers which are currently located on these
servers,
- * but should not be located there.
+ * When move a table to a group, all regions of it must be moved to group
servers (to the group).
+ * When move a server to a group, some regions on it which are of tables
that do not belong to
+ * the target group must be moved (from the group).
+ * So MoveType.TO means TableName set, and MoveType.FROM means server
Address set.
*
- * @param servers the servers that will move to new group
- * @param targetGroupName the target group name
- * @throws IOException if moving the server and tables fail
+ * @param set it's a table set or a server set
+ * @param targetGroupName target group name
+ * @param type type of move regions
+ * @param <T> the type of elements in Set
+ * @throws IOException if move haven't succeed even after max number of
retries
*/
- private void moveServerRegionsFromGroup(Set<Address> servers, String
targetGroupName)
+ private <T> void moveRegionsToOrFromGroup(Set<T> set, String
targetGroupName, MoveType type)
throws IOException {
+ Set<T> newSet = new HashSet<>(set);
boolean hasRegionsToMove;
+ int retry = 0;
+ IOException toThrow = null;
+ Set<String> failedRegions = new HashSet<>();
RSGroupInfo targetGrp = getRSGroupInfo(targetGroupName);
- Set<Address> allSevers = new HashSet<>(servers);
do {
hasRegionsToMove = false;
- for (Iterator<Address> iter = allSevers.iterator(); iter.hasNext();) {
- Address rs = iter.next();
- // Get regions that are associated with this server and filter regions
by group tables.
- for (RegionInfo region : getRegions(rs)) {
- 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;
+ for (Iterator<T> iter = newSet.iterator(); iter.hasNext(); ) {
+ T el = iter.next();
+ List<RegionInfo> toMoveRegions = new ArrayList<>();
+ if (type == MoveType.TO) {
+ // means element type of set is TableName
+ assert el instanceof TableName;
+ if (master.getAssignmentManager().isTableDisabled((TableName) el)) {
+ LOG.debug("Skipping move regions because the table {} is
disabled", el);
+ }else {
+ // Get regions of these tables and filter regions by group servers.
+ for (RegionInfo region :
+
master.getAssignmentManager().getRegionStates().getRegionsOfTable((TableName)
el)) {
+ ServerName sn =
+
master.getAssignmentManager().getRegionStates().getRegionServerOfRegion(region);
+ if (!targetGrp.containsServer(sn.getAddress())) {
+ toMoveRegions.add(region);
+ }
+ }
+ }
+ }
+ if (type == MoveType.FROM) {
+ // means element type of set is Address
+ assert el instanceof Address;
+ // Get regions that are associated with these servers and filter
regions by group tables.
+ for (RegionInfo region : getRegions((Address) el)) {
+ if (!targetGrp.containsTable(region.getTable())) {
+ toMoveRegions.add(region);
}
- hasRegionsToMove = true;
}
}
+ //move regions
+ for (RegionInfo region : toMoveRegions) {
+ LOG.info("Moving server region {}, which do not belong to RSGroup
{}",
+ region.getShortNameToLog(), targetGroupName);
+ try {
+ this.master.getAssignmentManager().move(region);
+ failedRegions.remove(region.getRegionNameAsString());
+ } catch (IOException ioe) {
+ LOG.debug("Move region {} from group failed, will retry, current
retry time is {}",
+ region.getShortNameToLog(), retry, ioe);
+ toThrow = ioe;
+ failedRegions.add(region.getRegionNameAsString());
+ }
+ if (master.getAssignmentManager().getRegionStates().
+ getRegionState(region).isFailedOpen()) {
+ continue;
+ }
+ hasRegionsToMove = true;
+ }
if (!hasRegionsToMove) {
- LOG.info("Server {} has no more regions to move for RSGroup",
rs.getHostname());
+ LOG.info("There are no more regions of {} to move for RSGroup {}",
el, targetGroupName);
iter.remove();
}
}
+ retry++;
try {
rsGroupInfoManager.wait(1000);
} catch (InterruptedException e) {
LOG.warn("Sleep interrupted", e);
Thread.currentThread().interrupt();
}
- } while (hasRegionsToMove);
- }
+ } while (hasRegionsToMove && retry <= moveMaxRetry);
Review comment:
Do you means 'retry'? See 282.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
