Author: kihwal Date: Mon Dec 2 20:31:55 2013 New Revision: 1547181 URL: http://svn.apache.org/r1547181 Log: svn merge -c 1547173 merging from trunk to branch-0.23 to fix: HDFS-5557. Write pipeline recovery for the last packet in the block may cause rejection of valid replicas.
Modified: hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestClientProtocolForPipelineRecovery.java Modified: hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1547181&r1=1547180&r2=1547181&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt (original) +++ hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt Mon Dec 2 20:31:55 2013 @@ -62,6 +62,9 @@ Release 0.23.10 - UNRELEASED HDFS-5526. Datanode cannot roll back to previous layout version (kihwal) + HDFS-5557. Write pipeline recovery for the last packet in the block may + cause rejection of valid replicas. (kihwal) + Release 0.23.9 - 2013-07-08 INCOMPATIBLE CHANGES Modified: hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java?rev=1547181&r1=1547180&r2=1547181&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java Mon Dec 2 20:31:55 2013 @@ -796,7 +796,6 @@ class DFSOutputStream extends FSOutputSu // We also need to set lastAckedSeqno to the end-of-block Packet's seqno, so that // a client waiting on close() will be aware that the flush finished. synchronized (dataQueue) { - assert dataQueue.size() == 1; Packet endOfBlockPacket = dataQueue.remove(); // remove the end of block packet assert endOfBlockPacket.lastPacketInBlock; assert lastAckedSeqno == endOfBlockPacket.seqno - 1; @@ -981,7 +980,7 @@ class DFSOutputStream extends FSOutputSu // set up the pipeline again with the remaining nodes if (failPacket) { // for testing - success = createBlockOutputStream(nodes, newGS-1, isRecovery); + success = createBlockOutputStream(nodes, newGS, isRecovery); failPacket = false; try { // Give DNs time to send in bad reports. In real situations, Modified: hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java?rev=1547181&r1=1547180&r2=1547181&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java Mon Dec 2 20:31:55 2013 @@ -211,6 +211,8 @@ public class BlockInfoUnderConstruction * @param genStamp The final generation stamp for the block. */ public void setGenerationStampAndVerifyReplicas(long genStamp) { + // Set the generation stamp for the block. + setGenerationStamp(genStamp); if (replicas == null) return; @@ -220,12 +222,9 @@ public class BlockInfoUnderConstruction if (genStamp != r.getGenerationStamp()) { r.getExpectedLocation().removeBlock(this); NameNode.blockStateChangeLog.info("BLOCK* Removing stale replica " - + "from location: " + r); + + "from location: " + r.getExpectedLocation()); } } - - // Set the generation stamp for the block. - setGenerationStamp(genStamp); } /** @@ -240,6 +239,8 @@ public class BlockInfoUnderConstruction + block.getBlockId() + ", expected id = " + getBlockId()); blockUCState = BlockUCState.COMMITTED; this.set(getBlockId(), block.getNumBytes(), block.getGenerationStamp()); + // Sort out invalid replicas. + setGenerationStampAndVerifyReplicas(block.getGenerationStamp()); } /** Modified: hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java?rev=1547181&r1=1547180&r2=1547181&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java Mon Dec 2 20:31:55 2013 @@ -1324,13 +1324,15 @@ public class BlockManager { * Besides the block in question, it provides the ReplicaState * reported by the datanode in the block report. */ - private static class StatefulBlockInfo { + static class StatefulBlockInfo { final BlockInfoUnderConstruction storedBlock; + final Block reportedBlock; final ReplicaState reportedState; StatefulBlockInfo(BlockInfoUnderConstruction storedBlock, - ReplicaState reportedState) { + Block reportedBlock, ReplicaState reportedState) { this.storedBlock = storedBlock; + this.reportedBlock = reportedBlock; this.reportedState = reportedState; } } @@ -1415,7 +1417,7 @@ public class BlockManager { // Process the blocks on each queue for (StatefulBlockInfo b : toUC) { - addStoredBlockUnderConstruction(b.storedBlock, node, b.reportedState); + addStoredBlockUnderConstruction(b, node); } for (Block b : toRemove) { removeStoredBlock(b, node); @@ -1591,7 +1593,7 @@ public class BlockManager { if (isBlockUnderConstruction(storedBlock, ucState, reportedState)) { toUC.add(new StatefulBlockInfo( - (BlockInfoUnderConstruction)storedBlock, reportedState)); + (BlockInfoUnderConstruction)storedBlock, block, reportedState)); return storedBlock; } @@ -1701,13 +1703,11 @@ public class BlockManager { } } - void addStoredBlockUnderConstruction( - BlockInfoUnderConstruction block, - DatanodeDescriptor node, - ReplicaState reportedState) - throws IOException { - block.addReplicaIfNotPresent(node, block, reportedState); - if (reportedState == ReplicaState.FINALIZED && block.findDatanode(node) < 0) { + void addStoredBlockUnderConstruction(StatefulBlockInfo ucBlock, + DatanodeDescriptor node) throws IOException { + BlockInfoUnderConstruction block = ucBlock.storedBlock; + block.addReplicaIfNotPresent(node, ucBlock.reportedBlock, ucBlock.reportedState); + if (ucBlock.reportedState == ReplicaState.FINALIZED && block.findDatanode(node) < 0) { addStoredBlock(block, node, null, true); } } @@ -2223,7 +2223,7 @@ public class BlockManager { : "The block should be only in one of the lists."; for (StatefulBlockInfo b : toUC) { - addStoredBlockUnderConstruction(b.storedBlock, node, b.reportedState); + addStoredBlockUnderConstruction(b, node); } for (BlockInfo b : toAdd) { addStoredBlock(b, node, delHintNode, true); Modified: hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestClientProtocolForPipelineRecovery.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestClientProtocolForPipelineRecovery.java?rev=1547181&r1=1547180&r2=1547181&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestClientProtocolForPipelineRecovery.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestClientProtocolForPipelineRecovery.java Mon Dec 2 20:31:55 2013 @@ -142,16 +142,10 @@ public class TestClientProtocolForPipeli Path file = new Path("dataprotocol1.dat"); Mockito.when(faultInjector.failPacket()).thenReturn(true); - try { - DFSTestUtil.createFile(fileSys, file, 1L, (short)numDataNodes, 0L); - } catch (IOException e) { - // completeFile() should fail. - Assert.assertTrue(e.getMessage().startsWith("Unable to close file")); - return; - } + DFSTestUtil.createFile(fileSys, file, 68000000L, (short)numDataNodes, 0L); - // At this point, NN let data corruption to happen. - // Before failing test, try reading the file. It should fail. + // At this point, NN should have accepted only valid replicas. + // Read should succeed. FSDataInputStream in = fileSys.open(file); try { int c = in.read(); @@ -161,8 +155,6 @@ public class TestClientProtocolForPipeli Assert.fail("Block is missing because the file was closed with" + " corrupt replicas."); } - Assert.fail("The file was closed with corrupt replicas, but read still" - + " works!"); } finally { DFSClientFaultInjector.instance = oldInjector; if (cluster != null) {