Add option for balancing a single table at a time when using host regex balancer #4523
Conversation
alancer This adds a new property balancer.host.regex.concurrent.tables which if true will process multiple tables at a time (this is the default). If set to false the balancer will stop processing after a table is found with migrations
cshannon
changed the title
| @@ -466,6 +475,13 @@ public long balance(BalanceParameters params) { | |||
| LOG.trace("Sample up to 10 outstanding migrations: {}", limitTen(migrations)); | |||
| } | |||
| return minBalanceTime; | |||
| } else if (!myConf.concurrentTables) { | |||
| LOG.warn("Not balancing tables due to {} existing migrations and {}} is set to false", | |||
There was a problem hiding this comment.
Should this be at debug? It seems that it will be common and would be expected to sort itself out eventually. It would only be a concern is it was persistent? Could the message include something to show that it may be making progress? Current id being processed?
Is this a log message that quickly flood the log?
There was a problem hiding this comment.
Yeah this should likely be debug or trace, I just copied it from above and didn't pay attention but that would definitely flood things pretty quickly and it is expected.
There was a problem hiding this comment.
This seems another case where we really could benefit from being able to log 1 of Number of 1 every T seconds occurrences of the same message. It is not helpful to keep repeating that same message frequently, but having it appear occasionally helps troubleshoot cases where something in not making progress - might be out of scope for this, but could be worthwhile.
This adds a new property
balancer.host.regex.concurrent.tableswhich if true will process multiple tables at a time (this is the default). If set to false the balancer will stop processing after a table is found with migrationsThis is a draft PR for now to start getting some feed back on the approach. I have implemented the 4 steps but not quite sure it's actually correct yet. I've been playing around with it for a little while tweaking things and trying to test but it hasn't been tested for real yet (other than the smiple test in BaseHostRegexTAbleLoadBalancerTest) yet.
I made the property a default of true for concurrent but we could negate that and make the property something like
balancer.host.regex.single.tableand default to false.This closes #4521