From e188bb12b0e715ab623e4e803aa5e69f381a99ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Elek?= Date: Tue, 18 Jun 2019 08:46:20 +0200 Subject: [PATCH] HDDS-1694. TestNodeReportHandler is failing with NPE Closes #978 --- .../hadoop/hdds/scm/node/SCMNodeManager.java | 23 ++++++++----------- .../scm/server/StorageContainerManager.java | 2 +- .../hdds/scm/node/TestContainerPlacement.java | 8 +++++-- .../hdds/scm/node/TestNodeReportHandler.java | 9 +++++++- 4 files changed, 25 insertions(+), 17 deletions(-) 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 9a5ea11b412..eaa2255cb0d 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 @@ -30,7 +30,7 @@ import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.node.states.NodeAlreadyExistsException; import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException; -import org.apache.hadoop.hdds.scm.server.StorageContainerManager; +import org.apache.hadoop.hdds.scm.server.SCMStorageConfig; import org.apache.hadoop.hdds.scm.VersionInfo; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeStat; @@ -94,13 +94,12 @@ public class SCMNodeManager implements NodeManager { LoggerFactory.getLogger(SCMNodeManager.class); private final NodeStateManager nodeStateManager; - private final String clusterID; private final VersionInfo version; private final CommandQueue commandQueue; private final SCMNodeMetrics metrics; // Node manager MXBean private ObjectName nmInfoBean; - private final StorageContainerManager scmManager; + private final SCMStorageConfig scmStorageConfig; private final NetworkTopology clusterMap; private final DNSToSwitchMapping dnsToSwitchMapping; private final boolean useHostname; @@ -108,18 +107,17 @@ public class SCMNodeManager implements NodeManager { /** * Constructs SCM machine Manager. */ - public SCMNodeManager(OzoneConfiguration conf, String clusterID, - StorageContainerManager scmManager, EventPublisher eventPublisher) - throws IOException { + public SCMNodeManager(OzoneConfiguration conf, + SCMStorageConfig scmStorageConfig, EventPublisher eventPublisher, + NetworkTopology networkTopology) { this.nodeStateManager = new NodeStateManager(conf, eventPublisher); - this.clusterID = clusterID; this.version = VersionInfo.getLatestVersion(); this.commandQueue = new CommandQueue(); - this.scmManager = scmManager; + this.scmStorageConfig = scmStorageConfig; LOG.info("Entering startup safe mode."); registerMXBean(); this.metrics = SCMNodeMetrics.create(this); - this.clusterMap = scmManager.getClusterMap(); + this.clusterMap = networkTopology; Class dnsToSwitchMappingClass = conf.getClass(DFSConfigKeys.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, TableMapping.class, DNSToSwitchMapping.class); @@ -221,9 +219,8 @@ public class SCMNodeManager implements NodeManager { return VersionResponse.newBuilder() .setVersion(this.version.getVersion()) .addValue(OzoneConsts.SCM_ID, - this.scmManager.getScmStorageConfig().getScmId()) - .addValue(OzoneConsts.CLUSTER_ID, this.scmManager.getScmStorageConfig() - .getClusterID()) + this.scmStorageConfig.getScmId()) + .addValue(OzoneConsts.CLUSTER_ID, this.scmStorageConfig.getClusterID()) .build(); } @@ -274,7 +271,7 @@ public class SCMNodeManager implements NodeManager { return RegisteredCommand.newBuilder().setErrorCode(ErrorCode.success) .setDatanodeUUID(datanodeDetails.getUuidString()) - .setClusterID(this.clusterID) + .setClusterID(this.scmStorageConfig.getClusterID()) .setHostname(datanodeDetails.getHostName()) .setIpAddress(datanodeDetails.getIpAddress()) .build(); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index ca60a5dd115..08712ccdbc5 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -378,7 +378,7 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl scmNodeManager = configurator.getScmNodeManager(); } else { scmNodeManager = new SCMNodeManager( - conf, scmStorageConfig.getClusterID(), this, eventQueue); + conf, scmStorageConfig, eventQueue, clusterMap); } ContainerPlacementPolicy containerPlacementPolicy = diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java index ad24a1079ab..ec0c4c34470 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java @@ -36,6 +36,7 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.events.SCMEvents; import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; import org.apache.hadoop.hdds.scm.pipeline.SCMPipelineManager; +import org.apache.hadoop.hdds.scm.server.SCMStorageConfig; import org.apache.hadoop.hdds.server.events.EventQueue; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.test.PathUtils; @@ -48,7 +49,6 @@ import org.mockito.Mockito; import java.io.File; import java.io.IOException; import java.util.List; -import java.util.UUID; import java.util.concurrent.TimeoutException; import static org.apache.hadoop.hdds.scm.ScmConfigKeys @@ -94,8 +94,12 @@ public class TestContainerPlacement { Mockito.mock(StaleNodeHandler.class)); eventQueue.addHandler(SCMEvents.DEAD_NODE, Mockito.mock(DeadNodeHandler.class)); + + SCMStorageConfig storageConfig = Mockito.mock(SCMStorageConfig.class); + Mockito.when(storageConfig.getClusterID()).thenReturn("cluster1"); + SCMNodeManager nodeManager = new SCMNodeManager(config, - UUID.randomUUID().toString(), null, eventQueue); + storageConfig, eventQueue, null); return nodeManager; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeReportHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeReportHandler.java index 1cb9bcdc963..88de27d9965 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeReportHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeReportHandler.java @@ -24,7 +24,9 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolPro import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.TestUtils; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; +import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.server.SCMDatanodeHeartbeatDispatcher.NodeReportFromDatanode; +import org.apache.hadoop.hdds.scm.server.SCMStorageConfig; import org.apache.hadoop.hdds.server.events.Event; import org.apache.hadoop.hdds.server.events.EventPublisher; import org.apache.hadoop.hdds.server.events.EventQueue; @@ -32,6 +34,7 @@ import org.apache.hadoop.test.GenericTestUtils; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,7 +53,11 @@ public class TestNodeReportHandler implements EventPublisher { @Before public void resetEventCollector() throws IOException { OzoneConfiguration conf = new OzoneConfiguration(); - nodeManager = new SCMNodeManager(conf, "cluster1", null, new EventQueue()); + SCMStorageConfig storageConfig = Mockito.mock(SCMStorageConfig.class); + Mockito.when(storageConfig.getClusterID()).thenReturn("cluster1"); + nodeManager = + new SCMNodeManager(conf, storageConfig, new EventQueue(), Mockito.mock( + NetworkTopology.class)); nodeReportHandler = new NodeReportHandler(nodeManager); }