-
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-25757 Move BaseLoadBalancer to hbase-balancer module #3191
Conversation
Put it here first. I think we could land some pre patches first before landing this patch, where the pre patches could be landed to branch-2 too. And we can not move the tests to hbase-balancer for now, as the in the base test class we make use of StochasticLoadBalancer, and it uses a NamedQueueRecorder which is in hbase-server. Will refactor in later PRs to also move StochasticLoadBalancer and related test code to hbase-balancer. |
💔 -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. |
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.
LGTM
@@ -75,7 +75,7 @@ public void updateFavoredNodesMap(RegionInfo region, List<ServerName> servers) { | |||
} | |||
|
|||
/** | |||
* @return the list of favored region server for this region based on the plan | |||
* Returns the list of favored region server for this region based on the plan |
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.
we do not use javadoc ?
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.
This generates a error prone warning, where we do not have a javadoc summary for the method but only a return block.
[MissingSummary] A summary fragment is required; consider using the value of the @return block as a summary fragment instead.
The suggestion is to just use the doc for @return
as the javadoc summary.
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.
Get it, Thanks!
|
FavoredNodeLoadBalancer is another story, @saintstack suggests that we just purge it from our code base as it never worked well... Plan to start a discuss thread in the mailing list to see if there are any users of this balancer. |
No description provided.