HBASE-10349 Table became unusable when master balanced its region after table was dropped

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

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

@@ -3144,9 +3144,6 @@ public class AssignmentManager extends ZooKeeperListener {
    * @param plan Plan to execute.
    */
   public void balance(final RegionPlan plan) {
-    synchronized (this.regionPlans) {
-      this.regionPlans.put(plan.getRegionName(), plan);
-    }
     HRegionInfo hri = plan.getRegionInfo();
     TableName tableName = hri.getTable();
     if (zkTable.isDisablingOrDisabledTable(tableName)) {
@@ -3154,7 +3151,24 @@ public class AssignmentManager extends ZooKeeperListener {
         + tableName);
       return;
     }
+
+    // Move the region only if it's assigned
+    String encodedName = hri.getEncodedName();
+    ReentrantLock lock = locker.acquireLock(encodedName);
+    try {
+      if (!regionStates.isRegionOnline(hri)) {
+        RegionState state = regionStates.getRegionState(encodedName);
+        LOG.info("Ignored moving region not assigned: " + hri + ", "
+          + (state == null ? "not in region states" : state));
+        return;
+      }
+      synchronized (this.regionPlans) {
+        this.regionPlans.put(plan.getRegionName(), plan);
+      }
       unassign(hri, false, plan.getDestination());
+    } finally {
+      lock.unlock();
+    }
   }
 
   public void stop() {

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

@@ -576,6 +576,23 @@ public class RegionStates {
     }
   }
 
+  /**
+   * A table is deleted. Remove its regions from all internal maps.
+   * We loop through all regions assuming we don't delete tables too much.
+   */
+  public synchronized void tableDeleted(final TableName tableName) {
+    Set<HRegionInfo> regionsToDelete = new HashSet<HRegionInfo>();
+    for (RegionState state: regionStates.values()) {
+      HRegionInfo region = state.getRegion();
+      if (region.getTable().equals(tableName)) {
+        regionsToDelete.add(region);
+      }
+    }
+    for (HRegionInfo region: regionsToDelete) {
+      deleteRegion(region);
+    }
+  }
+
   /**
    * Checking if a region was assigned to a server which is not online now.
    * If so, we should hold re-assign this region till SSH has split its hlogs.
@@ -746,4 +763,19 @@ public class RegionStates {
       return null;
     }
   }
+
+  /**
+   * Remove a region from all state maps.
+   */
+  private void deleteRegion(final HRegionInfo hri) {
+    String encodedName = hri.getEncodedName();
+    regionsInTransition.remove(encodedName);
+    regionStates.remove(encodedName);
+    lastAssignments.remove(encodedName);
+    ServerName sn = regionAssignments.remove(hri);
+    if (sn != null) {
+      Set<HRegionInfo> regions = serverHoldings.get(sn);
+      regions.remove(hri);
+    }
+  }
 }

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

@@ -117,7 +117,11 @@ public class DeleteTableHandler extends TableEventHandler {
       LOG.debug("Removing '" + tableName + "' descriptor.");
       this.masterServices.getTableDescriptors().remove(tableName);
 
-      // 7. If entry for this table in zk, and up in AssignmentManager, remove it.
+      // 7. Clean up regions of the table in RegionStates.
+      LOG.debug("Removing '" + tableName + "' from region states.");
+      states.tableDeleted(tableName);
+
+      // 8. If entry for this table in zk, and up in AssignmentManager, remove it.
       LOG.debug("Marking '" + tableName + "' as deleted.");
       am.getZKTable().setDeletedTable(tableName);
     }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java

@@ -1339,4 +1339,26 @@ public class TestAssignmentManager {
       am.shutdown();
     }
   }
+
+  /**
+   * If a table is deleted, we should not be able to balance it anymore.
+   * Otherwise, the region will be brought back.
+   * @throws Exception
+   */
+  @Test
+  public void testBalanceRegionOfDeletedTable() throws Exception {
+    CatalogTracker ct = Mockito.mock(CatalogTracker.class);
+    AssignmentManager am = new AssignmentManager(this.server, this.serverManager,
+      ct, balancer, null, null, master.getTableLockManager());
+    RegionStates regionStates = am.getRegionStates();
+    HRegionInfo hri = REGIONINFO;
+    regionStates.createRegionState(hri);
+    assertFalse(regionStates.isRegionInTransition(hri));
+    RegionPlan plan = new RegionPlan(hri, SERVERNAME_A, SERVERNAME_B);
+    // Fake table is deleted
+    regionStates.tableDeleted(hri.getTable());
+    am.balance(plan);
+    assertFalse("The region should not in transition",
+      regionStates.isRegionInTransition(hri));
+  }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java

@@ -43,6 +43,7 @@ import org.apache.hadoop.hbase.MiniHBaseCluster;
 import org.apache.hadoop.hbase.ServerLoad;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.UnknownRegionException;
 import org.apache.hadoop.hbase.catalog.MetaEditor;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.client.HTable;
@@ -272,6 +273,54 @@ public class TestAssignmentManagerOnCluster {
     }
   }
 
+  /**
+   * If a table is deleted, we should not be able to move it anymore.
+   * Otherwise, the region will be brought back.
+   * @throws Exception
+   */
+  @Test (timeout=50000)
+  public void testMoveRegionOfDeletedTable() throws Exception {
+    TableName table =
+        TableName.valueOf("testMoveRegionOfDeletedTable");
+    HBaseAdmin admin = TEST_UTIL.getHBaseAdmin();
+    try {
+      HRegionInfo hri = createTableAndGetOneRegion(table);
+
+      HMaster master = TEST_UTIL.getHBaseCluster().getMaster();
+      AssignmentManager am = master.getAssignmentManager();
+      RegionStates regionStates = am.getRegionStates();
+      ServerName serverName = regionStates.getRegionServerOfRegion(hri);
+      ServerName destServerName = null;
+      for (int i = 0; i < 3; i++) {
+        HRegionServer destServer = TEST_UTIL.getHBaseCluster().getRegionServer(i);
+        if (!destServer.getServerName().equals(serverName)) {
+          destServerName = destServer.getServerName();
+          break;
+        }
+      }
+      assertTrue(destServerName != null
+        && !destServerName.equals(serverName));
+
+      TEST_UTIL.deleteTable(table);
+
+      try {
+        admin.move(hri.getEncodedNameAsBytes(),
+          Bytes.toBytes(destServerName.getServerName()));
+        fail("We should not find the region");
+      } catch (IOException ioe) {
+        assertTrue(ioe instanceof UnknownRegionException);
+      }
+
+      am.balance(new RegionPlan(hri, serverName, destServerName));
+      assertFalse("The region should not be in transition",
+        regionStates.isRegionInTransition(hri));
+    } finally {
+      if (admin.tableExists(table)) {
+        TEST_UTIL.deleteTable(table);
+      }
+    }
+  }
+
   HRegionInfo createTableAndGetOneRegion(
       final TableName tableName) throws IOException, InterruptedException {
     HTableDescriptor desc = new HTableDescriptor(tableName);