From 4bd7aed0646c7280d207835aa810a7d6b328311d 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 bf730bc86e05..2377be6b256e 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 @@ -570,7 +570,7 @@ public void checkIfShouldMoveSystemRegionAsync() { 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 @@ -2289,16 +2289,6 @@ private void addToPendingAssignment(final HashMap 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 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 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 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. @@ -2329,12 +2313,11 @@ private List 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()