From 72d7641fb5edb5def3264e3fb6f668732ef37b86 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Thu, 13 May 2021 14:19:03 +0800 Subject: [PATCH] HBASE-25852 Move all the intialization work of LoadBalancer implementation to initialize method (#3248) (#3258) Signed-off-by: Michael Stack Signed-off-by: Yulin Niu --- .../rsgroup/RSGroupBasedLoadBalancer.java | 39 ++-- .../TestRSGroupBasedLoadBalancer.java | 2 + ...rWithStochasticLoadBalancerAsInternal.java | 8 +- .../favored/FavoredNodeLoadBalancer.java | 7 +- .../apache/hadoop/hbase/master/HMaster.java | 6 +- .../hadoop/hbase/master/LoadBalancer.java | 5 +- .../master/balancer/BaseLoadBalancer.java | 141 +++++--------- .../master/balancer/ClusterStatusChore.java | 2 +- .../balancer/FavoredStochasticBalancer.java | 9 +- .../balancer/MaintenanceLoadBalancer.java | 2 +- .../master/balancer/SimpleLoadBalancer.java | 48 ++++- .../balancer/StochasticLoadBalancer.java | 172 +++++++++--------- .../master/balancer/BalancerTestBase.java | 1 + .../master/balancer/BalancerTestBase2.java | 4 +- .../LoadOnlyFavoredStochasticBalancer.java | 9 +- .../master/balancer/TestBalancerDecision.java | 8 +- .../balancer/TestBalancerRejection.java | 6 +- .../balancer/TestSimpleLoadBalancer.java | 7 + .../TestStochasticBalancerJmxMetrics.java | 4 +- .../balancer/TestStochasticLoadBalancer.java | 12 +- ...tStochasticLoadBalancerBalanceCluster.java | 2 +- ...ochasticLoadBalancerHeterogeneousCost.java | 6 +- ...dBalancerRegionReplicaHighReplication.java | 2 +- ...ReplicaReplicationGreaterThanNumNodes.java | 2 +- ...ticLoadBalancerRegionReplicaSameHosts.java | 2 +- ...ticLoadBalancerRegionReplicaWithRacks.java | 2 +- 26 files changed, 242 insertions(+), 266 deletions(-) diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java index 188a74308d1..d736e94a27b 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.rsgroup; +import com.google.errorprone.annotations.RestrictedApi; import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; import java.util.ArrayList; @@ -70,7 +71,6 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Maps; public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { private static final Logger LOG = LoggerFactory.getLogger(RSGroupBasedLoadBalancer.class); - private ClusterMetrics clusterStatus; private MasterServices masterServices; private volatile RSGroupInfoManager rsGroupInfoManager; private LoadBalancer internalBalancer; @@ -92,12 +92,11 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { @InterfaceAudience.Private public RSGroupBasedLoadBalancer() {} + // must be called after calling initialize @Override - public void setClusterMetrics(ClusterMetrics sm) { - this.clusterStatus = sm; - if (internalBalancer != null) { - internalBalancer.setClusterMetrics(sm); - } + public synchronized void updateClusterMetrics(ClusterMetrics sm) { + assert internalBalancer != null; + internalBalancer.updateClusterMetrics(sm); } @Override @@ -105,11 +104,17 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { this.masterServices = masterServices; } + @RestrictedApi(explanation = "Should only be called in tests", link = "", + allowedOnPath = ".*/src/test/.*") + public void setRsGroupInfoManager(RSGroupInfoManager rsGroupInfoManager) { + this.rsGroupInfoManager = rsGroupInfoManager; + } + /** * Balance by RSGroup. */ @Override - public List balanceCluster( + public synchronized List balanceCluster( Map>> loadOfAllTable) throws IOException { if (!isOnline()) { throw new ConstraintException(RSGroupInfoManager.RSGROUP_TABLE_NAME + @@ -328,7 +333,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { throw new HBaseIOException(msg); } rsGroupInfoManager = cps.get(0).getGroupInfoManager(); - if(rsGroupInfoManager == null){ + if (rsGroupInfoManager == null) { String msg = "RSGroupInfoManager hasn't been initialized"; LOG.error(msg); throw new HBaseIOException(msg); @@ -342,17 +347,14 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { Configuration conf = masterServices.getConfiguration(); // Create the balancer Class balancerClass = conf.getClass(HBASE_RSGROUP_LOADBALANCER_CLASS, - StochasticLoadBalancer.class, LoadBalancer.class); + StochasticLoadBalancer.class, LoadBalancer.class); if (this.getClass().isAssignableFrom(balancerClass)) { LOG.warn("The internal balancer of RSGroupBasedLoadBalancer cannot be itself, " + - "falling back to the default LoadBalancer class"); + "falling back to the default LoadBalancer class"); balancerClass = LoadBalancerFactory.getDefaultLoadBalancerClass(); } internalBalancer = ReflectionUtils.newInstance(balancerClass); internalBalancer.setMasterServices(masterServices); - if (clusterStatus != null) { - internalBalancer.setClusterMetrics(clusterStatus); - } internalBalancer.initialize(); // init fallback groups this.fallbackEnabled = conf.getBoolean(FALLBACK_GROUP_ENABLE_KEY, false); @@ -379,7 +381,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { } @Override - public void onConfigurationChange(Configuration conf) { + public synchronized void onConfigurationChange(Configuration 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, @@ -391,19 +393,16 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { @Override public void stop(String why) { + internalBalancer.stop(why); } @Override public boolean isStopped() { - return false; - } - - public void setRsGroupInfoManager(RSGroupInfoManager rsGroupInfoManager) { - this.rsGroupInfoManager = rsGroupInfoManager; + return internalBalancer.isStopped(); } @Override - public void postMasterStartupInitialize() { + public synchronized void postMasterStartupInitialize() { this.internalBalancer.postMasterStartupInitialize(); } diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java index c901bbd7180..c2e9e0f34bd 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java @@ -41,6 +41,7 @@ import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.rsgroup.RSGroupBasedLoadBalancer; import org.apache.hadoop.hbase.rsgroup.RSGroupInfo; import org.apache.hadoop.hbase.testclassification.LargeTests; +import org.apache.hadoop.net.DNSToSwitchMapping; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -67,6 +68,7 @@ public class TestRSGroupBasedLoadBalancer extends RSGroupableBalancerTestBase { tableDescs = constructTableDesc(true); conf.set("hbase.regions.slop", "0"); conf.set("hbase.rsgroup.grouploadbalancer.class", SimpleLoadBalancer.class.getCanonicalName()); + conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class); loadBalancer = new RSGroupBasedLoadBalancer(); loadBalancer.setRsGroupInfoManager(getMockedGroupInfoManager()); loadBalancer.setMasterServices(getMockedMaster()); diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java index e2ef688af4d..f9b5c9c503f 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal.java @@ -42,6 +42,7 @@ import org.apache.hadoop.hbase.rsgroup.RSGroupBasedLoadBalancer; import org.apache.hadoop.hbase.rsgroup.RSGroupInfo; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.net.DNSToSwitchMapping; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -67,7 +68,8 @@ public class TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal conf.set("hbase.regions.slop", "0"); conf.setFloat("hbase.master.balancer.stochastic.readRequestCost", 10000f); conf.set("hbase.rsgroup.grouploadbalancer.class", - StochasticLoadBalancer.class.getCanonicalName()); + StochasticLoadBalancer.class.getCanonicalName()); + conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class); loadBalancer = new RSGroupBasedLoadBalancer(); loadBalancer.setRsGroupInfoManager(getMockedGroupInfoManager()); loadBalancer.setMasterServices(getMockedMaster()); @@ -113,7 +115,7 @@ public class TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal serverMetricsMap.put(serverC, mockServerMetricsWithReadRequests(serverC, regionsOnServerC, 0)); ClusterMetrics clusterStatus = mock(ClusterMetrics.class); when(clusterStatus.getLiveServerMetrics()).thenReturn(serverMetricsMap); - loadBalancer.setClusterMetrics(clusterStatus); + loadBalancer.updateClusterMetrics(clusterStatus); // ReadRequestCostFunction are Rate based, So doing setClusterMetrics again // this time, regions on serverA with more readRequestCount load @@ -128,7 +130,7 @@ public class TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal serverMetricsMap.put(serverC, mockServerMetricsWithReadRequests(serverC, regionsOnServerC, 0)); clusterStatus = mock(ClusterMetrics.class); when(clusterStatus.getLiveServerMetrics()).thenReturn(serverMetricsMap); - loadBalancer.setClusterMetrics(clusterStatus); + loadBalancer.updateClusterMetrics(clusterStatus); Map>> LoadOfAllTable = (Map) mockClusterServersWithTables(clusterState); 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 122baa08ab5..fdbd53115df 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 @@ -36,7 +36,6 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position; -import org.apache.hadoop.hbase.master.RackManager; import org.apache.hadoop.hbase.master.RegionPlan; import org.apache.hadoop.hbase.master.ServerManager; import org.apache.hadoop.hbase.master.SnapshotOfRegionAssignmentFromMeta; @@ -68,14 +67,12 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Sets; public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements FavoredNodesPromoter { private static final Logger LOG = LoggerFactory.getLogger(FavoredNodeLoadBalancer.class); - private RackManager rackManager; private FavoredNodesManager fnm; @Override - public synchronized void initialize() throws HBaseIOException { + public void initialize() { super.initialize(); this.fnm = services.getFavoredNodesManager(); - this.rackManager = new RackManager(getConf()); } @Override @@ -316,7 +313,7 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored regionsOnServer.add(region); } - public synchronized List getFavoredNodes(RegionInfo regionInfo) { + public List getFavoredNodes(RegionInfo regionInfo) { return this.fnm.getFavoredNodes(regionInfo); } 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 f969d424295..a4b5b1f7839 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 @@ -919,8 +919,8 @@ public class HMaster extends HRegionServer implements MasterServices { // initialize load balancer this.balancer.setMasterServices(this); - this.balancer.setClusterMetrics(getClusterMetricsWithoutCoprocessor()); this.balancer.initialize(); + this.balancer.updateClusterMetrics(getClusterMetricsWithoutCoprocessor()); // start up all service threads. status.setStatus("Initializing master service threads"); @@ -1003,7 +1003,7 @@ public class HMaster extends HRegionServer implements MasterServices { } // set cluster status again after user regions are assigned - this.balancer.setClusterMetrics(getClusterMetricsWithoutCoprocessor()); + this.balancer.updateClusterMetrics(getClusterMetricsWithoutCoprocessor()); // Start balancer and meta catalog janitor after meta and regions have been assigned. status.setStatus("Starting balancer and catalog janitor"); @@ -1718,7 +1718,7 @@ public class HMaster extends HRegionServer implements MasterServices { } //Give the balancer the current cluster state. - this.balancer.setClusterMetrics(getClusterMetricsWithoutCoprocessor()); + this.balancer.updateClusterMetrics(getClusterMetricsWithoutCoprocessor()); List plans = this.balancer.balanceCluster(assignments); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java index 38409909782..ae6df6e205d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java @@ -73,10 +73,9 @@ public interface LoadBalancer extends Stoppable, ConfigurationObserver { ServerName BOGUS_SERVER_NAME = ServerName.valueOf("localhost,1,1"); /** - * Set the current cluster status. This allows a LoadBalancer to map host name to a server - * @param st + * Set the current cluster status. This allows a LoadBalancer to map host name to a server */ - void setClusterMetrics(ClusterMetrics st); + void updateClusterMetrics(ClusterMetrics metrics); /** * Set the master service. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java index c24d0b87263..60b46c62e19 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java @@ -26,7 +26,6 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.NavigableMap; import java.util.Random; import java.util.Set; import java.util.TreeMap; @@ -57,12 +56,12 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Sets; /** * The base class for load balancers. It provides the the functions used to by - * {@link org.apache.hadoop.hbase.master.assignment.AssignmentManager} to assign regions - * in the edge cases. It doesn't provide an implementation of the - * actual balancing algorithm. - * + * {@link org.apache.hadoop.hbase.master.assignment.AssignmentManager} to assign regions in the edge + * cases. It doesn't provide an implementation of the actual balancing algorithm. */ @InterfaceAudience.Private +@edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "IS2_INCONSISTENT_SYNC", + justification = "All the unsynchronized access is before initialization") public abstract class BaseLoadBalancer implements LoadBalancer { private static final Logger LOG = LoggerFactory.getLogger(BaseLoadBalancer.class); @@ -87,8 +86,6 @@ public abstract class BaseLoadBalancer implements LoadBalancer { // slop for regions protected float slop; - // overallSlop to control simpleLoadBalancer's cluster level threshold - protected float overallSlop; protected RackManager rackManager; protected MetricsBalancer metricsBalancer = null; protected ClusterMetrics clusterStatus = null; @@ -121,38 +118,6 @@ public abstract class BaseLoadBalancer implements LoadBalancer { return services.getConfiguration(); } - protected void setConf(Configuration conf) { - setSlop(conf); - if (slop < 0) { - slop = 0; - } else if (slop > 1) { - slop = 1; - } - - if (overallSlop < 0) { - overallSlop = 0; - } else if (overallSlop > 1) { - overallSlop = 1; - } - - this.onlySystemTablesOnMaster = LoadBalancer.isSystemTablesOnlyOnMaster(conf); - - this.rackManager = new RackManager(getConf()); - useRegionFinder = conf.getBoolean("hbase.master.balancer.uselocality", true); - if (useRegionFinder) { - regionFinder = new RegionLocationFinder(); - regionFinder.setConf(conf); - } - this.isByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); - // Print out base configs. Don't print overallSlop since it for simple balancer exclusively. - LOG.info("slop={}, systemTablesOnMaster={}", this.slop, this.onlySystemTablesOnMaster); - } - - protected void setSlop(Configuration conf) { - this.slop = conf.getFloat("hbase.regions.slop", (float) 0.2); - this.overallSlop = conf.getFloat("hbase.regions.overallSlop", slop); - } - /** * Check if a region belongs to some system table. * If so, the primary replica may be expected to be put on the master regionserver. @@ -243,7 +208,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { } @Override - public synchronized void setClusterMetrics(ClusterMetrics st) { + public synchronized void updateClusterMetrics(ClusterMetrics st) { this.clusterStatus = st; if (useRegionFinder) { regionFinder.setClusterMetrics(st); @@ -255,14 +220,10 @@ public abstract class BaseLoadBalancer implements LoadBalancer { public void setMasterServices(MasterServices masterServices) { masterServerName = masterServices.getServerName(); this.services = masterServices; - setConf(services.getConfiguration()); - if (useRegionFinder) { - this.regionFinder.setServices(masterServices); - } } @Override - public void postMasterStartupInitialize() { + public synchronized void postMasterStartupInitialize() { if (services != null && regionFinder != null) { try { Set regions = @@ -274,57 +235,6 @@ public abstract class BaseLoadBalancer implements LoadBalancer { } } - public void setRackManager(RackManager rackManager) { - this.rackManager = rackManager; - } - - protected boolean needsBalance(TableName tableName, BalancerClusterState c) { - ClusterLoadState cs = new ClusterLoadState(c.clusterState); - if (cs.getNumServers() < MIN_SERVER_BALANCE) { - if (LOG.isDebugEnabled()) { - LOG.debug("Not running balancer because only " + cs.getNumServers() - + " active regionserver(s)"); - } - return false; - } - if (areSomeRegionReplicasColocated(c)) { - return true; - } - if(idleRegionServerExist(c)) { - return true; - } - - // Check if we even need to do any load balancing - // HBASE-3681 check sloppiness first - float average = cs.getLoadAverage(); // for logging - int floor = (int) Math.floor(average * (1 - slop)); - int ceiling = (int) Math.ceil(average * (1 + slop)); - if (!(cs.getMaxLoad() > ceiling || cs.getMinLoad() < floor)) { - NavigableMap> serversByLoad = cs.getServersByLoad(); - if (LOG.isTraceEnabled()) { - // If nothing to balance, then don't say anything unless trace-level logging. - LOG.trace("Skipping load balancing because balanced cluster; " + - "servers=" + cs.getNumServers() + - " regions=" + cs.getNumRegions() + " average=" + average + - " mostloaded=" + serversByLoad.lastKey().getLoad() + - " leastloaded=" + serversByLoad.firstKey().getLoad()); - } - return false; - } - return true; - } - - /** - * Subclasses should implement this to return true if the cluster has nodes that hosts - * multiple replicas for the same region, or, if there are multiple racks and the same - * rack hosts replicas of the same region - * @param c Cluster information - * @return whether region replicas are currently co-located - */ - protected boolean areSomeRegionReplicasColocated(BalancerClusterState c) { - return false; - } - protected final boolean idleRegionServerExist(BalancerClusterState c){ boolean isServerExistsWithMoreRegions = false; boolean isServerExistsWithZeroRegions = false; @@ -622,8 +532,38 @@ public abstract class BaseLoadBalancer implements LoadBalancer { return assignments; } + protected final float normalizeSlop(float slop) { + if (slop < 0) { + return 0; + } + if (slop > 1) { + return 1; + } + return slop; + } + + protected float getDefaultSlop() { + return 0.2f; + } + + protected void loadConf(Configuration conf) { + this.slop =normalizeSlop(conf.getFloat("hbase.regions.slop", getDefaultSlop())); + + this.onlySystemTablesOnMaster = LoadBalancer.isSystemTablesOnlyOnMaster(conf); + + this.rackManager = new RackManager(getConf()); + useRegionFinder = conf.getBoolean("hbase.master.balancer.uselocality", true); + if (useRegionFinder) { + regionFinder = new RegionLocationFinder(); + regionFinder.setConf(conf); + } + this.isByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); + LOG.info("slop={}, systemTablesOnMaster={}", this.slop, this.onlySystemTablesOnMaster); + } + @Override - public void initialize() throws HBaseIOException{ + public void initialize() { + loadConf(getConf()); } @Override @@ -641,7 +581,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { @Override public void stop(String why) { - LOG.info("Load Balancer stop requested: "+why); + LOG.info("Load Balancer stop requested: {}", why); stopped = true; } @@ -809,7 +749,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { * @see #balanceTable(TableName, Map) */ @Override - public final synchronized List + public synchronized final List balanceCluster(Map>> loadOfAllTable) { preBalanceCluster(loadOfAllTable); if (isByTable) { @@ -829,6 +769,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { } @Override - public void onConfigurationChange(Configuration conf) { + public synchronized void onConfigurationChange(Configuration conf) { + loadConf(conf); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterStatusChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterStatusChore.java index c0383faf375..04c0b9ba08e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterStatusChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ClusterStatusChore.java @@ -46,7 +46,7 @@ public class ClusterStatusChore extends ScheduledChore { @Override protected void chore() { try { - balancer.setClusterMetrics(master.getClusterMetricsWithoutCoprocessor()); + balancer.updateClusterMetrics(master.getClusterMetricsWithoutCoprocessor()); } catch (InterruptedIOException e) { LOG.warn("Ignoring interruption", e); } 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 6bbc07a3645..fd457eee871 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 @@ -79,16 +79,11 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements private FavoredNodesManager fnm; @Override - public void initialize() throws HBaseIOException { - configureGenerators(); - super.initialize(); - } - - protected void configureGenerators() { + protected List createCandidateGenerators() { List fnPickers = new ArrayList<>(2); fnPickers.add(new FavoredNodeLoadPicker()); fnPickers.add(new FavoredNodeLocalityPicker()); - setCandidateGenerators(fnPickers); + return fnPickers; } @Override 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 fc181fdfbc8..747d12e71ed 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 @@ -52,7 +52,7 @@ public class MaintenanceLoadBalancer implements LoadBalancer { } @Override - public void setClusterMetrics(ClusterMetrics st) { + public void updateClusterMetrics(ClusterMetrics st) { } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java index c5da3bf538b..c03f542acea 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hbase.master.balancer; +import com.google.errorprone.annotations.RestrictedApi; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -59,7 +60,8 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { private RegionPlan.RegionPlanComparator rpComparator = new RegionPlan.RegionPlanComparator(); private float avgLoadOverall; private List serverLoadList = new ArrayList<>(); - + // overallSlop to control simpleLoadBalancer's cluster level threshold + private float overallSlop; /** * Stores additional per-server information about the regions added/removed * during the run of the balancing algorithm. @@ -105,6 +107,8 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { /** * Pass RegionStates and allow balancer to set the current cluster load. */ + @RestrictedApi(explanation = "Should only be called in tests", link = "", + allowedOnPath = ".*(/src/test/.*|SimpleLoadBalancer).java") void setClusterLoad(Map>> clusterLoad) { serverLoadList.clear(); Map server2LoadMap = new HashMap<>(); @@ -133,11 +137,17 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { setClusterLoad(loadOfAllTable); } + @Override + protected void loadConf(Configuration conf) { + super.loadConf(conf); + this.overallSlop = conf.getFloat("hbase.regions.overallSlop", slop); + } + @Override public void onConfigurationChange(Configuration conf) { float originSlop = slop; float originOverallSlop = overallSlop; - super.setConf(conf); + loadConf(conf); LOG.info("Update configuration of SimpleLoadBalancer, previous slop is {}," + " current slop is {}, previous overallSlop is {}, current overallSlop is {}", originSlop, slop, originOverallSlop, overallSlop); @@ -172,6 +182,38 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { return true; } + private boolean needsBalance(BalancerClusterState c) { + ClusterLoadState cs = new ClusterLoadState(c.clusterState); + if (cs.getNumServers() < MIN_SERVER_BALANCE) { + if (LOG.isDebugEnabled()) { + LOG.debug( + "Not running balancer because only " + cs.getNumServers() + " active regionserver(s)"); + } + return false; + } + if (idleRegionServerExist(c)) { + return true; + } + + // Check if we even need to do any load balancing + // HBASE-3681 check sloppiness first + float average = cs.getLoadAverage(); // for logging + int floor = (int) Math.floor(average * (1 - slop)); + int ceiling = (int) Math.ceil(average * (1 + slop)); + if (!(cs.getMaxLoad() > ceiling || cs.getMinLoad() < floor)) { + NavigableMap> serversByLoad = cs.getServersByLoad(); + if (LOG.isTraceEnabled()) { + // If nothing to balance, then don't say anything unless trace-level logging. + LOG.trace("Skipping load balancing because balanced cluster; " + "servers=" + + cs.getNumServers() + " regions=" + cs.getNumRegions() + " average=" + average + + " mostloaded=" + serversByLoad.lastKey().getLoad() + " leastloaded=" + + serversByLoad.firstKey().getLoad()); + } + return false; + } + return true; + } + /** * Generate a global load balancing plan according to the specified map of * server information to the most loaded regions of each server. @@ -278,7 +320,7 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { // argument as defaults BalancerClusterState c = new BalancerClusterState(loadOfOneTable, null, this.regionFinder, this.rackManager); - if (!this.needsBalance(tableName, c) && !this.overallNeedsBalance()) { + if (!needsBalance(c) && !this.overallNeedsBalance()) { return null; } ClusterLoadState cs = new ClusterLoadState(loadOfOneTable); 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 23a2a4d3ba6..a52ea607a68 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 @@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalancerDecision; import org.apache.hadoop.hbase.client.BalancerRejection; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.master.RackManager; import org.apache.hadoop.hbase.master.RegionPlan; import org.apache.hadoop.hbase.namequeues.BalancerDecisionDetails; import org.apache.hadoop.hbase.namequeues.BalancerRejectionDetails; @@ -47,8 +48,6 @@ import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.common.collect.Lists; - /** *

This is a best effort load balancer. Given a Cost function F(C) => x It will * randomly try and mutate the cluster to Cprime. If F(Cprime) < F(C) then the @@ -100,8 +99,6 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Lists; * so that the balancer gets the full picture of all loads on the cluster.

*/ @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) -@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="IS2_INCONSISTENT_SYNC", - justification="Complaint is about costFunctions not being synchronized; not end of the world") public class StochasticLoadBalancer extends BaseLoadBalancer { private static final Logger LOG = LoggerFactory.getLogger(StochasticLoadBalancer.class); @@ -164,78 +161,6 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { super(new MetricsStochasticBalancer()); } - @Override - public void onConfigurationChange(Configuration conf) { - setConf(conf); - } - - @Override - protected synchronized void setConf(Configuration conf) { - super.setConf(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); - - numRegionLoadsToRemember = conf.getInt(KEEP_REGION_LOADS, numRegionLoadsToRemember); - minCostNeedBalance = conf.getFloat(MIN_COST_NEED_BALANCE_KEY, minCostNeedBalance); - if (localityCandidateGenerator == null) { - localityCandidateGenerator = new LocalityBasedCandidateGenerator(); - } - localityCost = new ServerLocalityCostFunction(conf); - rackLocalityCost = new RackLocalityCostFunction(conf); - - if (this.candidateGenerators == null) { - candidateGenerators = Lists.newArrayList(); - candidateGenerators.add(new RandomCandidateGenerator()); - candidateGenerators.add(new LoadCandidateGenerator()); - candidateGenerators.add(localityCandidateGenerator); - candidateGenerators.add(new RegionReplicaRackCandidateGenerator()); - } - regionLoadFunctions = new CostFromRegionLoadFunction[] { - new ReadRequestCostFunction(conf), - new WriteRequestCostFunction(conf), - new MemStoreSizeCostFunction(conf), - new StoreFileCostFunction(conf) - }; - regionReplicaHostCostFunction = new RegionReplicaHostCostFunction(conf); - regionReplicaRackCostFunction = new RegionReplicaRackCostFunction(conf); - - costFunctions = new ArrayList<>(); - addCostFunction(new RegionCountSkewCostFunction(conf)); - addCostFunction(new PrimaryRegionCountSkewCostFunction(conf)); - addCostFunction(new MoveCostFunction(conf)); - addCostFunction(localityCost); - addCostFunction(rackLocalityCost); - addCostFunction(new TableSkewCostFunction(conf)); - addCostFunction(regionReplicaHostCostFunction); - addCostFunction(regionReplicaRackCostFunction); - addCostFunction(regionLoadFunctions[0]); - addCostFunction(regionLoadFunctions[1]); - addCostFunction(regionLoadFunctions[2]); - addCostFunction(regionLoadFunctions[3]); - loadCustomCostFunctions(conf); - - curFunctionCosts = new double[costFunctions.size()]; - tempFunctionCosts = new double[costFunctions.size()]; - - isBalancerDecisionRecording = getConf() - .getBoolean(BaseLoadBalancer.BALANCER_DECISION_BUFFER_ENABLED, - BaseLoadBalancer.DEFAULT_BALANCER_DECISION_BUFFER_ENABLED); - isBalancerRejectionRecording = getConf() - .getBoolean(BaseLoadBalancer.BALANCER_REJECTION_BUFFER_ENABLED, - BaseLoadBalancer.DEFAULT_BALANCER_REJECTION_BUFFER_ENABLED); - - if (this.namedQueueRecorder == null && (isBalancerDecisionRecording - || isBalancerRejectionRecording)) { - this.namedQueueRecorder = NamedQueueRecorder.getInstance(getConf()); - } - - LOG.info("Loaded config; maxSteps=" + maxSteps + ", stepsPerRegion=" + stepsPerRegion + - ", maxRunningTime=" + maxRunningTime + ", isByTable=" + isByTable + ", CostFunctions=" + - Arrays.toString(getCostFunctionNames()) + " etc."); - } - private static CostFunction createCostFunction(Class clazz, Configuration conf) { try { @@ -267,10 +192,6 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { } } - protected void setCandidateGenerators(List customCandidateGenerators) { - this.candidateGenerators = customCandidateGenerators; - } - @RestrictedApi(explanation = "Should only be called in tests", link = "", allowedOnPath = ".*/src/test/.*") List getCandidateGenerators() { @@ -278,13 +199,80 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { } @Override - protected void setSlop(Configuration conf) { - this.slop = conf.getFloat("hbase.regions.slop", 0.001F); + protected float getDefaultSlop() { + return 0.001f; + } + + protected List createCandidateGenerators() { + List candidateGenerators = new ArrayList(4); + candidateGenerators.add(new RandomCandidateGenerator()); + candidateGenerators.add(new LoadCandidateGenerator()); + candidateGenerators.add(localityCandidateGenerator); + candidateGenerators.add(new RegionReplicaRackCandidateGenerator()); + return candidateGenerators; } @Override - public synchronized void setClusterMetrics(ClusterMetrics st) { - super.setClusterMetrics(st); + 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); + + numRegionLoadsToRemember = conf.getInt(KEEP_REGION_LOADS, numRegionLoadsToRemember); + minCostNeedBalance = conf.getFloat(MIN_COST_NEED_BALANCE_KEY, minCostNeedBalance); + if (localityCandidateGenerator == null) { + localityCandidateGenerator = new LocalityBasedCandidateGenerator(); + } + localityCost = new ServerLocalityCostFunction(conf); + rackLocalityCost = new RackLocalityCostFunction(conf); + + this.candidateGenerators = createCandidateGenerators(); + + regionLoadFunctions = new CostFromRegionLoadFunction[] { new ReadRequestCostFunction(conf), + new WriteRequestCostFunction(conf), new MemStoreSizeCostFunction(conf), + new StoreFileCostFunction(conf) }; + regionReplicaHostCostFunction = new RegionReplicaHostCostFunction(conf); + regionReplicaRackCostFunction = new RegionReplicaRackCostFunction(conf); + + costFunctions = new ArrayList<>(); + addCostFunction(new RegionCountSkewCostFunction(conf)); + addCostFunction(new PrimaryRegionCountSkewCostFunction(conf)); + addCostFunction(new MoveCostFunction(conf)); + addCostFunction(localityCost); + addCostFunction(rackLocalityCost); + addCostFunction(new TableSkewCostFunction(conf)); + addCostFunction(regionReplicaHostCostFunction); + addCostFunction(regionReplicaRackCostFunction); + addCostFunction(regionLoadFunctions[0]); + addCostFunction(regionLoadFunctions[1]); + addCostFunction(regionLoadFunctions[2]); + addCostFunction(regionLoadFunctions[3]); + loadCustomCostFunctions(conf); + + curFunctionCosts = new double[costFunctions.size()]; + tempFunctionCosts = new double[costFunctions.size()]; + + isBalancerDecisionRecording = conf.getBoolean(BaseLoadBalancer.BALANCER_DECISION_BUFFER_ENABLED, + BaseLoadBalancer.DEFAULT_BALANCER_DECISION_BUFFER_ENABLED); + isBalancerRejectionRecording = + conf.getBoolean(BaseLoadBalancer.BALANCER_REJECTION_BUFFER_ENABLED, + BaseLoadBalancer.DEFAULT_BALANCER_REJECTION_BUFFER_ENABLED); + + if (this.namedQueueRecorder == null && + (isBalancerDecisionRecording || isBalancerRejectionRecording)) { + this.namedQueueRecorder = NamedQueueRecorder.getInstance(conf); + } + + LOG.info("Loaded config; maxSteps=" + maxSteps + ", stepsPerRegion=" + stepsPerRegion + + ", maxRunningTime=" + maxRunningTime + ", isByTable=" + isByTable + ", CostFunctions=" + + Arrays.toString(getCostFunctionNames()) + " etc."); + } + + @Override + public synchronized void updateClusterMetrics(ClusterMetrics st) { + super.updateClusterMetrics(st); updateRegionLoad(); for(CostFromRegionLoadFunction cost : regionLoadFunctions) { cost.setClusterMetrics(st); @@ -313,8 +301,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { } } - @Override - protected synchronized boolean areSomeRegionReplicasColocated(BalancerClusterState c) { + private boolean areSomeRegionReplicasColocated(BalancerClusterState c) { regionReplicaHostCostFunction.init(c); if (regionReplicaHostCostFunction.cost() > 0) { return true; @@ -326,8 +313,9 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { return false; } - @Override - protected boolean needsBalance(TableName tableName, BalancerClusterState cluster) { + @RestrictedApi(explanation = "Should only be called in tests", link = "", + allowedOnPath = ".*(/src/test/.*|StochasticLoadBalancer).java") + boolean needsBalance(TableName tableName, BalancerClusterState cluster) { ClusterLoadState cs = new ClusterLoadState(cluster.clusterState); if (cs.getNumServers() < MIN_SERVER_BALANCE) { if (LOG.isDebugEnabled()) { @@ -398,6 +386,12 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { .generate(cluster); } + @RestrictedApi(explanation = "Should only be called in tests", link = "", + allowedOnPath = ".*/src/test/.*") + void setRackManager(RackManager rackManager) { + this.rackManager = rackManager; + } + /** * Given the cluster state this will try and approach an optimal balance. This * should always approach the optimal state given enough steps. @@ -660,7 +654,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { /** * Store the current region loads. */ - private synchronized void updateRegionLoad() { + private void updateRegionLoad() { // We create a new hashmap so that regions that are no longer there are removed. // However we temporarily need the old loads so we can use them to keep the rolling average. Map> oldLoads = loads; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java index 413899b9634..435a59132c2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java @@ -79,6 +79,7 @@ public class BalancerTestBase { MasterServices services = mock(MasterServices.class); when(services.getConfiguration()).thenReturn(conf); loadBalancer.setMasterServices(services); + loadBalancer.initialize(); } protected int[] largeCluster = new int[] { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase2.java index dc952dafece..e3515acff08 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase2.java @@ -29,12 +29,12 @@ public class BalancerTestBase2 extends BalancerTestBase { conf.setFloat("hbase.master.balancer.stochastic.localityCost", 0); conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 90 * 1000); // 90 sec conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); } @After public void after() { // reset config to make sure balancer run - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadOnlyFavoredStochasticBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadOnlyFavoredStochasticBalancer.java index 8aa65c196be..f7521c9250a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadOnlyFavoredStochasticBalancer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadOnlyFavoredStochasticBalancer.java @@ -17,19 +17,18 @@ */ package org.apache.hadoop.hbase.master.balancer; +import java.util.ArrayList; import java.util.List; -import org.apache.hbase.thirdparty.com.google.common.collect.Lists; - /** * Used for FavoredNode unit tests */ public class LoadOnlyFavoredStochasticBalancer extends FavoredStochasticBalancer { @Override - protected void configureGenerators() { - List fnPickers = Lists.newArrayList(); + protected List createCandidateGenerators() { + List fnPickers = new ArrayList<>(1); fnPickers.add(new FavoredNodeLoadPicker()); - setCandidateGenerators(fnPickers); + return fnPickers; } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerDecision.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerDecision.java index cfeeefefd6e..74419951f28 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerDecision.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerDecision.java @@ -21,7 +21,6 @@ package org.apache.hadoop.hbase.master.balancer; import java.util.Arrays; import java.util.List; import java.util.Map; - import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; @@ -56,7 +55,7 @@ public class TestBalancerDecision extends BalancerTestBase { @Test public void testBalancerDecisions() { conf.setBoolean("hbase.master.balancer.decision.buffer.enabled", true); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); try { @@ -64,7 +63,7 @@ public class TestBalancerDecision extends BalancerTestBase { boolean[] perTableBalancerConfigs = {true, false}; for (boolean isByTable : perTableBalancerConfigs) { conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); for (int[] mockCluster : clusterStateMocks) { Map> servers = mockClusterServers(mockCluster); Map>> LoadOfAllTable = @@ -93,7 +92,7 @@ public class TestBalancerDecision extends BalancerTestBase { // reset config conf.unset(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE); conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); } } @@ -101,5 +100,4 @@ public class TestBalancerDecision extends BalancerTestBase { return (Arrays.stream(cluster).anyMatch(x -> x > 1)) && (Arrays.stream(cluster) .anyMatch(x -> x < 1)); } - } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerRejection.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerRejection.java index 200efc12ab6..6b9628cdac7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerRejection.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBalancerRejection.java @@ -81,7 +81,7 @@ public class TestBalancerRejection extends BalancerTestBase { //enabled balancer rejection recording conf.setBoolean(BaseLoadBalancer.BALANCER_REJECTION_BUFFER_ENABLED, true); conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY, MockCostFunction.class.getName()); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); //Simulate 2 servers with 5 regions. Map> servers = mockClusterServers(new int[] { 5, 5 }); Map>> LoadOfAllTable = (Map) mockClusterServersWithTables(servers); @@ -94,7 +94,7 @@ public class TestBalancerRejection extends BalancerTestBase { //Reject case 2: Cost < minCostNeedBalance MockCostFunction.mockCost = 1; conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", Float.MAX_VALUE); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); Assert.assertNull(loadBalancer.balanceCluster(LoadOfAllTable)); //NamedQueue is an async Producer-consumer Pattern, waiting here until it completed @@ -112,7 +112,7 @@ public class TestBalancerRejection extends BalancerTestBase { }finally { conf.unset(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY); conf.unset(BaseLoadBalancer.BALANCER_REJECTION_BUFFER_ENABLED); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestSimpleLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestSimpleLoadBalancer.java index cbe952d8f67..ecad7a25503 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestSimpleLoadBalancer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestSimpleLoadBalancer.java @@ -19,6 +19,8 @@ package org.apache.hadoop.hbase.master.balancer; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.HashMap; @@ -31,6 +33,7 @@ import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.RegionPlan; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.SmallTests; @@ -65,6 +68,10 @@ public class TestSimpleLoadBalancer extends BalancerTestBase { conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class); conf.set("hbase.regions.slop", "0"); loadBalancer = new SimpleLoadBalancer(); + MasterServices services = mock(MasterServices.class); + when(services.getConfiguration()).thenReturn(conf); + loadBalancer.setMasterServices(services); + loadBalancer.initialize(); } // int[testnum][servernumber] -> numregions diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticBalancerJmxMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticBalancerJmxMetrics.java index 42a476817d3..523d8511ae2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticBalancerJmxMetrics.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticBalancerJmxMetrics.java @@ -133,7 +133,7 @@ public class TestStochasticBalancerJmxMetrics extends BalancerTestBase { loadBalancer = new StochasticLoadBalancer(); conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - loadBalancer.setConf(conf); + loadBalancer.initialize(); TableName tableName = HConstants.ENSEMBLE_TABLE_NAME; Map> clusterState = mockClusterServers(mockCluster_ensemble); @@ -162,7 +162,7 @@ public class TestStochasticBalancerJmxMetrics extends BalancerTestBase { loadBalancer = new StochasticLoadBalancer(); conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, true); - loadBalancer.setConf(conf); + loadBalancer.initialize(); // NOTE the size is normally set in setClusterMetrics, for test purpose, we set it manually // Tables: hbase:namespace, table1, table2 diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java index 9b6c00e9b2a..086e31c026d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java @@ -143,7 +143,7 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { when(clusterStatus.getLiveServerMetrics()).thenReturn(serverMetricsMap); // when(clusterStatus.getLoad(sn)).thenReturn(sl); - loadBalancer.setClusterMetrics(clusterStatus); + loadBalancer.updateClusterMetrics(clusterStatus); } String regionNameAsString = RegionInfo.getRegionNameAsString(Bytes.toBytes(REGION_KEY)); @@ -169,7 +169,7 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { boolean[] perTableBalancerConfigs = {true, false}; for (boolean isByTable : perTableBalancerConfigs) { conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); for (int[] mockCluster : clusterStateMocks) { Map> servers = mockClusterServers(mockCluster); Map>> LoadOfAllTable = @@ -183,7 +183,7 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { // reset config conf.unset(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE); conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); } } @@ -298,7 +298,7 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { @Test public void testCostAfterUndoAction() { final int runs = 10; - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); for (int[] mockCluster : clusterStateMocks) { BalancerClusterState cluster = mockCluster(mockCluster); loadBalancer.initCosts(cluster); @@ -418,13 +418,13 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY, DummyCostFunction.class.getName()); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); assertTrue(Arrays. asList(loadBalancer.getCostFunctionNames()). contains(DummyCostFunction.class.getSimpleName())); } finally { conf.unset(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerBalanceCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerBalanceCluster.java index 2f778c40eae..d1b597b757c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerBalanceCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerBalanceCluster.java @@ -54,7 +54,7 @@ public class TestStochasticLoadBalancerBalanceCluster extends BalancerTestBase { conf.setLong(StochasticLoadBalancer.MAX_STEPS_KEY, 2000000L); conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 90 * 1000); // 90 sec conf.setFloat("hbase.master.balancer.stochastic.maxMovePercent", 1.0f); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); for (int[] mockCluster : clusterStateMocks) { Map> servers = mockClusterServers(mockCluster); List list = convertToList(servers); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java index 5cc8275d6c8..1ee8a5b478b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java @@ -30,7 +30,6 @@ import java.util.Queue; import java.util.Random; import java.util.TreeMap; import java.util.concurrent.ThreadLocalRandom; - import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; @@ -78,11 +77,12 @@ public class TestStochasticLoadBalancerHeterogeneousCost extends BalancerTestBas BalancerTestBase.conf.set( HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, RULES_FILE); - BalancerTestBase.loadBalancer = new StochasticLoadBalancer(); + loadBalancer = new StochasticLoadBalancer(); MasterServices services = mock(MasterServices.class); when(services.getConfiguration()).thenReturn(conf); BalancerTestBase.loadBalancer.setMasterServices(services); - BalancerTestBase.loadBalancer.getCandidateGenerators().add(new FairRandomCandidateGenerator()); + loadBalancer.initialize(); + loadBalancer.getCandidateGenerators().add(new FairRandomCandidateGenerator()); } @Test diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaHighReplication.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaHighReplication.java index eefb70750b5..cf02e8ba77e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaHighReplication.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaHighReplication.java @@ -35,7 +35,7 @@ public class TestStochasticLoadBalancerRegionReplicaHighReplication extends Bala public void testRegionReplicasOnMidClusterHighReplication() { conf.setLong(StochasticLoadBalancer.MAX_STEPS_KEY, 4000000L); conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 120 * 1000); // 120 sec - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); int numNodes = 40; int numRegions = 6 * numNodes; int replication = 40; // 40 replicas per region, one for each server diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaReplicationGreaterThanNumNodes.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaReplicationGreaterThanNumNodes.java index fd0cc98ad98..07f525c1a9e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaReplicationGreaterThanNumNodes.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaReplicationGreaterThanNumNodes.java @@ -35,7 +35,7 @@ public class TestStochasticLoadBalancerRegionReplicaReplicationGreaterThanNumNod @Test public void testRegionReplicationOnMidClusterReplicationGreaterThanNumNodes() { conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 120 * 1000); // 120 sec - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); int numNodes = 40; int numRegions = 6 * 50; int replication = 50; // 50 replicas per region, more than numNodes diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaSameHosts.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaSameHosts.java index 470230deb13..0e7f419b841 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaSameHosts.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaSameHosts.java @@ -42,7 +42,7 @@ public class TestStochasticLoadBalancerRegionReplicaSameHosts extends BalancerTe conf.setLong(StochasticLoadBalancer.MAX_STEPS_KEY, 2000000L); conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 90 * 1000); // 90 sec conf.setFloat("hbase.master.balancer.stochastic.maxMovePercent", 1.0f); - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); int numHosts = 30; int numRegions = 30 * 30; int replication = 3; // 3 replicas per region diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaWithRacks.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaWithRacks.java index b7532ebaa95..ad53fc0752c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaWithRacks.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplicaWithRacks.java @@ -54,7 +54,7 @@ public class TestStochasticLoadBalancerRegionReplicaWithRacks extends BalancerTe conf.setLong(StochasticLoadBalancer.MAX_STEPS_KEY, 10000000L); conf.setFloat("hbase.master.balancer.stochastic.maxMovePercent", 1.0f); conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 120 * 1000); // 120 sec - loadBalancer.setConf(conf); + loadBalancer.onConfigurationChange(conf); int numNodes = 30; int numRegions = numNodes * 30; int replication = 3; // 3 replicas per region