[
https://issues.apache.org/jira/browse/HADOOP-18193?focusedWorklogId=768627&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768627
]
ASF GitHub Bot logged work on HADOOP-18193:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 10/May/22 18:07
Start Date: 10/May/22 18:07
Worklog Time Spent: 10m
Work Description: 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
Issue Time Tracking
-------------------
Worklog Id: (was: 768627)
Time Spent: 3h 40m (was: 3.5h)
> Support nested mount points in INodeTree
> ----------------------------------------
>
> Key: HADOOP-18193
> URL: https://issues.apache.org/jira/browse/HADOOP-18193
> Project: Hadoop Common
> Issue Type: Improvement
> Components: viewfs
> Affects Versions: 2.10.0
> Reporter: Lei Yang
> Assignee: Lei Yang
> Priority: Major
> Labels: pull-request-available
> Attachments: Nested Mount Point in ViewFs.pdf
>
> Time Spent: 3h 40m
> Remaining Estimate: 0h
>
> Defining following client mount table config is not supported in INodeTree
> and will throw FileAlreadyExistsException
>
> {code:java}
> fs.viewfs.mounttable.link./foo/bar=hdfs://nn1/foo/bar
> fs.viewfs.mounttable.link./foo=hdfs://nn02/foo
> {code}
> INodeTree has 2 methods that need change to support nested mount points.
> {code:java}
> createLink(): build INodeTree during fs init.
> resolve(): resolve path in INodeTree with viewfs apis.
> {code}
> ViewFileSystem and ViewFs maintains an INodeTree instance(fsState) in both
> classes and call fsState.resolve(..) to resolve path to specific mount point.
> INodeTree.resolve encapsulates the logic of nested mount point resolving. So
> no changes are expected in both classes.
> AC:
> # INodeTree.createlink should support creating nested mount
> points.(INodeTree is constructed during fs init)
> # INodeTree.resolve should support resolve path based on nested mount
> points. (INodeTree.resolve is used in viewfs apis)
> # No regression in existing ViewFileSystem and ViewFs apis.
> # Ensure some important apis are not broken with nested mount points.
> (Rename, getContentSummary, listStatus...)
>
> Spec:
> Please review attached pdf for spec about this feature.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
