diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index a14a1d8db0..56a85f4604 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -2696,6 +2696,9 @@ Release 2.7.3 - UNRELEASED HDFS-9672. o.a.h.hdfs.TestLeaseRecovery2 fails intermittently (Mingliang Liu via jitendra) + HDFS-9690. ClientProtocol.addBlock is not idempotent after HDFS-8071. + (szetszwo) + Release 2.7.2 - 2016-01-25 INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java index 914fbd9de0..6ba8e1ca96 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java @@ -184,17 +184,16 @@ static ValidateAddBlockResult validateAddBlock( src = fsn.dir.resolvePath(pc, src, pathComponents); FileState fileState = analyzeFileState(fsn, src, fileId, clientName, previous, onRetryBlock); - final INodeFile pendingFile = fileState.inode; - // Check if the penultimate block is minimally replicated - if (!fsn.checkFileProgress(src, pendingFile, false)) { - throw new NotReplicatedYetException("Not replicated yet: " + src); - } - if (onRetryBlock[0] != null && onRetryBlock[0].getLocations().length > 0) { // This is a retry. No need to generate new locations. // Use the last block if it has locations. return null; } + + final INodeFile pendingFile = fileState.inode; + if (!fsn.checkFileProgress(src, pendingFile, false)) { + throw new NotReplicatedYetException("Not replicated yet: " + src); + } if (pendingFile.getBlocks().length >= fsn.maxBlocksPerFile) { throw new IOException("File has reached the limit on maximum number of" + " blocks (" + DFSConfigKeys.DFS_NAMENODE_MAX_BLOCKS_PER_FILE_KEY diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java index e41c06a3a6..1f783f6ab6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java @@ -432,19 +432,37 @@ public void testIdempotentAllocateBlockAndClose() throws Exception { // Make the call to addBlock() get called twice, as if it were retried // due to an IPC issue. doAnswer(new Answer() { + private int getBlockCount(LocatedBlock ret) throws IOException { + LocatedBlocks lb = cluster.getNameNodeRpc().getBlockLocations(src, 0, Long.MAX_VALUE); + assertEquals(lb.getLastLocatedBlock().getBlock(), ret.getBlock()); + return lb.getLocatedBlocks().size(); + } + @Override public LocatedBlock answer(InvocationOnMock invocation) throws Throwable { - LocatedBlock ret = (LocatedBlock) invocation.callRealMethod(); - LocatedBlocks lb = cluster.getNameNodeRpc().getBlockLocations(src, 0, Long.MAX_VALUE); - int blockCount = lb.getLocatedBlocks().size(); - assertEquals(lb.getLastLocatedBlock().getBlock(), ret.getBlock()); - + LOG.info("Called addBlock: " + + Arrays.toString(invocation.getArguments())); + + // call first time + // warp NotReplicatedYetException with RemoteException as rpc does. + final LocatedBlock ret; + try { + ret = (LocatedBlock) invocation.callRealMethod(); + } catch(NotReplicatedYetException e) { + throw new RemoteException(e.getClass().getName(), e.getMessage()); + } + final int blockCount = getBlockCount(ret); + // Retrying should result in a new block at the end of the file. // (abandoning the old one) - LocatedBlock ret2 = (LocatedBlock) invocation.callRealMethod(); - lb = cluster.getNameNodeRpc().getBlockLocations(src, 0, Long.MAX_VALUE); - int blockCount2 = lb.getLocatedBlocks().size(); - assertEquals(lb.getLastLocatedBlock().getBlock(), ret2.getBlock()); + // It should not have NotReplicatedYetException. + final LocatedBlock ret2; + try { + ret2 = (LocatedBlock) invocation.callRealMethod(); + } catch(NotReplicatedYetException e) { + throw new AssertionError("Unexpected exception", e); + } + final int blockCount2 = getBlockCount(ret2); // We shouldn't have gained an extra block by the RPC. assertEquals(blockCount, blockCount2);