diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java index e19fbeb956..5411b5c8f4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java @@ -239,7 +239,7 @@ public class PendingMove { private DDatanode proxySource; private StorageGroup target; - private PendingMove(Source source, StorageGroup target) { + PendingMove(Source source, StorageGroup target) { this.source = source; this.target = target; } @@ -280,12 +280,18 @@ private boolean chooseBlockAndProxy() { /** * @return true if the given block is good for the tentative move. */ - private boolean markMovedIfGoodBlock(DBlock block, StorageType targetStorageType) { + boolean markMovedIfGoodBlock(DBlock block, StorageType targetStorageType) { synchronized (block) { synchronized (movedBlocks) { if (isGoodBlockCandidate(source, target, targetStorageType, block)) { if (block instanceof DBlockStriped) { reportedBlock = ((DBlockStriped) block).getInternalBlock(source); + if (reportedBlock == null) { + LOG.info( + "No striped internal block on source {}, block {}. Skipping.", + source, block); + return false; + } } else { reportedBlock = block; } @@ -497,7 +503,7 @@ public DBlockStriped(Block block, byte[] indices, short dataBlockNum, this.cellSize = cellSize; } - public DBlock getInternalBlock(StorageGroup storage) { + DBlock getInternalBlock(StorageGroup storage) { int idxInLocs = locations.indexOf(storage); if (idxInLocs == -1) { return null; @@ -516,7 +522,11 @@ public DBlock getInternalBlock(StorageGroup storage) { @Override public long getNumBytes(StorageGroup storage) { - return getInternalBlock(storage).getNumBytes(); + DBlock block = getInternalBlock(storage); + if (block == null) { + return 0; + } + return block.getNumBytes(); } } @@ -1305,7 +1315,7 @@ public static boolean checkForSuccess( * 2. the block does not have a replica/internalBlock on the target; * 3. doing the move does not reduce the number of racks that the block has */ - private boolean isGoodBlockCandidate(StorageGroup source, StorageGroup target, + boolean isGoodBlockCandidate(StorageGroup source, StorageGroup target, StorageType targetStorageType, DBlock block) { if (source.equals(target)) { return false; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java index f253b1ea87..3c624cde7b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java @@ -41,11 +41,16 @@ import org.apache.hadoop.hdfs.protocol.ErasureCodingPolicy; import org.junit.AfterClass; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyLong; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; import java.io.File; import java.io.IOException; @@ -1614,11 +1619,39 @@ private void doTestBalancerWithStripedFile(Configuration conf) throws Exception // verify locations of striped blocks locatedBlocks = client.getBlockLocations(fileName, 0, fileLen); StripedFileTestUtil.verifyLocatedStripedBlocks(locatedBlocks, groupSize); + + // Test handling NPE with striped blocks + testNullStripedBlocks(conf); + } finally { cluster.shutdown(); } } + private void testNullStripedBlocks(Configuration conf) throws IOException { + NameNodeConnector nnc = NameNodeConnector.newNameNodeConnectors( + DFSUtil.getInternalNsRpcUris(conf), + Balancer.class.getSimpleName(), Balancer.BALANCER_ID_PATH, conf, + BalancerParameters.DEFAULT.getMaxIdleIteration()).get(0); + Dispatcher dispatcher = new Dispatcher(nnc, Collections.emptySet(), + Collections. emptySet(), 1, 1, 0, + 1, 1, conf); + Dispatcher spyDispatcher = spy(dispatcher); + Dispatcher.PendingMove move = spyDispatcher.new PendingMove( + mock(Dispatcher.Source.class), + mock(Dispatcher.DDatanode.StorageGroup.class)); + Dispatcher.DBlockStriped block = mock(Dispatcher.DBlockStriped.class); + + doReturn(null).when(block).getInternalBlock(any()); + doReturn(true) + .when(spyDispatcher) + .isGoodBlockCandidate(any(), any(), any(), any()); + + when(move.markMovedIfGoodBlock(block, DEFAULT)).thenCallRealMethod(); + + assertFalse(move.markMovedIfGoodBlock(block, DEFAULT)); + } + /** * Test Balancer runs fine when logging in with a keytab in kerberized env. * Reusing testUnknownDatanode here for basic functionality testing.