HDFS-7104. Fix and clarify INodeInPath getter functions. Contributed by Zhe Zhang.

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

@@ -518,6 +518,8 @@ Release 2.6.0 - UNRELEASED
     HDFS-4165. Faulty sanity check in FsDirectory.unprotectedSetQuota.
     (Binglin Chang via suresh)
 
+    HDFS-7104. Fix and clarify INodeInPath getter functions. (Zhe Zhang via wang)
+
   OPTIMIZATIONS
 
     HDFS-6690. Deduplicate xattr names in memory. (wang)

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

@@ -133,7 +133,6 @@ public class INodesInPath {
    *        be thrown when the path refers to a symbolic link.
    * @return the specified number of existing INodes in the path
    */
-  // TODO: Eliminate null elements from inodes (to be provided by HDFS-7104)
   static INodesInPath resolve(final INodeDirectory startingDir,
       final byte[][] components, final int numOfINodes, 
       final boolean resolveLink) throws UnresolvedLinkException {
@@ -262,7 +261,8 @@ public class INodesInPath {
    */
   private boolean isSnapshot;
   /**
-   * Index of {@link INodeDirectoryWithSnapshot} for snapshot path, else -1
+   * index of the {@link Snapshot.Root} node in the inodes array,
+   * -1 for non-snapshot paths.
    */
   private int snapshotRootIndex;
   /**
@@ -312,15 +312,20 @@ public class INodesInPath {
   }
 
   /**
-   * @return the inodes array excluding the null elements.
+   * @return a new array of inodes excluding the null elements introduced by
+   * snapshot path elements. E.g., after resolving path "/dir/.snapshot",
+   * {@link #inodes} is {/, dir, null}, while the returned array only contains
+   * inodes of "/" and "dir". Note the length of the returned array is always
+   * equal to {@link #capacity}.
    */
   INode[] getINodes() {
-    if (capacity < inodes.length) {
+    if (capacity == inodes.length) {
-      INode[] newNodes = new INode[capacity];
+      return inodes;
-      System.arraycopy(inodes, 0, newNodes, 0, capacity);
-      inodes = newNodes;
     }
-    return inodes;
+
+    INode[] newNodes = new INode[capacity];
+    System.arraycopy(inodes, 0, newNodes, 0, capacity);
+    return newNodes;
   }
   
   /**
@@ -341,8 +346,8 @@ public class INodesInPath {
   }
   
   /**
-   * @return index of the {@link INodeDirectoryWithSnapshot} in
+   * @return index of the {@link Snapshot.Root} node in the inodes array,
-   *         {@link #inodes} for snapshot path, else -1.
+   * -1 for non-snapshot paths.
    */
   int getSnapshotRootIndex() {
     return this.snapshotRootIndex;

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java

@@ -215,7 +215,7 @@ public class TestSnapshotPathINodes {
     // snapshotRootIndex should be -1.
     assertSnapshot(nodesInPath, true, snapshot, -1);
     // Check the INode for file1 (snapshot file)
-    assertINodeFile(nodesInPath.getLastINode(), file1);
+    assertINodeFile(inodes[inodes.length - 1], file1);
     
     // Call getExistingPathINodes and request 2 INodes.
     nodesInPath = INodesInPath.resolve(fsdir.rootDir, components, 2, false);
@@ -224,7 +224,7 @@ public class TestSnapshotPathINodes {
     // There should be two INodes in inodes: s1 and snapshot of file1. Thus the
     // SnapshotRootIndex should be 0.
     assertSnapshot(nodesInPath, true, snapshot, 0);
-    assertINodeFile(nodesInPath.getLastINode(), file1);
+    assertINodeFile(inodes[inodes.length - 1], file1);
     
     // Resolve the path "/TestSnapshot/sub1/.snapshot"  
     String dotSnapshotPath = sub1.toString() + "/.snapshot";
@@ -239,7 +239,7 @@ public class TestSnapshotPathINodes {
     // No SnapshotRoot dir is included in the resolved inodes  
     assertSnapshot(nodesInPath, true, snapshot, -1);
     // The last INode should be the INode for sub1
-    final INode last = nodesInPath.getLastINode();
+    final INode last = inodes[inodes.length - 1];
     assertEquals(last.getFullPathName(), sub1.toString());
     assertFalse(last instanceof INodeFile);