You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/05/10 18:07:56 UTC

[GitHub] [hadoop] li-leyang commented on a diff in pull request #4181: HADOOP-18193:Support nested mount points in INodeTree

li-leyang commented on code in PR #4181:
URL: https://github.com/apache/hadoop/pull/4181#discussion_r852343467


##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestNestedMountPoint.java:
##########
@@ -0,0 +1,345 @@
+package org.apache.hadoop.fs.viewfs;
+
+import java.net.URI;
+import java.util.List;
+import java.util.function.Function;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.Path;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+
+public class TestNestedMountPoint {
+  private InodeTree inodeTree;
+  private Configuration conf;
+  private String mtName;
+  private URI fsUri;
+
+  static class TestNestMountPointFileSystem {
+    public URI getUri() {
+      return uri;
+    }
+
+    private URI uri;
+
+    TestNestMountPointFileSystem(URI uri) {
+      this.uri = uri;
+    }
+  }
+
+  static class TestNestMountPointInternalFileSystem extends TestNestMountPointFileSystem {
+    TestNestMountPointInternalFileSystem(URI uri) {
+      super(uri);
+    }
+  }
+
+  private static final URI LINKFALLBACK_TARGET = URI.create("hdfs://nn00");
+  private static final URI NN1_TARGET = URI.create("hdfs://nn01/a/b");
+  private static final URI NN2_TARGET = URI.create("hdfs://nn02/a/b/e");
+  private static final URI NN3_TARGET = URI.create("hdfs://nn03/a/b/c/d");
+  private static final URI NN4_TARGET = URI.create("hdfs://nn04/a/b/c/d/e");
+  private static final URI NN5_TARGET = URI.create("hdfs://nn05/b/c/d/e");
+  private static final URI NN6_TARGET = URI.create("hdfs://nn06/b/c/d/e/f");
+
+  @Before
+  public void setUp() throws Exception {
+    conf = new Configuration();
+    mtName = TestNestedMountPoint.class.getName();
+    ConfigUtil.setIsNestedMountPointSupported(conf, true);
+    ConfigUtil.addLink(conf, mtName, "/a/b", NN1_TARGET);
+    ConfigUtil.addLink(conf, mtName, "/a/b/e", NN2_TARGET);

Review Comment:
   Not sure what test case you are referring to
   /a/b is nested in /a/b/c/d which is nested in /a/b/c/d/e.
   
   Do you mean to add the test case like below:
   /a/b -> /a/b/c/d ->/a/b/c/d/e
   /a/b -> /a/b/e -> /a/b/e/f
   (/a/b is a dirlink has has two nested mount points)



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ConfigUtil.java:
##########
@@ -247,4 +247,22 @@ public static String getDefaultMountTableName(final Configuration conf) {
     return conf.get(Constants.CONFIG_VIEWFS_DEFAULT_MOUNT_TABLE_NAME_KEY,
         Constants.CONFIG_VIEWFS_DEFAULT_MOUNT_TABLE);
   }
+
+  /**
+   * Get the bool config whether nested mount point is supported.

Review Comment:
   this is to get config value from conf object. I will change it to "check".



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -133,6 +138,9 @@ public INode(String pathToNode, UserGroupInformation aUgi) {
     // and is read only.
     abstract boolean isInternalDir();
 
+    // Identity some type of inodes(nested mount point).

Review Comment:
   yeah, will expand this comment



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -212,6 +225,33 @@ void addLink(final String pathComponent, final INodeLink<T> link)
       }
       children.put(pathComponent, link);
     }
+
+    void addDirLink(final String pathComponent, final INodeDirLink<T> dirLink) {
+      children.put(pathComponent, dirLink);
+    }
+  }
+
+  /**
+   * Internal class to represent nested mount points in INodeTree. Nested mount points are non-leaf nodes in INode tree.

Review Comment:
   I have added in the comments.



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -682,6 +736,25 @@ protected InodeTree(final Configuration config, final String viewName,
     }
   }
 
+  private Collection<LinkEntry> getLinkEntries(List<LinkEntry> linkEntries) {
+    if (isNestedMountPointSupported) {

Review Comment:
   The main reason for sorting is to group nested mp together and create INodeDirLink for nested mp:
   Lets say we have 3 mount points sorted:
   /foo, /foo/bar and /foo/bar/baz
   
   When creating inode tree:
   1st pass: /foo -> create INodeLink for /foo
   2nd pass: /foo/bar -> /foo is already a INodeLink so /foo is nested, updating /foo as INodeDirLink, creating /foo/bar as INodeLink
   3nd pass: /foo/bar/baz -> /foo/bar is INodeLink so/foo/bar is also nested, updating /foo/bar to INodeDirLink, creating /foo/bar/baz as INodeLink.
   



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -212,6 +225,33 @@ void addLink(final String pathComponent, final INodeLink<T> link)
       }
       children.put(pathComponent, link);
     }
+
+    void addDirLink(final String pathComponent, final INodeDirLink<T> dirLink) {
+      children.put(pathComponent, dirLink);
+    }
+  }
+
+  /**
+   * Internal class to represent nested mount points in INodeTree. Nested mount points are non-leaf nodes in INode tree.
+   * Hence it inherits INodeDir. But it has additional attributes so wrap it with link.
+   * @param <T>
+   */
+  static class INodeDirLink<T> extends INodeDir<T> {

Review Comment:
   I have added nested mp semantics in the comment



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -212,6 +225,33 @@ void addLink(final String pathComponent, final INodeLink<T> link)
       }
       children.put(pathComponent, link);
     }
+
+    void addDirLink(final String pathComponent, final INodeDirLink<T> dirLink) {
+      children.put(pathComponent, dirLink);
+    }
+  }
+
+  /**
+   * Internal class to represent nested mount points in INodeTree. Nested mount points are non-leaf nodes in INode tree.
+   * Hence it inherits INodeDir. But it has additional attributes so wrap it with link.
+   * @param <T>
+   */
+  static class INodeDirLink<T> extends INodeDir<T> {

Review Comment:
   The class inheritance hierarchy at high level is:
   
   INode:
   Base class
   
   INodeDir -> INode
   Representing internal dir to mount point. 
   isLInk(): false
   isInternalDir(): true
   **isDirAndLink(): false**
   
   INodeLink -> INode
   Representing non-nested mount point
   IsLInk(): true
   isInternalDir(): false
   **isDirAndLink(): false**
   
   **INodeDirLink -> INodeDir
   Representing nested mount point
   isLink(): false
   isInternalDir(): true
   isDirAndLink(): true**
   
   Highlighted are newly added in the PR.



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -682,6 +748,32 @@ protected InodeTree(final Configuration config, final String viewName,
     }
   }
 
+  /**
+   * Get collection of linkEntry. If nested mount point is supported, sort mount point src path based on alphabetical order.

Review Comment:
   Fixed



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java:
##########
@@ -662,177 +790,177 @@ public void testResolvePathMountPoints() throws IOException {
         fsView.resolvePath(new Path("/internalDir/internalDir2/linkToDir3")));
 
   }
-  
+
   @Test
   public void testResolvePathThroughMountPoints() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/user/foo");
     Assert.assertEquals(new Path(targetTestRoot,"user/foo"),
         fsView.resolvePath(new Path("/user/foo")));
-    
+
     fsView.mkdirs(
         fileSystemTestHelper.getTestRootPath(fsView, "/user/dirX"));
     Assert.assertEquals(new Path(targetTestRoot,"user/dirX"),
         fsView.resolvePath(new Path("/user/dirX")));
 
-    
+
     fsView.mkdirs(
         fileSystemTestHelper.getTestRootPath(fsView, "/user/dirX/dirY"));
     Assert.assertEquals(new Path(targetTestRoot,"user/dirX/dirY"),
         fsView.resolvePath(new Path("/user/dirX/dirY")));
   }
 
-  @Test(expected=FileNotFoundException.class) 
+  @Test(expected=FileNotFoundException.class)
   public void testResolvePathDanglingLink() throws IOException {
     fsView.resolvePath(new Path("/danglingLink"));
   }
-  
-  @Test(expected=FileNotFoundException.class) 
+
+  @Test(expected=FileNotFoundException.class)
   public void testResolvePathMissingThroughMountPoints() throws IOException {
     fsView.resolvePath(new Path("/user/nonExisting"));
   }
-  
 
-  @Test(expected=FileNotFoundException.class) 
+
+  @Test(expected=FileNotFoundException.class)
   public void testResolvePathMissingThroughMountPoints2() throws IOException {
     fsView.mkdirs(
         fileSystemTestHelper.getTestRootPath(fsView, "/user/dirX"));
     fsView.resolvePath(new Path("/user/dirX/nonExisting"));
   }
-  
+
   /**
-   * Test modify operations (create, mkdir, rename, etc) 
+   * Test modify operations (create, mkdir, rename, etc)
    * on internal dirs of mount table
    * These operations should fail since the mount table is read-only or
    * because the internal dir that it is trying to create already
    * exits.
    */
- 
- 
+
+
   // Mkdir on existing internal mount table succeed except for /
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalMkdirSlash() throws IOException {
     fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView, "/"));
   }
-  
+
   public void testInternalMkdirExisting1() throws IOException {
-    Assert.assertTrue("mkdir of existing dir should succeed", 
+    Assert.assertTrue("mkdir of existing dir should succeed",
         fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView,
         "/internalDir")));
   }
 
   public void testInternalMkdirExisting2() throws IOException {
-    Assert.assertTrue("mkdir of existing dir should succeed", 
+    Assert.assertTrue("mkdir of existing dir should succeed",
         fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView,
         "/internalDir/linkToDir2")));
   }
-  
+
   // Mkdir for new internal mount table should fail
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalMkdirNew() throws IOException {
     fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView, "/dirNew"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalMkdirNew2() throws IOException {
     fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView, "/internalDir/dirNew"));
   }
-  
+
   // Create File on internal mount table should fail
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreate1() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/foo"); // 1 component
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreate2() throws IOException {  // 2 component
     fileSystemTestHelper.createFile(fsView, "/internalDir/foo");
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreateMissingDir() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/missingDir/foo");
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreateMissingDir2() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/missingDir/miss2/foo");
   }
-  
-  
-  @Test(expected=AccessControlException.class) 
+
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreateMissingDir3() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/internalDir/miss2/foo");
   }
-  
+
   // Delete on internal mount table should fail
-  
-  @Test(expected=FileNotFoundException.class) 
+
+  @Test(expected=FileNotFoundException.class)
   public void testInternalDeleteNonExisting() throws IOException {
     fsView.delete(new Path("/NonExisting"), false);
   }
-  @Test(expected=FileNotFoundException.class) 
+  @Test(expected=FileNotFoundException.class)
   public void testInternalDeleteNonExisting2() throws IOException {
     fsView.delete(new Path("/internalDir/NonExisting"), false);
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalDeleteExisting() throws IOException {
     fsView.delete(new Path("/internalDir"), false);
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalDeleteExisting2() throws IOException {
     fsView.getFileStatus(
             new Path("/internalDir/linkToDir2")).isDirectory();
     fsView.delete(new Path("/internalDir/linkToDir2"), false);
-  } 
-  
+  }
+
   @Test
   public void testMkdirOfMountLink() throws IOException {
     // data exists - mkdirs returns true even though no permission in internal
     // mount table
-    Assert.assertTrue("mkdir of existing mount link should succeed", 
+    Assert.assertTrue("mkdir of existing mount link should succeed",
         fsView.mkdirs(new Path("/data")));
   }
-  
-  
+
+
   // Rename on internal mount table should fail
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalRename1() throws IOException {
     fsView.rename(new Path("/internalDir"), new Path("/newDir"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRename2() throws IOException {
     fsView.getFileStatus(new Path("/internalDir/linkToDir2")).isDirectory();
     fsView.rename(new Path("/internalDir/linkToDir2"),
         new Path("/internalDir/dir1"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRename3() throws IOException {
     fsView.rename(new Path("/user"), new Path("/internalDir/linkToDir2"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRenameToSlash() throws IOException {
     fsView.rename(new Path("/internalDir/linkToDir2/foo"), new Path("/"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRenameFromSlash() throws IOException {
     fsView.rename(new Path("/"), new Path("/bar"));
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalSetOwner() throws IOException {
     fsView.setOwner(new Path("/internalDir"), "foo", "bar");
   }
-  
+
   @Test
   public void testCreateNonRecursive() throws IOException {
     Path path = fileSystemTestHelper.getTestRootPath(fsView, "/user/foo");
     fsView.createNonRecursive(path, false, 1024, (short)1, 1024L, null);
     FileStatus status = fsView.getFileStatus(new Path("/user/foo"));
     Assert.assertTrue("Created file should be type file",
-        fsView.isFile(new Path("/user/foo")));
+        fsView.getFileStatus(new Path("/user/foo")).isFile());
     Assert.assertTrue("Target of created file should be type file",
-        fsTarget.isFile(new Path(targetTestRoot, "user/foo")));
+        fsTarget.getFileStatus(new Path(targetTestRoot, "user/foo")).isFile());

Review Comment:
   removed



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -212,6 +225,33 @@ void addLink(final String pathComponent, final INodeLink<T> link)
       }
       children.put(pathComponent, link);
     }
+
+    void addDirLink(final String pathComponent, final INodeDirLink<T> dirLink) {
+      children.put(pathComponent, dirLink);
+    }
+  }
+
+  /**
+   * Internal class to represent nested mount points in INodeTree. Nested mount points are non-leaf nodes in INode tree.
+   * Hence it inherits INodeDir. But it has additional attributes so wrap it with link.
+   * @param <T>
+   */
+  static class INodeDirLink<T> extends INodeDir<T> {

Review Comment:
   isLink() is mostly used inside INodeTree createLink() and resolve() methods to detect whether the node is leaf(INodeLink) or not. 
   The current semantics without nested mp are all leaf nodes are INodeLink(isLink=true), all non-leaf nodes are INodeDir(isLink=false)
   With nested mp, INodeDirLink is a non-leaf node but also has link to it so whether to set isLink() to true is open to discussion. That is why i use another property isDirLink and use it anywhere.
   



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -403,24 +455,24 @@ private void createLink(final String src, final String target,
     final String fullPath = curInode.fullPath + (curInode == root ? "" : "/")
         + iPath;
     switch (linkType) {
-    case SINGLE:
-      newLink = new INodeLink<T>(fullPath, aUgi,
-          initAndGetTargetFs(), target);
-      break;
-    case SINGLE_FALLBACK:
-    case MERGE_SLASH:
-      // Link fallback and link merge slash configuration
-      // are handled specially at InodeTree.
-      throw new IllegalArgumentException("Unexpected linkType: " + linkType);
-    case MERGE:
-    case NFLY:
-      final String[] targetUris = StringUtils.getStrings(target);
-      newLink = new INodeLink<T>(fullPath, aUgi,
-          getTargetFileSystem(settings, StringUtils.stringToURI(targetUris)),
-          targetUris);
-      break;
-    default:
-      throw new IllegalArgumentException(linkType + ": Infeasible linkType");
+      case SINGLE:

Review Comment:
   Removed this change



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -682,6 +736,25 @@ protected InodeTree(final Configuration config, final String viewName,
     }
   }
 
+  private Collection<LinkEntry> getLinkEntries(List<LinkEntry> linkEntries) {
+    if (isNestedMountPointSupported) {

Review Comment:
   Added



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -377,9 +422,16 @@ private void createLink(final String src, final String target,
         nextInode = newDir;
       }
       if (nextInode.isLink()) {
-        // Error - expected a dir but got a link
-        throw new FileAlreadyExistsException("Path " + nextInode.fullPath +
-            " already exists as link");
+        if (isNestedMountPointSupported) {
+          // nested mount detected, add a new INodeDirLink that wraps existing INodeLink to INodeTree and override existing INodelink
+          INodeDirLink<T> dirLink = new INodeDirLink<T>(nextInode.fullPath, aUgi, (INodeLink<T>) nextInode);

Review Comment:
   I added more details in the jira. The dirLink will only take path and link. Dirlink is INodeDir traversal wise(there are couple of places in the code that assumes the non-leaf node is INodeDir so we cannot really DirLink a link here) and internalDirFs wouldn't apply to DirLink since it always has a nested mp associated with it.



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -212,6 +227,43 @@ void addLink(final String pathComponent, final INodeLink<T> link)
       }
       children.put(pathComponent, link);
     }
+
+    void addDirLink(final String pathComponent, final INodeDirLink<T> dirLink) {
+      children.put(pathComponent, dirLink);
+    }
+  }
+
+  /**
+   * Internal class to represent a INodeDir which also contains a INodeLink. This is used to support nested mount point

Review Comment:
   Fixed



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -212,6 +225,33 @@ void addLink(final String pathComponent, final INodeLink<T> link)
       }
       children.put(pathComponent, link);
     }
+
+    void addDirLink(final String pathComponent, final INodeDirLink<T> dirLink) {
+      children.put(pathComponent, dirLink);
+    }
+  }
+
+  /**
+   * Internal class to represent nested mount points in INodeTree. Nested mount points are non-leaf nodes in INode tree.
+   * Hence it inherits INodeDir. But it has additional attributes so wrap it with link.
+   * @param <T>
+   */
+  static class INodeDirLink<T> extends INodeDir<T> {

Review Comment:
   Added



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -682,6 +748,32 @@ protected InodeTree(final Configuration config, final String viewName,
     }
   }
 
+  /**
+   * Get collection of linkEntry. If nested mount point is supported, sort mount point src path based on alphabetical order.
+   * The purpose is to group nested path(shortest path always comes first) during INodeTree creation.

Review Comment:
   Fixed



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java:
##########
@@ -167,8 +167,21 @@ void setupMountPoints() {
         new Path(targetTestRoot, "missingTarget").toUri());
     ConfigUtil.addLink(conf, "/linkToAFile",
         new Path(targetTestRoot, "aFile").toUri());
+
+    // Enable nested mount point, ViewFilesystem should support both non-nested and nested mount points
+    ConfigUtil.setIsNestedMountPointSupported(conf, true);
+    ConfigUtil.addLink(conf, "/user/userA",
+        new Path(targetTestRoot, "user").toUri());
+    ConfigUtil.addLink(conf, "/user/userB",
+        new Path(targetTestRoot, "userB").toUri());
+    ConfigUtil.addLink(conf, "/data/dataA",
+        new Path(targetTestRoot, "dataA").toUri());
+    ConfigUtil.addLink(conf, "/data/dataB",
+        new Path(targetTestRoot, "user").toUri());
+    ConfigUtil.addLink(conf, "/internalDir/linkToDir2/linkToDir2",
+        new Path(targetTestRoot,"linkToDir2").toUri());

Review Comment:
   I created a new test class that extends ViewFileSystemBaseTest



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java:
##########
@@ -263,50 +276,79 @@ private void testOperationsThroughMountLinksInternal(boolean located)
     // Create file
     fileSystemTestHelper.createFile(fsView, "/user/foo");
     Assert.assertTrue("Created file should be type file",
-        fsView.isFile(new Path("/user/foo")));
+        fsView.getFileStatus(new Path("/user/foo")).isFile());
     Assert.assertTrue("Target of created file should be type file",
-        fsTarget.isFile(new Path(targetTestRoot,"user/foo")));
-    
+        fsTarget.getFileStatus(new Path(targetTestRoot,"user/foo")).isFile());
+
+    // Create file with nested mp
+    fileSystemTestHelper.createFile(fsView, "/user/userB/foo");
+    Assert.assertTrue("Created file should be type file",
+        fsView.getFileStatus(new Path("/user/userB/foo")).isFile());
+    Assert.assertTrue("Target of created file should be type file",
+        fsTarget.getFileStatus(new Path(targetTestRoot,"userB/foo")).isFile());
+
     // Delete the created file
     Assert.assertTrue("Delete should succeed",
         fsView.delete(new Path("/user/foo"), false));
     Assert.assertFalse("File should not exist after delete",
         fsView.exists(new Path("/user/foo")));
     Assert.assertFalse("Target File should not exist after delete",
         fsTarget.exists(new Path(targetTestRoot,"user/foo")));
-    
+
+    // Delete the create file with nested mp
+    Assert.assertTrue("Delete should succeed",
+        fsView.delete(new Path("/user/userB/foo"), false));
+    Assert.assertFalse("File should not exist after delete",
+        fsView.exists(new Path("/user/userB/foo")));
+    Assert.assertFalse("Target File should not exist after delete",
+        fsTarget.exists(new Path(targetTestRoot,"userB/foo")));
+
     // Create file with a 2 component dirs
     fileSystemTestHelper.createFile(fsView, "/internalDir/linkToDir2/foo");
     Assert.assertTrue("Created file should be type file",
-        fsView.isFile(new Path("/internalDir/linkToDir2/foo")));
+        fsView.getFileStatus(new Path("/internalDir/linkToDir2/foo")).isFile());
     Assert.assertTrue("Target of created file should be type file",
-        fsTarget.isFile(new Path(targetTestRoot,"dir2/foo")));
-    
+        fsTarget.getFileStatus(new Path(targetTestRoot,"dir2/foo")).isFile());
+
+    // Create file with a 2 component dirs with nested mp
+    fileSystemTestHelper.createFile(fsView, "/internalDir/linkToDir2/linkToDir2/foo");
+    Assert.assertTrue("Created file should be type file",
+        fsView.getFileStatus(new Path("/internalDir/linkToDir2/linkToDir2/foo")).isFile());
+    Assert.assertTrue("Target of created file should be type file",
+        fsTarget.getFileStatus(new Path(targetTestRoot,"linkToDir2/foo")).isFile());
+
     // Delete the created file
     Assert.assertTrue("Delete should succeed",
         fsView.delete(new Path("/internalDir/linkToDir2/foo"), false));
     Assert.assertFalse("File should not exist after delete",
         fsView.exists(new Path("/internalDir/linkToDir2/foo")));
     Assert.assertFalse("Target File should not exist after delete",
         fsTarget.exists(new Path(targetTestRoot,"dir2/foo")));
-    
-    
+
+    // Delete the created file with nested mp
+    Assert.assertTrue("Delete should succeed",
+        fsView.delete(new Path("/internalDir/linkToDir2/linkToDir2/foo"), false));
+    Assert.assertFalse("File should not exist after delete",
+        fsView.exists(new Path("/internalDir/linkToDir2/linkToDir2/foo")));
+    Assert.assertFalse("Target File should not exist after delete",
+        fsTarget.exists(new Path(targetTestRoot,"linkToDir2/foo")));
+
     // Create file with a 3 component dirs
     fileSystemTestHelper.createFile(fsView, "/internalDir/internalDir2/linkToDir3/foo");
     Assert.assertTrue("Created file should be type file",
-        fsView.isFile(new Path("/internalDir/internalDir2/linkToDir3/foo")));
+        fsView.getFileStatus(new Path("/internalDir/internalDir2/linkToDir3/foo")).isFile());

Review Comment:
   removed deprecated call changes



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java:
##########
@@ -839,44 +938,46 @@ public ResolveResult<T> resolve(final String p, final boolean resolveLastCompone
 
       if (nextInode.isLink()) {
         final INodeLink<T> link = (INodeLink<T>) nextInode;
-        final Path remainingPath;
-        if (i >= path.length - 1) {
-          remainingPath = SlashPath;
-        } else {
-          StringBuilder remainingPathStr =
-              new StringBuilder("/" + path[i + 1]);
-          for (int j = i + 2; j < path.length; ++j) {
-            remainingPathStr.append('/').append(path[j]);
-          }
-          remainingPath = new Path(remainingPathStr.toString());
-        }
+        final Path remainingPath = getRemainingPath(path, i + 1);
         resolveResult = new ResolveResult<T>(ResultKind.EXTERNAL_DIR,
             link.getTargetFileSystem(), nextInode.fullPath, remainingPath,
             true);
         return resolveResult;
       } else if (nextInode.isInternalDir()) {
         curInode = (INodeDir<T>) nextInode;
+        // track last resolved nest mount point.
+        if (isNestedMountPointSupported && nextInode.isDirAndLink()) {
+          lastResolvedDirLink = (INodeDirLink<T>) nextInode;
+          lastResolvedDirLinkIndex = i;
+        }
       }
     }
 
-    // We have resolved to an internal dir in mount table.
     Path remainingPath;
-    if (resolveLastComponent) {
+    if (isNestedMountPointSupported && lastResolvedDirLink != null) {
+      remainingPath = getRemainingPath(path, lastResolvedDirLinkIndex + 1);
+      resolveResult = new ResolveResult<T>(ResultKind.EXTERNAL_DIR, lastResolvedDirLink.getLink().getTargetFileSystem(),
+          lastResolvedDirLink.fullPath, remainingPath,true);
+    } else {
+      remainingPath = resolveLastComponent ? SlashPath : getRemainingPath(path, i);
+      resolveResult = new ResolveResult<T>(ResultKind.INTERNAL_DIR, curInode.getInternalDirFs(),
+          curInode.fullPath, remainingPath, false);
+    }
+    return resolveResult;
+  }
+
+  private Path getRemainingPath(String[] path, int startIndex) {

Review Comment:
   Added



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java:
##########
@@ -662,177 +790,177 @@ public void testResolvePathMountPoints() throws IOException {
         fsView.resolvePath(new Path("/internalDir/internalDir2/linkToDir3")));
 
   }
-  
+
   @Test
   public void testResolvePathThroughMountPoints() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/user/foo");
     Assert.assertEquals(new Path(targetTestRoot,"user/foo"),
         fsView.resolvePath(new Path("/user/foo")));
-    
+
     fsView.mkdirs(
         fileSystemTestHelper.getTestRootPath(fsView, "/user/dirX"));
     Assert.assertEquals(new Path(targetTestRoot,"user/dirX"),
         fsView.resolvePath(new Path("/user/dirX")));
 
-    
+
     fsView.mkdirs(
         fileSystemTestHelper.getTestRootPath(fsView, "/user/dirX/dirY"));
     Assert.assertEquals(new Path(targetTestRoot,"user/dirX/dirY"),
         fsView.resolvePath(new Path("/user/dirX/dirY")));
   }
 
-  @Test(expected=FileNotFoundException.class) 
+  @Test(expected=FileNotFoundException.class)
   public void testResolvePathDanglingLink() throws IOException {
     fsView.resolvePath(new Path("/danglingLink"));
   }
-  
-  @Test(expected=FileNotFoundException.class) 
+
+  @Test(expected=FileNotFoundException.class)
   public void testResolvePathMissingThroughMountPoints() throws IOException {
     fsView.resolvePath(new Path("/user/nonExisting"));
   }
-  
 
-  @Test(expected=FileNotFoundException.class) 
+
+  @Test(expected=FileNotFoundException.class)
   public void testResolvePathMissingThroughMountPoints2() throws IOException {
     fsView.mkdirs(
         fileSystemTestHelper.getTestRootPath(fsView, "/user/dirX"));
     fsView.resolvePath(new Path("/user/dirX/nonExisting"));
   }
-  
+
   /**
-   * Test modify operations (create, mkdir, rename, etc) 
+   * Test modify operations (create, mkdir, rename, etc)
    * on internal dirs of mount table
    * These operations should fail since the mount table is read-only or
    * because the internal dir that it is trying to create already
    * exits.
    */
- 
- 
+
+
   // Mkdir on existing internal mount table succeed except for /
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalMkdirSlash() throws IOException {
     fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView, "/"));
   }
-  
+
   public void testInternalMkdirExisting1() throws IOException {
-    Assert.assertTrue("mkdir of existing dir should succeed", 
+    Assert.assertTrue("mkdir of existing dir should succeed",
         fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView,
         "/internalDir")));
   }
 
   public void testInternalMkdirExisting2() throws IOException {
-    Assert.assertTrue("mkdir of existing dir should succeed", 
+    Assert.assertTrue("mkdir of existing dir should succeed",
         fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView,
         "/internalDir/linkToDir2")));
   }
-  
+
   // Mkdir for new internal mount table should fail
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalMkdirNew() throws IOException {
     fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView, "/dirNew"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalMkdirNew2() throws IOException {
     fsView.mkdirs(fileSystemTestHelper.getTestRootPath(fsView, "/internalDir/dirNew"));
   }
-  
+
   // Create File on internal mount table should fail
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreate1() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/foo"); // 1 component
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreate2() throws IOException {  // 2 component
     fileSystemTestHelper.createFile(fsView, "/internalDir/foo");
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreateMissingDir() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/missingDir/foo");
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreateMissingDir2() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/missingDir/miss2/foo");
   }
-  
-  
-  @Test(expected=AccessControlException.class) 
+
+
+  @Test(expected=AccessControlException.class)
   public void testInternalCreateMissingDir3() throws IOException {
     fileSystemTestHelper.createFile(fsView, "/internalDir/miss2/foo");
   }
-  
+
   // Delete on internal mount table should fail
-  
-  @Test(expected=FileNotFoundException.class) 
+
+  @Test(expected=FileNotFoundException.class)
   public void testInternalDeleteNonExisting() throws IOException {
     fsView.delete(new Path("/NonExisting"), false);
   }
-  @Test(expected=FileNotFoundException.class) 
+  @Test(expected=FileNotFoundException.class)
   public void testInternalDeleteNonExisting2() throws IOException {
     fsView.delete(new Path("/internalDir/NonExisting"), false);
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalDeleteExisting() throws IOException {
     fsView.delete(new Path("/internalDir"), false);
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalDeleteExisting2() throws IOException {
     fsView.getFileStatus(
             new Path("/internalDir/linkToDir2")).isDirectory();
     fsView.delete(new Path("/internalDir/linkToDir2"), false);
-  } 
-  
+  }
+
   @Test
   public void testMkdirOfMountLink() throws IOException {
     // data exists - mkdirs returns true even though no permission in internal
     // mount table
-    Assert.assertTrue("mkdir of existing mount link should succeed", 
+    Assert.assertTrue("mkdir of existing mount link should succeed",
         fsView.mkdirs(new Path("/data")));
   }
-  
-  
+
+
   // Rename on internal mount table should fail
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalRename1() throws IOException {
     fsView.rename(new Path("/internalDir"), new Path("/newDir"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRename2() throws IOException {
     fsView.getFileStatus(new Path("/internalDir/linkToDir2")).isDirectory();
     fsView.rename(new Path("/internalDir/linkToDir2"),
         new Path("/internalDir/dir1"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRename3() throws IOException {
     fsView.rename(new Path("/user"), new Path("/internalDir/linkToDir2"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRenameToSlash() throws IOException {
     fsView.rename(new Path("/internalDir/linkToDir2/foo"), new Path("/"));
   }
-  @Test(expected=AccessControlException.class) 
+  @Test(expected=AccessControlException.class)
   public void testInternalRenameFromSlash() throws IOException {
     fsView.rename(new Path("/"), new Path("/bar"));
   }
-  
-  @Test(expected=AccessControlException.class) 
+
+  @Test(expected=AccessControlException.class)
   public void testInternalSetOwner() throws IOException {
     fsView.setOwner(new Path("/internalDir"), "foo", "bar");
   }
-  
+
   @Test
   public void testCreateNonRecursive() throws IOException {
     Path path = fileSystemTestHelper.getTestRootPath(fsView, "/user/foo");
     fsView.createNonRecursive(path, false, 1024, (short)1, 1024L, null);
     FileStatus status = fsView.getFileStatus(new Path("/user/foo"));
     Assert.assertTrue("Created file should be type file",
-        fsView.isFile(new Path("/user/foo")));
+        fsView.getFileStatus(new Path("/user/foo")).isFile());

Review Comment:
   removed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org