-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-25518 Support separate child regions to different region servers #3001
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
saintstack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patch looks good. Some comments below.
| } | ||
|
|
||
| /** | ||
| * Create round robin assign procedures for the give regions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/give/given/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll change it.
| /** Default value for the balancer period */ | ||
| public static final int DEFAULT_HBASE_BALANCER_PERIOD = 300000; | ||
|
|
||
| /** Config for support separate child regions */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this config needs more explaination about what it enables. Does it need a default setting too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll add more explanation and a default value for it.
| ServerName parentServer) { | ||
| if(env.getMasterConfiguration().getBoolean(HConstants.HBASE_ENABLE_SEPARATE_CHILD_REGIONS, | ||
| false)){ | ||
| // keep daughter one on the parent RS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/keep daughter one/keep one daughter/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll change it.
|
|
||
| @Test | ||
| public void testSplitTableRegionAndSeparateChildRegions() throws Exception { | ||
| cleanupTest(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should make a new test suite with this test in it rather than do this cleanupTest and setupCluster... These methods are for before the test class and after... not for calling in the test as here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll change it.
| assertEquals("Table region not correct.", 2, tableRegions.size()); | ||
| Map<RegionInfo, ServerName> regionInfoMap = UTIL.getHBaseCluster().getMaster().getAssignmentManager() | ||
| .getRegionStates().getRegionAssignments(); | ||
| assertNotEquals(regionInfoMap.get(tableRegions.get(0).getRegionInfo()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can you be sure the roundrobin will not assign both daughters to same server?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the test cluster has three region servers, and before the round robin assignment, one daughter will be kept on the parent server, which will be excluded from the round robin servers set. At last, the other daughter and its replicas will be assigned to the other two servers. More than one servers in the destination server set will make round robin work well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
d5abc17 to
a5f6706
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Shout if you need input on how to fix the licensing issue. |
|
💔 -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. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
#3001) Signed-off-by: stack <stack@apache.org>
#3001) Signed-off-by: stack <stack@apache.org>
No description provided.