HDFS-13416. Ozone: TestNodeManager tests fail. Contributed by Bharat Viswanadham.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/e6da4d8d Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/e6da4d8d Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/e6da4d8d Branch: refs/heads/trunk Commit: e6da4d8da4f55f3b26842f7e92935957aec83160 Parents: d8fd922 Author: Nanda kumar <na...@apache.org> Authored: Wed Apr 11 14:36:51 2018 +0530 Committer: Nanda kumar <na...@apache.org> Committed: Wed Apr 11 14:36:51 2018 +0530 ---------------------------------------------------------------------- .../hadoop/hdds/protocol/DatanodeDetails.java | 11 +++++ .../hadoop/hdds/scm/node/SCMNodeManager.java | 6 +-- .../hadoop/hdds/scm/node/TestNodeManager.java | 51 +++++++++----------- 3 files changed, 38 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/e6da4d8d/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java ---------------------------------------------------------------------- diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java index 764b3cd..b2fa291 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java @@ -241,6 +241,17 @@ public final class DatanodeDetails implements Comparable<DatanodeDetails> { return this.getUuid().compareTo(that.getUuid()); } + @Override + public boolean equals(Object obj) { + return obj instanceof DatanodeDetails && + uuid.equals(((DatanodeDetails) obj).uuid); + } + + @Override + public int hashCode() { + return uuid.hashCode(); + } + /** * Returns DatanodeDetails.Builder instance. * http://git-wip-us.apache.org/repos/asf/hadoop/blob/e6da4d8d/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java ---------------------------------------------------------------------- diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index 0174c17..af68dba 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -829,9 +829,10 @@ public class SCMNodeManager DatanodeDetailsProto datanodeDetailsProto, SCMNodeReport nodeReport, ReportState containerReportState) { + Preconditions.checkNotNull(datanodeDetailsProto, "Heartbeat is missing " + + "DatanodeDetails."); DatanodeDetails datanodeDetails = DatanodeDetails .getFromProtoBuf(datanodeDetailsProto); - // Checking for NULL to make sure that we don't get // an exception from ConcurrentList. // This could be a problem in tests, if this function is invoked via @@ -846,7 +847,6 @@ public class SCMNodeManager } else { LOG.error("Datanode ID in heartbeat is null"); } - return commandQueue.getCommand(datanodeDetails.getUuid()); } @@ -875,7 +875,7 @@ public class SCMNodeManager */ @Override public SCMNodeMetric getNodeStat(DatanodeDetails datanodeDetails) { - return new SCMNodeMetric(nodeStats.get(datanodeDetails)); + return new SCMNodeMetric(nodeStats.get(datanodeDetails.getUuid())); } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/e6da4d8d/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeManager.java ---------------------------------------------------------------------- diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeManager.java index de6e30c..89ce12e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeManager.java @@ -35,7 +35,6 @@ import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.protocol.commands.SCMCommand; import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.test.PathUtils; -import org.hamcrest.CoreMatchers; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -71,7 +70,6 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.STALE; import static org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.SCMCmdType; import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.StringStartsWith.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -423,8 +421,8 @@ public class TestNodeManager { try (SCMNodeManager nodeManager = createNodeManager(conf)) { - List<DatanodeDetails> nodeList = createNodeSet(nodeManager, nodeCount, - "Node"); + List<DatanodeDetails> nodeList = createNodeSet(nodeManager, nodeCount); + DatanodeDetails staleNode = TestUtils.getDatanodeDetails(nodeManager); @@ -484,22 +482,20 @@ public class TestNodeManager { } /** - * Asserts that we log an error for null in datanode ID. + * Check for NPE when datanodeDetails is passed null for sendHeartbeat. * * @throws IOException * @throws InterruptedException * @throws TimeoutException */ @Test - public void testScmLogErrorOnNullDatanode() throws IOException, + public void testScmCheckForErrorOnNullDatanodeDetails() throws IOException, InterruptedException, TimeoutException { try (SCMNodeManager nodeManager = createNodeManager(getConf())) { - GenericTestUtils.LogCapturer logCapturer = - GenericTestUtils.LogCapturer.captureLogs(SCMNodeManager.LOG); nodeManager.sendHeartbeat(null, null, reportState); - logCapturer.stopCapturing(); - assertThat(logCapturer.getOutput(), - containsString("Datanode ID in heartbeat is null")); + } catch (NullPointerException npe) { + GenericTestUtils.assertExceptionContains("Heartbeat is missing " + + "DatanodeDetails.", npe); } } @@ -568,11 +564,11 @@ public class TestNodeManager { */ try (SCMNodeManager nodeManager = createNodeManager(conf)) { DatanodeDetails healthyNode = - TestUtils.getDatanodeDetails(nodeManager, "HealthyNode"); + TestUtils.getDatanodeDetails(nodeManager); DatanodeDetails staleNode = - TestUtils.getDatanodeDetails(nodeManager, "StaleNode"); + TestUtils.getDatanodeDetails(nodeManager); DatanodeDetails deadNode = - TestUtils.getDatanodeDetails(nodeManager, "DeadNode"); + TestUtils.getDatanodeDetails(nodeManager); nodeManager.sendHeartbeat( healthyNode.getProtoBufMessage(), null, reportState); nodeManager.sendHeartbeat( @@ -708,15 +704,14 @@ public class TestNodeManager { * Create a set of Nodes with a given prefix. * * @param count - number of nodes. - * @param prefix - A prefix string that can be used in verification. * @return List of Nodes. */ private List<DatanodeDetails> createNodeSet(SCMNodeManager nodeManager, int - count, String - prefix) { + count) { List<DatanodeDetails> list = new LinkedList<>(); for (int x = 0; x < count; x++) { - list.add(TestUtils.getDatanodeDetails(nodeManager, prefix + x)); + list.add(TestUtils.getDatanodeDetails(nodeManager, UUID.randomUUID() + .toString())); } return list; } @@ -758,11 +753,11 @@ public class TestNodeManager { try (SCMNodeManager nodeManager = createNodeManager(conf)) { List<DatanodeDetails> healthyNodeList = createNodeSet(nodeManager, - healthyCount, "Healthy"); + healthyCount); List<DatanodeDetails> staleNodeList = createNodeSet(nodeManager, - staleCount, "Stale"); - List<DatanodeDetails> deadNodeList = createNodeSet(nodeManager, deadCount, - "Dead"); + staleCount); + List<DatanodeDetails> deadNodeList = createNodeSet(nodeManager, + deadCount); Runnable healthyNodeTask = () -> { try { @@ -810,9 +805,11 @@ public class TestNodeManager { List<DatanodeDetails> deadList = nodeManager.getNodes(DEAD); for (DatanodeDetails node : deadList) { - assertThat(node.getHostName(), CoreMatchers.startsWith("Dead")); + assertTrue(deadNodeList.contains(node)); } + + // Checking stale nodes is tricky since they have to move between // healthy and stale to avoid becoming dead nodes. So we search for // that state for a while, if we don't find that state waitfor will @@ -849,9 +846,9 @@ public class TestNodeManager { try (SCMNodeManager nodeManager = createNodeManager(conf)) { List<DatanodeDetails> healthyList = createNodeSet(nodeManager, - healthyCount, "h"); + healthyCount); List<DatanodeDetails> staleList = createNodeSet(nodeManager, - staleCount, "s"); + staleCount); Runnable healthyNodeTask = () -> { try { @@ -911,7 +908,7 @@ public class TestNodeManager { try (SCMNodeManager nodeManager = createNodeManager(conf)) { List<DatanodeDetails> healthyList = createNodeSet(nodeManager, - healthyCount, "h"); + healthyCount); GenericTestUtils.LogCapturer logCapturer = GenericTestUtils.LogCapturer.captureLogs(SCMNodeManager.LOG); Runnable healthyNodeTask = () -> { @@ -1108,7 +1105,7 @@ public class TestNodeManager { // Compare the result from // NodeManager#getNodeStats and NodeManager#getNodeStat SCMNodeStat stat1 = nodeManager.getNodeStats(). - get(datanodeDetails); + get(datanodeDetails.getUuid()); SCMNodeStat stat2 = nodeManager.getNodeStat(datanodeDetails).get(); assertEquals(stat1, stat2); --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
