From 1883889e26fd1bac73630f48d1c6e9adab0230e4 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Sat, 3 Jul 2021 22:41:28 +0530 Subject: [PATCH] HBASE-22923 Consider minVersionToMoveSysTables while moving region and creating regionPlan (ADDENDUM) (#3455) Signed-off-by: David Manning Signed-off-by: Bharath Vissapragada --- .../master/assignment/AssignmentManager.java | 31 +++++-------------- 1 file changed, 7 insertions(+), 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 index c00bf7a8c14..5622a58f966 100644 --- 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 @@ -577,7 +577,7 @@ public class AssignmentManager { List 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 @@ public class AssignmentManager { } } - /** - * 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 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 class AssignmentManager { * "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 getExcludedServersForSystemTable( - boolean checkForMinVersion) { + public List 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 @@ public class AssignmentManager { } 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()