diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 635ee13b902..abd569ec2b1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -48,7 +48,11 @@ Release 2.0.1-alpha - UNRELEASED (todd) HDFS-2885. Remove "federation" from the nameservice config options. - (Tsz Wo (Nicholas) via eli) + (Tsz Wo (Nicholas) Sze via eli) + + HDFS-3394. Do not use generic in INodeFile.getLastBlock(): the run-time + ClassCastException check is useless since generic type information is only + available in compile-time. (szetszwo) OPTIMIZATIONS diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java index f7c33cad011..e3eecadce0c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java @@ -19,9 +19,6 @@ package org.apache.hadoop.hdfs.server.blockmanagement; import java.io.IOException; -import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfoUnderConstruction; -import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; -import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor; import org.apache.hadoop.fs.ContentSummary; /** @@ -31,19 +28,24 @@ import org.apache.hadoop.fs.ContentSummary; public interface BlockCollection { /** * Get the last block of the collection. - * Make sure it has the right type. */ - public T getLastBlock() throws IOException; + public BlockInfo getLastBlock() throws IOException; /** * Get content summary. */ public ContentSummary computeContentSummary(); - /** @return the number of blocks */ + /** + * @return the number of blocks + */ public int numBlocks(); + /** + * Get the blocks. + */ public BlockInfo[] getBlocks(); + /** * Get preferred block size for the collection * @return preferred block size in bytes @@ -57,7 +59,7 @@ public interface BlockCollection { public short getReplication(); /** - * Get name of collection. + * Get the name of the collection. */ public String getName(); } 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 656c2ff0237..7e37cbc2cc0 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 @@ -439,7 +439,7 @@ public class BlockManager { * @throws IOException if the block does not have at least a minimal number * of replicas reported from data-nodes. */ - private boolean commitBlock(final BlockInfoUnderConstruction block, + private static boolean commitBlock(final BlockInfoUnderConstruction block, final Block commitBlock) throws IOException { if (block.getBlockUCState() == BlockUCState.COMMITTED) return false; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/MutableBlockCollection.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/MutableBlockCollection.java index 2b5b3e4dd27..41975d3371d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/MutableBlockCollection.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/MutableBlockCollection.java @@ -19,26 +19,20 @@ package org.apache.hadoop.hdfs.server.blockmanagement; import java.io.IOException; -import org.apache.hadoop.classification.InterfaceAudience; -import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfoUnderConstruction; -import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; -import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor; -import org.apache.hadoop.fs.ContentSummary; - /** * This interface is used by the block manager to expose a * few characteristics of a collection of Block/BlockUnderConstruction. */ public interface MutableBlockCollection extends BlockCollection { /** - * Set block + * Set the block at the given index. */ - public void setBlock(int idx, BlockInfo blk); + public void setBlock(int index, BlockInfo blk); /** - * Convert the last block of the collection to an under-construction block. - * Set its locations. + * Convert the last block of the collection to an under-construction block + * and set the locations. */ public BlockInfoUnderConstruction setLastBlock(BlockInfo lastBlock, - DatanodeDescriptor[] targets) throws IOException; + DatanodeDescriptor[] locations) throws IOException; } 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 90c6742d266..4b576232bf3 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 @@ -2768,9 +2768,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats, throw new IOException(message); } - // no we know that the last block is not COMPLETE, and + // The last block is not COMPLETE, and // that the penultimate block if exists is either COMPLETE or COMMITTED - BlockInfoUnderConstruction lastBlock = pendingFile.getLastBlock(); + final BlockInfo lastBlock = pendingFile.getLastBlock(); BlockUCState lastBlockState = lastBlock.getBlockUCState(); BlockInfo penultimateBlock = pendingFile.getPenultimateBlock(); boolean penultimateBlockMinReplication; @@ -2814,13 +2814,15 @@ public class FSNamesystem implements Namesystem, FSClusterStats, throw new AlreadyBeingCreatedException(message); case UNDER_CONSTRUCTION: case UNDER_RECOVERY: + final BlockInfoUnderConstruction uc = (BlockInfoUnderConstruction)lastBlock; // setup the last block locations from the blockManager if not known - if(lastBlock.getNumExpectedLocations() == 0) - lastBlock.setExpectedLocations(blockManager.getNodes(lastBlock)); + if (uc.getNumExpectedLocations() == 0) { + uc.setExpectedLocations(blockManager.getNodes(lastBlock)); + } // start recovery of the last block for this file long blockRecoveryId = nextGenerationStamp(); lease = reassignLease(lease, src, recoveryLeaseHolder, pendingFile); - lastBlock.initializeBlockRecovery(blockRecoveryId); + uc.initializeBlockRecovery(blockRecoveryId); leaseManager.renewLease(lease); // Cannot close file right now, since the last block requires recovery. // This may potentially cause infinite loop in lease recovery @@ -4557,15 +4559,16 @@ public class FSNamesystem implements Namesystem, FSClusterStats, LOG.info("updatePipeline(" + oldBlock + ") successfully to " + newBlock); } - /** @see updatePipeline(String, ExtendedBlock, ExtendedBlock, DatanodeID[]) */ + /** @see #updatePipeline(String, ExtendedBlock, ExtendedBlock, DatanodeID[]) */ private void updatePipelineInternal(String clientName, ExtendedBlock oldBlock, ExtendedBlock newBlock, DatanodeID[] newNodes) throws IOException { assert hasWriteLock(); // check the vadility of the block and lease holder name - final INodeFileUnderConstruction pendingFile = - checkUCBlock(oldBlock, clientName); - final BlockInfoUnderConstruction blockinfo = pendingFile.getLastBlock(); + final INodeFileUnderConstruction pendingFile + = checkUCBlock(oldBlock, clientName); + final BlockInfoUnderConstruction blockinfo + = (BlockInfoUnderConstruction)pendingFile.getLastBlock(); // check new GS & length: this is not expected if (newBlock.getGenerationStamp() <= blockinfo.getGenerationStamp() || diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java index e940b61ab9c..c9d26e4b542 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java @@ -177,14 +177,14 @@ abstract class INode implements Comparable { return (short)PermissionStatusFormat.MODE.retrieve(permission); } /** Set the {@link FsPermission} of this {@link INode} */ - protected void setPermission(FsPermission permission) { + void setPermission(FsPermission permission) { updatePermissionStatus(PermissionStatusFormat.MODE, permission.toShort()); } /** * Check whether it's a directory */ - public abstract boolean isDirectory(); + abstract boolean isDirectory(); /** * Collect all the blocks in all children of this INode. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java index b3485ecc8ee..3bfb3359060 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java @@ -41,9 +41,9 @@ public class INodeFile extends INode implements BlockCollection { //Format: [16 bits for replication][48 bits for PreferredBlockSize] static final long HEADERMASK = 0xffffL << BLOCKBITS; - protected long header; + private long header; - protected BlockInfo blocks[] = null; + BlockInfo blocks[] = null; INodeFile(PermissionStatus permissions, int nrBlocks, short replication, long modificationTime, @@ -52,12 +52,7 @@ public class INodeFile extends INode implements BlockCollection { modificationTime, atime, preferredBlockSize); } - protected INodeFile() { - blocks = null; - header = 0; - } - - protected INodeFile(PermissionStatus permissions, BlockInfo[] blklist, + INodeFile(PermissionStatus permissions, BlockInfo[] blklist, short replication, long modificationTime, long atime, long preferredBlockSize) { super(permissions, modificationTime, atime); @@ -71,47 +66,40 @@ public class INodeFile extends INode implements BlockCollection { * Since this is a file, * the {@link FsAction#EXECUTE} action, if any, is ignored. */ - protected void setPermission(FsPermission permission) { + void setPermission(FsPermission permission) { super.setPermission(permission.applyUMask(UMASK)); } - public boolean isDirectory() { + boolean isDirectory() { return false; } - /** - * Get block replication for the file - * @return block replication value - */ + /** @return the replication factor of the file. */ + @Override public short getReplication() { return (short) ((header & HEADERMASK) >> BLOCKBITS); } - public void setReplication(short replication) { + void setReplication(short replication) { if(replication <= 0) throw new IllegalArgumentException("Unexpected value for the replication"); header = ((long)replication << BLOCKBITS) | (header & ~HEADERMASK); } - /** - * Get preferred block size for the file - * @return preferred block size in bytes - */ + /** @return preferred block size (in bytes) of the file. */ + @Override public long getPreferredBlockSize() { - return header & ~HEADERMASK; + return header & ~HEADERMASK; } - public void setPreferredBlockSize(long preferredBlkSize) - { + private void setPreferredBlockSize(long preferredBlkSize) { if((preferredBlkSize < 0) || (preferredBlkSize > ~HEADERMASK )) throw new IllegalArgumentException("Unexpected value for the block size"); header = (header & HEADERMASK) | (preferredBlkSize & ~HEADERMASK); } - /** - * Get file blocks - * @return file blocks - */ + /** @return the blocks of the file. */ + @Override public BlockInfo[] getBlocks() { return this.blocks; } @@ -152,9 +140,7 @@ public class INodeFile extends INode implements BlockCollection { } } - /** - * Set file block - */ + /** Set the block of the file at the given index. */ public void setBlock(int idx, BlockInfo blk) { this.blocks[idx] = blk; } @@ -171,6 +157,7 @@ public class INodeFile extends INode implements BlockCollection { return 1; } + @Override public String getName() { // Get the full path name of this inode. return getFullPathName(); @@ -215,7 +202,7 @@ public class INodeFile extends INode implements BlockCollection { return diskspaceConsumed(blocks); } - long diskspaceConsumed(Block[] blkArr) { + private long diskspaceConsumed(Block[] blkArr) { long size = 0; if(blkArr == null) return 0; @@ -245,26 +232,12 @@ public class INodeFile extends INode implements BlockCollection { return blocks[blocks.length - 2]; } - /** - * Get the last block of the file. - * Make sure it has the right type. - */ - public T getLastBlock() throws IOException { - if (blocks == null || blocks.length == 0) - return null; - T returnBlock = null; - try { - @SuppressWarnings("unchecked") // ClassCastException is caught below - T tBlock = (T)blocks[blocks.length - 1]; - returnBlock = tBlock; - } catch(ClassCastException cce) { - throw new IOException("Unexpected last block type: " - + blocks[blocks.length - 1].getClass().getSimpleName()); - } - return returnBlock; + @Override + public BlockInfo getLastBlock() throws IOException { + return blocks == null || blocks.length == 0? null: blocks[blocks.length-1]; } - /** @return the number of blocks */ + @Override public int numBlocks() { return blocks == null ? 0 : blocks.length; }