bbeaudreault commented on a change in pull request #3660:
URL: https://github.com/apache/hbase/pull/3660#discussion_r701991971
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -206,13 +219,13 @@ protected float getDefaultSlop() {
@Override
protected void loadConf(Configuration conf) {
super.loadConf(conf);
- maxSteps = conf.getInt(MAX_STEPS_KEY, maxSteps);
- stepsPerRegion = conf.getInt(STEPS_PER_REGION_KEY, stepsPerRegion);
- maxRunningTime = conf.getLong(MAX_RUNNING_TIME_KEY, maxRunningTime);
- runMaxSteps = conf.getBoolean(RUN_MAX_STEPS_KEY, runMaxSteps);
+ maxSteps = conf.getInt(MAX_STEPS_KEY, DEFAULT_MAX_STEPS);
Review comment:
Previously we used the existing class member variable, which is
initialized above, as the default values here. This works fine for the first
call to loadConf. If you were to add an override to one of these keys in your
hbase-site.xml, and then later delete that override and refresh, the true
default value would not be set. Instead you'd be stuck with your overridden
version forever until restarting the HMaster.
I updated this to follow the more usual HBase/Hadoop convention of defining
defaults as static fields, which fixes this bug and my test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]