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.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1341141 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -194,7 +194,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
 

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java

@@ -19,9 +19,6 @@
 
 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 @@
 public interface BlockCollection {
   /**
    * Get the last block of the collection.
-   * Make sure it has the right type.
    */
-  public <T extends BlockInfo> 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();
 }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java

@@ -437,7 +437,7 @@ public boolean checkMinReplication(Block block) {
    * @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;

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/MutableBlockCollection.java

@@ -19,26 +19,20 @@
 
 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;
 }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -2783,9 +2783,9 @@ boolean internalReleaseLease(Lease lease, String src,
       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;
@@ -2829,13 +2829,15 @@ boolean internalReleaseLease(Lease lease, String src,
       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
@@ -4572,15 +4574,16 @@ void updatePipeline(String clientName, ExtendedBlock oldBlock,
     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() ||

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java

@@ -177,14 +177,14 @@ protected short getFsPermissionShort() {
     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.

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 @@ protected INodeFile(PermissionStatus permissions, BlockInfo[] blklist,
    * 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 @@ void addBlock(BlockInfo newblock) {
     }
   }
 
-  /**
-   * 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 @@ int collectSubtreeBlocksAndClear(List<Block> v) {
     return 1;
   }
   
+  @Override
   public String getName() {
     // Get the full path name of this inode.
     return getFullPathName();
@@ -215,7 +202,7 @@ long diskspaceConsumed() {
     return diskspaceConsumed(blocks);
   }
   
-  long diskspaceConsumed(Block[] blkArr) {
+  private long diskspaceConsumed(Block[] blkArr) {
     long size = 0;
     if(blkArr == null) 
       return 0;
@@ -245,26 +232,12 @@ BlockInfo getPenultimateBlock() {
     return blocks[blocks.length - 2];
   }
 
-  /**
-   * Get the last block of the file.
-   * Make sure it has the right type.
-   */
-  public <T extends BlockInfo> 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;
   }