HBASE-3586 Improve the selection of regions to balance

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1079025 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -91,8 +91,6 @@ Release 0.91.0 - Unreleased
    HBASE-3564  DemoClient.pl - a demo client in Perl
    HBASE-3560  the hbase-default entry of "hbase.defaults.for.version" 
                causes tests not to run via not-maven
-   HBASE-3586  Improve the selection of regions to balance (Ted Yu via Andrew
-               Purtell)  
    HBASE-3513  upgrade thrift to 0.5.0 and use mvn version
 
   TASK
@@ -138,12 +136,13 @@ Release 0.90.2 - Unreleased
    HBASE-3591  completebulkload doesn't honor generic -D options
    HBASE-3594  Rest server fails because of missing asm jar
    HBASE-3582  Allow HMaster and HRegionServer to login from keytab
-               when on secure Hadoop
+               hen on secure Hadoop
    HBASE-3608  MemstoreFlusher error message doesnt include exception!
 
   IMPROVEMENTS
    HBASE-3542  MultiGet methods in Thrift
    HBASE-3285  Hlog recovery takes too much time
+   HBASE-3586  Improve the selection of regions to balance
 
 Release 0.90.1 - Unreleased
 

src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java

@@ -58,7 +58,7 @@ import org.apache.hadoop.hbase.HServerInfo;
  */
 public class LoadBalancer {
   private static final Log LOG = LogFactory.getLog(LoadBalancer.class);
-  private static final Random rand = new Random();
+  private static final Random RANDOM = new Random(System.currentTimeMillis());
 
   static class RegionPlanComparator implements Comparator<RegionPlan> {
     @Override
@@ -195,7 +195,7 @@ public class LoadBalancer {
         break;
       }
       serversOverloaded++;
-      List<HRegionInfo> regions = server.getValue();
+      List<HRegionInfo> regions = randomize(server.getValue());
       int numToOffload = Math.min(regionCount - max, regions.size());
       int numTaken = 0;
       for (int i = regions.size() - 1; i >= 0; i--) {
@@ -209,8 +209,6 @@ public class LoadBalancer {
       serverBalanceInfo.put(serverInfo,
           new BalanceInfo(numToOffload, (-1)*numTaken));
     }
-    // put young regions at the beginning of regionsToMove
-    Collections.sort(regionsToMove, rpComparator);
 
     // Walk down least loaded, filling each to the min
     int serversUnderloaded = 0; // number of servers that get new regions
@@ -335,6 +333,15 @@ public class LoadBalancer {
     return regionsToMove;
   }
 
+  /**
+   * @param regions
+   * @return Randomization of passed <code>regions</code>
+   */
+  static List<HRegionInfo> randomize(final List<HRegionInfo> regions) {
+    Collections.shuffle(regions, RANDOM);
+    return regions;
+  }
+
   /**
    * Stores additional per-server information about the regions added/removed
    * during the run of the balancing algorithm.
@@ -396,7 +403,7 @@ public class LoadBalancer {
     int max = (int)Math.ceil((float)numRegions/numServers);
     int serverIdx = 0;
     if (numServers > 1) {
-      serverIdx = rand.nextInt(numServers);
+      serverIdx = RANDOM.nextInt(numServers);
     }
     int regionIdx = 0;
     for (int j = 0; j < numServers; j++) {
@@ -444,7 +451,7 @@ public class LoadBalancer {
       if (server != null) {
         assignments.get(server).add(region.getKey());
       } else {
-        assignments.get(servers.get(rand.nextInt(assignments.size()))).add(
+        assignments.get(servers.get(RANDOM.nextInt(assignments.size()))).add(
             region.getKey());
       }
     }
@@ -575,7 +582,7 @@ public class LoadBalancer {
     Map<HRegionInfo,HServerInfo> assignments =
       new TreeMap<HRegionInfo,HServerInfo>();
     for(HRegionInfo region : regions) {
-      assignments.put(region, servers.get(rand.nextInt(servers.size())));
+      assignments.put(region, servers.get(RANDOM.nextInt(servers.size())));
     }
     return assignments;
   }
@@ -585,7 +592,7 @@ public class LoadBalancer {
       LOG.warn("Wanted to do random assignment but no servers to assign to");
       return null;
     }
-    return servers.get(rand.nextInt(servers.size()));
+    return servers.get(RANDOM.nextInt(servers.size()));
   }
 
   /**

src/test/java/org/apache/hadoop/hbase/master/TestLoadBalancer.java

@@ -19,6 +19,7 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import static org.junit.Assert.*;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -34,6 +35,8 @@ import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
 
+import junit.framework.Assert;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.HRegionInfo;
@@ -42,6 +45,7 @@ import org.apache.hadoop.hbase.HServerInfo;
 import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.master.LoadBalancer.RegionPlan;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.tools.ant.taskdefs.PathConvert.MapEntry;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -134,6 +138,38 @@ public class TestLoadBalancer {
       new int [] { 12, 100 },
   };
 
+  @Test
+  public void testRandomizer() {
+    for(int [] mockCluster : clusterStateMocks) {
+      if (mockCluster.length < 5) continue;
+      Map<HServerInfo, List<HRegionInfo>> servers =
+        mockClusterServers(mockCluster);
+      for (Map.Entry<HServerInfo, List<HRegionInfo>> e: servers.entrySet()) {
+        List<HRegionInfo> original = e.getValue();
+        if (original.size() < 5) continue;
+        // Try ten times in case random chances upon original order more than
+        // one or two times in a row.
+        boolean same = true;
+        for (int i = 0; i < 10 && same; i++) {
+          List<HRegionInfo> copy = new ArrayList<HRegionInfo>(original);
+          System.out.println("Randomizing before " + copy.size());
+          for (HRegionInfo hri: copy) {
+            System.out.println(hri.getEncodedName());
+          }
+          List<HRegionInfo> randomized = LoadBalancer.randomize(copy);
+          System.out.println("Randomizing after " + randomized.size());
+          for (HRegionInfo hri: randomized) {
+            System.out.println(hri.getEncodedName());
+          }
+          if (original.equals(randomized)) continue;
+          same = false;
+          break;
+        }
+        assertFalse(same);
+      }
+    }
+  }
+
   /**
    * Test the load balancing algorithm.
    *
@@ -410,7 +446,7 @@ public class TestLoadBalancer {
       Bytes.putInt(start, 0, numRegions << 1);
       Bytes.putInt(end, 0, (numRegions << 1) + 1);
       HRegionInfo hri = new HRegionInfo(
-          new HTableDescriptor(Bytes.toBytes("table")), start, end);
+          new HTableDescriptor(Bytes.toBytes("table" + i)), start, end);
       regions.add(hri);
     }
     return regions;