HBASE-22923 min version of RegionServer to move system table regions (#…

…3439) (#3438)

Signed-off-by: Andrew Purtell <[email protected]>
Signed-off-by: Bharath Vissapragada <[email protected]>

hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java

@@ -161,6 +161,31 @@ public class AssignmentManager {
   private final RegionStates regionStates = new RegionStates();
   private final RegionStateStore regionStateStore;
 
+  /**
+   * When the operator uses this configuration option, any version between
+   * the current cluster version and the value of "hbase.min.version.move.system.tables"
+   * does not trigger any auto-region movement. Auto-region movement here
+   * refers to auto-migration of system table regions to newer server versions.
+   * It is assumed that the configured range of versions does not require special
+   * handling of moving system table regions to higher versioned RegionServer.
+   * This auto-migration is done by {@link #checkIfShouldMoveSystemRegionAsync()}.
+   * Example: Let's assume the cluster is on version 1.4.0 and we have
+   * set "hbase.min.version.move.system.tables" as "2.0.0". Now if we upgrade
+   * one RegionServer on 1.4.0 cluster to 1.6.0 (< 2.0.0), then AssignmentManager will
+   * not move hbase:meta, hbase:namespace and other system table regions
+   * to newly brought up RegionServer 1.6.0 as part of auto-migration.
+   * However, if we upgrade one RegionServer on 1.4.0 cluster to 2.2.0 (> 2.0.0),
+   * then AssignmentManager will move all system table regions to newly brought
+   * up RegionServer 2.2.0 as part of auto-migration done by
+   * {@link #checkIfShouldMoveSystemRegionAsync()}.
+   * "hbase.min.version.move.system.tables" is introduced as part of HBASE-22923.
+   */
+  private final String minVersionToMoveSysTables;
+
+  private static final String MIN_VERSION_MOVE_SYS_TABLES_CONFIG =
+    "hbase.min.version.move.system.tables";
+  private static final String DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG = "";
+
   private final Map<ServerName, Set<byte[]>> rsReports = new HashMap<>();
 
   private final boolean shouldAssignRegionsWithFavoredNodes;
@@ -209,6 +234,8 @@ public AssignmentManager(final MasterServices master) {
     } else {
       this.deadMetricChore = null;
     }
+    minVersionToMoveSysTables = conf.get(MIN_VERSION_MOVE_SYS_TABLES_CONFIG,
+      DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG);
   }
 
   public void start() throws IOException, KeeperException {
@@ -543,7 +570,7 @@ public void checkIfShouldMoveSystemRegionAsync() {
           List<RegionPlan> plans = new ArrayList<>();
           // TODO: I don't think this code does a good job if all servers in cluster have same
           // version. It looks like it will schedule unnecessary moves.
-          for (ServerName server : getExcludedServersForSystemTable()) {
+          for (ServerName server : getExcludedServersForSystemTable(true)) {
             if (master.getServerManager().isServerDead(server)) {
               // TODO: See HBASE-18494 and HBASE-18495. Though getExcludedServersForSystemTable()
               // considers only online servers, the server could be queued for dead server
@@ -2263,28 +2290,60 @@ private void addToPendingAssignment(final HashMap<RegionInfo, RegionStateNode> r
   }
 
   /**
-   * Get a list of servers that this region cannot be assigned to.
-   * For system tables, we must assign them to a server with highest version.
+   * For a given cluster with mixed versions of servers, get a list of
+   * servers with lower versions, where system table regions should not be
+   * assigned to.
+   * For system table, we must assign regions to a server with highest version.
    */
   public List<ServerName> getExcludedServersForSystemTable() {
+    return getExcludedServersForSystemTable(false);
+  }
+
+  /**
+   * For a given cluster with mixed versions of servers, get a list of
+   * servers with lower versions, where system table regions should not be
+   * assigned to.
+   * For system table, we must assign regions to a server with highest version.
+   * However, we can disable this exclusion using config:
+   * "hbase.min.version.move.system.tables" if checkForMinVersion is true.
+   * Detailed explanation available with definition of minVersionToMoveSysTables.
+   *
+   * @param checkForMinVersion If false, return a list of servers with lower version. If true,
+   *   compare higher version with minVersionToMoveSysTables. Only if higher version is greater
+   *   than minVersionToMoveSysTables, this method returns list of servers with lower version. If
+   *   higher version is less than or equal to minVersionToMoveSysTables, returns empty list.
+   *   An example is provided with definition of minVersionToMoveSysTables.
+   * @return List of Excluded servers for System table regions.
+   */
+  private List<ServerName> getExcludedServersForSystemTable(
+      boolean checkForMinVersion) {
     // TODO: This should be a cached list kept by the ServerManager rather than calculated on each
     // move or system region assign. The RegionServerTracker keeps list of online Servers with
     // RegionServerInfo that includes Version.
     List<Pair<ServerName, String>> serverList = master.getServerManager().getOnlineServersList()
-        .stream()
-        .map((s)->new Pair<>(s, master.getRegionServerVersion(s)))
-        .collect(Collectors.toList());
+      .stream()
+      .map(s->new Pair<>(s, master.getRegionServerVersion(s)))
+      .collect(Collectors.toList());
     if (serverList.isEmpty()) {
       return Collections.emptyList();
     }
     String highestVersion = Collections.max(serverList,
       (o1, o2) -> VersionInfo.compareVersion(o1.getSecond(), o2.getSecond())).getSecond();
+    if (checkForMinVersion) {
+      if (!DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG.equals(minVersionToMoveSysTables)) {
+        int comparedValue = VersionInfo.compareVersion(minVersionToMoveSysTables, highestVersion);
+        if (comparedValue > 0) {
+          return Collections.emptyList();
+        }
+      }
+    }
     return serverList.stream()
-        .filter((p)->!p.getSecond().equals(highestVersion))
-        .map(Pair::getFirst)
-        .collect(Collectors.toList());
+      .filter(pair -> !pair.getSecond().equals(highestVersion))
+      .map(Pair::getFirst)
+      .collect(Collectors.toList());
   }
 
+
   MasterServices getMaster() {
     return master;
   }