-
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-25032 Do not assign regions to region server which has not call… #3268
Conversation
As The trick is straight forward, in Added two UTs, one is to make sure we do not assign regions to a region server which has not called Thanks. |
🎊 +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.
nice patch, simple and neat, makes sense to me.
Set<ServerName> destServers = new HashSet<>(); | ||
onlineServers.forEach((sn, sm) -> { | ||
if (sm.getLastReportTimestamp() > 0) { | ||
// This means we have already called regionServerReport at leaset once, then let's include |
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.
nit: 'least' typo.
Test failures seem related? |
Yes, should be related. Let me see how to fix the UTs. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
TestRSGroupKillRS is broken. Still digging... |
🎊 +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.
Nice.
One nit.
@@ -76,7 +76,7 @@ | |||
private MasterServices masterServices; | |||
private FavoredNodesManager favoredNodesManager; | |||
private volatile RSGroupInfoManager rsGroupInfoManager; | |||
private LoadBalancer internalBalancer; | |||
private volatile LoadBalancer internalBalancer; |
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 belongs in this patch?
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.
In general, no.
But this is also a fix on the balancer implementation and this patch will be applied to all active branches I believe, so I think it is OK to also include these minor changes.
…ed regionServerReport yet
🎊 +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. |
…ed regionServerReport yet (#3268) Signed-off-by: Bharath Vissapragada <[email protected]> Signed-off-by: Michael Stack <[email protected]>
…ed regionServerReport yet (#3268) Signed-off-by: Bharath Vissapragada <[email protected]> Signed-off-by: Michael Stack <[email protected]>
…ed regionServerReport yet (#3268) Signed-off-by: Bharath Vissapragada <[email protected]> Signed-off-by: Michael Stack <[email protected]>
…which has not called regionServerReport yet (apache#3268) Signed-off-by: Bharath Vissapragada <[email protected]> Signed-off-by: Michael Stack <[email protected]>
…which has not called regionServerReport yet (apache#3268) Signed-off-by: Bharath Vissapragada <[email protected]> Signed-off-by: Michael Stack <[email protected]>
…which has not called regionServerReport yet (apache#3268) Signed-off-by: Bharath Vissapragada <[email protected]> Signed-off-by: Michael Stack <[email protected]>
…ed regionServerReport yet