svn commit: r1459581 - in /hbase/branches/0.95/hbase-server/src: main/java/org/apache/hadoop/hbase/master/HMaster.java test/java/org/apache/hadoop/hbase/master/TestMaster.java

[jx...@apache.org]

Author: jxiang
Date: Thu Mar 21 23:28:33 2013
New Revision: 1459581

URL: http://svn.apache.org/r1459581
Log:
HBASE-8160 HMaster#move doesn't check if master initialized

Modified:
    hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
    hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java

Modified: hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java?rev=1459581&r1=1459580&r2=1459581&view=diff
==============================================================================
--- hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java (original)
+++ hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Thu Mar 21 23:28:33 2013
@@ -293,8 +293,11 @@ Server {
   private volatile boolean abort = false;
   // flag set after we become the active master (used for testing)
   private volatile boolean isActiveMaster = false;
-  // flag set after we complete initialization once active (used for testing)
-  private volatile boolean initialized = false;
+
+  // flag set after we complete initialization once active,
+  // it is not private since it's used in unit tests
+  volatile boolean initialized = false;
+
   // flag set after we complete assignMeta.
   private volatile boolean serverShutdownHandlerEnabled = false;
 
@@ -1397,11 +1400,21 @@ Server {
       LOG.warn("moveRegion specifier type: expected: " + RegionSpecifierType.ENCODED_REGION_NAME
         + " actual: " + type);
     }
+
+    try {
+      move(encodedRegionName, destServerName);
+    } catch (IOException ioe) {
+      throw new ServiceException(ioe);
+    }
+    return mrr;
+  }
+
+  void move(final byte[] encodedRegionName,
+      final byte[] destServerName) throws UnknownRegionException {
     RegionState regionState = assignmentManager.getRegionStates().
       getRegionState(Bytes.toString(encodedRegionName));
     if (regionState == null) {
-      throw new ServiceException(
-        new UnknownRegionException(Bytes.toStringBinary(encodedRegionName)));
+      throw new UnknownRegionException(Bytes.toStringBinary(encodedRegionName));
     }
 
     HRegionInfo hri = regionState.getRegion();
@@ -1417,7 +1430,7 @@ Server {
       if (dest.equals(regionState.getServerName())) {
         LOG.debug("Skipping move of region " + hri.getRegionNameAsString()
           + " because region already assigned to the same server " + dest + ".");
-        return mrr;
+        return;
       }
     }
 
@@ -1425,9 +1438,10 @@ Server {
     RegionPlan rp = new RegionPlan(hri, regionState.getServerName(), dest);
 
     try {
+      checkInitialized();
       if (this.cpHost != null) {
         if (this.cpHost.preMove(hri, rp.getSource(), rp.getDestination())) {
-          return mrr;
+          return;
         }
       }
       LOG.info("Added move plan " + rp + ", running balancer");
@@ -1439,9 +1453,8 @@ Server {
       UnknownRegionException ure = new UnknownRegionException(
         Bytes.toStringBinary(encodedRegionName));
       ure.initCause(ioe);
-      throw new ServiceException(ure);
+      throw ure;
     }
-    return mrr;
   }
 
   @Override

Modified: hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java?rev=1459581&r1=1459580&r2=1459581&view=diff
==============================================================================
--- hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java (original)
+++ hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java Thu Mar 21 23:28:33 2013
@@ -23,12 +23,14 @@ import org.apache.commons.logging.LogFac
 import org.apache.hadoop.hbase.*;
 import org.apache.hadoop.hbase.catalog.MetaReader;
 import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.exceptions.PleaseHoldException;
 import org.apache.hadoop.hbase.executor.EventHandler;
 import org.apache.hadoop.hbase.executor.EventHandler.EventHandlerListener;
 import org.apache.hadoop.hbase.executor.EventType;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Pair;
 
+import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -52,7 +54,7 @@ public class TestMaster {
   @BeforeClass
   public static void beforeAllTests() throws Exception {
     // Start a cluster of two regionservers.
-    TEST_UTIL.startMiniCluster(1);
+    TEST_UTIL.startMiniCluster(2);
   }
 
   @AfterClass
@@ -113,6 +115,22 @@ public class TestMaster {
     }
   }
 
+  @Test
+  public void testMoveRegionWhenNotInitialized() {
+    MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
+    HMaster m = cluster.getMaster();
+    try {
+      m.initialized = false; // fake it, set back later
+      HRegionInfo meta = HRegionInfo.FIRST_META_REGIONINFO;
+      m.move(meta.getEncodedNameAsBytes(), null);
+      fail("Region should not be moved since master is not initialized");
+    } catch (IOException ioe) {
+      assertTrue(ioe.getCause() instanceof PleaseHoldException);
+    } finally {
+      m.initialized = true;
+    }
+  }
+
   static class RegionSplitListener implements EventHandlerListener {
     CountDownLatch split, proceed;