From 22cee7e6816d1cc2fcba8caaf0db1b697ef6e04d Mon Sep 17 00:00:00 2001 From: Sean Busbey Date: Thu, 23 Apr 2015 22:51:35 -0500 Subject: [PATCH] HBASE-13546 handle nulls in MasterAddressTracker when there is no master active. Conflicts: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java --- .../hbase/zookeeper/MasterAddressTracker.java | 7 +- .../regionserver/TestMasterAddressTracker.java | 80 +++++++++++++++----- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java index d71a893..abfe73e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java @@ -104,6 +104,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker { public static ServerName getMasterAddress(final ZooKeeperWatcher zkw) throws KeeperException, IOException { byte [] data = ZKUtil.getData(zkw, zkw.getMasterAddressZNode()); + // TODO javadoc claims we return null in this case. :/ if (data == null){ throw new IOException("Can't get master address from ZooKeeper; znode data == null"); } @@ -123,7 +124,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker { * @param zkw The ZooKeeperWatcher to use. * @param znode Where to create the znode; could be at the top level or it * could be under backup masters - * @param master ServerName of the current master + * @param master ServerName of the current master must not be null. * @return true if node created, false if not; a watch is set in both cases * @throws KeeperException */ @@ -142,7 +143,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker { } /** - * @param sn + * @param sn must not be null * @return Content of the master znode as a serialized pb with the pb * magic as prefix. */ @@ -159,6 +160,8 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker { /** * delete the master znode if its content is same as the parameter + * @param zkw must not be null + * @param content must not be null */ public static boolean deleteIfEquals(ZooKeeperWatcher zkw, final String content) { if (content == null){ diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java index c48757c..e709336 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.regionserver; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.concurrent.Semaphore; @@ -34,6 +35,8 @@ import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.Rule; +import org.junit.rules.TestName; import org.junit.experimental.categories.Category; @Category(MediumTests.class) @@ -42,6 +45,9 @@ public class TestMasterAddressTracker { private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + @Rule + public TestName name = new TestName(); + @BeforeClass public static void setUpBeforeClass() throws Exception { TEST_UTIL.startMiniZKCluster(); @@ -51,16 +57,28 @@ public class TestMasterAddressTracker { public static void tearDownAfterClass() throws Exception { TEST_UTIL.shutdownMiniZKCluster(); } - /** - * Unit tests that uses ZooKeeper but does not use the master-side methods - * but rather acts directly on ZK. - * @throws Exception - */ + @Test - public void testMasterAddressTrackerFromZK() throws Exception { + public void testDeleteIfEquals() throws Exception { + final ServerName sn = ServerName.valueOf("localhost", 1234, System.currentTimeMillis()); + final MasterAddressTracker addressTracker = setupMasterTracker(sn); + try { + assertFalse("shouldn't have deleted wrong master server.", + MasterAddressTracker.deleteIfEquals(addressTracker.getWatcher(), "some other string.")); + } finally { + assertTrue("Couldn't clean up master", + MasterAddressTracker.deleteIfEquals(addressTracker.getWatcher(), sn.toString())); + } + } + /** + * create an address tracker instance + * @param sn if not-null set the active master + */ + private MasterAddressTracker setupMasterTracker(final ServerName sn) + throws Exception { ZooKeeperWatcher zk = new ZooKeeperWatcher(TEST_UTIL.getConfiguration(), - "testMasterAddressTrackerFromZK", null); + name.getMethodName(), null); ZKUtil.createAndFailSilent(zk, zk.baseZNode); // Should not have a master yet @@ -73,21 +91,43 @@ public class TestMasterAddressTracker { NodeCreationListener listener = new NodeCreationListener(zk, zk.getMasterAddressZNode()); zk.registerListener(listener); + if (sn != null) { + LOG.info("Creating master node"); + MasterAddressTracker.setMasterAddress(zk, zk.getMasterAddressZNode(), sn); + + // Wait for the node to be created + LOG.info("Waiting for master address manager to be notified"); + listener.waitForCreation(); + LOG.info("Master node created"); + } + return addressTracker; + } + + /** + * Unit tests that uses ZooKeeper but does not use the master-side methods + * but rather acts directly on ZK. + * @throws Exception + */ + @Test + public void testMasterAddressTrackerFromZK() throws Exception { // Create the master node with a dummy address - String host = "localhost"; - int port = 1234; - ServerName sn = ServerName.valueOf(host, port, System.currentTimeMillis()); - LOG.info("Creating master node"); - MasterAddressTracker.setMasterAddress(zk, zk.getMasterAddressZNode(), sn); - - // Wait for the node to be created - LOG.info("Waiting for master address manager to be notified"); - listener.waitForCreation(); - LOG.info("Master node created"); - assertTrue(addressTracker.hasMaster()); - ServerName pulledAddress = addressTracker.getMasterAddress(); - assertTrue(pulledAddress.equals(sn)); + final ServerName sn = ServerName.valueOf("localhost", 1234, System.currentTimeMillis()); + final MasterAddressTracker addressTracker = setupMasterTracker(sn); + try { + assertTrue(addressTracker.hasMaster()); + ServerName pulledAddress = addressTracker.getMasterAddress(); + assertTrue(pulledAddress.equals(sn)); + } finally { + assertTrue("Couldn't clean up master", + MasterAddressTracker.deleteIfEquals(addressTracker.getWatcher(), sn.toString())); + } + } + @Test + public void testNoMaster() throws Exception { + final MasterAddressTracker addressTracker = setupMasterTracker(null); + assertFalse(addressTracker.hasMaster()); + assertNull("should get null master when none active.", addressTracker.getMasterAddress()); } public static class NodeCreationListener extends ZooKeeperListener { -- 1.7.10.2 (Apple Git-33)