HBASE-22923 Consider minVersionToMoveSysTables while moving region and creating regionPlan (ADDENDUM) (apache#3455)

Signed-off-by: David Manning <[email protected]>
Signed-off-by: Bharath Vissapragada <[email protected]>

Author: virajjasani

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

@@ -577,7 +577,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(true)) {
+          for (ServerName server : getExcludedServersForSystemTable()) {
             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
@@ -2302,16 +2302,6 @@ private void addToPendingAssignment(final HashMap<RegionInfo, RegionStateNode> r
     }
   }
 
-  /**
-   * 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
@@ -2321,15 +2311,9 @@ public List<ServerName> getExcludedServersForSystemTable() {
    * "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) {
+  public List<ServerName> getExcludedServersForSystemTable() {
     // 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.
@@ -2342,12 +2326,11 @@ private List<ServerName> getExcludedServersForSystemTable(
     }
     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();
-        }
+    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()