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 764b3cd1af..b2fa291d5a 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 int compareTo(DatanodeDetails that) { 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. * 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 0174c1754f..af68dba637 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 List sendHeartbeat( 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 List sendHeartbeat( } else { LOG.error("Datanode ID in heartbeat is null"); } - return commandQueue.getCommand(datanodeDetails.getUuid()); } @@ -875,7 +875,7 @@ public Map getNodeStats() { */ @Override public SCMNodeMetric getNodeStat(DatanodeDetails datanodeDetails) { - return new SCMNodeMetric(nodeStats.get(datanodeDetails)); + return new SCMNodeMetric(nodeStats.get(datanodeDetails.getUuid())); } @Override 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 de6e30c947..89ce12e5d6 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.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 .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 void testScmDetectStaleAndDeadNode() throws IOException, try (SCMNodeManager nodeManager = createNodeManager(conf)) { - List nodeList = createNodeSet(nodeManager, nodeCount, - "Node"); + List nodeList = createNodeSet(nodeManager, nodeCount); + DatanodeDetails staleNode = TestUtils.getDatanodeDetails(nodeManager); @@ -484,22 +482,20 @@ public void testScmDetectStaleAndDeadNode() throws IOException, } /** - * 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 void testScmClusterIsInExpectedState1() throws IOException, */ 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 @@ private void heartbeatNodeSet(SCMNodeManager manager, * 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 createNodeSet(SCMNodeManager nodeManager, int - count, String - prefix) { + count) { List 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 void testScmClusterIsInExpectedState2() throws IOException, try (SCMNodeManager nodeManager = createNodeManager(conf)) { List healthyNodeList = createNodeSet(nodeManager, - healthyCount, "Healthy"); + healthyCount); List staleNodeList = createNodeSet(nodeManager, - staleCount, "Stale"); - List deadNodeList = createNodeSet(nodeManager, deadCount, - "Dead"); + staleCount); + List deadNodeList = createNodeSet(nodeManager, + deadCount); Runnable healthyNodeTask = () -> { try { @@ -810,9 +805,11 @@ public void testScmClusterIsInExpectedState2() throws IOException, List 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 void testScmCanHandleScale() throws IOException, try (SCMNodeManager nodeManager = createNodeManager(conf)) { List healthyList = createNodeSet(nodeManager, - healthyCount, "h"); + healthyCount); List staleList = createNodeSet(nodeManager, - staleCount, "s"); + staleCount); Runnable healthyNodeTask = () -> { try { @@ -911,7 +908,7 @@ public void testScmLogsHeartbeatFlooding() throws IOException, try (SCMNodeManager nodeManager = createNodeManager(conf)) { List healthyList = createNodeSet(nodeManager, - healthyCount, "h"); + healthyCount); GenericTestUtils.LogCapturer logCapturer = GenericTestUtils.LogCapturer.captureLogs(SCMNodeManager.LOG); Runnable healthyNodeTask = () -> { @@ -1108,7 +1105,7 @@ public void testScmNodeReportUpdate() throws IOException, // 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);