HBASE-22923 Consider minVersionToMoveSysTables while moving region an…

…d creating regionPlan (ADDENDUM) (#3455) Signed-off-by: David Manning <[email protected]> Signed-off-by: Bharath Vissapragada <[email protected]>
apache · Jul 3, 2021 · 4bd7aed · 4bd7aed
1 parent fbafc92
commit 4bd7aed
Showing 1 changed file with 7 additions and 24 deletions.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
@@ -570,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(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
@@ -2289,16 +2289,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
@@ -2308,15 +2298,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.
@@ -2329,12 +2313,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()