diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index b93191796c5..f95cb345646 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -520,6 +520,8 @@ Release 2.8.0 - UNRELEASED HDFS-8983. NameNode support for protected directories. (Arpit Agarwal) + HDFS-8980. Remove unnecessary block replacement in INodeFile. (jing9) + OPTIMIZATIONS HDFS-8026. Trace FSOutputSummer#writeChecksumChunks rather than diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java index 17e115669a1..db5d42abc7f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java @@ -39,7 +39,7 @@ import static org.apache.hadoop.hdfs.server.namenode.INodeId.INVALID_INODE_ID; * the block are stored. */ @InterfaceAudience.Private -public abstract class BlockInfo extends Block +public abstract class BlockInfo extends Block implements LightWeightGSet.LinkedElement { public static final BlockInfo[] EMPTY_ARRAY = {}; @@ -209,12 +209,6 @@ public abstract class BlockInfo extends Block */ abstract boolean removeStorage(DatanodeStorageInfo storage); - /** - * Replace the current BlockInfo with the new one in corresponding - * DatanodeStorageInfo's linked list - */ - abstract void replaceBlock(BlockInfo newBlock); - /** * Find specified DatanodeStorageInfo. * @return DatanodeStorageInfo or null if not found. @@ -375,19 +369,12 @@ public abstract class BlockInfo extends Block } /** - * Convert an under construction block to a complete block. - * - * @return BlockInfo - a complete block. - * @throws IOException if the state of the block - * (the generation stamp and the length) has not been committed by - * the client or it does not have at least a minimal number of replicas - * reported from data-nodes. + * Convert an under construction block to complete. */ - BlockInfo convertToCompleteBlock() throws IOException { + void convertToCompleteBlock() { assert getBlockUCState() != BlockUCState.COMPLETE : "Trying to convert a COMPLETE block"; uc = null; - return this; } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java index 42934c3abb7..94fb222068f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java @@ -104,19 +104,4 @@ public class BlockInfoContiguous extends BlockInfo { } return 0; } - - @Override - void replaceBlock(BlockInfo newBlock) { - assert newBlock instanceof BlockInfoContiguous; - for (int i = this.numNodes() - 1; i >= 0; i--) { - final DatanodeStorageInfo storage = this.getStorageInfo(i); - final boolean removed = storage.removeBlock(this); - assert removed : "currentBlock not found."; - - final DatanodeStorageInfo.AddBlockResult result = storage.addBlock( - newBlock); - assert result == DatanodeStorageInfo.AddBlockResult.ADDED : - "newBlock already exists."; - } - } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java index 36a7f5d0dc2..4aaef616b04 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java @@ -640,37 +640,34 @@ public class BlockManager implements BlockStatsMXBean { return false; // already completed (e.g. by syncBlock) final boolean b = commitBlock(lastBlock, commitBlock); - if(countNodes(lastBlock).liveReplicas() >= minReplication) - completeBlock(bc, bc.numBlocks()-1, false); + if (countNodes(lastBlock).liveReplicas() >= minReplication) { + completeBlock(lastBlock, false); + } return b; } /** * Convert a specified block of the file to a complete block. - * @param bc file - * @param blkIndex block index in the file * @throws IOException if the block does not have at least a minimal number * of replicas reported from data-nodes. */ - private BlockInfo completeBlock(final BlockCollection bc, - final int blkIndex, boolean force) throws IOException { - if(blkIndex < 0) - return null; - BlockInfo curBlock = bc.getBlocks()[blkIndex]; - if(curBlock.isComplete()) - return curBlock; + private void completeBlock(BlockInfo curBlock, boolean force) + throws IOException { + if (curBlock.isComplete()) { + return; + } int numNodes = curBlock.numNodes(); - if (!force && numNodes < minReplication) - throw new IOException("Cannot complete block: " + - "block does not satisfy minimal replication requirement."); - if(!force && curBlock.getBlockUCState() != BlockUCState.COMMITTED) + if (!force && numNodes < minReplication) { + throw new IOException("Cannot complete block: " + + "block does not satisfy minimal replication requirement."); + } + if (!force && curBlock.getBlockUCState() != BlockUCState.COMMITTED) { throw new IOException( "Cannot complete block: block has not been COMMITTED by the client"); - BlockInfo completeBlock = curBlock.convertToCompleteBlock(); - // replace penultimate block in file - bc.setBlock(blkIndex, completeBlock); - + } + + curBlock.convertToCompleteBlock(); // Since safe-mode only counts complete blocks, and we now have // one more complete block, we need to adjust the total up, and // also count it as safe, if we have at least the minimum replica @@ -680,33 +677,18 @@ public class BlockManager implements BlockStatsMXBean { namesystem.adjustSafeModeBlockTotals(0, 1); namesystem.incrementSafeBlockCount( Math.min(numNodes, minReplication)); - - // replace block in the blocksMap - return blocksMap.replaceBlock(completeBlock); } - private BlockInfo completeBlock(final BlockCollection bc, - final BlockInfo block, boolean force) throws IOException { - BlockInfo[] fileBlocks = bc.getBlocks(); - for(int idx = 0; idx < fileBlocks.length; idx++) - if(fileBlocks[idx] == block) { - return completeBlock(bc, idx, force); - } - return block; - } - /** * Force the given block in the given file to be marked as complete, * regardless of whether enough replicas are present. This is necessary * when tailing edit logs as a Standby. */ - public BlockInfo forceCompleteBlock(final BlockCollection bc, - final BlockInfo block) throws IOException { + public void forceCompleteBlock(final BlockInfo block) throws IOException { block.commitBlock(block); - return completeBlock(bc, block, true); + completeBlock(block, true); } - /** * Convert the last block of the file to an under construction block.

* The block is converted only if the file has blocks and the last one @@ -2494,7 +2476,7 @@ public class BlockManager implements BlockStatsMXBean { int numCurrentReplica = countLiveNodes(storedBlock); if (storedBlock.getBlockUCState() == BlockUCState.COMMITTED && numCurrentReplica >= minReplication) { - completeBlock(getBlockCollection(storedBlock), storedBlock, false); + completeBlock(storedBlock, false); } else if (storedBlock.isComplete() && result == AddBlockResult.ADDED) { // check whether safe replication is reached for the block // only complete blocks are counted towards that. @@ -2568,7 +2550,7 @@ public class BlockManager implements BlockStatsMXBean { if(storedBlock.getBlockUCState() == BlockUCState.COMMITTED && numLiveReplicas >= minReplication) { - storedBlock = completeBlock(bc, storedBlock, false); + completeBlock(storedBlock, false); } else if (storedBlock.isComplete() && result == AddBlockResult.ADDED) { // check whether safe replication is reached for the block // only complete blocks are counted towards that diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java index 33c68f39461..9189c3261ad 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java @@ -209,20 +209,4 @@ class BlocksMap { int getCapacity() { return capacity; } - - /** - * Replace a block in the block map by a new block. - * The new block and the old one have the same key. - * @param newBlock - block for replacement - * @return new block - */ - BlockInfo replaceBlock(BlockInfo newBlock) { - BlockInfo currentBlock = blocks.get(newBlock); - assert currentBlock != null : "the block if not in blocksMap"; - // replace block in data-node lists - currentBlock.replaceBlock(newBlock); - // replace block in the map itself - blocks.put(newBlock); - return newBlock; - } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java index a430ebd8664..24738fa3fe3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java @@ -506,7 +506,7 @@ public class FSEditLogLoader { } INodeFile oldFile = INodeFile.valueOf(fsDir.getINode(path), path); // add the new block to the INodeFile - addNewBlock(fsDir, addBlockOp, oldFile); + addNewBlock(addBlockOp, oldFile); break; } case OP_SET_REPLICATION: { @@ -943,7 +943,7 @@ public class FSEditLogLoader { /** * Add a new block into the given INodeFile */ - private void addNewBlock(FSDirectory fsDir, AddBlockOp op, INodeFile file) + private void addNewBlock(AddBlockOp op, INodeFile file) throws IOException { BlockInfo[] oldBlocks = file.getBlocks(); Block pBlock = op.getPenultimateBlock(); @@ -963,7 +963,7 @@ public class FSEditLogLoader { oldLastBlock.setNumBytes(pBlock.getNumBytes()); if (!oldLastBlock.isComplete()) { - fsNamesys.getBlockManager().forceCompleteBlock(file, oldLastBlock); + fsNamesys.getBlockManager().forceCompleteBlock(oldLastBlock); fsNamesys.getBlockManager().processQueuedMessagesForBlock(pBlock); } } else { // the penultimate block is null @@ -1016,7 +1016,7 @@ public class FSEditLogLoader { if (!oldBlock.isComplete() && (!isLastBlock || op.shouldCompleteLastBlock())) { changeMade = true; - fsNamesys.getBlockManager().forceCompleteBlock(file, oldBlock); + fsNamesys.getBlockManager().forceCompleteBlock(oldBlock); } if (changeMade) { // The state or gen-stamp of the block has changed. So, we may be