HBASE-22414 Interruption of moving regions in RSGroup will cause regi…

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java

@@ -27,6 +27,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
+
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.ServerName;
@@ -60,9 +62,13 @@ public class RSGroupAdminServer implements RSGroupAdmin {
   private MasterServices master;
   private final RSGroupInfoManager rsGroupInfoManager;
 
+  private String FAILED_MOVE_MAX_RETRY = "hbase.rsgroup.move.max.retry";
+  private int moveMaxRetry;
+
   public RSGroupAdminServer(MasterServices master, RSGroupInfoManager rsGroupInfoManager) {
     this.master = master;
     this.rsGroupInfoManager = rsGroupInfoManager;
+    this.moveMaxRetry = master.getConfiguration().getInt(FAILED_MOVE_MAX_RETRY, 50);
   }
 
   @Override
@@ -202,68 +208,90 @@ private void checkServersAndTables(Set<Address> servers, Set<TableName> tables,
    * @throws IOException if moving the server and tables fail
    */
   private void moveServerRegionsFromGroup(Set<Address> servers, String targetGroupName)
-      throws IOException {
+    throws IOException {
+    moveRegionsBetweenGroups(servers, targetGroupName,
+      rs -> getRegions(rs),
+      info -> {
+        try {
+          RSGroupInfo group = getRSGroupInfo(targetGroupName);
+          return group.containsTable(info.getTable());
+        } catch (IOException e) {
+          e.printStackTrace();
+          return false;
+        }
+      },
+      rs -> rs.getHostname());
+  }
+
+  /**
+   * 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 moveTableRegionsToGroup(Set<TableName> tables, String targetGroupName)
+    throws IOException {
+    moveRegionsBetweenGroups(tables, targetGroupName,
+      table -> master.getAssignmentManager().getRegionStates().getRegionsOfTable(table),
+      info -> {
+        try {
+          RSGroupInfo group = getRSGroupInfo(targetGroupName);
+          ServerName sn =
+            master.getAssignmentManager().getRegionStates().getRegionServerOfRegion(info);
+          return group.containsServer(sn.getAddress());
+        } catch (IOException e) {
+          e.printStackTrace();
+          return false;
+        }
+      },
+      table -> table.getNameWithNamespaceInclAsString());
+  }
+
+  private <T> void moveRegionsBetweenGroups(Set<T> regionsOwners, String targetGroupName,
+    Function<T, List<RegionInfo>> getRegionsInfo,
+    Function<RegionInfo,Boolean> validation,
+    Function<T,String> getOwnerName) throws IOException {
     boolean hasRegionsToMove;
-    RSGroupInfo targetGrp = getRSGroupInfo(targetGroupName);
-    Set<Address> allSevers = new HashSet<>(servers);
+    int retry = 0;
+    Set<T> allOwners = new HashSet<>(regionsOwners);
     do {
       hasRegionsToMove = false;
-      for (Iterator<Address> iter = allSevers.iterator(); iter.hasNext();) {
-        Address rs = iter.next();
+      for (Iterator<T> iter = allOwners.iterator(); iter.hasNext();) {
+        T owner = 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);
+        for (RegionInfo region : getRegionsInfo.apply(owner)) {
+          if (!validation.apply(region)) {
+            LOG.info("Moving region {}, which do not belong to RSGroup {}",
+              region.getShortNameToLog(), targetGroupName);
+            try {
+              this.master.getAssignmentManager().move(region);
+            }catch (IOException ioe){
+              LOG.error("Move region {} from group failed, will retry, current retry time is {}",
+                region.getShortNameToLog(), retry, ioe);
+            }
             if (master.getAssignmentManager().getRegionStates().
-                getRegionState(region).isFailedOpen()) {
+              getRegionState(region).isFailedOpen()) {
               continue;
             }
             hasRegionsToMove = true;
           }
         }
 
         if (!hasRegionsToMove) {
-          LOG.info("Server {} has no more regions to move for RSGroup", rs.getHostname());
+          LOG.info("No more regions to move from {} to RSGroup", getOwnerName.apply(owner));
           iter.remove();
         }
       }
+
+      retry++;
       try {
         rsGroupInfoManager.wait(1000);
       } catch (InterruptedException e) {
         LOG.warn("Sleep interrupted", e);
         Thread.currentThread().interrupt();
       }
-    } while (hasRegionsToMove);
-  }
-
-  /**
-   * 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 moveTableRegionsToGroup(Set<TableName> 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 (!targetGrp.containsServer(sn.getAddress())) {
-          LOG.info("Moving region {} to RSGroup {}", region.getShortNameToLog(), targetGroupName);
-          master.getAssignmentManager().move(region);
-        }
-      }
-    }
+    } while (hasRegionsToMove && retry <= 50);
   }
 
   @edu.umd.cs.findbugs.annotations.SuppressWarnings(