From 9c78f870d8e926684516f629292451784de3dd16 Mon Sep 17 00:00:00 2001 From: Jing Zhao Date: Fri, 25 Oct 2013 18:46:57 +0000 Subject: [PATCH] HDFS-5257. Merge change r1535811 from trunk. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1535813 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../apache/hadoop/hdfs/DFSOutputStream.java | 5 ++++ .../hdfs/server/namenode/FSNamesystem.java | 19 ++++++++---- .../server/namenode/TestAddBlockRetry.java | 30 +++++++++++++++++++ 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 3ac2f2d8dff..fa44a8f140a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -140,6 +140,9 @@ Release 2.3.0 - UNRELEASED HDFS-5400. DFS_CLIENT_MMAP_CACHE_THREAD_RUNS_PER_TIMEOUT constant is set to the wrong value. (Colin Patrick McCabe) + HDFS-5257. addBlock() retry should return LocatedBlock with locations else client + will get AIOBE. (Vinay via jing9) + Release 2.2.1 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java index ec5c5396aed..78f150eb6d1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java @@ -1102,6 +1102,11 @@ private DatanodeInfo[] nextBlockOutputStream() throws IOException { // private boolean createBlockOutputStream(DatanodeInfo[] nodes, long newGS, boolean recoveryFlag) { + if (nodes.length == 0) { + DFSClient.LOG.info("nodes are empty for write pipeline of block " + + block); + return false; + } Status pipelineStatus = SUCCESS; String firstBadLink = ""; if (DFSClient.LOG.isDebugEnabled()) { 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 eedefdbd621..92843e20154 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 @@ -2490,8 +2490,8 @@ LocatedBlock getAdditionalBlock(String src, long fileId, String clientName, final INodeFileUnderConstruction pendingFile = (INodeFileUnderConstruction) inodes[inodes.length - 1]; - if(onRetryBlock[0] != null) { - // This is a retry. Just return the last block. + if (onRetryBlock[0] != null && onRetryBlock[0].getLocations().length > 0) { + // This is a retry. Just return the last block if having locations. return onRetryBlock[0]; } if (pendingFile.getBlocks().length >= maxBlocksPerFile) { @@ -2528,9 +2528,18 @@ LocatedBlock getAdditionalBlock(String src, long fileId, String clientName, final INodeFileUnderConstruction pendingFile = (INodeFileUnderConstruction) inodes[inodes.length - 1]; - if(onRetryBlock[0] != null) { - // This is a retry. Just return the last block. - return onRetryBlock[0]; + if (onRetryBlock[0] != null) { + if (onRetryBlock[0].getLocations().length > 0) { + // This is a retry. Just return the last block if having locations. + return onRetryBlock[0]; + } else { + // add new chosen targets to already allocated block and return + BlockInfo lastBlockInFile = pendingFile.getLastBlock(); + ((BlockInfoUnderConstruction) lastBlockInFile) + .setExpectedLocations(targets); + offset = pendingFile.computeFileSize(); + return makeLocatedBlock(lastBlockInFile, targets, offset); + } } // commit the last block and complete it if it has minimum replicas diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java index 399dfc210d3..3114c824cea 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.spy; @@ -139,4 +140,33 @@ public DatanodeDescriptor[] answer(InvocationOnMock invocation) assertEquals("Wrong replication", REPLICATION, lb1.getLocations().length); assertEquals("Blocks are not equal", lb1.getBlock(), lb2.getBlock()); } + + /* + * Since NameNode will not persist any locations of the block, addBlock() + * retry call after restart NN should re-select the locations and return to + * client. refer HDFS-5257 + */ + @Test + public void testAddBlockRetryShouldReturnBlockWithLocations() + throws Exception { + final String src = "/testAddBlockRetryShouldReturnBlockWithLocations"; + NamenodeProtocols nameNodeRpc = cluster.getNameNodeRpc(); + // create file + nameNodeRpc.create(src, FsPermission.getFileDefault(), "clientName", + new EnumSetWritable(EnumSet.of(CreateFlag.CREATE)), true, + (short) 3, 1024); + // start first addBlock() + LOG.info("Starting first addBlock for " + src); + LocatedBlock lb1 = nameNodeRpc.addBlock(src, "clientName", null, null, + INodeId.GRANDFATHER_INODE_ID, null); + assertTrue("Block locations should be present", + lb1.getLocations().length > 0); + + cluster.restartNameNode(); + nameNodeRpc = cluster.getNameNodeRpc(); + LocatedBlock lb2 = nameNodeRpc.addBlock(src, "clientName", null, null, + INodeId.GRANDFATHER_INODE_ID, null); + assertEquals("Blocks are not equal", lb1.getBlock(), lb2.getBlock()); + assertTrue("Wrong locations with retry", lb2.getLocations().length > 0); + } }