[GitHub] [hbase] saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase
saintstack commented on a change in pull request #1165: HBASE-22514 Move
rsgroup feature into core of HBase
URL: https://github.com/apache/hbase/pull/1165#discussion_r383988388
##
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##
@@ -2262,4 +2265,104 @@ boolean snapshotCleanupSwitch(final boolean on, final
boolean synchronous)
*/
boolean isSnapshotCleanupEnabled() throws IOException;
+ /**
+ * Creates a new RegionServer group with the given name
+ * @param groupName the name of the group
+ * @throws IOException if a remote or network exception occurs
+ */
+ void addRSGroup(String groupName) throws IOException;
+
+ /**
+ * Get group info for the given group name
+ * @param groupName the group name
+ * @return group info
+ * @throws IOException if a remote or network exception occurs
+ */
+ RSGroupInfo getRSGroup(String groupName) throws IOException;
+
+ /**
+ * Get group info for the given hostPort
+ * @param hostPort HostPort to get RSGroupInfo for
+ * @throws IOException if a remote or network exception occurs
+ */
+ RSGroupInfo getRSGroup(Address hostPort) throws IOException;
+
+ /**
+ * Get group info for the given table
+ * @param tableName table name to get RSGroupInfo for
+ * @throws IOException if a remote or network exception occurs
+ */
+ RSGroupInfo getRSGroup(TableName tableName) throws IOException;
+
+ /**
+ * Lists current set of RegionServer groups
+ * @throws IOException if a remote or network exception occurs
+ */
+ List listRSGroups() throws IOException;
+
+ /**
+ * Get all tables in this RegionServer group.
+ * @param groupName the group name
+ * @throws IOException if a remote or network exception occurs
+ * @see #getConfiguredNamespacesAndTablesInRSGroup(String)
+ */
+ List listTablesInRSGroup(String groupName) throws IOException;
+
+ /**
+ * Get the namespaces and tables which have this RegionServer group in
descriptor.
+ *
+ * The difference between this method and {@link
#listTablesInRSGroup(String)} is that, this
+ * method will not include the table which is actually in this RegionServr
group but without the
+ * RegionServer group configuration in its {@link TableDescriptor}. For
example, we have a group
+ * 'A', and we make namespace 'nsA' in this group, then all the tables under
this namespace will
+ * in the group 'A', but this method will not return these tables but only
the namespace 'nsA',
+ * while the {@link #listTablesInRSGroup(String)} will return all these
tables.
+ * @param groupName the group name
+ * @throws IOException if a remote or network exception occurs
+ * @see #listTablesInRSGroup(String)
+ */
+ Pair, List>
getConfiguredNamespacesAndTablesInRSGroup(String groupName)
Review comment:
The 'Configure' seems redundant to me given the suffix on method name is
'InRSGroup'.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase
saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase URL: https://github.com/apache/hbase/pull/1165#discussion_r383986883 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java ## @@ -2262,4 +2265,104 @@ boolean snapshotCleanupSwitch(final boolean on, final boolean synchronous) */ boolean isSnapshotCleanupEnabled() throws IOException; + /** + * Creates a new RegionServer group with the given name + * @param groupName the name of the group + * @throws IOException if a remote or network exception occurs + */ + void addRSGroup(String groupName) throws IOException; Review comment: Ok. So, your argument is that folks using current data structures and methods will have an easier time if the names of methods, etc., have same basic pattern in the new context. Ok. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase
saintstack commented on a change in pull request #1165: HBASE-22514 Move
rsgroup feature into core of HBase
URL: https://github.com/apache/hbase/pull/1165#discussion_r378448167
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##
@@ -3498,13 +3511,13 @@ public SplitOrMergeTracker getSplitOrMergeTracker() {
}
@Override
- public LoadBalancer getLoadBalancer() {
+ public RSGroupBasedLoadBalancer getLoadBalancer() {
Review comment:
Yeah, can't just return 'LoadBalancer' only now LB handles RSGroups too?
(Maybe not possible for RSGroupLB...)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase
saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase URL: https://github.com/apache/hbase/pull/1165#discussion_r378438971 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java ## @@ -2262,4 +2265,104 @@ boolean snapshotCleanupSwitch(final boolean on, final boolean synchronous) */ boolean isSnapshotCleanupEnabled() throws IOException; + /** + * Creates a new RegionServer group with the given name + * @param groupName the name of the group + * @throws IOException if a remote or network exception occurs + */ + void addRSGroup(String groupName) throws IOException; Review comment: Do we still need 'RS'? Should this be addGroup or addRegionServerGroup? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase
saintstack commented on a change in pull request #1165: HBASE-22514 Move
rsgroup feature into core of HBase
URL: https://github.com/apache/hbase/pull/1165#discussion_r378447342
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##
@@ -382,7 +386,7 @@ public void run() {
private final LockManager lockManager = new LockManager(this);
- private LoadBalancer balancer;
+ private RSGroupBasedLoadBalancer balancer;
Review comment:
This balancer acts like old balancer when RSGroups is off? If so, just
rename RSGroupBasedLB as LB?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase
saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase URL: https://github.com/apache/hbase/pull/1165#discussion_r378439617 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java ## @@ -2262,4 +2265,104 @@ boolean snapshotCleanupSwitch(final boolean on, final boolean synchronous) */ boolean isSnapshotCleanupEnabled() throws IOException; + /** + * Creates a new RegionServer group with the given name + * @param groupName the name of the group + * @throws IOException if a remote or network exception occurs + */ + void addRSGroup(String groupName) throws IOException; Review comment: Hmm... Yeah, Group is too generic so s/RS/RegionGroup/ ? Big change I suppose. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #1165: HBASE-22514 Move rsgroup feature into core of HBase
saintstack commented on a change in pull request #1165: HBASE-22514 Move
rsgroup feature into core of HBase
URL: https://github.com/apache/hbase/pull/1165#discussion_r378440187
##
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##
@@ -2262,4 +2265,104 @@ boolean snapshotCleanupSwitch(final boolean on, final
boolean synchronous)
*/
boolean isSnapshotCleanupEnabled() throws IOException;
+ /**
+ * Creates a new RegionServer group with the given name
+ * @param groupName the name of the group
+ * @throws IOException if a remote or network exception occurs
+ */
+ void addRSGroup(String groupName) throws IOException;
+
+ /**
+ * Get group info for the given group name
+ * @param groupName the group name
+ * @return group info
+ * @throws IOException if a remote or network exception occurs
+ */
+ RSGroupInfo getRSGroup(String groupName) throws IOException;
+
+ /**
+ * Get group info for the given hostPort
+ * @param hostPort HostPort to get RSGroupInfo for
+ * @throws IOException if a remote or network exception occurs
+ */
+ RSGroupInfo getRSGroup(Address hostPort) throws IOException;
+
+ /**
+ * Get group info for the given table
+ * @param tableName table name to get RSGroupInfo for
+ * @throws IOException if a remote or network exception occurs
+ */
+ RSGroupInfo getRSGroup(TableName tableName) throws IOException;
+
+ /**
+ * Lists current set of RegionServer groups
+ * @throws IOException if a remote or network exception occurs
+ */
+ List listRSGroups() throws IOException;
+
+ /**
+ * Get all tables in this RegionServer group.
+ * @param groupName the group name
+ * @throws IOException if a remote or network exception occurs
+ * @see #getConfiguredNamespacesAndTablesInRSGroup(String)
+ */
+ List listTablesInRSGroup(String groupName) throws IOException;
+
+ /**
+ * Get the namespaces and tables which have this RegionServer group in
descriptor.
+ *
+ * The difference between this method and {@link
#listTablesInRSGroup(String)} is that, this
+ * method will not include the table which is actually in this RegionServr
group but without the
+ * RegionServer group configuration in its {@link TableDescriptor}. For
example, we have a group
+ * 'A', and we make namespace 'nsA' in this group, then all the tables under
this namespace will
+ * in the group 'A', but this method will not return these tables but only
the namespace 'nsA',
+ * while the {@link #listTablesInRSGroup(String)} will return all these
tables.
+ * @param groupName the group name
+ * @throws IOException if a remote or network exception occurs
+ * @see #listTablesInRSGroup(String)
+ */
+ Pair, List>
getConfiguredNamespacesAndTablesInRSGroup(String groupName)
Review comment:
Purge 'Configured' from this method name?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
