From 527288ef891dc26019d003bd85ddfd50eb4f3b7b Mon Sep 17 00:00:00 2001 From: Inigo Goiri Date: Thu, 6 Sep 2018 16:47:54 -0700 Subject: [PATCH] HDFS-13836. RBF: Handle mount table znode with null value. Contributed by yanghuafeng. --- .../hadoop/util/curator/ZKCuratorManager.java | 10 +++- .../util/curator/TestZKCuratorManager.java | 23 ++++++++ .../store/driver/TestStateStoreZK.java | 53 +++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java index 8276b6e29c6..d164138a39f 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java @@ -196,7 +196,10 @@ public final class ZKCuratorManager { */ public String getStringData(final String path) throws Exception { byte[] bytes = getData(path); - return new String(bytes, Charset.forName("UTF-8")); + if (bytes != null) { + return new String(bytes, Charset.forName("UTF-8")); + } + return null; } /** @@ -208,7 +211,10 @@ public final class ZKCuratorManager { */ public String getStringData(final String path, Stat stat) throws Exception { byte[] bytes = getData(path, stat); - return new String(bytes, Charset.forName("UTF-8")); + if (bytes != null) { + return new String(bytes, Charset.forName("UTF-8")); + } + return null; } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestZKCuratorManager.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestZKCuratorManager.java index 486e89a6265..a2156ee6d93 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestZKCuratorManager.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestZKCuratorManager.java @@ -19,6 +19,7 @@ package org.apache.hadoop.util.curator; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.Arrays; @@ -30,6 +31,7 @@ import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.util.ZKUtil; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Stat; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -97,6 +99,27 @@ public class TestZKCuratorManager { assertEquals(2, children.size()); } + @Test + public void testGetStringData() throws Exception { + String node1 = "/node1"; + String node2 = "/node2"; + assertFalse(curator.exists(node1)); + curator.create(node1); + assertNull(curator.getStringData(node1)); + + byte[] setData = "setData".getBytes("UTF-8"); + curator.setData(node1, setData, -1); + assertEquals("setData", curator.getStringData(node1)); + + Stat stat = new Stat(); + assertFalse(curator.exists(node2)); + curator.create(node2); + assertNull(curator.getStringData(node2, stat)); + + curator.setData(node2, setData, -1); + assertEquals("setData", curator.getStringData(node2, stat)); + + } @Test public void testTransaction() throws Exception { List zkAcl = ZKUtil.parseACLs(CommonConfigurationKeys.ZK_ACL_DEFAULT); diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreZK.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreZK.java index 3cf7c9198a9..f8be9f0a05b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreZK.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreZK.java @@ -18,6 +18,10 @@ package org.apache.hadoop.hdfs.server.federation.store.driver; import static org.apache.hadoop.hdfs.server.federation.store.FederationStateStoreTestUtils.getStateStoreConfiguration; +import static org.apache.hadoop.hdfs.server.federation.store.driver.impl.StateStoreZooKeeperImpl.FEDERATION_STORE_ZK_PARENT_PATH; +import static org.apache.hadoop.hdfs.server.federation.store.driver.impl.StateStoreZooKeeperImpl.FEDERATION_STORE_ZK_PARENT_PATH_DEFAULT; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import java.io.IOException; import java.util.concurrent.TimeUnit; @@ -29,7 +33,14 @@ import org.apache.curator.test.TestingServer; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys; +import org.apache.hadoop.hdfs.server.federation.store.StateStoreUtils; import org.apache.hadoop.hdfs.server.federation.store.driver.impl.StateStoreZooKeeperImpl; +import org.apache.hadoop.hdfs.server.federation.store.records.BaseRecord; +import org.apache.hadoop.hdfs.server.federation.store.records.DisabledNameservice; +import org.apache.hadoop.hdfs.server.federation.store.records.MembershipState; +import org.apache.hadoop.hdfs.server.federation.store.records.MountTable; +import org.apache.hadoop.hdfs.server.federation.store.records.RouterState; +import org.apache.zookeeper.CreateMode; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -42,6 +53,7 @@ public class TestStateStoreZK extends TestStateStoreDriverBase { private static TestingServer curatorTestingServer; private static CuratorFramework curatorFramework; + private static String baseZNode; @BeforeClass public static void setupCluster() throws Exception { @@ -61,6 +73,9 @@ public class TestStateStoreZK extends TestStateStoreDriverBase { // Disable auto-repair of connection conf.setLong(RBFConfigKeys.FEDERATION_STORE_CONNECTION_TEST_MS, TimeUnit.HOURS.toMillis(1)); + + baseZNode = conf.get(FEDERATION_STORE_ZK_PARENT_PATH, + FEDERATION_STORE_ZK_PARENT_PATH_DEFAULT); getStateStore(conf); } @@ -78,6 +93,44 @@ public class TestStateStoreZK extends TestStateStoreDriverBase { removeAll(getStateStoreDriver()); } + private String generateFakeZNode( + Class recordClass) throws IOException { + String nodeName = StateStoreUtils.getRecordName(recordClass); + String primaryKey = "test"; + + if (nodeName != null) { + return baseZNode + "/" + nodeName + "/" + primaryKey; + } + return null; + } + + private void testGetNullRecord(StateStoreDriver driver) throws Exception { + testGetNullRecord(driver, MembershipState.class); + testGetNullRecord(driver, MountTable.class); + testGetNullRecord(driver, RouterState.class); + testGetNullRecord(driver, DisabledNameservice.class); + } + + private void testGetNullRecord( + StateStoreDriver driver, Class recordClass) throws Exception { + driver.removeAll(recordClass); + + String znode = generateFakeZNode(recordClass); + assertNull(curatorFramework.checkExists().forPath(znode)); + + curatorFramework.create().withMode(CreateMode.PERSISTENT) + .withACL(null).forPath(znode, null); + assertNotNull(curatorFramework.checkExists().forPath(znode)); + + driver.get(recordClass); + assertNull(curatorFramework.checkExists().forPath(znode)); + } + + @Test + public void testGetNullRecord() throws Exception { + testGetNullRecord(getStateStoreDriver()); + } + @Test public void testInsert() throws IllegalArgumentException, IllegalAccessException, IOException {