-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-22822 : Un/Re-schedule balancer chore with balance_switch #468
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. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Please review @apurtell @Reidddddd @anoopsjohn |
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.
Just a nit in the below. Not important. LGTM.
* | ||
* @param on boolean value indicates whether to turn the balancer on | ||
*/ | ||
void turnBalancer(boolean on) { |
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/turnBalancer/switchBalancer/ ?
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
IIRC, And there's one script One step back, current impl brings no harm to master nor good, but the new impl looks like may bring a new uncertainty. I incline to keep it still, and so, leave my -0. |
graceful_stop.sh turns on balancer at the end conditionally. If -nob arg is provided, it won't: here |
@Reidddddd |
Hmm, right. But it only works for master and branch-2.
Just rolling restart, maybe. One place I'm not sure is RSGroup's move_regions op. |
💔 -1 overall
This message was automatically generated. |
Notice there are lots tests failed. Have you already confirmed that they are all irrelevant to this change? thanks @virajjasani |
@xcangCRM yes, these test failures are irrelevant, second last build has +1 overall including unit tests. |
💔 -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. |
I agree that it should not keep doing this in shorter time. @Reidddddd would you like me to close this PR? |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Thanks! |
Closing the PR as per suggestion by @Reidddddd Thanks for the review @Reidddddd @saintstack |
💔 -1 overall
This message was automatically generated. |
balance_switch turns on/off balancer. When it is turned off, we don't remove balancer chore from scheduled chores hence it keeps running only to eventually find out that it is not supposed to perform any action(if balancer was turned off). We can unschedule the chore to prevent the chore() execution and reschedule it when it is turned on by balance_switch.
This should also facilitate running balancer immediately after triggering balance_switch true(due to default initial delay: 0), and then chore would continue running as per duration provided in hbase.balancer.period.