Repository: hadoop Updated Branches: refs/heads/branch-2 c6b9c8d69 -> fd9a7f84b
HDFS-9313. Possible NullPointerException in BlockManager if no excess replica can be chosen. (mingma) (cherry picked from commit d565480da2f646b40c3180e1ccb2935c9863dfef) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/fd9a7f84 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/fd9a7f84 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/fd9a7f84 Branch: refs/heads/branch-2 Commit: fd9a7f84bcf6664443568a16df1d1101393e6901 Parents: c6b9c8d Author: Ming Ma <min...@apache.org> Authored: Mon Nov 2 19:36:37 2015 -0800 Committer: Ming Ma <min...@apache.org> Committed: Mon Nov 2 19:37:38 2015 -0800 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../blockmanagement/BlockPlacementPolicy.java | 8 +++-- .../BlockPlacementPolicyDefault.java | 6 ++++ .../blockmanagement/TestReplicationPolicy.java | 31 ++++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/fd9a7f84/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 6da1264..8d2ddf5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -1373,6 +1373,9 @@ Release 2.8.0 - UNRELEASED HDFS-9329. TestBootstrapStandby#testRateThrottling is flaky because fsimage size is smaller than IO buffer size. (zhz) + HDFS-9313. Possible NullPointerException in BlockManager if no excess + replica can be chosen. (mingma) + Release 2.7.2 - UNRELEASED INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/fd9a7f84/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java index 69a49aa..e75efa0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java @@ -23,8 +23,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.StorageType; @@ -35,13 +33,17 @@ import org.apache.hadoop.net.NetworkTopology; import org.apache.hadoop.net.Node; import org.apache.hadoop.util.ReflectionUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * This interface is used for choosing the desired number of targets * for placing block replicas. */ @InterfaceAudience.Private public abstract class BlockPlacementPolicy { - static final Log LOG = LogFactory.getLog(BlockPlacementPolicy.class); + static final Logger LOG = LoggerFactory.getLogger( + BlockPlacementPolicy.class); @InterfaceAudience.Private public static class NotEnoughReplicasException extends Exception { http://git-wip-us.apache.org/repos/asf/hadoop/blob/fd9a7f84/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java index d9b8d60..2723ed9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java @@ -981,6 +981,12 @@ public class BlockPlacementPolicyDefault extends BlockPlacementPolicy { excessTypes); } firstOne = false; + if (cur == null) { + LOG.warn("No excess replica can be found. excessTypes: {}." + + " moreThanOne: {}. exactlyOne: {}.", excessTypes, moreThanOne, + exactlyOne); + break; + } // adjust rackmap, moreThanOne, and exactlyOne adjustSetsWithChosenReplica(rackMap, moreThanOne, exactlyOne, cur); http://git-wip-us.apache.org/repos/asf/hadoop/blob/fd9a7f84/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java index e321864..60aa9ef 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java @@ -1000,6 +1000,14 @@ public class TestReplicationPolicy extends BaseReplicationPolicyTest { BlockStoragePolicySuite POLICY_SUITE = BlockStoragePolicySuite .createDefaultSuite(); BlockStoragePolicy storagePolicy = POLICY_SUITE.getDefaultPolicy(); + DatanodeStorageInfo excessSSD = DFSTestUtil.createDatanodeStorageInfo( + "Storage-excess-SSD-ID", "localhost", + storages[0].getDatanodeDescriptor().getNetworkLocation(), + "foo.com", StorageType.SSD); + updateHeartbeatWithUsage(excessSSD.getDatanodeDescriptor(), + 2* HdfsServerConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, + 2* HdfsServerConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 0L, 0L, 0, + 0); // use delete hint case. @@ -1022,6 +1030,29 @@ public class TestReplicationPolicy extends BaseReplicationPolicyTest { excessReplicas = replicator.chooseReplicasToDelete(nonExcess, 3, excessTypes, storages[3].getDatanodeDescriptor(), null); assertTrue(excessReplicas.contains(excessStorage)); + + + // The block was initially created on excessSSD(rack r1), + // storages[4](rack r3) and storages[5](rack r3) with + // ONESSD_STORAGE_POLICY_NAME storage policy. + // Right after balancer moves the block from storages[5] to + // storages[3](rack r2), the application changes the storage policy from + // ONESSD_STORAGE_POLICY_NAME to HOT_STORAGE_POLICY_ID. In this case, + // no replica can be chosen as the excessive replica as + // chooseReplicasToDelete only considers storages[4] and storages[5] that + // are the same rack. But neither's storage type is SSD. + // TODO BlockPlacementPolicyDefault should be able to delete excessSSD. + nonExcess.clear(); + nonExcess.add(excessSSD); + nonExcess.add(storages[3]); + nonExcess.add(storages[4]); + nonExcess.add(storages[5]); + excessTypes = storagePolicy.chooseExcess((short) 3, + DatanodeStorageInfo.toStorageTypes(nonExcess)); + excessReplicas = replicator.chooseReplicasToDelete(nonExcess, 3, + excessTypes, storages[3].getDatanodeDescriptor(), + storages[5].getDatanodeDescriptor()); + assertTrue(excessReplicas.size() == 0); } @Test