From a37040bdf5d24a9dab1670e2f4336ebf1bb0936f Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43419279@qq.com> Date: Mon, 31 May 2021 18:01:39 +0800 Subject: [PATCH 1/9] HDFS-16043. HDFS: Delete performance optimization --- .../org/apache/hadoop/hdfs/DFSConfigKeys.java | 10 ++ .../server/blockmanagement/BlockManager.java | 97 +++++++++++++++++++ .../hdfs/server/namenode/FSNamesystem.java | 3 +- .../namenode/metrics/NameNodeMetrics.java | 6 ++ 4 files changed, 115 insertions(+), 1 deletion(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java index 14f9cd7730e2f..f3994247ececd 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java @@ -476,6 +476,16 @@ public class DFSConfigKeys extends CommonConfigurationKeys { public static final String DFS_NAMENODE_STARTUP_DELAY_BLOCK_DELETION_SEC_KEY = "dfs.namenode.startup.delay.block.deletion.sec"; public static final long DFS_NAMENODE_STARTUP_DELAY_BLOCK_DELETION_SEC_DEFAULT = 0L; + /** Clear the lock time of the deleted block. */ + public static final String DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_KEY = + "dfs.namenode.delete.block.lock.time.ms"; + public static final long DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_DEFAULT = 500; + + /** Clear the lock release interval of deleted blocks. */ + public static final String DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_KEY = + "dfs.namenode.delete.block.unlock.sleep.interval.ms"; + public static final long DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_DEFAULT = 100; + /** Block deletion increment. */ public static final String DFS_NAMENODE_BLOCK_DELETION_INCREMENT_KEY = "dfs.namenode.block.deletion.increment"; 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 3348afa6e738d..64d17c062d918 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 @@ -47,6 +47,7 @@ import java.util.concurrent.FutureTask; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicLong; import javax.management.ObjectName; @@ -191,6 +192,9 @@ public class BlockManager implements BlockStatsMXBean { private volatile long lowRedundancyBlocksCount = 0L; private volatile long scheduledReplicationBlocksCount = 0L; + private final long deleteBlockLockTimeMs; + private final long deleteBlockUnlockIntervalTimeMs; + /** flag indicating whether replication queues have been initialized */ private boolean initializedReplQueues; @@ -324,6 +328,12 @@ public long getTotalECBlockGroups() { * {@link #redundancyThread} has run at least one full iteration. */ private final AtomicLong lastRedundancyCycleTS = new AtomicLong(-1); + /** + * markedDeleteBlockScrubber thread for handling async delete blocks. + */ + private final Daemon markedDeleteBlockScrubberThread = + new Daemon(new MarkedDeleteBlockScrubber()); + /** Block report thread for handling async reports. */ private final BlockReportProcessingThread blockReportThread; @@ -422,6 +432,12 @@ public long getTotalECBlockGroups() { */ private int numBlocksPerIteration; + /** + * The blocks of deleted files are put into the queue, + * and the cleanup thread processes these blocks periodically + */ + private final ConcurrentLinkedQueue> markedDeleteQueue; + /** * Progress of the Reconstruction queues initialisation. */ @@ -476,6 +492,14 @@ public BlockManager(final Namesystem namesystem, boolean haEnabled, startupDelayBlockDeletionInMs, blockIdManager); + markedDeleteQueue = new ConcurrentLinkedQueue<>(); + deleteBlockLockTimeMs = conf.getLong( + DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_KEY, + DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_DEFAULT); + deleteBlockUnlockIntervalTimeMs = conf.getLong( + DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_KEY, + DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_DEFAULT); + // Compute the map capacity by allocating 2% of total memory blocksMap = new BlocksMap( LightWeightGSet.computeCapacity(2.0, "BlocksMap")); @@ -725,6 +749,8 @@ public void activate(Configuration conf, long blockTotal) { datanodeManager.activate(conf); this.redundancyThread.setName("RedundancyMonitor"); this.redundancyThread.start(); + this.markedDeleteBlockScrubberThread.setName("MarkedDeleteBlockScrubberThread"); + this.markedDeleteBlockScrubberThread.start(); this.blockReportThread.start(); mxBeanName = MBeans.register("NameNode", "BlockStats", this); bmSafeMode.activate(blockTotal); @@ -4911,6 +4937,73 @@ public long getLastRedundancyMonitorTS() { return lastRedundancyCycleTS.get(); } + /** + * Periodically deletes the marked block. + */ + private class MarkedDeleteBlockScrubber implements Runnable { + Iterator toDeleteIterator = null; + boolean isSleep; + + private void toRemove(long time) { + // Reentrant write lock, Release the lock when the remove is + // complete + if (checkToDeleteIterator()) { + namesystem.writeLock(); + try { + while (toDeleteIterator.hasNext()) { + removeBlock(toDeleteIterator.next()); + if (Time.now() - time > deleteBlockLockTimeMs) { + LOG.info("Clear markedDeleteQueue over " + + deleteBlockLockTimeMs + " millisecond to release the write lock"); + isSleep = true; + break; + } + } + } finally { + namesystem.writeUnlock(); + } + } + } + + private boolean checkToDeleteIterator() { + return toDeleteIterator != null && toDeleteIterator.hasNext(); + } + + @Override + public void run() { + LOG.info("Start MarkedDeleteBlockScrubber thread"); + while (namesystem.isRunning()) { + if (!markedDeleteQueue.isEmpty() || checkToDeleteIterator()) { + namesystem.writeLock(); + try { + NameNodeMetrics metrics = NameNode.getNameNodeMetrics(); + metrics.setDeleteBlocksQueued(markedDeleteQueue.size()); + isSleep = false; + long startTime = Time.now(); + toRemove(startTime); + while (!markedDeleteQueue.isEmpty()) { + List markedDeleteList = markedDeleteQueue.poll(); + if (markedDeleteList != null) { + toDeleteIterator = markedDeleteList.listIterator(); + } + toRemove(startTime); + if (isSleep) { + break; + } + } + } finally { + namesystem.writeUnlock(); + } + } + try { + TimeUnit.MILLISECONDS.sleep(deleteBlockUnlockIntervalTimeMs); + } catch (InterruptedException e) { + LOG.info("Stopping MarkedDeleteBlockScrubber."); + } + } + } + } + /** * Periodically calls computeBlockRecoveryWork(). */ @@ -5259,6 +5352,10 @@ public BlockIdManager getBlockIdManager() { return blockIdManager; } + public ConcurrentLinkedQueue> getMarkedDeleteQueue() { + return markedDeleteQueue; + } + public long nextGenerationStamp(boolean legacyBlock) throws IOException { return blockIdManager.nextGenerationStamp(legacyBlock); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 610303a22a366..75b071ce15258 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -3385,7 +3385,8 @@ boolean delete(String src, boolean recursive, boolean logRetryCache) getEditLog().logSync(); logAuditEvent(ret, operationName, src); if (toRemovedBlocks != null) { - removeBlocks(toRemovedBlocks); // Incremental deletion of blocks + blockManager.getMarkedDeleteQueue().add( + toRemovedBlocks.getToDeleteList()); } return ret; } 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 fd1fab7a7fa32..1d67a6e5da445 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 @@ -89,6 +89,8 @@ public class NameNodeMetrics { MutableCounterLong blockOpsBatched; @Metric("Number of pending edits") MutableGaugeInt pendingEditsCount; + @Metric("Number of delete blocks Queued") + MutableGaugeInt deleteBlocksQueued; @Metric("Number of file system operations") public long totalFileOps(){ @@ -341,6 +343,10 @@ public void setBlockOpsQueued(int size) { blockOpsQueued.set(size); } + public void setDeleteBlocksQueued(int size) { + deleteBlocksQueued.set(size); + } + public void addBlockOpsBatched(int count) { blockOpsBatched.incr(count); } From 58a52aa81e8c598aad52e02d0eaccef548727938 Mon Sep 17 00:00:00 2001 From: zhuxiangyi Date: Thu, 10 Jun 2021 16:51:03 +0800 Subject: [PATCH 2/9] add test --- .../server/blockmanagement/BlockManager.java | 13 ++++--- .../src/main/resources/hdfs-default.xml | 16 ++++++++ .../hdfs/TestReadStripedFileWithDecoding.java | 5 ++- .../blockmanagement/BlockManagerTestUtil.java | 18 ++++++++- .../server/namenode/TestFileTruncate.java | 26 ++++++++----- .../hdfs/server/namenode/TestMetaSave.java | 2 + .../server/namenode/TestNameNodeMXBean.java | 39 +++++++++++++------ .../server/namenode/ha/TestHASafeMode.java | 4 ++ .../namenode/metrics/TestNameNodeMetrics.java | 2 + .../snapshot/TestSnapshotDeletion.java | 9 +++-- 10 files changed, 102 insertions(+), 32 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 64d17c062d918..0b38d3fc4941b 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 @@ -434,7 +434,7 @@ public long getTotalECBlockGroups() { /** * The blocks of deleted files are put into the queue, - * and the cleanup thread processes these blocks periodically + * and the cleanup thread processes these blocks periodically. */ private final ConcurrentLinkedQueue> markedDeleteQueue; @@ -749,7 +749,8 @@ public void activate(Configuration conf, long blockTotal) { datanodeManager.activate(conf); this.redundancyThread.setName("RedundancyMonitor"); this.redundancyThread.start(); - this.markedDeleteBlockScrubberThread.setName("MarkedDeleteBlockScrubberThread"); + this.markedDeleteBlockScrubberThread. + setName("MarkedDeleteBlockScrubberThread"); this.markedDeleteBlockScrubberThread.start(); this.blockReportThread.start(); mxBeanName = MBeans.register("NameNode", "BlockStats", this); @@ -4941,8 +4942,8 @@ public long getLastRedundancyMonitorTS() { * Periodically deletes the marked block. */ private class MarkedDeleteBlockScrubber implements Runnable { - Iterator toDeleteIterator = null; - boolean isSleep; + private Iterator toDeleteIterator = null; + private boolean isSleep; private void toRemove(long time) { // Reentrant write lock, Release the lock when the remove is @@ -4953,8 +4954,8 @@ private void toRemove(long time) { while (toDeleteIterator.hasNext()) { removeBlock(toDeleteIterator.next()); if (Time.now() - time > deleteBlockLockTimeMs) { - LOG.info("Clear markedDeleteQueue over " - + deleteBlockLockTimeMs + " millisecond to release the write lock"); + LOG.info("Clear markedDeleteQueue over " + deleteBlockLockTimeMs + + " millisecond to release the write lock"); isSleep = true; break; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml index e766a13787f22..de00284f9e2e0 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml @@ -6090,6 +6090,22 @@ + + dfs.namenode.delete.block.lock.time.ms + 1000 + + The maximum write lock time for clearing the blocks of deleted files. + + + + + dfs.namenode.delete.block.unlock.sleep.interval.ms + 1000 + + The interval for clearing the blocks of deleted files. + + + dfs.namenode.rpc-address.auxiliary-ports diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadStripedFileWithDecoding.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadStripedFileWithDecoding.java index 2fb9212f35429..132eb611a2dce 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadStripedFileWithDecoding.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadStripedFileWithDecoding.java @@ -23,6 +23,7 @@ import org.apache.hadoop.hdfs.protocol.LocatedStripedBlock; import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor; import org.apache.hadoop.hdfs.server.datanode.DataNode; import org.apache.hadoop.hdfs.server.datanode.DataNodeTestUtils; @@ -127,7 +128,7 @@ public void testReportBadBlock() throws IOException { } @Test - public void testInvalidateBlock() throws IOException { + public void testInvalidateBlock() throws IOException, InterruptedException { final Path file = new Path("/invalidate"); final int length = 10; final byte[] bytes = StripedFileTestUtil.generateBytes(length); @@ -151,6 +152,8 @@ public void testInvalidateBlock() throws IOException { try { // delete the file dfs.delete(file, true); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem().getBlockManager()); // check the block is added to invalidateBlocks final FSNamesystem fsn = cluster.getNamesystem(); final BlockManager bm = fsn.getBlockManager(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerTestUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerTestUtil.java index 790d09334654e..4fa320ac29e6c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerTestUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerTestUtil.java @@ -39,6 +39,9 @@ import org.apache.hadoop.util.Preconditions; public class BlockManagerTestUtil { + + static final long SLEEP_TIME = 1000; + public static void setNodeReplicationLimit(final BlockManager blockManager, final int limit) { blockManager.maxReplicationStreams = limit; @@ -178,7 +181,20 @@ public static HeartbeatManager getHeartbeatManager( */ public static CorruptReplicasMap getCorruptReplicas(final BlockManager blockManager){ return blockManager.corruptReplicas; - + + } + + /** + * Wait for the processing of the marked deleted block to complete. + */ + public static void waitForMarkedDeleteQueueIsEmpty( + BlockManager blockManager) throws InterruptedException { + while (true) { + if (blockManager.getMarkedDeleteQueue().isEmpty()) { + return; + } + Thread.sleep(SLEEP_TIME); + } } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java index 57f5ea33eb0ee..94cd7a8516128 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.util.concurrent.ThreadLocalRandom; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.apache.hadoop.hdfs.server.datanode.DataNodeFaultInjector; import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.test.LambdaTestUtils; @@ -488,7 +489,8 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) throws IOException { * remaining snapshots are still readable. */ @Test - public void testSnapshotWithTruncates() throws IOException { + public void testSnapshotWithTruncates() + throws IOException, InterruptedException { testSnapshotWithTruncates(0, 1, 2); testSnapshotWithTruncates(0, 2, 1); testSnapshotWithTruncates(1, 0, 2); @@ -497,7 +499,8 @@ public void testSnapshotWithTruncates() throws IOException { testSnapshotWithTruncates(2, 1, 0); } - void testSnapshotWithTruncates(int ... deleteOrder) throws IOException { + void testSnapshotWithTruncates(int ... deleteOrder) + throws IOException, InterruptedException { fs.mkdirs(parent); fs.setQuota(parent, 100, 1000); fs.allowSnapshot(parent); @@ -590,6 +593,8 @@ void testSnapshotWithTruncates(int ... deleteOrder) throws IOException { assertThat(contentSummary.getLength(), is(6L)); fs.delete(src, false); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem().getBlockManager()); assertBlockNotPresent(firstBlk); // Diskspace consumed should be 0 bytes * 3. [] @@ -671,10 +676,10 @@ public void testTruncateFailure() throws IOException { "File does not exist", expected); } - + fs.setPermission(p, FsPermission.createImmutable((short)0664)); { - final UserGroupInformation fooUgi = + final UserGroupInformation fooUgi = UserGroupInformation.createUserForTesting("foo", new String[]{"foo"}); try { final FileSystem foofs = DFSTestUtil.getFileSystemAs(fooUgi, conf); @@ -755,11 +760,11 @@ public void testTruncateWithDataNodesRestart() throws Exception { LocatedBlock newBlock = getLocatedBlocks(p).getLastLocatedBlock(); /* - * For non copy-on-truncate, the truncated block id is the same, but the + * For non copy-on-truncate, the truncated block id is the same, but the * GS should increase. * The truncated block will be replicated to dn0 after it restarts. */ - assertEquals(newBlock.getBlock().getBlockId(), + assertEquals(newBlock.getBlock().getBlockId(), oldBlock.getBlock().getBlockId()); assertEquals(newBlock.getBlock().getGenerationStamp(), oldBlock.getBlock().getGenerationStamp() + 1); @@ -811,7 +816,7 @@ public void testCopyOnTruncateWithDataNodesRestart() throws Exception { * For copy-on-truncate, new block is made with new block id and new GS. * The replicas of the new block is 2, then it will be replicated to dn1. */ - assertNotEquals(newBlock.getBlock().getBlockId(), + assertNotEquals(newBlock.getBlock().getBlockId(), oldBlock.getBlock().getBlockId()); assertEquals(newBlock.getBlock().getGenerationStamp(), oldBlock.getBlock().getGenerationStamp() + 1); @@ -864,10 +869,10 @@ public void testTruncateWithDataNodesRestartImmediately() throws Exception { LocatedBlock newBlock = getLocatedBlocks(p).getLastLocatedBlock(); /* - * For non copy-on-truncate, the truncated block id is the same, but the + * For non copy-on-truncate, the truncated block id is the same, but the * GS should increase. */ - assertEquals(newBlock.getBlock().getBlockId(), + assertEquals(newBlock.getBlock().getBlockId(), oldBlock.getBlock().getBlockId()); assertEquals(newBlock.getBlock().getGenerationStamp(), oldBlock.getBlock().getGenerationStamp() + 1); @@ -1256,7 +1261,8 @@ public void testTruncateWithRollingUpgrade() throws Exception { cluster.getNamesystem().getFSDirectory().getBlockManager() .getTotalBlocks()); fs.delete(p, true); - + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem().getBlockManager()); assertEquals("block num should 0", 0, cluster.getNamesystem().getFSDirectory().getBlockManager() .getTotalBlocks()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestMetaSave.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestMetaSave.java index c88570b56e0e0..4387f7679bce3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestMetaSave.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestMetaSave.java @@ -139,6 +139,8 @@ public void testMetasaveAfterDelete() nnRpc.delete("/filestatus0", true); nnRpc.delete("/filestatus1", true); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem().getBlockManager()); nnRpc.metaSave("metasaveAfterDelete.out.txt"); // Verification diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java index 81c9cb8670066..15619207cac67 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java @@ -1031,7 +1031,8 @@ public Boolean get() { @Test public void testTotalBlocksMetrics() throws Exception { MiniDFSCluster cluster = null; - FSNamesystem namesystem = null; + FSNamesystem activeNn = null; + FSNamesystem backUpNn = null; DistributedFileSystem fs = null; try { Configuration conf = new HdfsConfiguration(); @@ -1046,12 +1047,16 @@ public void testTotalBlocksMetrics() throws Exception { conf.setLong(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, blockSize); cluster = new MiniDFSCluster.Builder(conf) - .numDataNodes(totalSize).build(); - namesystem = cluster.getNamesystem(); - fs = cluster.getFileSystem(); + .nnTopology(MiniDFSNNTopology.simpleHAFederatedTopology(1)). + numDataNodes(totalSize).build(); + cluster.waitActive(); + cluster.transitionToActive(0); + activeNn = cluster.getNamesystem(0); + backUpNn = cluster.getNamesystem(1); + fs = cluster.getFileSystem(0); fs.enableErasureCodingPolicy( StripedFileTestUtil.getDefaultECPolicy().getName()); - verifyTotalBlocksMetrics(0L, 0L, namesystem.getTotalBlocks()); + verifyTotalBlocksMetrics(0L, 0L, activeNn.getTotalBlocks()); // create small file Path replDirPath = new Path("/replicated"); @@ -1068,7 +1073,7 @@ public void testTotalBlocksMetrics() throws Exception { final int smallLength = cellSize * dataBlocks; final byte[] smallBytes = StripedFileTestUtil.generateBytes(smallLength); DFSTestUtil.writeFile(fs, ecFileSmall, smallBytes); - verifyTotalBlocksMetrics(1L, 1L, namesystem.getTotalBlocks()); + verifyTotalBlocksMetrics(1L, 1L, activeNn.getTotalBlocks()); // create learge file Path replFileLarge = new Path(replDirPath, "replfile_large"); @@ -1079,15 +1084,20 @@ public void testTotalBlocksMetrics() throws Exception { final int largeLength = blockSize * totalSize + smallLength; final byte[] largeBytes = StripedFileTestUtil.generateBytes(largeLength); DFSTestUtil.writeFile(fs, ecFileLarge, largeBytes); - verifyTotalBlocksMetrics(3L, 3L, namesystem.getTotalBlocks()); + verifyTotalBlocksMetrics(3L, 3L, activeNn.getTotalBlocks()); // delete replicated files fs.delete(replDirPath, true); - verifyTotalBlocksMetrics(0L, 3L, namesystem.getTotalBlocks()); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); + verifyTotalBlocksMetrics(0L, 3L, activeNn.getTotalBlocks()); // delete ec files fs.delete(ecDirPath, true); - verifyTotalBlocksMetrics(0L, 0L, namesystem.getTotalBlocks()); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); + verifyTotalBlocksMetrics(0L, 0L, activeNn.getTotalBlocks()); + verifyTotalBlocksMetrics(0L, 0L, backUpNn.getTotalBlocks()); } finally { if (fs != null) { try { @@ -1096,9 +1106,16 @@ public void testTotalBlocksMetrics() throws Exception { throw e; } } - if (namesystem != null) { + if (activeNn != null) { + try { + activeNn.close(); + } catch (Exception e) { + throw e; + } + } + if (backUpNn != null) { try { - namesystem.close(); + backUpNn.close(); } catch (Exception e) { throw e; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java index 537e6a34bd0b1..37e279dfa5ee7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java @@ -347,6 +347,8 @@ public void testBlocksRemovedBeforeStandbyRestart() throws Exception { // once it starts up banner("Removing the blocks without rolling the edit log"); fs.delete(new Path("/test"), true); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); BlockManagerTestUtil.computeAllPendingWork( nn0.getNamesystem().getBlockManager()); cluster.triggerHeartbeats(); @@ -386,6 +388,8 @@ public void testBlocksRemovedWhileInSafeMode() throws Exception { // ACKed when due to block removals. banner("Removing the blocks without rolling the edit log"); fs.delete(new Path("/test"), true); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); BlockManagerTestUtil.computeAllPendingWork( nn0.getNamesystem().getBlockManager()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java index 349b7ac24112d..aaedb8288e410 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java @@ -661,6 +661,8 @@ public void testExcessBlocks() throws Exception { // verify ExcessBlocks metric is decremented and // excessReplicateMap is cleared after deleting a file fs.delete(file, true); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem().getBlockManager()); rb = getMetrics(NS_METRICS); assertGauge("ExcessBlocks", 0L, rb); assertEquals(0L, bm.getExcessBlocksCount()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java index b85530c3cbee6..0eb094c48094a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java @@ -44,6 +44,7 @@ import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport; import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.apache.hadoop.hdfs.server.namenode.FSDirectory; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.apache.hadoop.hdfs.server.namenode.INode; @@ -1128,7 +1129,8 @@ public void testHANNRestartAfterSnapshotDeletion() throws Exception { } @Test - public void testCorrectNumberOfBlocksAfterRestart() throws IOException { + public void testCorrectNumberOfBlocksAfterRestart() + throws IOException, InterruptedException { final Path foo = new Path("/foo"); final Path bar = new Path(foo, "bar"); final Path file = new Path(foo, "file"); @@ -1149,9 +1151,10 @@ public void testCorrectNumberOfBlocksAfterRestart() throws IOException { hdfs.delete(bar, true); hdfs.delete(foo, true); - long numberOfBlocks = cluster.getNamesystem().getBlocksTotal(); cluster.restartNameNode(0); - assertEquals(numberOfBlocks, cluster.getNamesystem().getBlocksTotal()); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem().getBlockManager()); + assertEquals(0, cluster.getNamesystem().getBlocksTotal()); } /* From e3f28c35e41a6e353204608a076030b385cf67b2 Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43412979@qq.com> Date: Mon, 21 Jun 2021 18:35:08 +0800 Subject: [PATCH 3/9] deal with check failed and comments. --- .../org/apache/hadoop/hdfs/DFSConfigKeys.java | 10 -------- .../server/blockmanagement/BlockManager.java | 24 +++++++------------ .../src/main/resources/hdfs-default.xml | 16 ------------- .../hdfs/TestBlocksScheduledCounter.java | 2 ++ .../TestComputeInvalidateWork.java | 2 ++ .../impl/TestLazyPersistLockedMemory.java | 3 +++ .../namenode/TestDecommissioningStatus.java | 2 ++ .../server/namenode/TestFileTruncate.java | 14 +++++------ .../namenode/TestLargeDirectoryDelete.java | 3 +++ .../server/namenode/TestNameNodeMXBean.java | 10 ++++---- 10 files changed, 33 insertions(+), 53 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java index f3994247ececd..14f9cd7730e2f 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java @@ -476,16 +476,6 @@ public class DFSConfigKeys extends CommonConfigurationKeys { public static final String DFS_NAMENODE_STARTUP_DELAY_BLOCK_DELETION_SEC_KEY = "dfs.namenode.startup.delay.block.deletion.sec"; public static final long DFS_NAMENODE_STARTUP_DELAY_BLOCK_DELETION_SEC_DEFAULT = 0L; - /** Clear the lock time of the deleted block. */ - public static final String DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_KEY = - "dfs.namenode.delete.block.lock.time.ms"; - public static final long DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_DEFAULT = 500; - - /** Clear the lock release interval of deleted blocks. */ - public static final String DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_KEY = - "dfs.namenode.delete.block.unlock.sleep.interval.ms"; - public static final long DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_DEFAULT = 100; - /** Block deletion increment. */ public static final String DFS_NAMENODE_BLOCK_DELETION_INCREMENT_KEY = "dfs.namenode.block.deletion.increment"; 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 0b38d3fc4941b..af816e02b835a 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 @@ -192,8 +192,8 @@ public class BlockManager implements BlockStatsMXBean { private volatile long lowRedundancyBlocksCount = 0L; private volatile long scheduledReplicationBlocksCount = 0L; - private final long deleteBlockLockTimeMs; - private final long deleteBlockUnlockIntervalTimeMs; + private final long deleteBlockLockTimeMs = 500; + private final long deleteBlockUnlockIntervalTimeMs = 100; /** flag indicating whether replication queues have been initialized */ private boolean initializedReplQueues; @@ -493,12 +493,6 @@ public BlockManager(final Namesystem namesystem, boolean haEnabled, blockIdManager); markedDeleteQueue = new ConcurrentLinkedQueue<>(); - deleteBlockLockTimeMs = conf.getLong( - DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_KEY, - DFS_NAMENODE_DELETE_BLOCK_LOCK_TIME_MS_DEFAULT); - deleteBlockUnlockIntervalTimeMs = conf.getLong( - DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_KEY, - DFS_NAMENODE_DELETE_BLOCK_UNLOCK_SLEEP_INTERVAL_MS_DEFAULT); // Compute the map capacity by allocating 2% of total memory blocksMap = new BlocksMap( @@ -4954,8 +4948,6 @@ private void toRemove(long time) { while (toDeleteIterator.hasNext()) { removeBlock(toDeleteIterator.next()); if (Time.now() - time > deleteBlockLockTimeMs) { - LOG.info("Clear markedDeleteQueue over " + deleteBlockLockTimeMs - + " millisecond to release the write lock"); isSleep = true; break; } @@ -4982,24 +4974,26 @@ public void run() { isSleep = false; long startTime = Time.now(); toRemove(startTime); - while (!markedDeleteQueue.isEmpty()) { + while (!isSleep && !markedDeleteQueue.isEmpty()) { List markedDeleteList = markedDeleteQueue.poll(); if (markedDeleteList != null) { toDeleteIterator = markedDeleteList.listIterator(); } toRemove(startTime); - if (isSleep) { - break; - } } } finally { namesystem.writeUnlock(); } } + if (isSleep) { + LOG.info("Clear markedDeleteQueue over " + deleteBlockLockTimeMs + + " millisecond to release the write lock"); + } try { - TimeUnit.MILLISECONDS.sleep(deleteBlockUnlockIntervalTimeMs); + Thread.sleep(deleteBlockUnlockIntervalTimeMs); } catch (InterruptedException e) { LOG.info("Stopping MarkedDeleteBlockScrubber."); + Thread.currentThread().interrupt(); } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml index de00284f9e2e0..e766a13787f22 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml @@ -6090,22 +6090,6 @@ - - dfs.namenode.delete.block.lock.time.ms - 1000 - - The maximum write lock time for clearing the blocks of deleted files. - - - - - dfs.namenode.delete.block.unlock.sleep.interval.ms - 1000 - - The interval for clearing the blocks of deleted files. - - - dfs.namenode.rpc-address.auxiliary-ports diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java index 95d6825d29740..d86700b39b110 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java @@ -190,6 +190,8 @@ public void testScheduledBlocksCounterDecrementOnDeletedBlock() // 4. delete the file dfs.delete(filePath, true); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); int blocksScheduled = 0; for (DatanodeDescriptor descriptor : dnList) { if (descriptor.getBlocksScheduled() != 0) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestComputeInvalidateWork.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestComputeInvalidateWork.java index d7920a75c1353..4ae0316fa7a22 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestComputeInvalidateWork.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestComputeInvalidateWork.java @@ -253,6 +253,8 @@ public void testDatanodeReRegistration() throws Exception { } dfs.delete(path, false); dfs.delete(ecFile, false); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); namesystem.writeLock(); InvalidateBlocks invalidateBlocks; int totalStripedDataBlocks = totalBlockGroups * (ecPolicy.getNumDataUnits() diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestLazyPersistLockedMemory.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestLazyPersistLockedMemory.java index 2d54c480461c5..699854cd1759c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestLazyPersistLockedMemory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestLazyPersistLockedMemory.java @@ -26,6 +26,7 @@ import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSOutputStream; import org.apache.hadoop.hdfs.DFSTestUtil; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.apache.hadoop.hdfs.server.datanode.DataNodeTestUtils; import org.apache.hadoop.hdfs.server.datanode.fsdataset.FsDatasetSpi; import org.apache.hadoop.test.GenericTestUtils; @@ -175,6 +176,8 @@ public void testWritePipelineFailure() // Delete the file and ensure locked RAM goes to zero. fs.delete(path, false); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); DataNodeTestUtils.triggerBlockReport(cluster.getDataNodes().get(0)); waitForLockedBytesUsed(fsd, 0); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java index e8bd8377a3c85..420aa8c1af79c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java @@ -445,6 +445,8 @@ public void testDecommissionStatusAfterDNRestart() throws Exception { // Delete the under-replicated file, which should let the // DECOMMISSION_IN_PROGRESS node become DECOMMISSIONED AdminStatesBaseTest.cleanupFile(fileSys, f); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); BlockManagerTestUtil.recheckDecommissionState(dm); // Block until the admin's monitor updates the number of tracked nodes. waitForDecommissionedNodes(dm.getDatanodeAdminManager(), 0); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java index 94cd7a8516128..31b65c5eab8e3 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java @@ -676,10 +676,10 @@ public void testTruncateFailure() throws IOException { "File does not exist", expected); } - + fs.setPermission(p, FsPermission.createImmutable((short)0664)); { - final UserGroupInformation fooUgi = + final UserGroupInformation fooUgi = UserGroupInformation.createUserForTesting("foo", new String[]{"foo"}); try { final FileSystem foofs = DFSTestUtil.getFileSystemAs(fooUgi, conf); @@ -760,11 +760,11 @@ public void testTruncateWithDataNodesRestart() throws Exception { LocatedBlock newBlock = getLocatedBlocks(p).getLastLocatedBlock(); /* - * For non copy-on-truncate, the truncated block id is the same, but the + * For non copy-on-truncate, the truncated block id is the same, but the * GS should increase. * The truncated block will be replicated to dn0 after it restarts. */ - assertEquals(newBlock.getBlock().getBlockId(), + assertEquals(newBlock.getBlock().getBlockId(), oldBlock.getBlock().getBlockId()); assertEquals(newBlock.getBlock().getGenerationStamp(), oldBlock.getBlock().getGenerationStamp() + 1); @@ -816,7 +816,7 @@ public void testCopyOnTruncateWithDataNodesRestart() throws Exception { * For copy-on-truncate, new block is made with new block id and new GS. * The replicas of the new block is 2, then it will be replicated to dn1. */ - assertNotEquals(newBlock.getBlock().getBlockId(), + assertNotEquals(newBlock.getBlock().getBlockId(), oldBlock.getBlock().getBlockId()); assertEquals(newBlock.getBlock().getGenerationStamp(), oldBlock.getBlock().getGenerationStamp() + 1); @@ -869,10 +869,10 @@ public void testTruncateWithDataNodesRestartImmediately() throws Exception { LocatedBlock newBlock = getLocatedBlocks(p).getLastLocatedBlock(); /* - * For non copy-on-truncate, the truncated block id is the same, but the + * For non copy-on-truncate, the truncated block id is the same, but the * GS should increase. */ - assertEquals(newBlock.getBlock().getBlockId(), + assertEquals(newBlock.getBlock().getBlockId(), oldBlock.getBlock().getBlockId()); assertEquals(newBlock.getBlock().getGenerationStamp(), oldBlock.getBlock().getGenerationStamp() + 1); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLargeDirectoryDelete.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLargeDirectoryDelete.java index df36322e9f77a..9736086950f9b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLargeDirectoryDelete.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLargeDirectoryDelete.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.Random; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.conf.Configuration; @@ -140,6 +141,8 @@ protected void execute() throws Throwable { final long start = Time.now(); mc.getFileSystem().delete(new Path("/root"), true); // recursive delete + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + mc.getNamesystem(0).getBlockManager()); final long end = Time.now(); threads[0].endThread(); threads[1].endThread(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java index 15619207cac67..d670025bf5069 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java @@ -1032,7 +1032,7 @@ public Boolean get() { public void testTotalBlocksMetrics() throws Exception { MiniDFSCluster cluster = null; FSNamesystem activeNn = null; - FSNamesystem backUpNn = null; + FSNamesystem standbyNn = null; DistributedFileSystem fs = null; try { Configuration conf = new HdfsConfiguration(); @@ -1052,7 +1052,7 @@ public void testTotalBlocksMetrics() throws Exception { cluster.waitActive(); cluster.transitionToActive(0); activeNn = cluster.getNamesystem(0); - backUpNn = cluster.getNamesystem(1); + standbyNn = cluster.getNamesystem(1); fs = cluster.getFileSystem(0); fs.enableErasureCodingPolicy( StripedFileTestUtil.getDefaultECPolicy().getName()); @@ -1097,7 +1097,7 @@ public void testTotalBlocksMetrics() throws Exception { BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( cluster.getNamesystem(0).getBlockManager()); verifyTotalBlocksMetrics(0L, 0L, activeNn.getTotalBlocks()); - verifyTotalBlocksMetrics(0L, 0L, backUpNn.getTotalBlocks()); + verifyTotalBlocksMetrics(0L, 0L, standbyNn.getTotalBlocks()); } finally { if (fs != null) { try { @@ -1113,9 +1113,9 @@ public void testTotalBlocksMetrics() throws Exception { throw e; } } - if (backUpNn != null) { + if (standbyNn != null) { try { - backUpNn.close(); + standbyNn.close(); } catch (Exception e) { throw e; } From 20ec1eca432323f52540bdcbd90739f1a3bf4e9d Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43412979@qq.com> Date: Fri, 25 Jun 2021 00:54:20 +0800 Subject: [PATCH 4/9] deal the checkstyle --- .../apache/hadoop/hdfs/server/namenode/TestFileTruncate.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java index 31b65c5eab8e3..16309ce89e9f0 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java @@ -499,7 +499,7 @@ public void testSnapshotWithTruncates() testSnapshotWithTruncates(2, 1, 0); } - void testSnapshotWithTruncates(int ... deleteOrder) + void testSnapshotWithTruncates(int... deleteOrder) throws IOException, InterruptedException { fs.mkdirs(parent); fs.setQuota(parent, 100, 1000); From 9bd2ced05acb827ed0f8ecc7e05272d26d3dcb4f Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43412979@qq.com> Date: Sat, 26 Jun 2021 22:00:23 +0800 Subject: [PATCH 5/9] deal the comments. --- .../server/blockmanagement/BlockManager.java | 10 +++---- .../hdfs/server/namenode/FSNamesystem.java | 26 +++++++++++-------- .../org/apache/hadoop/hdfs/TestDFSRename.java | 3 +++ .../apache/hadoop/hdfs/TestFileCreation.java | 5 ++++ .../server/namenode/TestFileTruncate.java | 21 ++++++++++++--- 5 files changed, 46 insertions(+), 19 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 af816e02b835a..ab61d3fb45bbc 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 @@ -4939,7 +4939,7 @@ private class MarkedDeleteBlockScrubber implements Runnable { private Iterator toDeleteIterator = null; private boolean isSleep; - private void toRemove(long time) { + private void remove(long time) { // Reentrant write lock, Release the lock when the remove is // complete if (checkToDeleteIterator()) { @@ -4947,7 +4947,7 @@ private void toRemove(long time) { try { while (toDeleteIterator.hasNext()) { removeBlock(toDeleteIterator.next()); - if (Time.now() - time > deleteBlockLockTimeMs) { + if (Time.monotonicNow() - time > deleteBlockLockTimeMs) { isSleep = true; break; } @@ -4972,14 +4972,14 @@ public void run() { NameNodeMetrics metrics = NameNode.getNameNodeMetrics(); metrics.setDeleteBlocksQueued(markedDeleteQueue.size()); isSleep = false; - long startTime = Time.now(); - toRemove(startTime); + long startTime = Time.monotonicNow(); + remove(startTime); while (!isSleep && !markedDeleteQueue.isEmpty()) { List markedDeleteList = markedDeleteQueue.poll(); if (markedDeleteList != null) { toDeleteIterator = markedDeleteList.listIterator(); } - toRemove(startTime); + remove(startTime); } } finally { namesystem.writeUnlock(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 75b071ce15258..425e596dace14 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -98,6 +98,7 @@ import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_SNAPSHOT_DIFF_LISTING_LIMIT_DEFAULT; import static org.apache.hadoop.hdfs.DFSUtil.isParentEntry; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicLong; import org.apache.commons.text.CaseUtils; @@ -2373,8 +2374,8 @@ boolean truncate(String src, long newLength, String clientName, } getEditLog().logSync(); if (!toRemoveBlocks.getToDeleteList().isEmpty()) { - removeBlocks(toRemoveBlocks); - toRemoveBlocks.clear(); + blockManager.getMarkedDeleteQueue().add( + toRemoveBlocks.getToDeleteList()); } logAuditEvent(true, operationName, src, null, status); } catch (AccessControlException e) { @@ -2821,8 +2822,8 @@ private HdfsFileStatus startFileInt(String src, if (!skipSync) { getEditLog().logSync(); if (toRemoveBlocks != null) { - removeBlocks(toRemoveBlocks); - toRemoveBlocks.clear(); + blockManager.getMarkedDeleteQueue().add( + toRemoveBlocks.getToDeleteList()); } } } @@ -3345,8 +3346,8 @@ void renameTo(final String src, final String dst, assert res != null; BlocksMapUpdateInfo collectedBlocks = res.collectedBlocks; if (!collectedBlocks.getToDeleteList().isEmpty()) { - removeBlocks(collectedBlocks); - collectedBlocks.clear(); + blockManager.getMarkedDeleteQueue().add( + collectedBlocks.getToDeleteList()); } logAuditEvent(true, operationName + " (options=" + @@ -3400,7 +3401,7 @@ FSPermissionChecker getPermissionChecker() * From the given list, incrementally remove the blocks from blockManager * Writelock is dropped and reacquired every blockDeletionIncrement to * ensure that other waiters on the lock can get in. See HDFS-2938 - * + * * @param blocks * An instance of {@link BlocksMapUpdateInfo} which contains a list * of blocks that need to be removed from blocksMap @@ -3419,7 +3420,7 @@ void removeBlocks(BlocksMapUpdateInfo blocks) { } } } - + /** * Remove leases and inodes related to a given path * @param removedUCFiles INodes whose leases need to be released @@ -4628,7 +4629,8 @@ private void clearCorruptLazyPersistFiles() INodesInPath.fromINode((INodeFile) bc), false); changed |= toRemoveBlocks != null; if (toRemoveBlocks != null) { - removeBlocks(toRemoveBlocks); // Incremental deletion of blocks + blockManager.getMarkedDeleteQueue().add( + toRemoveBlocks.getToDeleteList()); } } } finally { @@ -7339,7 +7341,8 @@ void deleteSnapshot(String snapshotRoot, String snapshotName, // Breaking the pattern as removing blocks have to happen outside of the // global lock if (blocksToBeDeleted != null) { - removeBlocks(blocksToBeDeleted); + blockManager.getMarkedDeleteQueue().add( + blocksToBeDeleted.getToDeleteList()); } logAuditEvent(true, operationName, rootPath, null, null); } @@ -7365,7 +7368,8 @@ public void gcDeletedSnapshot(String snapshotRoot, String snapshotName) } finally { writeUnlock(operationName, getLockReportInfoSupplier(rootPath)); } - removeBlocks(blocksToBeDeleted); + blockManager.getMarkedDeleteQueue().add( + blocksToBeDeleted.getToDeleteList()); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRename.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRename.java index fe2eee28b751e..427dc43d3bb15 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRename.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRename.java @@ -30,6 +30,7 @@ import org.apache.hadoop.fs.Options.Rename; import org.apache.hadoop.hdfs.protocol.LocatedBlocks; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter; import org.apache.hadoop.test.GenericTestUtils; @@ -161,6 +162,8 @@ public void testRenameWithOverwrite() throws Exception { assertTrue(bm.getStoredBlock(lbs.getLocatedBlocks().get(0).getBlock(). getLocalBlock()) != null); dfs.rename(srcPath, dstPath, Rename.OVERWRITE); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertTrue(bm.getStoredBlock(lbs.getLocatedBlocks().get(0).getBlock(). getLocalBlock()) == null); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileCreation.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileCreation.java index a7cf68b10168f..406e1e6699179 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileCreation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileCreation.java @@ -75,6 +75,7 @@ import org.apache.hadoop.hdfs.protocol.LocatedBlock; import org.apache.hadoop.hdfs.protocol.LocatedBlocks; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.apache.hadoop.hdfs.server.datanode.DataNode; import org.apache.hadoop.hdfs.server.datanode.DataNodeTestUtils; import org.apache.hadoop.hdfs.server.datanode.SimulatedFSDataset; @@ -1356,6 +1357,8 @@ public void testFileCreationWithOverwrite() throws Exception { assertBlocks(bm, oldBlocks, true); out = dfs.create(filePath, true); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); byte[] newData = AppendTestUtil.randomBytes(seed, fileSize); try { out.write(newData); @@ -1363,6 +1366,8 @@ public void testFileCreationWithOverwrite() throws Exception { out.close(); } dfs.deleteOnExit(filePath); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); LocatedBlocks newBlocks = NameNodeAdapter.getBlockLocations( nn, file, 0, fileSize); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java index 16309ce89e9f0..a80fc136e5dc0 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java @@ -320,7 +320,8 @@ public void testTruncateWithOtherOperations() throws IOException { } @Test - public void testSnapshotWithAppendTruncate() throws IOException { + public void testSnapshotWithAppendTruncate() + throws IOException, InterruptedException { testSnapshotWithAppendTruncate(0, 1, 2); testSnapshotWithAppendTruncate(0, 2, 1); testSnapshotWithAppendTruncate(1, 0, 2); @@ -334,7 +335,8 @@ public void testSnapshotWithAppendTruncate() throws IOException { * Delete snapshots in the specified order and verify that * remaining snapshots are still readable. */ - void testSnapshotWithAppendTruncate(int ... deleteOrder) throws IOException { + void testSnapshotWithAppendTruncate(int ... deleteOrder) + throws IOException, InterruptedException { FSDirectory fsDir = cluster.getNamesystem().getFSDirectory(); fs.mkdirs(parent); fs.setQuota(parent, 100, 1000); @@ -382,6 +384,8 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) throws IOException { // Truncate to block boundary int newLength = length[0] + BLOCK_SIZE / 2; boolean isReady = fs.truncate(src, newLength); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertTrue("Recovery is not expected.", isReady); assertFileLength(snapshotFiles[2], length[2]); assertFileLength(snapshotFiles[1], length[1]); @@ -435,7 +439,8 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) throws IOException { numINodes, fsDir.getInodeMapSize()); fs.deleteSnapshot(parent, ss[3]); - + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertBlockExists(firstBlk); assertBlockExists(lastBlk); assertBlockNotPresent(replacedBlk); @@ -458,6 +463,8 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) throws IOException { assertThat(contentSummary.getSpaceConsumed(), is(48L)); fs.deleteSnapshot(parent, ss[deleteOrder[1]]); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertFileLength(snapshotFiles[deleteOrder[2]], length[deleteOrder[2]]); assertBlockExists(firstBlk); contentSummary = fs.getContentSummary(parent); @@ -473,6 +480,8 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) throws IOException { numINodes, fsDir .getInodeMapSize()); fs.deleteSnapshot(parent, ss[deleteOrder[2]]); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertBlockNotPresent(firstBlk); assertBlockNotPresent(lastBlk); @@ -547,6 +556,8 @@ void testSnapshotWithTruncates(int... deleteOrder) assertThat(contentSummary.getSpaceConsumed(), is(42L)); fs.deleteSnapshot(parent, ss[deleteOrder[0]]); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertFileLength(snapshotFiles[deleteOrder[1]], length[deleteOrder[1]]); assertFileLength(snapshotFiles[deleteOrder[2]], length[deleteOrder[2]]); assertFileLength(src, length[2]); @@ -564,6 +575,8 @@ void testSnapshotWithTruncates(int... deleteOrder) } fs.deleteSnapshot(parent, ss[deleteOrder[1]]); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertFileLength(snapshotFiles[deleteOrder[2]], length[deleteOrder[2]]); assertFileLength(src, length[2]); assertBlockExists(firstBlk); @@ -584,6 +597,8 @@ void testSnapshotWithTruncates(int... deleteOrder) } fs.deleteSnapshot(parent, ss[deleteOrder[2]]); + BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( + cluster.getNamesystem(0).getBlockManager()); assertFileLength(src, length[2]); assertBlockExists(firstBlk); From 8e53a10643436d326b343d6fb71ca61a70304ae1 Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43412979@qq.com> Date: Sun, 26 Sep 2021 16:27:26 +0800 Subject: [PATCH 6/9] add closing threads and remove redundant locks --- .../server/blockmanagement/BlockManager.java | 20 +++++++++++-------- 1 file changed, 12 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 ab61d3fb45bbc..a6ac19d6bfa84 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 @@ -759,8 +759,10 @@ public void close() { try { redundancyThread.interrupt(); blockReportThread.interrupt(); + markedDeleteBlockScrubberThread.interrupt(); redundancyThread.join(3000); blockReportThread.join(3000); + markedDeleteBlockScrubberThread.join(3000); } catch (InterruptedException ie) { } datanodeManager.close(); @@ -4940,8 +4942,6 @@ private class MarkedDeleteBlockScrubber implements Runnable { private boolean isSleep; private void remove(long time) { - // Reentrant write lock, Release the lock when the remove is - // complete if (checkToDeleteIterator()) { namesystem.writeLock(); try { @@ -4965,24 +4965,28 @@ private boolean checkToDeleteIterator() { @Override public void run() { LOG.info("Start MarkedDeleteBlockScrubber thread"); - while (namesystem.isRunning()) { + while (namesystem.isRunning() && + !Thread.currentThread().isInterrupted()) { if (!markedDeleteQueue.isEmpty() || checkToDeleteIterator()) { - namesystem.writeLock(); try { NameNodeMetrics metrics = NameNode.getNameNodeMetrics(); metrics.setDeleteBlocksQueued(markedDeleteQueue.size()); isSleep = false; long startTime = Time.monotonicNow(); remove(startTime); - while (!isSleep && !markedDeleteQueue.isEmpty()) { + while (!isSleep && !markedDeleteQueue.isEmpty() && + !Thread.currentThread().isInterrupted()) { List markedDeleteList = markedDeleteQueue.poll(); if (markedDeleteList != null) { toDeleteIterator = markedDeleteList.listIterator(); } remove(startTime); } - } finally { - namesystem.writeUnlock(); + } catch (Exception e){ + LOG.warn("MarkedDeleteBlockScrubber encountered an exception" + + " during the block deletion process, " + + " the deletion of the block will retry in {} millisecond.", + deleteBlockUnlockIntervalTimeMs, e); } } if (isSleep) { @@ -4993,7 +4997,7 @@ public void run() { Thread.sleep(deleteBlockUnlockIntervalTimeMs); } catch (InterruptedException e) { LOG.info("Stopping MarkedDeleteBlockScrubber."); - Thread.currentThread().interrupt(); + break; } } } From de7524b5cbf3159959eed990a31e82c8ef337464 Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43412979@qq.com> Date: Fri, 7 Jan 2022 15:57:39 +0800 Subject: [PATCH 7/9] fix log level --- .../hadoop/hdfs/server/blockmanagement/BlockManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 a6ac19d6bfa84..73fc62a4e536d 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 @@ -4990,8 +4990,8 @@ public void run() { } } if (isSleep) { - LOG.info("Clear markedDeleteQueue over " + deleteBlockLockTimeMs - + " millisecond to release the write lock"); + LOG.debug("Clear markedDeleteQueue over {}" + + " millisecond to release the write lock", deleteBlockLockTimeMs); } try { Thread.sleep(deleteBlockUnlockIntervalTimeMs); From 0d1f846778d58a915b96a05494e35e81d8ea966c Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43412979@qq.com> Date: Wed, 12 Jan 2022 19:47:21 +0800 Subject: [PATCH 8/9] add pendingDeleteBlocksCount metrics and remove removeBlocks func --- .../server/blockmanagement/BlockManager.java | 11 +++++- .../hdfs/server/namenode/FSNamesystem.java | 39 ++++--------------- .../namenode/metrics/NameNodeMetrics.java | 10 +++++ 3 files changed, 27 insertions(+), 33 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 73fc62a4e536d..4b0d933963feb 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 @@ -4940,6 +4940,7 @@ public long getLastRedundancyMonitorTS() { private class MarkedDeleteBlockScrubber implements Runnable { private Iterator toDeleteIterator = null; private boolean isSleep; + private NameNodeMetrics metrics; private void remove(long time) { if (checkToDeleteIterator()) { @@ -4947,6 +4948,7 @@ private void remove(long time) { try { while (toDeleteIterator.hasNext()) { removeBlock(toDeleteIterator.next()); + metrics.decrPendingDeleteBlocksCount(); if (Time.monotonicNow() - time > deleteBlockLockTimeMs) { isSleep = true; break; @@ -4969,7 +4971,7 @@ public void run() { !Thread.currentThread().isInterrupted()) { if (!markedDeleteQueue.isEmpty() || checkToDeleteIterator()) { try { - NameNodeMetrics metrics = NameNode.getNameNodeMetrics(); + metrics = NameNode.getNameNodeMetrics(); metrics.setDeleteBlocksQueued(markedDeleteQueue.size()); isSleep = false; long startTime = Time.monotonicNow(); @@ -5351,10 +5353,17 @@ public BlockIdManager getBlockIdManager() { return blockIdManager; } + @VisibleForTesting public ConcurrentLinkedQueue> getMarkedDeleteQueue() { return markedDeleteQueue; } + public void addBLocksToMarkedDeleteQueue(List blockInfos) { + markedDeleteQueue.add(blockInfos); + NameNode.getNameNodeMetrics(). + incrPendingDeleteBlocksCount(blockInfos.size()); + } + public long nextGenerationStamp(boolean legacyBlock) throws IOException { return blockIdManager.nextGenerationStamp(legacyBlock); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 425e596dace14..a61028bde7c78 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -98,7 +98,6 @@ import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_SNAPSHOT_DIFF_LISTING_LIMIT_DEFAULT; import static org.apache.hadoop.hdfs.DFSUtil.isParentEntry; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicLong; import org.apache.commons.text.CaseUtils; @@ -2374,7 +2373,7 @@ boolean truncate(String src, long newLength, String clientName, } getEditLog().logSync(); if (!toRemoveBlocks.getToDeleteList().isEmpty()) { - blockManager.getMarkedDeleteQueue().add( + blockManager.addBLocksToMarkedDeleteQueue( toRemoveBlocks.getToDeleteList()); } logAuditEvent(true, operationName, src, null, status); @@ -2822,7 +2821,7 @@ private HdfsFileStatus startFileInt(String src, if (!skipSync) { getEditLog().logSync(); if (toRemoveBlocks != null) { - blockManager.getMarkedDeleteQueue().add( + blockManager.addBLocksToMarkedDeleteQueue( toRemoveBlocks.getToDeleteList()); } } @@ -3346,7 +3345,7 @@ void renameTo(final String src, final String dst, assert res != null; BlocksMapUpdateInfo collectedBlocks = res.collectedBlocks; if (!collectedBlocks.getToDeleteList().isEmpty()) { - blockManager.getMarkedDeleteQueue().add( + blockManager.addBLocksToMarkedDeleteQueue( collectedBlocks.getToDeleteList()); } @@ -3386,7 +3385,7 @@ boolean delete(String src, boolean recursive, boolean logRetryCache) getEditLog().logSync(); logAuditEvent(ret, operationName, src); if (toRemovedBlocks != null) { - blockManager.getMarkedDeleteQueue().add( + blockManager.addBLocksToMarkedDeleteQueue( toRemovedBlocks.getToDeleteList()); } return ret; @@ -3397,30 +3396,6 @@ FSPermissionChecker getPermissionChecker() return dir.getPermissionChecker(); } - /** - * From the given list, incrementally remove the blocks from blockManager - * Writelock is dropped and reacquired every blockDeletionIncrement to - * ensure that other waiters on the lock can get in. See HDFS-2938 - * - * @param blocks - * An instance of {@link BlocksMapUpdateInfo} which contains a list - * of blocks that need to be removed from blocksMap - */ - void removeBlocks(BlocksMapUpdateInfo blocks) { - List toDeleteList = blocks.getToDeleteList(); - Iterator iter = toDeleteList.iterator(); - while (iter.hasNext()) { - writeLock(); - try { - for (int i = 0; i < blockDeletionIncrement && iter.hasNext(); i++) { - blockManager.removeBlock(iter.next()); - } - } finally { - writeUnlock("removeBlocks"); - } - } - } - /** * Remove leases and inodes related to a given path * @param removedUCFiles INodes whose leases need to be released @@ -4629,7 +4604,7 @@ private void clearCorruptLazyPersistFiles() INodesInPath.fromINode((INodeFile) bc), false); changed |= toRemoveBlocks != null; if (toRemoveBlocks != null) { - blockManager.getMarkedDeleteQueue().add( + blockManager.addBLocksToMarkedDeleteQueue( toRemoveBlocks.getToDeleteList()); } } @@ -7341,7 +7316,7 @@ void deleteSnapshot(String snapshotRoot, String snapshotName, // Breaking the pattern as removing blocks have to happen outside of the // global lock if (blocksToBeDeleted != null) { - blockManager.getMarkedDeleteQueue().add( + blockManager.addBLocksToMarkedDeleteQueue( blocksToBeDeleted.getToDeleteList()); } logAuditEvent(true, operationName, rootPath, null, null); @@ -7368,7 +7343,7 @@ public void gcDeletedSnapshot(String snapshotRoot, String snapshotName) } finally { writeUnlock(operationName, getLockReportInfoSupplier(rootPath)); } - blockManager.getMarkedDeleteQueue().add( + blockManager.addBLocksToMarkedDeleteQueue( blocksToBeDeleted.getToDeleteList()); } 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 1d67a6e5da445..f0cf00238b5ee 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 @@ -91,6 +91,8 @@ public class NameNodeMetrics { MutableGaugeInt pendingEditsCount; @Metric("Number of delete blocks Queued") MutableGaugeInt deleteBlocksQueued; + @Metric("Number of pending deletion blocks") + MutableGaugeInt pendingDeleteBlocksCount; @Metric("Number of file system operations") public long totalFileOps(){ @@ -347,6 +349,14 @@ public void setDeleteBlocksQueued(int size) { deleteBlocksQueued.set(size); } + public void incrPendingDeleteBlocksCount(int size) { + pendingDeleteBlocksCount.incr(size); + } + + public void decrPendingDeleteBlocksCount() { + pendingDeleteBlocksCount.decr(); + } + public void addBlockOpsBatched(int count) { blockOpsBatched.incr(count); } From f6f2793310eff7c0678d027c912c95dcc3482972 Mon Sep 17 00:00:00 2001 From: zhuxiangyi <43412979@qq.com> Date: Thu, 13 Jan 2022 12:29:54 +0800 Subject: [PATCH 9/9] fix the checkstyle --- .../server/blockmanagement/BlockManager.java | 2 -- .../hdfs/server/namenode/TestFileTruncate.java | 16 +--------------- 2 files changed, 1 insertion(+), 17 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 4b0d933963feb..2fbda6fa187f2 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 @@ -491,9 +491,7 @@ public BlockManager(final Namesystem namesystem, boolean haEnabled, datanodeManager.getBlockInvalidateLimit(), startupDelayBlockDeletionInMs, blockIdManager); - markedDeleteQueue = new ConcurrentLinkedQueue<>(); - // Compute the map capacity by allocating 2% of total memory blocksMap = new BlocksMap( LightWeightGSet.computeCapacity(2.0, "BlocksMap")); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java index a80fc136e5dc0..13bc00f0944e3 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java @@ -335,7 +335,7 @@ public void testSnapshotWithAppendTruncate() * Delete snapshots in the specified order and verify that * remaining snapshots are still readable. */ - void testSnapshotWithAppendTruncate(int ... deleteOrder) + void testSnapshotWithAppendTruncate(int... deleteOrder) throws IOException, InterruptedException { FSDirectory fsDir = cluster.getNamesystem().getFSDirectory(); fs.mkdirs(parent); @@ -391,11 +391,9 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) assertFileLength(snapshotFiles[1], length[1]); assertFileLength(snapshotFiles[0], length[0]); assertBlockNotPresent(appendedBlk); - // Diskspace consumed should be 16 bytes * 3. [blk 1,2,3 SS:4] contentSummary = fs.getContentSummary(parent); assertThat(contentSummary.getSpaceConsumed(), is(48L)); - // Truncate full block again newLength = length[0] - BLOCK_SIZE / 2; isReady = fs.truncate(src, newLength); @@ -403,11 +401,9 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) assertFileLength(snapshotFiles[2], length[2]); assertFileLength(snapshotFiles[1], length[1]); assertFileLength(snapshotFiles[0], length[0]); - // Diskspace consumed should be 16 bytes * 3. [blk 1,2 SS:3,4] contentSummary = fs.getContentSummary(parent); assertThat(contentSummary.getSpaceConsumed(), is(48L)); - // Truncate half of the last block newLength -= BLOCK_SIZE / 2; isReady = fs.truncate(src, newLength); @@ -418,15 +414,12 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) assertFileLength(snapshotFiles[0], length[0]); Block replacedBlk = getLocatedBlocks(src).getLastLocatedBlock() .getBlock().getLocalBlock(); - // Diskspace consumed should be 16 bytes * 3. [blk 1,6 SS:2,3,4] contentSummary = fs.getContentSummary(parent); assertThat(contentSummary.getSpaceConsumed(), is(54L)); - snapshotDir = fs.createSnapshot(parent, ss[3]); snapshotFiles[3] = new Path(snapshotDir, truncateFile); length[3] = newLength; - // Delete file. Should still be able to read snapshots int numINodes = fsDir.getInodeMapSize(); isReady = fs.delete(src, false); @@ -437,18 +430,15 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) assertFileLength(snapshotFiles[0], length[0]); assertEquals("Number of INodes should not change", numINodes, fsDir.getInodeMapSize()); - fs.deleteSnapshot(parent, ss[3]); BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( cluster.getNamesystem(0).getBlockManager()); assertBlockExists(firstBlk); assertBlockExists(lastBlk); assertBlockNotPresent(replacedBlk); - // Diskspace consumed should be 16 bytes * 3. [SS:1,2,3,4] contentSummary = fs.getContentSummary(parent); assertThat(contentSummary.getSpaceConsumed(), is(48L)); - // delete snapshots in the specified order fs.deleteSnapshot(parent, ss[deleteOrder[0]]); assertFileLength(snapshotFiles[deleteOrder[1]], length[deleteOrder[1]]); @@ -457,11 +447,9 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) assertBlockExists(lastBlk); assertEquals("Number of INodes should not change", numINodes, fsDir.getInodeMapSize()); - // Diskspace consumed should be 16 bytes * 3. [SS:1,2,3,4] contentSummary = fs.getContentSummary(parent); assertThat(contentSummary.getSpaceConsumed(), is(48L)); - fs.deleteSnapshot(parent, ss[deleteOrder[1]]); BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( cluster.getNamesystem(0).getBlockManager()); @@ -478,13 +466,11 @@ void testSnapshotWithAppendTruncate(int ... deleteOrder) } assertEquals("Number of INodes should not change", numINodes, fsDir .getInodeMapSize()); - fs.deleteSnapshot(parent, ss[deleteOrder[2]]); BlockManagerTestUtil.waitForMarkedDeleteQueueIsEmpty( cluster.getNamesystem(0).getBlockManager()); assertBlockNotPresent(firstBlk); assertBlockNotPresent(lastBlk); - // Diskspace consumed should be 0 bytes * 3. [] contentSummary = fs.getContentSummary(parent); assertThat(contentSummary.getSpaceConsumed(), is(0L));