From 60549a7a1dd237e66936419b04140b76bde19a28 Mon Sep 17 00:00:00 2001 From: magnum Date: Thu, 21 May 2026 08:54:48 +0900 Subject: [PATCH] HDFS-17917. Fix empty block list enqueue in markedDeleteQueue --- .../server/blockmanagement/BlockManager.java | 12 +++-- .../namenode/metrics/NameNodeMetrics.java | 12 +++-- .../blockmanagement/TestBlockManager.java | 46 +++++++++++++++++++ 3 files changed, 62 insertions(+), 8 deletions(-) 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 15a9923a4957e7..8fb87b708ffe6e 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 @@ -5343,13 +5343,12 @@ private boolean checkToDeleteIterator() { @Override public void run() { + metrics = NameNode.getNameNodeMetrics(); LOG.info("Start MarkedDeleteBlockScrubber thread"); while (namesystem.isRunning() && !Thread.currentThread().isInterrupted()) { if (!markedDeleteQueue.isEmpty() || checkToDeleteIterator()) { try { - metrics = NameNode.getNameNodeMetrics(); - metrics.setDeleteBlocksQueued(markedDeleteQueue.size()); isSleep = false; long startTime = Time.monotonicNow(); remove(startTime); @@ -5357,6 +5356,7 @@ public void run() { !Thread.currentThread().isInterrupted()) { List markedDeleteList = markedDeleteQueue.poll(); if (markedDeleteList != null) { + metrics.decrDeleteBlocksQueued(); toDeleteIterator = markedDeleteList.listIterator(); } remove(startTime); @@ -5742,9 +5742,13 @@ public ConcurrentLinkedQueue> getMarkedDeleteQueue() { } public void addBLocksToMarkedDeleteQueue(List blockInfos) { + if (blockInfos == null || blockInfos.isEmpty()) { + return; + } + NameNodeMetrics metrics = NameNode.getNameNodeMetrics(); + metrics.incrDeleteBlocksQueued(); + metrics.incrPendingDeleteBlocksCount(blockInfos.size()); markedDeleteQueue.add(blockInfos); - NameNode.getNameNodeMetrics(). - incrPendingDeleteBlocksCount(blockInfos.size()); } public long nextGenerationStamp(boolean legacyBlock) throws IOException { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/metrics/NameNodeMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/metrics/NameNodeMetrics.java index f0cf00238b5ee6..f658518d8c2d34 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/metrics/NameNodeMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/metrics/NameNodeMetrics.java @@ -345,14 +345,18 @@ public void setBlockOpsQueued(int size) { blockOpsQueued.set(size); } - public void setDeleteBlocksQueued(int size) { - deleteBlocksQueued.set(size); - } - public void incrPendingDeleteBlocksCount(int size) { pendingDeleteBlocksCount.incr(size); } + public void incrDeleteBlocksQueued() { + deleteBlocksQueued.incr(); + } + + public void decrDeleteBlocksQueued() { + deleteBlocksQueued.decr(); + } + public void decrPendingDeleteBlocksCount() { pendingDeleteBlocksCount.decr(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java index c289a611d0cbee..499fc0cbc9c9e0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java @@ -2338,4 +2338,50 @@ public void delayDeleteReplica() { DataNodeFaultInjector.set(oldInjector); } } + + @Test + public void testDoesNotEnqueueBlocksWhenDeletingEmptyDir() throws Exception { + Configuration conf = new Configuration(); + // Pause the MarkedDeleteBlockScrubber so the queue state can be observed + // deterministically right after the delete call. + conf.setLong(DFSConfigKeys.DFS_NAMENODE_BLOCK_DELETION_UNLOCK_INTERVAL_MS, + TimeUnit.HOURS.toMillis(1)); + + try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build()) { + cluster.waitActive(); + BlockManager blockManager = cluster.getNamesystem().getBlockManager(); + DistributedFileSystem fs = cluster.getFileSystem(); + + Path dir = new Path("/test/dir"); + fs.mkdirs(new Path(dir, "leaf")); + fs.delete(dir, true); + + assertTrue(blockManager.getMarkedDeleteQueue().isEmpty(), + "Deleting an empty directory must not enqueue an empty block list"); + } + } + + @Test + public void testEnqueueBlocksWhenDeletingFile() throws Exception { + Configuration conf = new Configuration(); + // Pause the MarkedDeleteBlockScrubber so we can observe the enqueued batch + // before the background thread drains it. + conf.setLong(DFSConfigKeys.DFS_NAMENODE_BLOCK_DELETION_UNLOCK_INTERVAL_MS, + TimeUnit.HOURS.toMillis(1)); + + try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build()) { + cluster.waitActive(); + BlockManager blockManager = cluster.getNamesystem().getBlockManager(); + DistributedFileSystem fs = cluster.getFileSystem(); + + Path file = new Path("/test/file"); + DFSTestUtil.createFile(fs, file, 1024L, (short) 1, 0L); + DFSTestUtil.waitReplication(fs, file, (short) 1); + + fs.delete(file, false); + + assertEquals(1, blockManager.getMarkedDeleteQueue().size(), + "Deleting a file with blocks should enqueue exactly one batch"); + } + } }