svn commit: r1344352 - in /hbase/branches/0.94/src: main/java/org/apache/hadoop/hbase/master/AssignmentManager.java test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java

[ra...@apache.org]

Author: ramkrishna
Date: Wed May 30 17:09:20 2012
New Revision: 1344352

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

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

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java?rev=1344352&r1=1344351&r2=1344352&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Wed May 30 17:09:20 2012
@@ -383,6 +383,12 @@ 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/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java?rev=1344352&r1=1344351&r2=1344352&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java Wed May 30 17:09:20 2012
@@ -17,9 +17,11 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -28,6 +30,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;
@@ -56,11 +59,13 @@ import org.apache.hadoop.hbase.util.Byte
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.hadoop.hbase.util.Threads;
 import org.apache.hadoop.hbase.util.Writables;
+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.hadoop.hbase.zookeeper.ZKTable.TableState;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.KeeperException.NodeExistsException;
 import org.junit.After;
 import org.junit.AfterClass;
@@ -658,6 +663,39 @@ public class TestAssignmentManager {
       am.shutdown();
     }
   }
+  
+  /**
+   * 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.
@@ -872,6 +910,11 @@ public class TestAssignmentManager {
     public ServerName getRegionServerOfRegion(HRegionInfo hri) {
       return SERVERNAME_A;
     }
+    
+    /** reset the watcher */
+    void setWatcher(ZooKeeperWatcher watcher) {
+      this.watcher = watcher;
+    }
 
     /**
      * @return ExecutorService used by this instance.