From ba4cb91211717c6ac808df441616efb1b08c7486 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Thu, 6 May 2021 16:11:46 +0800 Subject: [PATCH] HBASE-25851 Make LoadBalancer not extend Configurable interface (#3233) Signed-off-by: Yulin Niu --- .../hadoop/hbase/master/LoadBalancer.java | 3 +- .../master/balancer/BaseLoadBalancer.java | 19 ++--- .../master/balancer/ClusterInfoProvider.java | 5 ++ .../balancer/DummyClusterInfoProvider.java | 83 +++++++++++++++++++ .../master/balancer/TestBaseLoadBalancer.java | 22 ++--- .../TestRegionHDFSBlockLocationFinder.java | 27 +----- .../favored/FavoredNodeLoadBalancer.java | 11 +-- .../apache/hadoop/hbase/master/HMaster.java | 1 - .../balancer/FavoredStochasticBalancer.java | 6 +- .../master/balancer/LoadBalancerFactory.java | 4 +- .../balancer/MaintenanceLoadBalancer.java | 4 +- .../balancer/MasterClusterInfoProvider.java | 6 ++ .../balancer/StochasticLoadBalancer.java | 8 +- .../rsgroup/RSGroupBasedLoadBalancer.java | 23 +---- .../balancer/RSGroupableBalancerTestBase.java | 4 + .../TestRSGroupBasedLoadBalancer.java | 8 +- ...rWithStochasticLoadBalancerAsInternal.java | 4 - 17 files changed, 135 insertions(+), 103 deletions(-) create mode 100644 hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/DummyClusterInfoProvider.java diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java index a6f5357d379..e78ad696014 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java @@ -22,7 +22,6 @@ import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; import java.util.List; import java.util.Map; -import org.apache.hadoop.conf.Configurable; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ClusterMetrics; import org.apache.hadoop.hbase.ServerName; @@ -45,7 +44,7 @@ import org.apache.yetus.audience.InterfaceAudience; * This class produces plans for the {@code AssignmentManager} to execute. */ @InterfaceAudience.Private -public interface LoadBalancer extends Configurable, Stoppable, ConfigurationObserver { +public interface LoadBalancer extends Stoppable, ConfigurationObserver { // Used to signal to the caller that the region(s) cannot be assigned // We deliberately use 'localhost' so the operation will fail fast diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java index 93a733126a1..85c61ff508a 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java @@ -78,7 +78,6 @@ public abstract class BaseLoadBalancer implements LoadBalancer { protected float slop; // overallSlop to control simpleLoadBalancer's cluster level threshold protected float overallSlop; - protected Configuration config; protected RackManager rackManager; protected MetricsBalancer metricsBalancer = null; protected ClusterMetrics clusterStatus = null; @@ -100,9 +99,11 @@ public abstract class BaseLoadBalancer implements LoadBalancer { this.metricsBalancer = (metricsBalancer != null) ? metricsBalancer : new MetricsBalancer(); } - @Override - public void setConf(Configuration conf) { - this.config = conf; + protected final Configuration getConf() { + return provider.getConfiguration(); + } + + protected void setConf(Configuration conf) { setSlop(conf); if (slop < 0) { slop = 0; @@ -116,8 +117,8 @@ public abstract class BaseLoadBalancer implements LoadBalancer { overallSlop = 1; } - this.rackManager = new RackManager(getConf()); - useRegionFinder = config.getBoolean("hbase.master.balancer.uselocality", true); + this.rackManager = new RackManager(conf); + useRegionFinder = conf.getBoolean("hbase.master.balancer.uselocality", true); if (useRegionFinder) { regionFinder = new RegionHDFSBlockLocationFinder(); regionFinder.setConf(conf); @@ -132,11 +133,6 @@ public abstract class BaseLoadBalancer implements LoadBalancer { this.overallSlop = conf.getFloat("hbase.regions.overallSlop", slop); } - @Override - public Configuration getConf() { - return this.config; - } - @Override public synchronized void setClusterMetrics(ClusterMetrics st) { this.clusterStatus = st; @@ -149,6 +145,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { @Override public void setClusterInfoProvider(ClusterInfoProvider provider) { this.provider = provider; + setConf(provider.getConfiguration()); if (useRegionFinder) { this.regionFinder.setClusterInfoProvider(provider); } diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterInfoProvider.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterInfoProvider.java index 0686cf8a0df..908bf9c36b6 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterInfoProvider.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterInfoProvider.java @@ -38,6 +38,11 @@ import org.apache.yetus.audience.InterfaceAudience; @InterfaceAudience.Private public interface ClusterInfoProvider { + /** + * Get the configuration. + */ + Configuration getConfiguration(); + /** * Get all the regions of this cluster. *

diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/DummyClusterInfoProvider.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/DummyClusterInfoProvider.java new file mode 100644 index 00000000000..c56b1b8e39f --- /dev/null +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/DummyClusterInfoProvider.java @@ -0,0 +1,83 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.master.balancer; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HDFSBlocksDistribution; +import org.apache.hadoop.hbase.ServerMetrics; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.TableDescriptor; + +public class DummyClusterInfoProvider implements ClusterInfoProvider { + + private final Configuration conf; + + public DummyClusterInfoProvider(Configuration conf) { + this.conf = conf; + } + + @Override + public Configuration getConfiguration() { + return conf; + } + + @Override + public List getAssignedRegions() { + return Collections.emptyList(); + } + + @Override + public TableDescriptor getTableDescriptor(TableName tableName) throws IOException { + return null; + } + + @Override + public int getNumberOfTables() throws IOException { + return 0; + } + + @Override + public HDFSBlocksDistribution computeHDFSBlocksDistribution(Configuration conf, + TableDescriptor tableDescriptor, RegionInfo regionInfo) throws IOException { + return new HDFSBlocksDistribution(); + } + + @Override + public boolean hasRegionReplica(Collection regions) throws IOException { + return false; + } + + @Override + public List getOnlineServersListWithPredicator(List servers, + Predicate filter) { + return Collections.emptyList(); + } + + @Override + public Map> getSnapShotOfAssignment(Collection regions) { + return Collections.emptyMap(); + } +} diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java index a6ae2acf88a..e515eadc26b 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java @@ -20,8 +20,6 @@ package org.apache.hadoop.hbase.master.balancer; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -34,11 +32,13 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.apache.commons.lang3.ArrayUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.ServerMetrics; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; @@ -91,9 +91,7 @@ public class TestBaseLoadBalancer extends BalancerTestBase { Configuration conf = HBaseConfiguration.create(); conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class); loadBalancer = new MockBalancer(); - loadBalancer.setConf(conf); - ClusterInfoProvider provider = mock(ClusterInfoProvider.class); - loadBalancer.setClusterInfoProvider(provider); + loadBalancer.setClusterInfoProvider(new DummyClusterInfoProvider(conf)); // Set up the rack topologies (5 machines per rack) rackManager = mock(RackManager.class); @@ -218,12 +216,14 @@ public class TestBaseLoadBalancer extends BalancerTestBase { LoadBalancer balancer = new MockBalancer(); Configuration conf = HBaseConfiguration.create(); conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class); - balancer.setConf(conf); - ClusterInfoProvider provider = mock(ClusterInfoProvider.class); - when( - provider.getOnlineServersListWithPredicator(anyList(), any())) - .thenReturn(idleServers); - balancer.setClusterInfoProvider(provider); + balancer.setClusterInfoProvider(new DummyClusterInfoProvider(conf) { + + @Override + public List getOnlineServersListWithPredicator(List servers, + Predicate filter) { + return idleServers; + } + }); RegionInfo hri1 = RegionInfoBuilder.newBuilder(TableName.valueOf(name.getMethodName())) .setStartKey(Bytes.toBytes("key1")) .setEndKey(Bytes.toBytes("key2")) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRegionHDFSBlockLocationFinder.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRegionHDFSBlockLocationFinder.java index f51764fd39f..8e129e347e2 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRegionHDFSBlockLocationFinder.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRegionHDFSBlockLocationFinder.java @@ -27,13 +27,10 @@ import static org.mockito.Mockito.when; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Random; -import java.util.function.Predicate; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ClusterMetrics; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -97,7 +94,7 @@ public class TestRegionHDFSBlockLocationFinder { @Before public void setUp() { finder = new RegionHDFSBlockLocationFinder(); - finder.setClusterInfoProvider(new ClusterInfoProvider() { + finder.setClusterInfoProvider(new DummyClusterInfoProvider(null) { @Override public TableDescriptor getTableDescriptor(TableName tableName) throws IOException { @@ -114,28 +111,6 @@ public class TestRegionHDFSBlockLocationFinder { TableDescriptor tableDescriptor, RegionInfo regionInfo) throws IOException { return generate(regionInfo); } - - @Override - public boolean hasRegionReplica(Collection regions) throws IOException { - return false; - } - - @Override - public List getOnlineServersListWithPredicator(List servers, - Predicate filter) { - return Collections.emptyList(); - } - - @Override - public Map> - getSnapShotOfAssignment(Collection regions) { - return Collections.emptyMap(); - } - - @Override - public int getNumberOfTables() { - return 0; - } }); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java index 17b2863bdb4..cf356ad237e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java @@ -29,7 +29,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.ServerMetrics; @@ -72,14 +71,8 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored private MasterServices services; private RackManager rackManager; - private Configuration conf; private FavoredNodesManager fnm; - @Override - public void setConf(Configuration conf) { - this.conf = conf; - } - public void setMasterServices(MasterServices services) { this.services = services; } @@ -87,10 +80,8 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored @Override public synchronized void initialize() throws HBaseIOException { super.initialize(); - super.setConf(conf); this.fnm = services.getFavoredNodesManager(); - this.rackManager = new RackManager(conf); - super.setConf(conf); + this.rackManager = new RackManager(getConf()); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index b3875befd64..8ca7370ebab 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -684,7 +684,6 @@ public class HMaster extends HRegionServer implements MasterServices { LoadBalancer.class); } this.balancer = new RSGroupBasedLoadBalancer(); - this.balancer.setConf(conf); this.loadBalancerTracker = new LoadBalancerTracker(zooKeeper, this); this.loadBalancerTracker.start(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java index 742863def01..c6081e5acf5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java @@ -35,6 +35,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.ThreadLocalRandom; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.ServerMetrics; import org.apache.hadoop.hbase.ServerName; @@ -303,10 +304,11 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements metricsBalancer.incrMiscInvocations(); + Configuration conf = getConf(); List favoredNodes = fnm.getFavoredNodes(regionInfo); if (favoredNodes == null || favoredNodes.isEmpty()) { // Generate new favored nodes and return primary - FavoredNodeAssignmentHelper helper = new FavoredNodeAssignmentHelper(servers, getConf()); + FavoredNodeAssignmentHelper helper = new FavoredNodeAssignmentHelper(servers, conf); helper.initialize(); try { favoredNodes = helper.generateFavoredNodes(regionInfo); @@ -323,7 +325,7 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements destination = onlineServers.get(ThreadLocalRandom.current().nextInt(onlineServers.size())); } - boolean alwaysAssign = getConf().getBoolean(FAVORED_ALWAYS_ASSIGN_REGIONS, true); + boolean alwaysAssign = conf.getBoolean(FAVORED_ALWAYS_ASSIGN_REGIONS, true); if (destination == null && alwaysAssign) { LOG.warn("Can't generate FN for region: " + regionInfo + " falling back"); destination = super.randomAssignment(regionInfo, servers); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerFactory.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerFactory.java index bfda12e335c..5af0180cf14 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerFactory.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerFactory.java @@ -46,8 +46,6 @@ public class LoadBalancerFactory { Class balancerKlass = conf.getClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, getDefaultLoadBalancerClass(), LoadBalancer.class); - LoadBalancer balancer = ReflectionUtils.newInstance(balancerKlass); - balancer.setConf(conf); - return balancer; + return ReflectionUtils.newInstance(balancerKlass); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java index 11a21029735..f9d06bacafa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java @@ -24,7 +24,6 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.conf.Configured; import org.apache.hadoop.hbase.ClusterMetrics; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; @@ -37,7 +36,7 @@ import org.apache.yetus.audience.InterfaceAudience; * a balancer which is only used in maintenance mode. */ @InterfaceAudience.Private -public class MaintenanceLoadBalancer extends Configured implements LoadBalancer { +public class MaintenanceLoadBalancer implements LoadBalancer { private volatile boolean stopped = false; @@ -122,5 +121,4 @@ public class MaintenanceLoadBalancer extends Configured implements LoadBalancer @Override public void updateBalancerStatus(boolean status) { } - } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MasterClusterInfoProvider.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MasterClusterInfoProvider.java index 3f0c618447c..afcfb34c8ba 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MasterClusterInfoProvider.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MasterClusterInfoProvider.java @@ -49,6 +49,11 @@ public class MasterClusterInfoProvider implements ClusterInfoProvider { this.services = services; } + @Override + public Configuration getConfiguration() { + return services.getConfiguration(); + } + @Override public List getAssignedRegions() { AssignmentManager am = services.getAssignmentManager(); @@ -100,4 +105,5 @@ public class MasterClusterInfoProvider implements ClusterInfoProvider { public int getNumberOfTables() throws IOException { return services.getTableDescriptors().getAll().size(); } + } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java index 9a7823daa50..74929b5d3fc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java @@ -170,7 +170,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { } @Override - public synchronized void setConf(Configuration conf) { + protected synchronized void setConf(Configuration conf) { super.setConf(conf); maxSteps = conf.getInt(MAX_STEPS_KEY, maxSteps); stepsPerRegion = conf.getInt(STEPS_PER_REGION_KEY, stepsPerRegion); @@ -221,16 +221,16 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { curFunctionCosts= new double[costFunctions.size()]; tempFunctionCosts= new double[costFunctions.size()]; - isBalancerDecisionRecording = getConf() + isBalancerDecisionRecording = conf .getBoolean(BaseLoadBalancer.BALANCER_DECISION_BUFFER_ENABLED, BaseLoadBalancer.DEFAULT_BALANCER_DECISION_BUFFER_ENABLED); - isBalancerRejectionRecording = getConf() + isBalancerRejectionRecording = conf .getBoolean(BaseLoadBalancer.BALANCER_REJECTION_BUFFER_ENABLED, BaseLoadBalancer.DEFAULT_BALANCER_REJECTION_BUFFER_ENABLED); if (this.namedQueueRecorder == null && (isBalancerDecisionRecording || isBalancerRejectionRecording)) { - this.namedQueueRecorder = NamedQueueRecorder.getInstance(getConf()); + this.namedQueueRecorder = NamedQueueRecorder.getInstance(conf); } LOG.info("Loaded config; maxSteps=" + maxSteps + ", stepsPerRegion=" + stepsPerRegion + diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java index fe82e2579be..f3dc80b0cb4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java @@ -73,7 +73,6 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Maps; public class RSGroupBasedLoadBalancer implements LoadBalancer { private static final Logger LOG = LoggerFactory.getLogger(RSGroupBasedLoadBalancer.class); - private Configuration config; private ClusterMetrics clusterStatus; private MasterServices masterServices; private FavoredNodesManager favoredNodesManager; @@ -97,19 +96,6 @@ public class RSGroupBasedLoadBalancer implements LoadBalancer { @InterfaceAudience.Private public RSGroupBasedLoadBalancer() {} - @Override - public Configuration getConf() { - return config; - } - - @Override - public void setConf(Configuration conf) { - this.config = conf; - if(internalBalancer != null) { - internalBalancer.setConf(conf); - } - } - @Override public void setClusterMetrics(ClusterMetrics sm) { this.clusterStatus = sm; @@ -347,10 +333,11 @@ public class RSGroupBasedLoadBalancer implements LoadBalancer { } // Create the balancer + Configuration conf = masterServices.getConfiguration(); Class balancerClass; - String balancerClassName = config.get(HBASE_RSGROUP_LOADBALANCER_CLASS); + String balancerClassName = conf.get(HBASE_RSGROUP_LOADBALANCER_CLASS); if (balancerClassName == null) { - balancerClass = config.getClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, + balancerClass = conf.getClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, LoadBalancerFactory.getDefaultLoadBalancerClass(), LoadBalancer.class); } else { try { @@ -364,7 +351,6 @@ public class RSGroupBasedLoadBalancer implements LoadBalancer { balancerClass = LoadBalancerFactory.getDefaultLoadBalancerClass(); } internalBalancer = ReflectionUtils.newInstance(balancerClass); - internalBalancer.setConf(config); internalBalancer.setClusterInfoProvider(new MasterClusterInfoProvider(masterServices)); if(clusterStatus != null) { internalBalancer.setClusterMetrics(clusterStatus); @@ -383,7 +369,7 @@ public class RSGroupBasedLoadBalancer implements LoadBalancer { internalBalancer.initialize(); // init fallback groups - this.fallbackEnabled = config.getBoolean(FALLBACK_GROUP_ENABLE_KEY, false); + this.fallbackEnabled = conf.getBoolean(FALLBACK_GROUP_ENABLE_KEY, false); } public boolean isOnline() { @@ -408,7 +394,6 @@ public class RSGroupBasedLoadBalancer implements LoadBalancer { @Override public void onConfigurationChange(Configuration conf) { - this.config = conf; boolean newFallbackEnabled = conf.getBoolean(FALLBACK_GROUP_ENABLE_KEY, false); if (fallbackEnabled != newFallbackEnabled) { LOG.info("Changing the value of {} from {} to {}", FALLBACK_GROUP_ENABLE_KEY, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/RSGroupableBalancerTestBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/RSGroupableBalancerTestBase.java index 1ac1622297d..d4d50adc8c7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/RSGroupableBalancerTestBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/RSGroupableBalancerTestBase.java @@ -33,6 +33,8 @@ import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableDescriptors; import org.apache.hadoop.hbase.TableName; @@ -70,6 +72,7 @@ public class RSGroupableBalancerTestBase extends BalancerTestBase { static Map tableDescs; int[] regionAssignment = new int[] { 2, 5, 7, 10, 4, 3, 1 }; static int regionId = 0; + static Configuration conf = HBaseConfiguration.create(); /** * Invariant is that all servers of a group have load between floor(avg) and @@ -406,6 +409,7 @@ public class RSGroupableBalancerTestBase extends BalancerTestBase { Mockito.when(services.getTableDescriptors()).thenReturn(tds); AssignmentManager am = Mockito.mock(AssignmentManager.class); Mockito.when(services.getAssignmentManager()).thenReturn(am); + Mockito.when(services.getConfiguration()).thenReturn(conf); return services; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java index 0da0e14a371..6e53d6d58df 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java @@ -30,9 +30,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import org.apache.commons.lang3.StringUtils; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; @@ -68,13 +66,11 @@ public class TestRSGroupBasedLoadBalancer extends RSGroupableBalancerTestBase { servers = generateServers(7); groupMap = constructGroupInfo(servers, groups); tableDescs = constructTableDesc(true); - Configuration conf = HBaseConfiguration.create(); conf.set("hbase.regions.slop", "0"); conf.set("hbase.rsgroup.grouploadbalancer.class", SimpleLoadBalancer.class.getCanonicalName()); loadBalancer = new RSGroupBasedLoadBalancer(); loadBalancer.setRsGroupInfoManager(getMockedGroupInfoManager()); loadBalancer.setMasterServices(getMockedMaster()); - loadBalancer.setConf(conf); loadBalancer.initialize(); } @@ -89,9 +85,8 @@ public class TestRSGroupBasedLoadBalancer extends RSGroupableBalancerTestBase { // Test with/without per table balancer. boolean[] perTableBalancerConfigs = { true, false }; for (boolean isByTable : perTableBalancerConfigs) { - Configuration conf = loadBalancer.getConf(); conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); Map> servers = mockClusterServers(); ArrayListMultimap list = convertToGroupBasedMap(servers); LOG.info("Mock Cluster : " + printStats(list)); @@ -193,7 +188,6 @@ public class TestRSGroupBasedLoadBalancer extends RSGroupableBalancerTestBase { assertFalse(loadBalancer.isFallbackEnabled()); // change FALLBACK_GROUP_ENABLE_KEY from false to true - Configuration conf = loadBalancer.getConf(); conf.setBoolean(RSGroupBasedLoadBalancer.FALLBACK_GROUP_ENABLE_KEY, true); loadBalancer.onConfigurationChange(conf); assertTrue(loadBalancer.isFallbackEnabled()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java index 4802847c15a..8db4ee2608e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java @@ -29,10 +29,8 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ClusterMetrics; import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.RegionMetrics; import org.apache.hadoop.hbase.ServerMetrics; import org.apache.hadoop.hbase.ServerName; @@ -66,7 +64,6 @@ public class TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal servers = generateServers(3); groupMap = constructGroupInfo(servers, groups); tableDescs = constructTableDesc(false); - Configuration conf = HBaseConfiguration.create(); conf.set("hbase.regions.slop", "0"); conf.setFloat("hbase.master.balancer.stochastic.readRequestCost", 10000f); conf.set("hbase.rsgroup.grouploadbalancer.class", @@ -74,7 +71,6 @@ public class TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal loadBalancer = new RSGroupBasedLoadBalancer(); loadBalancer.setRsGroupInfoManager(getMockedGroupInfoManager()); loadBalancer.setMasterServices(getMockedMaster()); - loadBalancer.setConf(conf); loadBalancer.initialize(); }