svn commit: r1327364 - in /hbase/trunk/src: main/java/org/apache/hadoop/hbase/master/AssignmentManager.java test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java

[te...@apache.org]

Author: tedyu
Date: Wed Apr 18 03:15:24 2012
New Revision: 1327364

URL: http://svn.apache.org/viewvc?rev=1327364&view=rev
Log:
HBASE-5733 AssignmentManager#processDeadServersAndRegionsInTransition can fail with NPE (Uma Maheswara Rao G)

Modified:
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
    hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java?rev=1327364&r1=1327363&r2=1327364&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Wed Apr 18 03:15:24 2012
@@ -402,6 +402,13 @@ public class AssignmentManager extends Z
   throws KeeperException, IOException, InterruptedException {
     List<String> nodes = ZKUtil.listChildrenAndWatchForNewChildren(watcher,
       watcher.assignmentZNode);
+    
+    if (nodes == null) {
+      String errorMessage = "Failed to get the children from ZK";
+      master.abort(errorMessage, new IOException(errorMessage));
+      return;
+    }
+    
     // Run through all regions.  If they are not assigned and not in RIT, then
     // its a clean cluster startup, else its a failover.
     for (Map.Entry<HRegionInfo, ServerName> e: this.regions.entrySet()) {

Modified: hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java?rev=1327364&r1=1327363&r2=1327364&view=diff
==============================================================================
--- hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (original)
+++ hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java Wed Apr 18 03:15:24 2012
@@ -17,8 +17,10 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -27,6 +29,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
@@ -39,10 +42,10 @@ import org.apache.hadoop.hbase.catalog.C
 import org.apache.hadoop.hbase.client.HConnection;
 import org.apache.hadoop.hbase.client.HConnectionTestingUtility;
 import org.apache.hadoop.hbase.client.Result;
-import org.apache.hadoop.hbase.executor.EventHandler.EventType;
 import org.apache.hadoop.hbase.executor.ExecutorService;
-import org.apache.hadoop.hbase.executor.ExecutorService.ExecutorType;
 import org.apache.hadoop.hbase.executor.RegionTransitionData;
+import org.apache.hadoop.hbase.executor.EventHandler.EventType;
+import org.apache.hadoop.hbase.executor.ExecutorService.ExecutorType;
 import org.apache.hadoop.hbase.master.handler.ServerShutdownHandler;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.ClientProtocol;
@@ -54,10 +57,12 @@ import org.apache.hadoop.hbase.regionser
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.hadoop.hbase.util.Threads;
+import org.apache.hadoop.hbase.zookeeper.RecoverableZooKeeper;
 import org.apache.hadoop.hbase.zookeeper.ZKAssign;
 import org.apache.hadoop.hbase.zookeeper.ZKUtil;
 import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.KeeperException.NodeExistsException;
 import org.junit.After;
 import org.junit.AfterClass;
@@ -478,6 +483,39 @@ public class TestAssignmentManager {
   }
 
   /**
+   * Tests the processDeadServersAndRegionsInTransition should not fail with NPE
+   * when it failed to get the children. Let's abort the system in this
+   * situation
+   * @throws ServiceException 
+   */
+  @Test(timeout = 5000)
+  public void testProcessDeadServersAndRegionsInTransitionShouldNotFailWithNPE()
+      throws IOException, KeeperException, InterruptedException, ServiceException {
+    final RecoverableZooKeeper recoverableZk = Mockito
+        .mock(RecoverableZooKeeper.class);
+    AssignmentManagerWithExtrasForTesting am = setUpMockedAssignmentManager(
+        this.server, this.serverManager);
+    Watcher zkw = new ZooKeeperWatcher(HBaseConfiguration.create(), "unittest",
+        null) {
+      public RecoverableZooKeeper getRecoverableZooKeeper() {
+        return recoverableZk;
+      }
+    };
+    ((ZooKeeperWatcher) zkw).registerListener(am);
+    Mockito.doThrow(new InterruptedException()).when(recoverableZk)
+        .getChildren("/hbase/unassigned", zkw);
+    am.setWatcher((ZooKeeperWatcher) zkw);
+    try {
+      am.processDeadServersAndRegionsInTransition();
+      fail("Expected to abort");
+    } catch (NullPointerException e) {
+      fail("Should not throw NPE");
+    } catch (RuntimeException e) {
+      assertEquals("Aborted", e.getLocalizedMessage());
+    }
+  }
+  
+  /**
    * Creates a new ephemeral node in the SPLITTING state for the specified region.
    * Create it ephemeral in case regionserver dies mid-split.
    *
@@ -610,7 +648,12 @@ public class TestAssignmentManager {
       while (this.gate.get()) Threads.sleep(1);
       super.processRegionsInTransition(data, regionInfo, deadServers, expectedVersion);
     }
-
+    
+    /** reset the watcher */
+    void setWatcher(ZooKeeperWatcher watcher) {
+      this.watcher = watcher;
+    }
+    
     /**
      * @return ExecutorService used by this instance.
      */