From 68c939668540671a925d102c79d9958f739067e4 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 Amend HBASE-21439 Update RSGroup Test too Signed-off-by: tedyu Signed-off-by: Andrew Purtell Amending-Author: Andrew Purtell --- .../org/apache/hadoop/hbase/HRegionInfo.java | 22 +++++++++++++++++++ ...rWithStochasticLoadBalancerAsInternal.java | 2 +- .../balancer/StochasticLoadBalancer.java | 7 +++--- .../balancer/TestStochasticLoadBalancer.java | 8 ++++--- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java index 8d93655041e..de503b5e82c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java @@ -48,6 +48,8 @@ import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.PairOfSameType; import org.apache.hadoop.io.DataInputBuffer; +import edu.umd.cs.findbugs.annotations.CheckForNull; + /** * Information about a region. A region is a range of keys in the whole keyspace of a table, an * identifier (a timestamp) for differentiating between subset ranges (after region split) @@ -189,6 +191,26 @@ public class HRegionInfo implements Comparable { return encodedName; } + @InterfaceAudience.Private + public static String getRegionNameAsString(byte[] regionName) { + return getRegionNameAsString(null, regionName); + } + + @InterfaceAudience.Private + public static String getRegionNameAsString(@CheckForNull HRegionInfo ri, byte[] regionName) { + if (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) + "." + encodeRegionName(regionName); + } else { + return Bytes.toStringBinary(regionName) + "." + ri.getEncodedName(); + } + } + /** * @return Return a short, printable name for this region (usually encoded name) for us logging. */ 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 771b59f035f..4f4162c91d1 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 @@ -84,7 +84,7 @@ public class TestRSGroupBasedLoadBalancerWithStochasticLoadBalancerAsInternal when(rl.getWriteRequestsCount()).thenReturn(0L); when(rl.getMemStoreSizeMB()).thenReturn(0); when(rl.getStorefileSizeMB()).thenReturn(0); - regionLoadMap.put(info.getEncodedNameAsBytes(), rl); + regionLoadMap.put(info.getRegionName(), rl); } when(serverMetrics.getRegionsLoad()).thenReturn(regionLoadMap); return serverMetrics; 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 2cfe2dc950b..84cf30a454b 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 @@ -51,7 +51,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 com.google.common.base.Optional; @@ -545,7 +544,8 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { continue; } for (Entry entry : sl.getRegionsLoad().entrySet()) { - Deque rLoads = oldLoads.get(Bytes.toString(entry.getKey())); + String regionNameAsString = HRegionInfo.getRegionNameAsString(entry.getKey()); + Deque rLoads = oldLoads.get(regionNameAsString); if (rLoads == null) { // There was nothing there rLoads = new ArrayDeque(); @@ -553,8 +553,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { rLoads.remove(); } rLoads.add(entry.getValue()); - loads.put(Bytes.toString(entry.getKey()), 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 ad459c12318..40bec1905f9 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.setClusterStatus(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 = HRegionInfo.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) { RegionLoad rl = loads.remove();