From 8ce48631034737a84c8853bc3bf6e35de5e6fb05 Mon Sep 17 00:00:00 2001 From: Ming Ma Date: Thu, 2 Mar 2017 11:06:48 -0800 Subject: [PATCH] HDFS-11412. Maintenance minimum replication config value allowable range should be [0, DefaultReplication]. (Manoj Govindassamy via mingma) --- .../server/blockmanagement/BlockManager.java | 15 ++- .../apache/hadoop/hdfs/MiniDFSCluster.java | 8 ++ .../hadoop/hdfs/TestMaintenanceState.java | 111 ++++++++++++++++++ 3 files changed, 129 insertions(+), 5 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 b06996a10fb..5d868e5545d 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 @@ -436,12 +436,12 @@ public class BlockManager implements BlockStatsMXBean { + DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_KEY + " = " + minMaintenanceR + " < 0"); } - if (minMaintenanceR > minR) { + if (minMaintenanceR > defaultReplication) { throw new IOException("Unexpected configuration parameters: " + DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_KEY + " = " + minMaintenanceR + " > " - + DFSConfigKeys.DFS_NAMENODE_REPLICATION_MIN_KEY - + " = " + minR); + + DFSConfigKeys.DFS_REPLICATION_KEY + + " = " + defaultReplication); } this.minReplicationToBeInMaintenance = (short)minMaintenanceR; @@ -749,6 +749,11 @@ public class BlockManager implements BlockStatsMXBean { return minReplicationToBeInMaintenance; } + private short getMinMaintenanceStorageNum(BlockInfo block) { + return (short) Math.min(minReplicationToBeInMaintenance, + block.getReplication()); + } + /** * Commit a block of a file * @@ -3718,7 +3723,7 @@ public class BlockManager implements BlockStatsMXBean { boolean isNeededReplicationForMaintenance(BlockInfo storedBlock, NumberReplicas numberReplicas) { return storedBlock.isComplete() && (numberReplicas.liveReplicas() < - getMinReplicationToBeInMaintenance() || + getMinMaintenanceStorageNum(storedBlock) || !isPlacementPolicySatisfied(storedBlock)); } @@ -3744,7 +3749,7 @@ public class BlockManager implements BlockStatsMXBean { final short expectedRedundancy = getExpectedReplicaNum(block); return (short)Math.max(expectedRedundancy - numberReplicas.maintenanceReplicas(), - getMinReplicationToBeInMaintenance()); + getMinMaintenanceStorageNum(block)); } public short getExpectedReplicaNum(BlockInfo block) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java index 8a9b21374bb..d7b7692e52f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java @@ -807,6 +807,14 @@ public class MiniDFSCluster implements AutoCloseable { int replication = conf.getInt(DFS_REPLICATION_KEY, 3); conf.setInt(DFS_REPLICATION_KEY, Math.min(replication, numDataNodes)); + int maintenanceMinReplication = conf.getInt( + DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_KEY, + DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_DEFAULT); + if (maintenanceMinReplication == + DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_DEFAULT) { + conf.setInt(DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_KEY, + Math.min(maintenanceMinReplication, numDataNodes)); + } int safemodeExtension = conf.getInt( DFS_NAMENODE_SAFEMODE_EXTENSION_TESTING_KEY, 0); conf.setInt(DFS_NAMENODE_SAFEMODE_EXTENSION_KEY, safemodeExtension); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestMaintenanceState.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestMaintenanceState.java index f3e2a0b65ef..24321533d13 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestMaintenanceState.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestMaintenanceState.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hdfs; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.ArrayList; @@ -30,6 +31,7 @@ import java.util.List; import java.util.Map; import com.google.common.collect.Lists; +import org.junit.Assert; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.fs.FileSystem; @@ -66,6 +68,38 @@ public class TestMaintenanceState extends AdminStatesBaseTest { minMaintenanceR); } + /** + * Test valid value range for the config namenode.maintenance.replication.min. + */ + @Test (timeout = 60000) + public void testMaintenanceMinReplConfigRange() { + LOG.info("Setting testMaintenanceMinReplConfigRange"); + + // Case 1: Maintenance min replication less allowed minimum 0 + setMinMaintenanceR(-1); + try { + startCluster(1, 1); + fail("Cluster start should fail when 'dfs.namenode.maintenance" + + ".replication.min=-1'"); + } catch (IOException e) { + LOG.info("Expected exception: " + e); + } + + // Case 2: Maintenance min replication greater + // allowed max of DFSConfigKeys.DFS_REPLICATION_KEY + int defaultRepl = getConf().getInt( + DFSConfigKeys.DFS_REPLICATION_KEY, + DFSConfigKeys.DFS_REPLICATION_DEFAULT); + setMinMaintenanceR(defaultRepl + 1); + try { + startCluster(1, 1); + fail("Cluster start should fail when 'dfs.namenode.maintenance" + + ".replication.min > " + defaultRepl + "'"); + } catch (IOException e) { + LOG.info("Expected exception: " + e); + } + } + /** * Verify a node can transition from AdminStates.ENTERING_MAINTENANCE to * AdminStates.NORMAL. @@ -406,6 +440,83 @@ public class TestMaintenanceState extends AdminStatesBaseTest { cleanupFile(fileSys, file); } + /** + * Test file block replication lesser than maintenance minimum. + */ + @Test(timeout = 360000) + public void testFileBlockReplicationAffectingMaintenance() + throws Exception { + int defaultReplication = getConf().getInt(DFSConfigKeys + .DFS_REPLICATION_KEY, DFSConfigKeys.DFS_REPLICATION_DEFAULT); + int defaultMaintenanceMinRepl = getConf().getInt(DFSConfigKeys + .DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_KEY, + DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_DEFAULT); + + // Case 1: + // * Maintenance min larger than default min replication + // * File block replication larger than maintenance min + // * Initial data nodes not sufficient to remove all maintenance nodes + // as file block replication is greater than maintenance min. + // * Data nodes added later for the state transition to progress + int maintenanceMinRepl = defaultMaintenanceMinRepl + 1; + int fileBlockReplication = maintenanceMinRepl + 1; + int numAddedDataNodes = 1; + int numInitialDataNodes = (maintenanceMinRepl * 2 - numAddedDataNodes); + Assert.assertTrue(maintenanceMinRepl <= defaultReplication); + testFileBlockReplicationImpl(maintenanceMinRepl, + numInitialDataNodes, numAddedDataNodes, fileBlockReplication); + + // Case 2: + // * Maintenance min larger than default min replication + // * File block replication lesser than maintenance min + // * Initial data nodes after removal of maintenance nodes is still + // sufficient for the file block replication. + // * No new data nodes to be added, still the state transition happens + maintenanceMinRepl = defaultMaintenanceMinRepl + 1; + fileBlockReplication = maintenanceMinRepl - 1; + numAddedDataNodes = 0; + numInitialDataNodes = (maintenanceMinRepl * 2 - numAddedDataNodes); + testFileBlockReplicationImpl(maintenanceMinRepl, + numInitialDataNodes, numAddedDataNodes, fileBlockReplication); + } + + private void testFileBlockReplicationImpl( + int maintenanceMinRepl, int numDataNodes, int numNewDataNodes, + int fileBlockRepl) + throws Exception { + setup(); + LOG.info("Starting testLargerMinMaintenanceReplication - maintMinRepl: " + + maintenanceMinRepl + ", numDNs: " + numDataNodes + ", numNewDNs: " + + numNewDataNodes + ", fileRepl: " + fileBlockRepl); + LOG.info("Setting maintenance minimum replication: " + maintenanceMinRepl); + setMinMaintenanceR(maintenanceMinRepl); + startCluster(1, numDataNodes); + + final Path file = new Path("/testLargerMinMaintenanceReplication.dat"); + + FileSystem fileSys = getCluster().getFileSystem(0); + writeFile(fileSys, file, fileBlockRepl, 1); + final DatanodeInfo[] nodes = getFirstBlockReplicasDatanodeInfos(fileSys, + file); + + ArrayList nodeUuids = new ArrayList<>(); + for (int i = 0; i < maintenanceMinRepl && i < nodes.length; i++) { + nodeUuids.add(nodes[i].getDatanodeUuid()); + } + + List maintenanceDNs = takeNodeOutofService(0, nodeUuids, + Long.MAX_VALUE, null, null, AdminStates.ENTERING_MAINTENANCE); + + for (int i = 0; i < numNewDataNodes; i++) { + getCluster().startDataNodes(getConf(), 1, true, null, null); + } + getCluster().waitActive(); + refreshNodes(0); + waitNodeState(maintenanceDNs, AdminStates.IN_MAINTENANCE); + cleanupFile(fileSys, file); + teardown(); + } + /** * Transition from IN_MAINTENANCE to DECOMMISSIONED. */