From 57ab8544921d925a5982bc20e05d8146c266fd75 Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Tue, 8 Oct 2019 14:14:14 +0530 Subject: [PATCH] HDFS-14859. Prevent unnecessary evaluation of costly operation getNumLiveDataNodes when dfs.namenode.safemode.min.datanodes is not zero. Contributed by Srinivasu Majeti. --- .../blockmanagement/BlockManagerSafeMode.java | 16 ++-- .../TestBlockManagerSafeMode.java | 86 +++++++++++++++++++ 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerSafeMode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerSafeMode.java index 3f59b64d0cd..7998fd7f098 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerSafeMode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerSafeMode.java @@ -573,12 +573,18 @@ class BlockManagerSafeMode { assert namesystem.hasWriteLock(); // Calculating the number of live datanodes is time-consuming // in large clusters. Skip it when datanodeThreshold is zero. - int datanodeNum = 0; - if (datanodeThreshold > 0) { - datanodeNum = blockManager.getDatanodeManager().getNumLiveDataNodes(); - } + // We need to evaluate getNumLiveDataNodes only when + // (blockSafe >= blockThreshold) is true and hence moving evaluation + // of datanodeNum conditional to isBlockThresholdMet as well synchronized (this) { - return blockSafe >= blockThreshold && datanodeNum >= datanodeThreshold; + boolean isBlockThresholdMet = (blockSafe >= blockThreshold); + boolean isDatanodeThresholdMet = true; + if (isBlockThresholdMet && datanodeThreshold > 0) { + int datanodeNum = blockManager.getDatanodeManager(). + getNumLiveDataNodes(); + isDatanodeThresholdMet = (datanodeNum >= datanodeThreshold); + } + return isBlockThresholdMet && isDatanodeThresholdMet; } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManagerSafeMode.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManagerSafeMode.java index 7be900baa40..5317d679109 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManagerSafeMode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManagerSafeMode.java @@ -377,6 +377,87 @@ public class TestBlockManagerSafeMode { assertFalse(bmSafeMode.isInSafeMode()); } + /** + * Test block manager won't leave safe mode if datanode threshold is not met + * only if datanodeThreshold is configured > 0. + */ + @Test(timeout = 30000) + public void testDatanodeThreshodShouldBeMetOnlyIfConfigured() + throws Exception { + bmSafeMode.activate(BLOCK_TOTAL); + + //Blocks received is set to threshold + setBlockSafe(BLOCK_THRESHOLD); + + //datanodeThreshold is configured to 1 but not all DNs registered . + // Expecting safe mode . + when(dn.getNumLiveDataNodes()).thenReturn(1); + setDatanodeThreshold(1); + bmSafeMode.checkSafeMode(); + assertTrue(bmSafeMode.isInSafeMode()); + + //datanodeThreshold is configured to 1 and all DNs registered . + // Not expecting safe mode . + when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM); + setDatanodeThreshold(1); + bmSafeMode.checkSafeMode(); + waitForExtensionPeriod(); + assertFalse(bmSafeMode.isInSafeMode()); + + //datanodeThreshold is configured to 0 but not all DNs registered . + // Not Expecting safe mode . + bmSafeMode.activate(BLOCK_TOTAL); + setBlockSafe(BLOCK_THRESHOLD); + when(dn.getNumLiveDataNodes()).thenReturn(1); + setDatanodeThreshold(0); + bmSafeMode.checkSafeMode(); + assertFalse(bmSafeMode.isInSafeMode()); + + //datanodeThreshold is configured to 0 and all DNs registered . + // Not Expecting safe mode . + bmSafeMode.activate(BLOCK_TOTAL); + setBlockSafe(BLOCK_THRESHOLD); + when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM); + setDatanodeThreshold(0); + bmSafeMode.checkSafeMode(); + assertFalse(bmSafeMode.isInSafeMode()); + + //Blocks received set to below threshold and all combinations + //of datanodeThreshold should result in safe mode. + + + //datanodeThreshold is configured to 1 but not all DNs registered . + // Expecting safe mode . + bmSafeMode.activate(BLOCK_TOTAL); + setBlockSafe(BLOCK_THRESHOLD-1); + setSafeModeStatus(BMSafeModeStatus.PENDING_THRESHOLD); + when(dn.getNumLiveDataNodes()).thenReturn(1); + setDatanodeThreshold(1); + bmSafeMode.checkSafeMode(); + assertTrue(bmSafeMode.isInSafeMode()); + + //datanodeThreshold is configured to 1 and all DNs registered . + // Expecting safe mode . + when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM); + setDatanodeThreshold(1); + bmSafeMode.checkSafeMode(); + assertTrue(bmSafeMode.isInSafeMode()); + + //datanodeThreshold is configured to 0 but not all DNs registered . + // Expecting safe mode . + when(dn.getNumLiveDataNodes()).thenReturn(1); + setDatanodeThreshold(0); + bmSafeMode.checkSafeMode(); + assertTrue(bmSafeMode.isInSafeMode()); + + //datanodeThreshold is configured to 0 and all DNs registered . + // Expecting safe mode . + when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM); + setDatanodeThreshold(0); + bmSafeMode.checkSafeMode(); + assertTrue(bmSafeMode.isInSafeMode()); + } + /** * Test block manager won't leave safe mode if there are blocks with * generation stamp (GS) in future. @@ -604,6 +685,11 @@ public class TestBlockManagerSafeMode { Whitebox.setInternalState(bmSafeMode, "blockSafe", blockSafe); } + private void setDatanodeThreshold(int dnSafeModeThreshold) { + Whitebox.setInternalState(bmSafeMode, "datanodeThreshold", + dnSafeModeThreshold); + } + private long getblockSafe() { return (long)Whitebox.getInternalState(bmSafeMode, "blockSafe"); }