From 0875fa0634e0c78fc0776520602eeb35ee623f80 Mon Sep 17 00:00:00 2001 From: Ben Lau Date: Mon, 5 Nov 2018 15:34:08 -0800 Subject: [PATCH] HBASE-21439 RegionLoads aren't being used in RegionLoad cost functions Signed-off-by: tedyu Signed-off-by: Andrew Purtell Conflicts: hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java --- .../hadoop/hbase/client/RegionInfo.java | 22 +++++++++++++++++++ .../hbase/client/RegionInfoBuilder.java | 10 +-------- .../balancer/StochasticLoadBalancer.java | 6 ++--- .../balancer/TestStochasticLoadBalancer.java | 8 ++++--- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java index 7f5d399eaaf..5bb4aef2df6 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java @@ -18,6 +18,8 @@ */ package org.apache.hadoop.hbase.client; +import edu.umd.cs.findbugs.annotations.CheckForNull; + import java.io.DataInputStream; import java.io.IOException; import java.util.ArrayList; @@ -271,6 +273,26 @@ public interface RegionInfo { return encodedName; } + @InterfaceAudience.Private + static String getRegionNameAsString(byte[] regionName) { + return getRegionNameAsString(null, regionName); + } + + @InterfaceAudience.Private + static String getRegionNameAsString(@CheckForNull RegionInfo ri, byte[] regionName) { + if (RegionInfo.hasEncodedName(regionName)) { + // new format region names already have their encoded name. + return Bytes.toStringBinary(regionName); + } + + // old format. regionNameStr doesn't have the region name. + if (ri == null) { + return Bytes.toStringBinary(regionName) + "." + RegionInfo.encodeRegionName(regionName); + } else { + return Bytes.toStringBinary(regionName) + "." + ri.getEncodedName(); + } + } + /** * @return Return a String of short, printable names for hris * (usually encoded name) for us logging. diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java index 3de98606f39..cd9e40b9524 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java @@ -287,15 +287,7 @@ public class RegionInfoBuilder { */ @Override public String getRegionNameAsString() { - if (RegionInfo.hasEncodedName(this.regionName)) { - // new format region names already have their encoded name. - return Bytes.toStringBinary(this.regionName); - } - - // old format. regionNameStr doesn't have the region name. - // - // - return Bytes.toStringBinary(this.regionName) + "." + this.getEncodedName(); + return RegionInfo.getRegionNameAsString(this, this.regionName); } /** @return the encoded region name */ 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 b2c6629d4ed..d25d1ecdc3d 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 @@ -45,7 +45,6 @@ import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.AssignRe import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.LocalityType; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.MoveRegionAction; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.SwapRegionsAction; -import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; @@ -531,14 +530,15 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { clusterStatus.getLiveServerMetrics().forEach((ServerName sn, ServerMetrics sm) -> { sm.getRegionMetrics().forEach((byte[] regionName, RegionMetrics rm) -> { - Deque rLoads = oldLoads.get(Bytes.toString(regionName)); + String regionNameAsString = RegionInfo.getRegionNameAsString(regionName); + Deque rLoads = oldLoads.get(regionNameAsString); if (rLoads == null) { rLoads = new ArrayDeque<>(numRegionLoadsToRemember + 1); } else if (rLoads.size() >= numRegionLoadsToRemember) { rLoads.remove(); } rLoads.add(new BalancerRegionLoad(rm)); - loads.put(Bytes.toString(regionName), rLoads); + loads.put(regionNameAsString, rLoads); }); }); 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 149bf681d09..41c3c4e3c42 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 @@ -142,10 +142,12 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { loadBalancer.setClusterMetrics(clusterStatus); } - assertTrue(loadBalancer.loads.get(REGION_KEY) != null); - assertTrue(loadBalancer.loads.get(REGION_KEY).size() == 15); - Queue loads = loadBalancer.loads.get(REGION_KEY); + String regionNameAsString = RegionInfo.getRegionNameAsString(Bytes.toBytes(REGION_KEY)); + assertTrue(loadBalancer.loads.get(regionNameAsString) != null); + assertTrue(loadBalancer.loads.get(regionNameAsString).size() == 15); + + Queue loads = loadBalancer.loads.get(regionNameAsString); int i = 0; while(loads.size() > 0) { BalancerRegionLoad rl = loads.remove();