HBASE-9837 Forward port HBASE-9080 'Retain assignment should be used when re-enabling table(s)'

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

hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java

@@ -59,7 +59,7 @@ public class EnableTableHandler extends EventHandler {
   private final AssignmentManager assignmentManager;
   private final TableLockManager tableLockManager;
   private final CatalogTracker catalogTracker;
-  private boolean retainAssignment = false;
+  private boolean skipTableStateCheck = false;
   private TableLock tableLock;
 
   public EnableTableHandler(Server server, TableName tableName,
@@ -70,7 +70,7 @@ public class EnableTableHandler extends EventHandler {
     this.catalogTracker = catalogTracker;
     this.assignmentManager = assignmentManager;
     this.tableLockManager = tableLockManager;
-    this.retainAssignment = skipTableStateCheck;
+    this.skipTableStateCheck = skipTableStateCheck;
   }
 
   public EnableTableHandler prepare()
@@ -85,7 +85,7 @@ public class EnableTableHandler extends EventHandler {
       // Check if table exists
       if (!MetaReader.tableExists(catalogTracker, tableName)) {
         // retainAssignment is true only during recovery.  In normal case it is false
-        if (!this.retainAssignment) {
+        if (!this.skipTableStateCheck) {
           throw new TableNotFoundException(tableName);
         } 
         try {
@@ -101,7 +101,7 @@ public class EnableTableHandler extends EventHandler {
       // the table at the same time. Ensure only the first request is honored
       // After that, no other requests can be accepted until the table reaches
       // DISABLED or ENABLED.
-      if (!retainAssignment) {
+      if (!skipTableStateCheck) {
         try {
           if (!this.assignmentManager.getZKTable().checkDisabledAndSetEnablingTable
             (this.tableName)) {
@@ -186,7 +186,7 @@ public class EnableTableHandler extends EventHandler {
     LOG.info("Table '" + this.tableName + "' has " + countOfRegionsInTable
       + " regions, of which " + regionsCount + " are offline.");
     BulkEnabler bd = new BulkEnabler(this.server, regions, countOfRegionsInTable,
-        this.retainAssignment);
+        true);
     try {
       if (bd.bulkAssign()) {
         done = true;
@@ -223,7 +223,7 @@ public class EnableTableHandler extends EventHandler {
       HRegionInfo hri = regionLocation.getFirst();
       ServerName sn = regionLocation.getSecond();
       if (regionStates.isRegionOffline(hri)) {
-        if (this.retainAssignment && sn != null && serverManager.isServerOnline(sn)) {
+        if (sn != null && serverManager.isServerOnline(sn)) {
           this.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, sn));
         }
         regions.add(hri);
@@ -244,43 +244,29 @@ public class EnableTableHandler extends EventHandler {
     private final List<HRegionInfo> regions;
     // Count of regions in table at time this assign was launched.
     private final int countOfRegionsInTable;
-    private final boolean retainAssignment;
 
     BulkEnabler(final Server server, final List<HRegionInfo> regions,
         final int countOfRegionsInTable, boolean retainAssignment) {
       super(server);
       this.regions = regions;
       this.countOfRegionsInTable = countOfRegionsInTable;
-      this.retainAssignment = retainAssignment;
     }
 
     @Override
     protected void populatePool(ExecutorService pool) throws IOException {
-      boolean roundRobinAssignment = this.server.getConfiguration().getBoolean(
-          "hbase.master.enabletable.roundrobin", false);
-
       // In case of masterRestart always go with single assign.  Going thro
       // roundRobinAssignment will use bulkassign which may lead to double assignment.
-      if (retainAssignment || !roundRobinAssignment) {
+      for (HRegionInfo region : regions) {
-        for (HRegionInfo region : regions) {
+        if (assignmentManager.getRegionStates()
-          if (assignmentManager.getRegionStates()
+            .isRegionInTransition(region)) {
-              .isRegionInTransition(region)) {
+          continue;
-            continue;
+        }
+        final HRegionInfo hri = region;
+        pool.execute(Trace.wrap("BulkEnabler.populatePool",new Runnable() {
+          public void run() {
+            assignmentManager.assign(hri, true);
           }
-          final HRegionInfo hri = region;
+        }));
-          pool.execute(Trace.wrap("BulkEnabler.populatePool",new Runnable() {
-            public void run() {
-              assignmentManager.assign(hri, true);
-            }
-          }));
-        }
-      } else {
-        try {
-          assignmentManager.assign(regions);
-        } catch (InterruptedException e) {
-          LOG.warn("Assignment was interrupted");
-          Thread.currentThread().interrupt();
-        }
       }
     }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java

@@ -58,6 +58,7 @@ import org.apache.hadoop.hbase.TableNotEnabledException;
 import org.apache.hadoop.hbase.TableNotFoundException;
 import org.apache.hadoop.hbase.ZooKeeperConnectionException;
 import org.apache.hadoop.hbase.catalog.CatalogTracker;
+import org.apache.hadoop.hbase.catalog.MetaReader;
 import org.apache.hadoop.hbase.constraint.ConstraintException;
 import org.apache.hadoop.hbase.executor.EventHandler;
 import org.apache.hadoop.hbase.master.AssignmentManager;
@@ -848,12 +849,12 @@ public class TestAdmin {
   }
 
   /**
-   * Test round-robin assignment on enableTable.
+   * Test retain assignment on enableTable.
    *
    * @throws IOException
    */
   @Test (timeout=300000)
-  public void testEnableTableRoundRobinAssignment() throws IOException {
+  public void testEnableTableRetainAssignment() throws IOException {
     byte[] tableName = Bytes.toBytes("testEnableTableAssignment");
     byte[][] splitKeys = { new byte[] { 1, 1, 1 }, new byte[] { 2, 2, 2 },
         new byte[] { 3, 3, 3 }, new byte[] { 4, 4, 4 }, new byte[] { 5, 5, 5 },
@@ -870,38 +871,16 @@ public class TestAdmin {
         + "but only found " + regions.size(), expectedRegions, regions.size());
     // Disable table.
     admin.disableTable(tableName);
-    // Enable table, use round-robin assignment to assign regions.
+    // Enable table, use retain assignment to assign regions.
     admin.enableTable(tableName);
+    Map<HRegionInfo, ServerName> regions2 = ht.getRegionLocations();
 
     // Check the assignment.
-    HTable metaTable = new HTable(TEST_UTIL.getConfiguration(),
+    assertEquals(regions.size(), regions2.size());
-        TableName.META_TABLE_NAME);
+    for (Map.Entry<HRegionInfo, ServerName> entry : regions.entrySet()) {
-    List<HRegionInfo> regionInfos = admin.getTableRegions(tableName);
+      assertEquals(regions2.get(entry.getKey()), entry.getValue());
-    Map<String, Integer> serverMap = new HashMap<String, Integer>();
+    }
-    for (HRegionInfo hri : regionInfos) {
-      Get get = new Get(hri.getRegionName());
-      Result result = metaTable.get(get);
-      String server = Bytes.toString(result.getValue(HConstants.CATALOG_FAMILY,
-          HConstants.SERVER_QUALIFIER));
-      Integer regioncount = serverMap.get(server);
-      if (regioncount == null) {
-        regioncount = 0;
-      }
-      regioncount++;
-      serverMap.put(server, regioncount);
     }
-    metaTable.close();
-    List<Map.Entry<String, Integer>> entryList = new ArrayList<Map.Entry<String, Integer>>(
-        serverMap.entrySet());
-    Collections.sort(entryList, new Comparator<Map.Entry<String, Integer>>() {
-      public int compare(Map.Entry<String, Integer> oa,
-          Map.Entry<String, Integer> ob) {
-        return (oa.getValue() - ob.getValue());
-      }
-    });
-    assertTrue(entryList.size() == 3);
-    assertTrue((entryList.get(2).getValue() - entryList.get(0).getValue()) < 2);
-  }
 
   /**
    * Multi-family scenario. Tests forcing split from client and