HBASE-2064 Cannot disable a table if at the same the Master is moving

its regions around
HBASE-2065  Cannot disable a table if any of its region is opening 
            at the same time


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

CHANGES.txt

@@ -135,6 +135,10 @@ Release 0.21.0 - Unreleased
                hbase.version
    HBASE-2054  memstore size 0 is >= than blocking -2.0g size
    HBASE-2057  Cluster won't stop
+   HBASE-2064  Cannot disable a table if at the same the Master is moving 
+               its regions around
+   HBASE-2065  Cannot disable a table if any of its region is opening 
+               at the same time
 
   IMPROVEMENTS
    HBASE-1760  Cleanup TODOs in HTable

src/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java

@@ -103,8 +103,15 @@ class ProcessRegionOpen extends ProcessRegionStatusChange {
           master.getRegionManager().metaScannerThread.interrupt();
         }
       }
-      // If updated successfully, remove from pending list.
-      master.getRegionManager().removeRegion(regionInfo);
+      // If updated successfully, remove from pending list if the state
+      // is consistent. For example, a disable could be called before the
+      // synchronization.
+      if(master.getRegionManager().
+          isOfflined(regionInfo.getRegionNameAsString())) {
+        LOG.warn("We opened a region while it was asked to be closed.");
+      } else {
+        master.getRegionManager().removeRegion(regionInfo);
+      }
       return true;
     }
   }

src/java/org/apache/hadoop/hbase/master/RegionManager.java

@@ -945,7 +945,7 @@ public class RegionManager implements HConstants {
         s = new RegionState(info);
         regionsInTransition.put(info.getRegionNameAsString(), s);
       }
-      if (force || (!s.isPendingOpen() && !s.isOpen())) {
+      if (force || (!s.isPendingOpen() || !s.isOpen())) {
         s.setUnassigned();
       }
     }
@@ -1018,7 +1018,7 @@ public class RegionManager implements HConstants {
    * @param regionInfo
    * @param setOffline
    */
-  public void setClosing(final String serverName, final HRegionInfo regionInfo,
+  public void setClosing(String serverName, final HRegionInfo regionInfo,
       final boolean setOffline) {
     synchronized (this.regionsInTransition) {
       RegionState s =
@@ -1026,6 +1026,11 @@ public class RegionManager implements HConstants {
       if (s == null) {
         s = new RegionState(regionInfo);
       }
+      // If region was asked to open before getting here, we could be taking
+      // the wrong server name
+      if(s.isPendingOpen()) {
+        serverName = s.getServerName();
+      }
       s.setClosing(serverName, setOffline);
       this.regionsInTransition.put(regionInfo.getRegionNameAsString(), s);
     }
@@ -1595,10 +1600,10 @@ public class RegionManager implements HConstants {
     }
     
     synchronized void setClosed() {
-      if (!pendingClose && !pendingOpen) {
+      if (!pendingClose && !pendingOpen && !closing) {
         throw new IllegalStateException(
             "Cannot set a region to be closed if it was not already marked as" +
-            " pending close or pending open. State: " + toString());
+            " pending close, pending open or closing. State: " + toString());
       }
       this.unassigned = false;
       this.pendingOpen = false;

src/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java

@@ -789,11 +789,6 @@ public class HLog implements HConstants, Syncable {
     }
     try {
       this.editsSize.addAndGet(logKey.heapSize() + logEdit.heapSize());
-      if (LOG.isDebugEnabled() && 
-          (this.numEntries.get() % this.flushlogentries == 0)) {
-        LOG.debug("edit=" + this.numEntries.get() + ", write=" +
-          logKey.toString());
-      }
       this.writer.append(new HLog.Entry(logKey, logEdit));
       long took = System.currentTimeMillis() - now;
       if (took > 1000) {

src/test/hbase-site.xml

@@ -99,7 +99,7 @@
   </property>
   <property>
     <name>hbase.regionserver.optionalcacheflushinterval</name>
-    <value>10000</value>
+    <value>1000</value>
     <description>
     Amount of time to wait since the last time a region was flushed before
     invoking an optional cache flush. Default 60,000.

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

@@ -22,6 +22,7 @@ package org.apache.hadoop.hbase.client;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.IOException;
 import java.util.Map;
@@ -44,6 +45,7 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+
 /**
  * Class to test HBaseAdmin.
  * Spins up the minicluster once at test start and then takes it down afterward.
@@ -56,6 +58,9 @@ public class TestAdmin {
 
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
+    TEST_UTIL.getConfiguration().setInt("hbase.regionserver.msginterval", 100);
+    TEST_UTIL.getConfiguration().setInt("hbase.client.pause", 250);
+    TEST_UTIL.getConfiguration().setInt("hbase.client.retries.number", 6);
     TEST_UTIL.startMiniCluster(3);
   }
 
@@ -103,7 +108,7 @@ public class TestAdmin {
     }
     assertEquals(true, ok);
     this.admin.enableTable(table);
-    
+
     //Test that table is enabled
     try {
       ht.get(get);
@@ -112,7 +117,7 @@ public class TestAdmin {
     }
     assertEquals(true, ok);
   }
-  
+
   @Test
   public void testTableExist() throws IOException {
     final byte [] table = Bytes.toBytes("testTableExist");
@@ -126,7 +131,7 @@ public class TestAdmin {
 
   /**
    * Tests forcing split from client and having scanners successfully ride over split.
-   * @throws Exception 
+   * @throws Exception
    * @throws IOException
    */
   @Test
@@ -164,7 +169,7 @@ public class TestAdmin {
     }
     scanner.close();
     assertEquals(rowCount, rows);
-    
+
     // Have an outstanding scan going on to make sure we can scan over splits.
     scan = new Scan();
     scanner = table.getScanner(scan);
@@ -337,7 +342,7 @@ public class TestAdmin {
   /**
    * Test that user table names can contain '-' and '.' so long as they do not
    * start with same. HBASE-771
-   * @throws IOException 
+   * @throws IOException
    */
   @Test
   public void testTableNames() throws IOException {
@@ -368,7 +373,7 @@ public class TestAdmin {
 
   /**
    * For HADOOP-2579
-   * @throws IOException 
+   * @throws IOException
    */
   @Test (expected=TableExistsException.class)
   public void testTableNotFoundExceptionWithATable() throws IOException {
@@ -379,11 +384,34 @@ public class TestAdmin {
 
   /**
    * For HADOOP-2579
-   * @throws IOException 
+   * @throws IOException
    */
   @Test (expected=TableNotFoundException.class)
   public void testTableNotFoundExceptionWithoutAnyTables() throws IOException {
     new HTable(TEST_UTIL.getConfiguration(),
         "testTableNotFoundExceptionWithoutAnyTables");
   }
-}
+
+  @Test
+  public void testHundredsOfTable() throws IOException{
+    final int times = 100;
+    byte [] name = Bytes.toBytes("testHundredsOfTable");
+    HColumnDescriptor fam1 = new HColumnDescriptor("fam1");
+    HColumnDescriptor fam2 = new HColumnDescriptor("fam2");
+    HColumnDescriptor fam3 = new HColumnDescriptor("fam3");
+
+    for(int i = 0; i < times; i++) {
+      HTableDescriptor htd = new HTableDescriptor("table"+i);
+      htd.addFamily(fam1);
+      htd.addFamily(fam2);
+      htd.addFamily(fam3);
+      this.admin.createTable(htd);
+    }
+
+    for(int i = 0; i < times; i++) {
+      String tableName = "table"+i;
+      this.admin.disableTable(tableName);
+      this.admin.deleteTable(tableName);
+    }
+  }
+}