-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-22707 [HBCK2] MasterRpcServices assigns method should try to reload regions from meta if the passed regions isn't found under AssignmentManager RegionsStateStore #403
Conversation
…load regions from meta if the passed regions isn't found under AssignmentManager RegionsStateStore
…load regions from meta if the passed regions isn't found under AssignmentManager RegionsStateStore
💔 -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. |
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
Show resolved
Hide resolved
💔 -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. |
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.
Really good.
+1 after addressing nits below.
hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
Show resolved
Hide resolved
regionStateStore.visitMetaForRegion(regionEncodedName, visitor); | ||
RegionInfo regionInfo = regionStates.getRegionState(regionEncodedName) == null ? null : | ||
regionStates.getRegionState(regionEncodedName).getRegion(); | ||
return regionInfo; |
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.
Change this from:
RegionInfo regionInfo = regionStates.getRegionState(regionEncodedName) == null ? null :
regionStates.getRegionState(regionEncodedName).getRegion();
return regionInfo;
to:
return regionStates.getRegionState(regionEncodedName) == null ? null :
regionStates.getRegionState(regionEncodedName).getRegion();
RegionInfo regionInfo = regionStates.getRegionState(regionEncodedName) == null ? null : | ||
regionStates.getRegionState(regionEncodedName).getRegion(); | ||
return regionInfo; | ||
}catch(IOException e){ |
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.
Need space after '}' and before '{'.
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
Show resolved
Hide resolved
throws IOException { | ||
Result result = MetaTableAccessor. | ||
scanByRegionEncodedName(master.getConnection(), regionEncodedName); | ||
if(result != null) { |
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.
Need space after 'if'.
💔 -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. |
Thanks for the suggestions @saintstack @jatsakthi ! I had pushed a new commit addressing the last comments. Am still going to add UTs for the newly added public methods. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Pushed new commits with UTs for the new public methods added on this PR. Let me know on your thoughts, @jatsakthi @saintstack ! |
💔 -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. |
💔 -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. |
💔 -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. |
💔 -1 overall
This message was automatically generated. |
Some refactoring for code re-use in AssignmentManager and MasterRpcServices, new convenient methods added in MetaTableAccessor and RegionStateStore.