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/04/15 22:05:48 UTC

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

mccormickt12 commented on code in PR #4181:
URL: https://github.com/apache/hadoop/pull/4181#discussion_r851530313


##########
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:
   each dirlink, only has one nested mount point. Could could add one dirlink that has multiple nested mount points under it.



##########
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);
+    ConfigUtil.addLink(conf, mtName, "/a/b/c/d", NN3_TARGET);
+    ConfigUtil.addLink(conf, mtName, "/a/b/c/d/e", NN4_TARGET);
+    ConfigUtil.addLink(conf, mtName, "/b/c/d/e", NN5_TARGET);
+    ConfigUtil.addLink(conf, mtName, "/b/c/d/e/f", NN6_TARGET);
+    ConfigUtil.addLinkFallback(conf, mtName, LINKFALLBACK_TARGET);
+
+    fsUri = new URI(FsConstants.VIEWFS_SCHEME, mtName, "/", null, null);
+
+    inodeTree = new InodeTree<TestNestedMountPoint.TestNestMountPointFileSystem>(conf,
+        mtName, fsUri, false) {
+      @Override
+      protected Function<URI, TestNestedMountPoint.TestNestMountPointFileSystem> initAndGetTargetFs() {
+        return new Function<URI, TestNestMountPointFileSystem>() {
+          @Override
+          public TestNestedMountPoint.TestNestMountPointFileSystem apply(URI uri) {
+            return new TestNestMountPointFileSystem(uri);
+          }
+        };
+      }
+
+      // For intenral dir fs
+      @Override
+      protected TestNestedMountPoint.TestNestMountPointInternalFileSystem getTargetFileSystem(
+          final INodeDir<TestNestedMountPoint.TestNestMountPointFileSystem> dir) {
+        return new TestNestMountPointInternalFileSystem(fsUri);
+      }
+
+      @Override
+      protected TestNestedMountPoint.TestNestMountPointInternalFileSystem getTargetFileSystem(
+          final String settings, final URI[] mergeFsURIList) {
+        return new TestNestMountPointInternalFileSystem(null);
+      }
+    };
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    inodeTree = null;
+  }
+
+  @Test
+  public void testPathResolveToLink() throws Exception {
+    // /a/b/c/d/e/f resolves to /a/b/c/d/e and /f
+    InodeTree.ResolveResult resolveResult = inodeTree.resolve("/a/b/c/d/e/f", true);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult.kind);
+    Assert.assertEquals("/a/b/c/d/e", resolveResult.resolvedPath);
+    Assert.assertEquals(new Path("/f"), resolveResult.remainingPath);
+    Assert.assertTrue(resolveResult.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN4_TARGET, ((TestNestMountPointFileSystem) resolveResult.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult.isLastInternalDirLink());
+
+    // /a/b/c/d/e resolves to /a/b/c/d/e and /
+    InodeTree.ResolveResult resolveResult2 = inodeTree.resolve("/a/b/c/d/e", true);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult2.kind);
+    Assert.assertEquals("/a/b/c/d/e", resolveResult2.resolvedPath);
+    Assert.assertEquals(new Path("/"), resolveResult2.remainingPath);
+    Assert.assertTrue(resolveResult2.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN4_TARGET, ((TestNestMountPointFileSystem) resolveResult2.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult2.isLastInternalDirLink());
+
+    // /a/b/c/d/e/f/g/h/i resolves to /a/b/c/d/e and /f/g/h/i
+    InodeTree.ResolveResult resolveResult3 = inodeTree.resolve("/a/b/c/d/e/f/g/h/i", true);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult3.kind);
+    Assert.assertEquals("/a/b/c/d/e", resolveResult3.resolvedPath);
+    Assert.assertEquals(new Path("/f/g/h/i"), resolveResult3.remainingPath);
+    Assert.assertTrue(resolveResult3.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN4_TARGET, ((TestNestMountPointFileSystem) resolveResult3.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult3.isLastInternalDirLink());
+  }
+
+  @Test
+  public void testPathResolveToLink_NotResolveLastComponent() throws Exception {
+    // /a/b/c/d/e/f resolves to /a/b/c/d/e and /f
+    InodeTree.ResolveResult resolveResult = inodeTree.resolve("/a/b/c/d/e/f", false);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult.kind);
+    Assert.assertEquals("/a/b/c/d/e", resolveResult.resolvedPath);
+    Assert.assertEquals(new Path("/f"), resolveResult.remainingPath);
+    Assert.assertTrue(resolveResult.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN4_TARGET, ((TestNestMountPointFileSystem) resolveResult.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult.isLastInternalDirLink());
+
+    // /a/b/c/d/e resolves to /a/b/c/d and /e
+    InodeTree.ResolveResult resolveResult2 = inodeTree.resolve("/a/b/c/d/e", false);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult2.kind);
+    Assert.assertEquals("/a/b/c/d", resolveResult2.resolvedPath);
+    Assert.assertEquals(new Path("/e"), resolveResult2.remainingPath);
+    Assert.assertTrue(resolveResult2.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN3_TARGET, ((TestNestMountPointFileSystem) resolveResult2.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult2.isLastInternalDirLink());
+
+    // /a/b/c/d/e/f/g/h/i resolves to /a/b/c/d/e and /f/g/h/i
+    InodeTree.ResolveResult resolveResult3 = inodeTree.resolve("/a/b/c/d/e/f/g/h/i", false);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult3.kind);
+    Assert.assertEquals("/a/b/c/d/e", resolveResult3.resolvedPath);
+    Assert.assertEquals(new Path("/f/g/h/i"), resolveResult3.remainingPath);
+    Assert.assertTrue(resolveResult3.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN4_TARGET, ((TestNestMountPointFileSystem) resolveResult3.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult3.isLastInternalDirLink());
+  }
+
+  @Test
+  public void testPathResolveToDirLink() throws Exception {
+    // /a/b/c/d/f resolves to /a/b/c/d, /f
+    InodeTree.ResolveResult resolveResult = inodeTree.resolve("/a/b/c/d/f", true);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult.kind);
+    Assert.assertEquals("/a/b/c/d", resolveResult.resolvedPath);
+    Assert.assertEquals(new Path("/f"), resolveResult.remainingPath);
+    Assert.assertTrue(resolveResult.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN3_TARGET, ((TestNestMountPointFileSystem) resolveResult.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult.isLastInternalDirLink());
+
+    // /a/b/c/d resolves to /a/b/c/d and /
+    InodeTree.ResolveResult resolveResult2 = inodeTree.resolve("/a/b/c/d", true);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult2.kind);
+    Assert.assertEquals("/a/b/c/d", resolveResult2.resolvedPath);
+    Assert.assertEquals(new Path("/"), resolveResult2.remainingPath);
+    Assert.assertTrue(resolveResult2.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN3_TARGET, ((TestNestMountPointFileSystem) resolveResult2.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult2.isLastInternalDirLink());
+
+    // /a/b/c/d/f/g/h/i resolves to /a/b/c/d and /f/g/h/i
+    InodeTree.ResolveResult resolveResult3 = inodeTree.resolve("/a/b/c/d/f/g/h/i", true);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult3.kind);
+    Assert.assertEquals("/a/b/c/d", resolveResult3.resolvedPath);
+    Assert.assertEquals(new Path("/f/g/h/i"), resolveResult3.remainingPath);
+    Assert.assertTrue(resolveResult3.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN3_TARGET, ((TestNestMountPointFileSystem) resolveResult3.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult3.isLastInternalDirLink());
+  }
+
+  @Test
+  public void testPathResolveToDirLink_NotResolveLastComponent() throws Exception {
+    // /a/b/c/d/f resolves to /a/b/c/d, /f
+    InodeTree.ResolveResult resolveResult = inodeTree.resolve("/a/b/c/d/f", false);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult.kind);
+    Assert.assertEquals("/a/b/c/d", resolveResult.resolvedPath);
+    Assert.assertEquals(new Path("/f"), resolveResult.remainingPath);
+    Assert.assertTrue(resolveResult.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN3_TARGET, ((TestNestMountPointFileSystem) resolveResult.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult.isLastInternalDirLink());
+
+    // /a/b/c/d resolves to /a/b and /c/d
+    InodeTree.ResolveResult resolveResult2 = inodeTree.resolve("/a/b/c/d", false);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult2.kind);
+    Assert.assertEquals("/a/b", resolveResult2.resolvedPath);
+    Assert.assertEquals(new Path("/c/d"), resolveResult2.remainingPath);
+    Assert.assertTrue(resolveResult2.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN1_TARGET, ((TestNestMountPointFileSystem) resolveResult2.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult2.isLastInternalDirLink());
+
+    // /a/b/c/d/f/g/h/i resolves to /a/b/c/d and /f/g/h/i
+    InodeTree.ResolveResult resolveResult3 = inodeTree.resolve("/a/b/c/d/f/g/h/i", false);
+    Assert.assertEquals(InodeTree.ResultKind.EXTERNAL_DIR, resolveResult3.kind);
+    Assert.assertEquals("/a/b/c/d", resolveResult3.resolvedPath);
+    Assert.assertEquals(new Path("/f/g/h/i"), resolveResult3.remainingPath);
+    Assert.assertTrue(resolveResult3.targetFileSystem instanceof TestNestMountPointFileSystem);
+    Assert.assertEquals(NN3_TARGET, ((TestNestMountPointFileSystem) resolveResult3.targetFileSystem).getUri());
+    Assert.assertTrue(resolveResult3.isLastInternalDirLink());
+  }
+
+  @Test
+  public void testMultiNestedMountPointsPathResolveToDirLink() throws Exception {
+    // /a/b/f resolves to /a/b and /f

Review Comment:
   Here you have a test that tests a top level dirlink correctly picks the correct link instead of a nested mount point.
   Since essentially unlimited nesting of these dirlinks is supported, can you add another test that tests that a nested mount point also handles this scenario correctly



-- 
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