From 7b7fbcf3b613f81973c7c90b0491c818f43503a9 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Tue, 29 Jun 2021 15:34:52 +0530 Subject: [PATCH 1/4] HBASE-22923 min version of RegionServer to move system table regions --- .../hbase/master/AssignmentManager.java | 65 ++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index f7cfc4cc0479..b27f79dba0c3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -253,6 +253,28 @@ public class AssignmentManager extends ZooKeeperListener { // are persisted in meta with a state store private final RegionStateStore regionStateStore; + /** + * Min version to consider for moving system tables regions to higher + * versioned RS. If RS has higher version than rest of the cluster but that + * version is less than this value, we should not move system table regions + * to that RS. If RS has higher version than rest of the cluster but that + * version is greater than or equal to this value, we should move system + * table regions to that RS. This is optional config and default value is + * empty string ({@link #DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG}). + * For instance, if we do not want meta region to be moved to RS with higher + * version until that version is >= 2.0.0, then we can configure + * "hbase.min.version.move.system.tables" as "2.0.0". + * When operator uses this config, it should be used with care, meaning + * we should be confident that even if user table regions come to RS with + * higher version (that rest of cluster), it would not cause any + * incompatibility issues. + */ + 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 = ""; + /** * For testing only! Set to true to skip handling of split and merge. */ @@ -348,6 +370,8 @@ public AssignmentManager(MasterServices server, ServerManager serverManager, scheduledThreadPoolExecutor.scheduleWithFixedDelay(new FailedOpenRetryRunnable(), failedOpenRetryPeriod, failedOpenRetryPeriod, TimeUnit.MILLISECONDS); } + minVersionToMoveSysTables = conf.get(MIN_VERSION_MOVE_SYS_TABLES_CONFIG, + DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG); } /** @@ -2564,6 +2588,45 @@ public int compare(Pair o1, Pair o2) { return res; } + /** + * Get a list of servers that this region can not assign to. + * For system table, we must assign them to a server with highest version. + * This method is same as {@link #getExcludedServersForSystemTable()} with + * the only difference is we can disable this exclusion using config: + * "hbase.min.version.move.system.tables". + * + * @return List of Excluded servers for System table regions. + */ + private List getExcludedServersForSystemTableUnlessAllowed() { + List> serverList = new ArrayList<>(); + for (ServerName s : serverManager.getOnlineServersList()) { + serverList.add(new Pair<>(s, server.getRegionServerVersion(s))); + } + if (serverList.isEmpty()) { + return Collections.emptyList(); + } + String highestVersion = Collections.max(serverList, + new Comparator>() { + @Override + public int compare(Pair o1, Pair o2) { + return VersionInfo.compareVersion(o1.getSecond(), o2.getSecond()); + } + }).getSecond(); + if (!DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG.equals(minVersionToMoveSysTables)) { + int comparedValue = + VersionInfo.compareVersion(minVersionToMoveSysTables, highestVersion); + if (comparedValue > 0) { + return Collections.emptyList(); + } + } + List res = new ArrayList<>(); + for (Pair pair : serverList) { + if (!pair.getSecond().equals(highestVersion)) { + res.add(pair.getFirst()); + } + } + return res; + } /** * @param region the region to assign @@ -2686,7 +2749,7 @@ public void run() { synchronized (checkIfShouldMoveSystemRegionLock) { // RS register on ZK after reports startup on master List regionsShouldMove = new ArrayList<>(); - for (ServerName server : getExcludedServersForSystemTable()) { + for (ServerName server : getExcludedServersForSystemTableUnlessAllowed()) { regionsShouldMove.addAll(getCarryingSystemTables(server)); } if (!regionsShouldMove.isEmpty()) { From 0cc95dde287e106ac39884626ac5e6002315bd37 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 30 Jun 2021 20:46:38 +0530 Subject: [PATCH 2/4] addressing comments by @apurtell and @bharathv --- .../hbase/master/AssignmentManager.java | 64 ++++++------------- 1 file changed, 20 insertions(+), 44 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index 7da8f43b659f..c293ec149916 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -255,20 +255,11 @@ public class AssignmentManager extends ZooKeeperListener { private final RegionStateStore regionStateStore; /** - * Min version to consider for moving system tables regions to higher - * versioned RS. If RS has higher version than rest of the cluster but that - * version is less than this value, we should not move system table regions - * to that RS. If RS has higher version than rest of the cluster but that - * version is greater than or equal to this value, we should move system - * table regions to that RS. This is optional config and default value is - * empty string ({@link #DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG}). - * For instance, if we do not want meta region to be moved to RS with higher - * version until that version is >= 2.0.0, then we can configure - * "hbase.min.version.move.system.tables" as "2.0.0". - * When operator uses this config, it should be used with care, meaning - * we should be confident that even if user table regions come to RS with - * higher version (that rest of cluster), it would not cause any - * incompatibility issues. + * When the operator uses this configuration option, any version between + * the current version and the new value of + * "hbase.min.version.move.system.tables" does not trigger any region movement. + * It is assumed that the configured range of versions do not require special + * handling of moving system table regions to higher versioned RegionServer. */ private final String minVersionToMoveSysTables; @@ -2575,38 +2566,21 @@ private int setOfflineInZooKeeper(final RegionState state, final ServerName dest * know the version. So in fact we will never assign a system region to a RS without registering on zk. */ public List getExcludedServersForSystemTable() { - List> serverList = new ArrayList<>(); - for (ServerName s : serverManager.getOnlineServersList()) { - serverList.add(new Pair<>(s, server.getRegionServerVersion(s))); - } - if (serverList.isEmpty()) { - return new ArrayList<>(); - } - String highestVersion = Collections.max(serverList, new Comparator>() { - @Override - public int compare(Pair o1, Pair o2) { - return VersionInfo.compareVersion(o1.getSecond(), o2.getSecond()); - } - }).getSecond(); - List res = new ArrayList<>(); - for (Pair pair : serverList) { - if (!pair.getSecond().equals(highestVersion)) { - res.add(pair.getFirst()); - } - } - return res; + return getExcludedServersForSystemTable(false); } /** * Get a list of servers that this region can not assign to. * For system table, we must assign them to a server with highest version. - * This method is same as {@link #getExcludedServersForSystemTable()} with - * the only difference is we can disable this exclusion using config: - * "hbase.min.version.move.system.tables". + * We can disable this exclusion using config: + * "hbase.min.version.move.system.tables" if checkForMinVersion is true. * + * @param checkForMinVersion if true, check for minVersionToMoveSysTables + * and decide moving system table regions accordingly. * @return List of Excluded servers for System table regions. */ - private List getExcludedServersForSystemTableUnlessAllowed() { + private List getExcludedServersForSystemTable( + boolean checkForMinVersion) { List> serverList = new ArrayList<>(); for (ServerName s : serverManager.getOnlineServersList()) { serverList.add(new Pair<>(s, server.getRegionServerVersion(s))); @@ -2621,11 +2595,13 @@ public int compare(Pair o1, Pair o2) { return VersionInfo.compareVersion(o1.getSecond(), o2.getSecond()); } }).getSecond(); - if (!DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG.equals(minVersionToMoveSysTables)) { - int comparedValue = - VersionInfo.compareVersion(minVersionToMoveSysTables, highestVersion); - if (comparedValue > 0) { - return Collections.emptyList(); + if (checkForMinVersion) { + if (!DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG.equals(minVersionToMoveSysTables)) { + int comparedValue = VersionInfo + .compareVersion(minVersionToMoveSysTables, highestVersion); + if (comparedValue > 0) { + return Collections.emptyList(); + } } } List res = new ArrayList<>(); @@ -2758,7 +2734,7 @@ public void run() { synchronized (checkIfShouldMoveSystemRegionLock) { // RS register on ZK after reports startup on master List regionsShouldMove = new ArrayList<>(); - for (ServerName server : getExcludedServersForSystemTableUnlessAllowed()) { + for (ServerName server : getExcludedServersForSystemTable(true)) { regionsShouldMove.addAll(getCarryingSystemTables(server)); } if (!regionsShouldMove.isEmpty()) { From b7ec7e044f1758acd4e475adab64cfc219a21625 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 30 Jun 2021 23:51:37 +0530 Subject: [PATCH 3/4] addressing comments by @saintstack --- .../hbase/master/AssignmentManager.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index c293ec149916..29d7bc11b17e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -256,10 +256,22 @@ public class AssignmentManager extends ZooKeeperListener { /** * When the operator uses this configuration option, any version between - * the current version and the new value of - * "hbase.min.version.move.system.tables" does not trigger any region movement. - * It is assumed that the configured range of versions do not require special + * 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; @@ -2560,8 +2572,10 @@ private int setOfflineInZooKeeper(final RegionState state, final ServerName dest } /** - * Get a list of servers that this region can not assign to. - * For system table, 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. * RS will report to master before register on zk, and only when RS have registered on zk we can * know the version. So in fact we will never assign a system region to a RS without registering on zk. */ @@ -2570,10 +2584,13 @@ public List getExcludedServersForSystemTable() { } /** - * Get a list of servers that this region can not assign to. - * For system table, we must assign them to a server with highest version. - * We can disable this exclusion using config: + * 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 true, check for minVersionToMoveSysTables * and decide moving system table regions accordingly. From 9986bfada9a30921cdeb5fca72c2fa42fa136c39 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Thu, 1 Jul 2021 00:06:23 +0530 Subject: [PATCH 4/4] addendum --- .../org/apache/hadoop/hbase/master/AssignmentManager.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index 29d7bc11b17e..28555c8c056b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -2592,8 +2592,11 @@ public List getExcludedServersForSystemTable() { * "hbase.min.version.move.system.tables" if checkForMinVersion is true. * Detailed explanation available with definition of minVersionToMoveSysTables. * - * @param checkForMinVersion if true, check for minVersionToMoveSysTables - * and decide moving system table regions accordingly. + * @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(