You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by te...@apache.org on 2017/01/26 18:45:09 UTC

hbase git commit: HBASE-17515 Reduce memory footprint of RegionLoads kept by StochasticLoadBalancer - revert due to pending discussion

Repository: hbase
Updated Branches:
  refs/heads/master 0ac5d4a71 -> e8979c67a


HBASE-17515 Reduce memory footprint of RegionLoads kept by StochasticLoadBalancer - revert due to pending discussion


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e8979c67
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e8979c67
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e8979c67

Branch: refs/heads/master
Commit: e8979c67aa9cf0995b5f1603db72175f3eb222d5
Parents: 0ac5d4a
Author: tedyu <yu...@gmail.com>
Authored: Thu Jan 26 10:44:54 2017 -0800
Committer: tedyu <yu...@gmail.com>
Committed: Thu Jan 26 10:44:54 2017 -0800

----------------------------------------------------------------------
 .../master/balancer/BalancerRegionLoad.java     | 57 --------------------
 .../hbase/master/balancer/BaseLoadBalancer.java | 10 ++--
 .../master/balancer/StochasticLoadBalancer.java | 37 ++++++-------
 .../balancer/TestStochasticLoadBalancer.java    | 12 ++---
 4 files changed, 30 insertions(+), 86 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/e8979c67/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerRegionLoad.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerRegionLoad.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerRegionLoad.java
deleted file mode 100644
index 1909815..0000000
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerRegionLoad.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/**
- * 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 org.apache.hadoop.hbase.RegionLoad;
-import org.apache.hadoop.hbase.classification.InterfaceStability;
-
-/**
- * Wrapper class for the few fields required by the {@link StochasticLoadBalancer}
- * from the full {@link RegionLoad}.
- */
-@InterfaceStability.Evolving
-class BalancerRegionLoad {
-  private final long readRequestsCount;
-  private final long writeRequestsCount;
-  private final int memStoreSizeMB;
-  private final int storefileSizeMB;
-
-  BalancerRegionLoad(RegionLoad regionLoad) {
-    readRequestsCount = regionLoad.getReadRequestsCount();
-    writeRequestsCount = regionLoad.getWriteRequestsCount();
-    memStoreSizeMB = regionLoad.getMemStoreSizeMB();
-    storefileSizeMB = regionLoad.getStorefileSizeMB();
-  }
-
-  public long getReadRequestsCount() {
-    return readRequestsCount;
-  }
-
-  public long getWriteRequestsCount() {
-    return writeRequestsCount;
-  }
-
-  public int getMemStoreSizeMB() {
-    return memStoreSizeMB;
-  }
-
-  public int getStorefileSizeMB() {
-    return storefileSizeMB;
-  }
-}

http://git-wip-us.apache.org/repos/asf/hbase/blob/e8979c67/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
----------------------------------------------------------------------
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 d4a390a..2a4a7bc 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
@@ -118,7 +118,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
 
     ArrayList<String> tables;
     HRegionInfo[] regions;
-    Deque<BalancerRegionLoad>[] regionLoads;
+    Deque<RegionLoad>[] regionLoads;
     private RegionLocationFinder regionFinder;
 
     int[][] regionLocations; //regionIndex -> list of serverIndex sorted by locality
@@ -166,7 +166,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
 
     protected Cluster(
         Map<ServerName, List<HRegionInfo>> clusterState,
-        Map<String, Deque<BalancerRegionLoad>> loads,
+        Map<String, Deque<RegionLoad>> loads,
         RegionLocationFinder regionFinder,
         RackManager rackManager) {
       this(null, clusterState, loads, regionFinder, rackManager);
@@ -176,7 +176,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
     protected Cluster(
         Collection<HRegionInfo> unassignedRegions,
         Map<ServerName, List<HRegionInfo>> clusterState,
-        Map<String, Deque<BalancerRegionLoad>> loads,
+        Map<String, Deque<RegionLoad>> loads,
         RegionLocationFinder regionFinder,
         RackManager rackManager) {
 
@@ -431,7 +431,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
 
     /** Helper for Cluster constructor to handle a region */
     private void registerRegion(HRegionInfo region, int regionIndex,
-        int serverIndex, Map<String, Deque<BalancerRegionLoad>> loads,
+        int serverIndex, Map<String, Deque<RegionLoad>> loads,
         RegionLocationFinder regionFinder) {
       String tableName = region.getTable().getNameAsString();
       if (!tablesToIndex.containsKey(tableName)) {
@@ -448,7 +448,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
 
       // region load
       if (loads != null) {
-        Deque<BalancerRegionLoad> rl = loads.get(region.getRegionNameAsString());
+        Deque<RegionLoad> rl = loads.get(region.getRegionNameAsString());
         // That could have failed if the RegionLoad is using the other regionName
         if (rl == null) {
           // Try getting the region load using encoded name.

http://git-wip-us.apache.org/repos/asf/hbase/blob/e8979c67/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
----------------------------------------------------------------------
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 226dee7..4fbae6e 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
@@ -113,7 +113,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
   private static final Random RANDOM = new Random(System.currentTimeMillis());
   private static final Log LOG = LogFactory.getLog(StochasticLoadBalancer.class);
 
-  Map<String, Deque<BalancerRegionLoad>> loads = new HashMap<String, Deque<BalancerRegionLoad>>();
+  Map<String, Deque<RegionLoad>> loads = new HashMap<String, Deque<RegionLoad>>();
 
   // values are defaults
   private int maxSteps = 1000000;
@@ -498,8 +498,8 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
   private synchronized 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<String, Deque<BalancerRegionLoad>> oldLoads = loads;
-    loads = new HashMap<String, Deque<BalancerRegionLoad>>();
+    Map<String, Deque<RegionLoad>> oldLoads = loads;
+    loads = new HashMap<String, Deque<RegionLoad>>();
 
     for (ServerName sn : clusterStatus.getServers()) {
       ServerLoad sl = clusterStatus.getLoad(sn);
@@ -507,15 +507,16 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
         continue;
       }
       for (Entry<byte[], RegionLoad> entry : sl.getRegionsLoad().entrySet()) {
-        Deque<BalancerRegionLoad> rLoads = oldLoads.get(Bytes.toString(entry.getKey()));
+        Deque<RegionLoad> rLoads = oldLoads.get(Bytes.toString(entry.getKey()));
         if (rLoads == null) {
           // There was nothing there
-          rLoads = new ArrayDeque<BalancerRegionLoad>();
+          rLoads = new ArrayDeque<RegionLoad>();
         } else if (rLoads.size() >= numRegionLoadsToRemember) {
           rLoads.remove();
         }
-        rLoads.add(new BalancerRegionLoad(entry.getValue()));
+        rLoads.add(entry.getValue());
         loads.put(Bytes.toString(entry.getKey()), rLoads);
+
       }
     }
 
@@ -1250,7 +1251,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
   abstract static class CostFromRegionLoadFunction extends CostFunction {
 
     private ClusterStatus clusterStatus = null;
-    private Map<String, Deque<BalancerRegionLoad>> loads = null;
+    private Map<String, Deque<RegionLoad>> loads = null;
     private double[] stats = null;
     CostFromRegionLoadFunction(Configuration conf) {
       super(conf);
@@ -1260,7 +1261,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
       this.clusterStatus = status;
     }
 
-    void setLoads(Map<String, Deque<BalancerRegionLoad>> l) {
+    void setLoads(Map<String, Deque<RegionLoad>> l) {
       this.loads = l;
     }
 
@@ -1280,7 +1281,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
 
         // for every region on this server get the rl
         for(int regionIndex:cluster.regionsPerServer[i]) {
-          Collection<BalancerRegionLoad> regionLoadList =  cluster.regionLoads[regionIndex];
+          Collection<RegionLoad> regionLoadList =  cluster.regionLoads[regionIndex];
 
           // Now if we found a region load get the type of cost that was requested.
           if (regionLoadList != null) {
@@ -1296,15 +1297,15 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
       return costFromArray(stats);
     }
 
-    protected double getRegionLoadCost(Collection<BalancerRegionLoad> regionLoadList) {
+    protected double getRegionLoadCost(Collection<RegionLoad> regionLoadList) {
       double cost = 0;
-      for (BalancerRegionLoad rl : regionLoadList) {
+      for (RegionLoad rl : regionLoadList) {
         cost += getCostFromRl(rl);
       }
       return cost / regionLoadList.size();
     }
 
-    protected abstract double getCostFromRl(BalancerRegionLoad rl);
+    protected abstract double getCostFromRl(RegionLoad rl);
   }
 
   /**
@@ -1319,11 +1320,11 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     }
 
     @Override
-    protected double getRegionLoadCost(Collection<BalancerRegionLoad> regionLoadList) {
+    protected double getRegionLoadCost(Collection<RegionLoad> regionLoadList) {
       double cost = 0;
       double previous = 0;
       boolean isFirst = true;
-      for (BalancerRegionLoad rl : regionLoadList) {
+      for (RegionLoad rl : regionLoadList) {
         double current = getCostFromRl(rl);
         if (isFirst) {
           isFirst = false;
@@ -1353,7 +1354,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     }
 
     @Override
-    protected double getCostFromRl(BalancerRegionLoad rl) {
+    protected double getCostFromRl(RegionLoad rl) {
       return rl.getReadRequestsCount();
     }
   }
@@ -1374,7 +1375,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     }
 
     @Override
-    protected double getCostFromRl(BalancerRegionLoad rl) {
+    protected double getCostFromRl(RegionLoad rl) {
       return rl.getWriteRequestsCount();
     }
   }
@@ -1552,7 +1553,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     }
 
     @Override
-    protected double getCostFromRl(BalancerRegionLoad rl) {
+    protected double getCostFromRl(RegionLoad rl) {
       return rl.getMemStoreSizeMB();
     }
   }
@@ -1572,7 +1573,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     }
 
     @Override
-    protected double getCostFromRl(BalancerRegionLoad rl) {
+    protected double getCostFromRl(RegionLoad rl) {
       return rl.getStorefileSizeMB();
     }
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/e8979c67/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java
----------------------------------------------------------------------
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 ba030c8..3d975b8 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
@@ -69,7 +69,7 @@ public class TestStochasticLoadBalancer extends BalancerTestBase {
       ServerLoad sl = mock(ServerLoad.class);
 
       RegionLoad rl = mock(RegionLoad.class);
-      when(rl.getStorefileSizeMB()).thenReturn(i);
+      when(rl.getStores()).thenReturn(i);
 
       Map<byte[], RegionLoad> regionLoadMap =
           new TreeMap<byte[], RegionLoad>(Bytes.BYTES_COMPARATOR);
@@ -85,11 +85,11 @@ public class TestStochasticLoadBalancer extends BalancerTestBase {
     assertTrue(loadBalancer.loads.get(REGION_KEY) != null);
     assertTrue(loadBalancer.loads.get(REGION_KEY).size() == 15);
 
-    Queue<BalancerRegionLoad> loads = loadBalancer.loads.get(REGION_KEY);
+    Queue<RegionLoad> loads = loadBalancer.loads.get(REGION_KEY);
     int i = 0;
     while(loads.size() > 0) {
-      BalancerRegionLoad rl = loads.remove();
-      assertEquals(i + (numClusterStatusToAdd - 15), rl.getStorefileSizeMB());
+      RegionLoad rl = loads.remove();
+      assertEquals(i + (numClusterStatusToAdd - 15), rl.getStores());
       i ++;
     }
   }
@@ -232,9 +232,9 @@ public class TestStochasticLoadBalancer extends BalancerTestBase {
 
   @Test
   public void testRegionLoadCost() {
-    List<BalancerRegionLoad> regionLoads = new ArrayList<>();
+    List<RegionLoad> regionLoads = new ArrayList<>();
     for (int i = 1; i < 5; i++) {
-      BalancerRegionLoad regionLoad = mock(BalancerRegionLoad.class);
+      RegionLoad regionLoad = mock(RegionLoad.class);
       when(regionLoad.getReadRequestsCount()).thenReturn(new Long(i));
       when(regionLoad.getStorefileSizeMB()).thenReturn(i);
       regionLoads.add(regionLoad);