You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2016/12/02 19:34:23 UTC

hbase git commit: HBASE-17194 Assign the new region to the idle server after splitting (ChiaPing Tsai)

Repository: hbase
Updated Branches:
  refs/heads/master 4a20361f1 -> 7775feda0


HBASE-17194 Assign the new region to the idle server after splitting (ChiaPing Tsai)


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

Branch: refs/heads/master
Commit: 7775feda05b0db63178c81910946adfec4c4c41f
Parents: 4a20361
Author: Michael Stack <st...@apache.org>
Authored: Fri Dec 2 11:34:14 2016 -0800
Committer: Michael Stack <st...@apache.org>
Committed: Fri Dec 2 11:34:14 2016 -0800

----------------------------------------------------------------------
 .../hadoop/hbase/master/ServerManager.java      | 24 +++++++-
 .../hbase/master/balancer/BaseLoadBalancer.java | 33 +++++++----
 .../master/balancer/TestBaseLoadBalancer.java   | 59 +++++++++++++++++---
 3 files changed, 96 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/7775feda/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
index b76cd7e..d15b87e 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
@@ -21,7 +21,6 @@ package org.apache.hadoop.hbase.master;
 import static org.apache.hadoop.hbase.util.CollectionUtils.computeIfAbsent;
 
 import com.google.common.annotations.VisibleForTesting;
-
 import java.io.IOException;
 import java.net.InetAddress;
 import java.util.ArrayList;
@@ -37,7 +36,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentNavigableMap;
 import java.util.concurrent.ConcurrentSkipListMap;
 import java.util.concurrent.CopyOnWriteArrayList;
-
+import java.util.function.Predicate;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
@@ -1074,6 +1073,27 @@ public class ServerManager {
   }
 
   /**
+   * @param keys The target server name
+   * @param idleServerPredicator Evaluates the server on the given load
+   * @return A copy of the internal list of online servers matched by the predicator
+   */
+  public List<ServerName> getOnlineServersListWithPredicator(List<ServerName> keys,
+    Predicate<ServerLoad> idleServerPredicator) {
+    List<ServerName> names = new ArrayList<>();
+    if (keys != null && idleServerPredicator != null) {
+      keys.forEach(name -> {
+        ServerLoad load = onlineServers.get(name);
+        if (load != null) {
+          if (idleServerPredicator.test(load)) {
+            names.add(name);
+          }
+        }
+      });
+    }
+    return names;
+  }
+
+  /**
    * @return A copy of the internal list of draining servers.
    */
   public List<ServerName> getDrainingServersList() {

http://git-wip-us.apache.org/repos/asf/hbase/blob/7775feda/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 807632c..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
@@ -17,6 +17,11 @@
  */
 package org.apache.hadoop.hbase.master.balancer;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -33,7 +38,7 @@ import java.util.NavigableMap;
 import java.util.Random;
 import java.util.Set;
 import java.util.TreeMap;
-
+import java.util.function.Predicate;
 import org.apache.commons.lang.NotImplementedException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -43,6 +48,7 @@ import org.apache.hadoop.hbase.HBaseIOException;
 import org.apache.hadoop.hbase.HDFSBlocksDistribution;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.RegionLoad;
+import org.apache.hadoop.hbase.ServerLoad;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.RegionReplicaUtil;
@@ -54,12 +60,6 @@ import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.Action.T
 import org.apache.hadoop.hbase.security.access.AccessControlLists;
 import org.apache.hadoop.util.StringUtils;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Joiner;
-import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Lists;
-import 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.AssignmentManager} to assign regions
@@ -73,6 +73,9 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
 
   private static final List<HRegionInfo> EMPTY_REGION_LIST = new ArrayList<HRegionInfo>(0);
 
+  static final Predicate<ServerLoad> IDLE_SERVER_PREDICATOR
+    = load -> load.getNumberOfRegions() == 0;
+
   protected final RegionLocationFinder regionFinder = new RegionLocationFinder();
 
   private static class DefaultRackManager extends RackManager {
@@ -1323,6 +1326,11 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
         rackManager);
   }
 
+  private List<ServerName> findIdleServers(List<ServerName> servers) {
+    return this.services.getServerManager()
+            .getOnlineServersListWithPredicator(servers, IDLE_SERVER_PREDICATOR);
+  }
+
   /**
    * Used to assign a single region to a random server.
    */
@@ -1346,10 +1354,15 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
     if (numServers == 1) { // Only one server, nothing fancy we can do here
       return servers.get(0);
     }
-
+    List<ServerName> idleServers = findIdleServers(servers);
+    if (idleServers.size() == 1) {
+      return idleServers.get(0);
+    }
+    final List<ServerName> finalServers = idleServers.isEmpty() ?
+            servers : idleServers;
     List<HRegionInfo> regions = Lists.newArrayList(regionInfo);
-    Cluster cluster = createCluster(servers, regions, false);
-    return randomAssignment(cluster, regionInfo, servers);
+    Cluster cluster = createCluster(finalServers, regions, false);
+    return randomAssignment(cluster, regionInfo, finalServers);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/7775feda/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
index b0b0a2b..9e3bb17 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
@@ -17,12 +17,9 @@
  */
 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 com.google.common.collect.Lists;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -30,7 +27,6 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
-
 import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -45,17 +41,21 @@ import org.apache.hadoop.hbase.master.LoadBalancer;
 import org.apache.hadoop.hbase.master.MasterServices;
 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.balancer.BaseLoadBalancer.Cluster;
 import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.MoveRegionAction;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.net.DNSToSwitchMapping;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.mockito.Mockito;
-
-import com.google.common.collect.Lists;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 @Category({MasterTests.class, MediumTests.class})
 public class TestBaseLoadBalancer extends BalancerTestBase {
@@ -211,6 +211,49 @@ public class TestBaseLoadBalancer extends BalancerTestBase {
     assertRetainedAssignment(existing, listOfServerNames, assignment);
   }
 
+  @Test (timeout=30000)
+  public void testRandomAssignment() throws Exception {
+    for (int i = 1; i != 5; ++i) {
+      LOG.info("run testRandomAssignment() with idle servers:" + i);
+      testRandomAssignment(i);
+    }
+  }
+
+  private void testRandomAssignment(int numberOfIdleServers) throws Exception {
+    assert numberOfIdleServers > 0;
+    List<ServerName> idleServers = new ArrayList<>(numberOfIdleServers);
+    for (int i = 0; i != numberOfIdleServers; ++i) {
+      idleServers.add(ServerName.valueOf("server-" + i, 1000, 1L));
+    }
+    List<ServerName> allServers = new ArrayList<>(idleServers.size() + 1);
+    allServers.add(ServerName.valueOf("server-" + numberOfIdleServers, 1000, 1L));
+    allServers.addAll(idleServers);
+    LoadBalancer balancer = new MockBalancer() {
+      @Override
+      public boolean shouldBeOnMaster(HRegionInfo region) {
+        return false;
+      }
+    };
+    Configuration conf = HBaseConfiguration.create();
+    conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class);
+    balancer.setConf(conf);
+    ServerManager sm = Mockito.mock(ServerManager.class);
+    Mockito.when(sm.getOnlineServersListWithPredicator(allServers, BaseLoadBalancer.IDLE_SERVER_PREDICATOR))
+           .thenReturn(idleServers);
+    MasterServices services = Mockito.mock(MasterServices.class);
+    Mockito.when(services.getServerManager()).thenReturn(sm);
+    balancer.setMasterServices(services);
+    HRegionInfo hri1 = new HRegionInfo(
+        TableName.valueOf("table"), "key1".getBytes(), "key2".getBytes(),
+        false, 100);
+    assertNull(balancer.randomAssignment(hri1, Collections.EMPTY_LIST));
+    assertNull(balancer.randomAssignment(hri1, null));
+    for (int i = 0; i != 3; ++i) {
+      ServerName sn = balancer.randomAssignment(hri1, allServers);
+      assertTrue("actual:" + sn + ", except:" + idleServers, idleServers.contains(sn));
+    }
+  }
+
   @Test (timeout=180000)
   public void testRegionAvailability() throws Exception {
     // Create a cluster with a few servers, assign them to specific racks