HBASE-22514 Move rsgroup feature into core of HBase #1165
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
saintstack
approved these changes
Feb 12, 2020
| * @param groupName the name of the group | ||
| * @throws IOException if a remote or network exception occurs | ||
| */ | ||
| void addRSGroup(String groupName) throws IOException; |
There was a problem hiding this comment.
Do we still need 'RS'? Should this be addGroup or addRegionServerGroup?
There was a problem hiding this comment.
Hmm... Yeah, Group is too generic so s/RS/RegionGroup/ ? Big change I suppose.
There was a problem hiding this comment.
The IA.Public data structure is call RSGroupInfo so...
There was a problem hiding this 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.
| * @throws IOException if a remote or network exception occurs | ||
| * @see #listTablesInRSGroup(String) | ||
| */ | ||
| Pair<List<String>, List<TableName>> getConfiguredNamespacesAndTablesInRSGroup(String groupName) |
There was a problem hiding this comment.
Purge 'Configured' from this method name?
There was a problem hiding this comment.
But it is the 'configured' namespaces and tables. For example, you have namespace 'N' under rs group 'R', this method will only return namespace 'N', and will not return the tables under namespace 'N'...
There was a problem hiding this comment.
The 'Configure' seems redundant to me given the suffix on method name is 'InRSGroup'.
There was a problem hiding this comment.
There is a listTablesInRSGroup method above, which will all the tables in the rs group, including the tables which are included becasue of their namespace is configued to be in the rs group...
| @@ -382,7 +386,7 @@ public void run() { | |||
|
|
|||
| private final LockManager lockManager = new LockManager(this); | |||
|
|
|||
| private LoadBalancer balancer; | |||
| private RSGroupBasedLoadBalancer balancer; | |||
There was a problem hiding this comment.
This balancer acts like old balancer when RSGroups is off? If so, just rename RSGroupBasedLB as LB?
| @@ -3498,13 +3511,13 @@ public SplitOrMergeTracker getSplitOrMergeTracker() { | |||
| } | |||
|
|
|||
| @Override | |||
| public LoadBalancer getLoadBalancer() { | |||
| public RSGroupBasedLoadBalancer getLoadBalancer() { | |||
There was a problem hiding this comment.
Yeah, can't just return 'LoadBalancer' only now LB handles RSGroups too? (Maybe not possible for RSGroupLB...)
This comment has been minimized.
This comment has been minimized.
busbey
requested changes
Feb 22, 2020
| @@ -87,7 +102,7 @@ public boolean containsServer(Address hostPort) { | |||
| /** | |||
| * Get list of servers. | |||
| */ | |||
| public Set<Address> getServers() { | |||
| public SortedSet<Address> getServers() { | |||
There was a problem hiding this comment.
Can we avoid changing the method signature on this IA.Public method?
There was a problem hiding this comment.
I think this one is fine? SortedSet is Set, and this is a new major release
This comment has been minimized.
This comment has been minimized.
|
Unit test failures look related? |
Let me take a look. All the UTs are not in a good state recently, even for master, the flakey job list are increased a lot... |
|
OK I think it is a problem introduced by the newly introduced master based meta look up feature. If I shutdown the running master and start a new one, then I must recreate the hbase client as it only records the old master address so it will not send request to the new one and cause the client to retry for ever... |
|
Plan to switch back to ZKConnectionRegistry first. Let me open an issue for the ref guide update. |
This comment has been minimized.
This comment has been minimized.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
The find bugs error is OOM... |
…aded (#362) Signed-off-by: Guanghao Zhang <zghao@apache.org>
Signed-off-by: Guanghao Zhang <zghao@apache.org>
…move hbase-rsgroup module (#399) Signed-off-by: Zheng Hu <openinx@gmail.com>
Signed-off-by: Guanghao Zhang <zghao@apache.org>
…ive servers (#464) Signed-off-by: Guanghao Zhang <zghao@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
…BASE-22695 (#498) Signed-off-by: Guanghao Zhang <zghao@apache.org>
Amending-Author: Duo Zhang <zhangduo@apache.org> Signed-off-by: stack <stack@apache.org>
…nd (#599) Signed-off-by: Guanghao Zhang <zghao@apache.org>
…always enabled (#595) Signed-off-by: Guanghao Zhang <zghao@apache.org>
Signed-off-by: Peter Somogyi <psomogyi@apache.org>
…roup part (#776) Signed-off-by: Duo Zhang <zhangduo@apache.org>
busbey
approved these changes
Feb 29, 2020
…657) Signed-off-by: Duo Zhang <zhangduo@apache.org>
…ed in HBASE-22932 (#813) Signed-off-by: Guanghao Zhang <zghao@apache.org>
…1117) Signed-off-by: Guanghao Zhang <zghao@apache.org>
Signed-off-by: Guanghao Zhang <zghao@apache.org>
…ethods (#1148) Signed-off-by: stack <stack@apache.org>
…ode base (#1152) Signed-off-by: stack <stack@apache.org> Signed-off-by: Viraj Jasani <vjasani@apache.org>
Signed-off-by: Sean Busbey <busbey@apache.org>
…ode base (#1224) Signed-off-by: Guanghao Zhang <zghao@apache.org>
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.