From 6eea28c3f3813594279b81c5be9cc3087bf3d99f Mon Sep 17 00:00:00 2001 From: He Xiaoqiao Date: Wed, 30 Mar 2022 14:56:04 +0800 Subject: [PATCH] HDFS-16498. Fix NPE for checkBlockReportLease #4057. Contributed by tomscut. Reviewed-by: Ayush Saxena Signed-off-by: He Xiaoqiao --- .../server/blockmanagement/BlockManager.java | 3 ++ .../blockmanagement/DatanodeManager.java | 5 +++ .../server/namenode/NameNodeRpcServer.java | 2 +- .../blockmanagement/TestBlockReportLease.java | 45 +++++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java index 494c2f01c9..7b666f3432 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java @@ -2751,6 +2751,9 @@ public boolean checkBlockReportLease(BlockReportContext context, return true; } DatanodeDescriptor node = datanodeManager.getDatanode(nodeID); + if (node == null) { + throw new UnregisteredNodeException(nodeID, null); + } final long startTime = Time.monotonicNow(); return blockReportLeaseManager.checkLease(node, startTime, context.getLeaseId()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java index 092ef6502a..a9850aa7f5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java @@ -2198,5 +2198,10 @@ public DatanodeStorageReport[] getDatanodeStorageReport( } return reports; } + + @VisibleForTesting + public Map getDatanodeMap() { + return datanodeMap; + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java index 4f400519ff..73957bcafe 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java @@ -1641,7 +1641,7 @@ public DatanodeCommand blockReport(final DatanodeRegistration nodeReg, } } } catch (UnregisteredNodeException une) { - LOG.debug("Datanode {} is attempting to report but not register yet.", + LOG.warn("Datanode {} is attempting to report but not register yet.", nodeReg); return RegisterCommand.REGISTER; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockReportLease.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockReportLease.java index a5acc14edd..d1ae0b600f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockReportLease.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockReportLease.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hdfs.server.protocol.FinalizeCommand; import org.apache.hadoop.hdfs.server.protocol.HeartbeatResponse; import org.apache.hadoop.hdfs.server.protocol.NamenodeProtocols; +import org.apache.hadoop.hdfs.server.protocol.RegisterCommand; import org.apache.hadoop.hdfs.server.protocol.SlowDiskReports; import org.apache.hadoop.hdfs.server.protocol.SlowPeerReports; import org.apache.hadoop.hdfs.server.protocol.StorageBlockReport; @@ -136,6 +137,50 @@ public void testCheckBlockReportLease() throws Exception { } } + @Test + public void testCheckBlockReportLeaseWhenDnUnregister() throws Exception { + HdfsConfiguration conf = new HdfsConfiguration(); + Random rand = new Random(); + + try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build()) { + FSNamesystem fsn = cluster.getNamesystem(); + BlockManager blockManager = fsn.getBlockManager(); + String poolId = cluster.getNamesystem().getBlockPoolId(); + NamenodeProtocols rpcServer = cluster.getNameNodeRpc(); + + // Remove the unique DataNode to simulate the unregistered situation. + // This is similar to starting NameNode, and DataNodes are not registered yet. + DataNode dn = cluster.getDataNodes().get(0); + blockManager.getDatanodeManager().getDatanodeMap().remove(dn.getDatanodeUuid()); + + // Trigger BlockReport. + DatanodeRegistration dnRegistration = dn.getDNRegistrationForBP(poolId); + StorageReport[] storages = dn.getFSDataset().getStorageReports(poolId); + ExecutorService pool = Executors.newFixedThreadPool(1); + BlockReportContext brContext = new BlockReportContext(1, 0, + rand.nextLong(), 1); + Future sendBRFuture = pool.submit(() -> { + // Build every storage with 100 blocks for sending report. + DatanodeStorage[] datanodeStorages + = new DatanodeStorage[storages.length]; + for (int i = 0; i < storages.length; i++) { + datanodeStorages[i] = storages[i].getStorage(); + } + StorageBlockReport[] reports = createReports(datanodeStorages, 100); + + // Send blockReport. + return rpcServer.blockReport(dnRegistration, poolId, reports, + brContext); + }); + + // When unregistered DataNode triggering the block report, will throw an + // UnregisteredNodeException. After NameNode processing, RegisterCommand + // is returned to the DataNode. + DatanodeCommand datanodeCommand = sendBRFuture.get(); + assertTrue(datanodeCommand instanceof RegisterCommand); + } + } + private StorageBlockReport[] createReports(DatanodeStorage[] dnStorages, int numBlocks) { int longsPerBlock = 3;