saintstack commented on a change in pull request #3439:
URL: https://github.com/apache/hbase/pull/3439#discussion_r661685304
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -163,6 +163,19 @@
private final RegionStates regionStates = new RegionStates();
private final RegionStateStore regionStateStore;
+ /**
+ * When the operator uses this configuration option, any version between
+ * the current version and the new value of
Review comment:
Is it 'new value' or just 'value'.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -163,6 +163,19 @@
private final RegionStates regionStates = new RegionStates();
private final RegionStateStore regionStateStore;
+ /**
+ * 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.
Review comment:
s/do/does/ and s/of//. Perhaps point at the location in the code where
this is done.
Can we add examples of how this would work -- the ones from the JIRA?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -2280,24 +2295,48 @@ private void addToPendingAssignment(final
HashMap<RegionInfo, RegionStateNode> r
* For system tables, we must assign them to a server with highest version.
*/
public List<ServerName> getExcludedServersForSystemTable() {
+ 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.
+ * 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.
Review comment:
This method doesn't do move? It just looks for servers to exclude....
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -163,6 +163,19 @@
private final RegionStates regionStates = new RegionStates();
private final RegionStateStore regionStateStore;
+ /**
+ * 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.
Review comment:
Do you want to explain what the 'region movement' is here? The
auto-migration of system tables to newer server versions. A dev reading this
w/o context might not tie the 'reigon movement' to the auto-movement to newer
versions.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -2280,24 +2295,48 @@ private void addToPendingAssignment(final
HashMap<RegionInfo, RegionStateNode> r
* For system tables, we must assign them to a server with highest version.
*/
public List<ServerName> getExcludedServersForSystemTable() {
Review comment:
Is this used?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -2280,24 +2295,48 @@ private void addToPendingAssignment(final
HashMap<RegionInfo, RegionStateNode> r
* For system tables, we must assign them to a server with highest version.
*/
public List<ServerName> getExcludedServersForSystemTable() {
+ return getExcludedServersForSystemTable(false);
+ }
+
+ /**
+ * Get a list of servers that this region can not assign to.
Review comment:
There doesn't seem to be a 'region' passed to this method.... So is it
any system table region?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
