[
https://issues.apache.org/jira/browse/HADOOP-18193?focusedWorklogId=767029&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-767029
]
ASF GitHub Bot logged work on HADOOP-18193:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 06/May/22 06:15
Start Date: 06/May/22 06:15
Worklog Time Spent: 10m
Work Description: virajith commented on code in PR #4181:
URL: https://github.com/apache/hadoop/pull/4181#discussion_r866488045
##########
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:
nit:
"support nested mount point where a INode is internalDir but would also
point to a mount link" ->
"support nested mount points where an INode is an internalDir and points to
a mount link."
##########
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:
nit: "nested path(shortest" -> "nested paths (shortest"
##########
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:
nit: "sort mount point src path based on alphabetical order." -> "sort mount
points based on alphabetical order of the src paths"
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java:
##########
@@ -460,30 +502,116 @@ public void testRenameAcrossMounts4() throws IOException
{
.assertIsFile(fsTarget, new Path(targetTestRoot, "data/fooBar"));
}
+ // rename across nested mount points that point to same target also fail
+ @Test
+ public void testRenameAcrossNestedMountPointSameTarget() throws IOException {
+ fileSystemTestHelper.createFile(fsView, "/user/foo");
+ try {
+ // Nested mount points point to the same target should fail
+ // /user -> /user
+ // /user/userA -> /user
+ // Rename strategy: SAME_MOUNTPOINT
+ fsView.rename(new Path("/user/foo"), new Path("/user/userA/foo"));
+ ContractTestUtils.fail("IOException is not thrown on rename operation");
+ } catch (IOException e) {
+ GenericTestUtils
+ .assertExceptionContains("Renames across Mount points not supported",
+ e);
+ }
+ }
+
+
+ // rename across nested mount points fail if the mount link targets are
different
+ // even if the targets are part of the same target FS
+ @Test
+ public void testRenameAcrossMountPointDifferentTarget() throws IOException {
+ fileSystemTestHelper.createFile(fsView, "/data/foo");
+ // /data -> /data
+ // /data/dataA -> /dataA
+ // Rename strategy: SAME_MOUNTPOINT
+ try {
+ fsView.rename(new Path("/data/foo"), new Path("/data/dataA/fooBar"));
+ ContractTestUtils.fail("IOException is not thrown on rename operation");
+ } catch (IOException e) {
+ GenericTestUtils
+ .assertExceptionContains("Renames across Mount points not supported",
+ e);
+ }
+ }
+
+ // RenameStrategy SAME_TARGET_URI_ACROSS_MOUNTPOINT enabled
+ // to rename across nested mount points that point to same target URI
+ @Test
+ public void testRenameAcrossNestedMountPointSameTargetUriAcrossMountPoint()
throws IOException {
+ // /user/foo -> /user
+ // /user/userA/fooBarBar -> /user
+ // Rename strategy: SAME_TARGET_URI_ACROSS_MOUNTPOINT
+ Configuration conf2 = new Configuration(conf);
+ conf2.set(Constants.CONFIG_VIEWFS_RENAME_STRATEGY,
+ ViewFileSystem.RenameStrategy.SAME_TARGET_URI_ACROSS_MOUNTPOINT
+ .toString());
+ FileSystem fsView2 = FileSystem.newInstance(FsConstants.VIEWFS_URI, conf2);
+ fileSystemTestHelper.createFile(fsView2, "/user/foo");
+ fsView2.rename(new Path("/user/foo"), new Path("/user/userA/fooBarBar"));
+ ContractTestUtils.assertPathDoesNotExist(fsView2, "src should not exist
after rename",
+ new Path("/user/foo"));
+ ContractTestUtils.assertPathDoesNotExist(fsTarget, "src should not exist
after rename",
+ new Path(targetTestRoot, "user/foo"));
+ ContractTestUtils.assertIsFile(fsView2,
fileSystemTestHelper.getTestRootPath(fsView2, "/user/userA/fooBarBar"));
+ ContractTestUtils.assertIsFile(fsTarget, new Path(targetTestRoot,
"user/fooBarBar"));
+ }
+
+ // RenameStrategy SAME_FILESYSTEM_ACROSS_MOUNTPOINT enabled
+ // to rename across mount points where the mount link targets are different
+ // but are part of the same target FS
+ @Test
+ public void testRenameAcrossNestedMountPointSameFileSystemAcrossMountPoint()
throws IOException {
+ // /data/foo -> /data
+ // /data/dataA/fooBar -> /dataA
+ // Rename strategy: SAME_FILESYSTEM_ACROSS_MOUNTPOINT
+ Configuration conf2 = new Configuration(conf);
+ conf2.set(Constants.CONFIG_VIEWFS_RENAME_STRATEGY,
+ ViewFileSystem.RenameStrategy.SAME_FILESYSTEM_ACROSS_MOUNTPOINT
+ .toString());
+ FileSystem fsView2 = FileSystem.newInstance(FsConstants.VIEWFS_URI, conf2);
+ fileSystemTestHelper.createFile(fsView2, "/data/foo");
+ fsView2.rename(new Path("/data/foo"), new Path("/data/dataB/fooBar"));
+ ContractTestUtils
+ .assertPathDoesNotExist(fsView2, "src should not exist after rename",
+ new Path("/data/foo"));
+ ContractTestUtils
+ .assertPathDoesNotExist(fsTarget, "src should not exist after rename",
+ new Path(targetTestRoot, "data/foo"));
+ ContractTestUtils.assertIsFile(fsView2,
+ fileSystemTestHelper.getTestRootPath(fsView2, "/user/fooBar"));
+ ContractTestUtils
+ .assertIsFile(fsTarget, new Path(targetTestRoot, "user/fooBar"));
+ }
+
static protected boolean SupportsBlocks = false; // local fs use 1 block
// override for HDFS
@Test
public void testGetBlockLocations() throws IOException {
Path targetFilePath = new Path(targetTestRoot,"data/largeFile");
- FileSystemTestHelper.createFile(fsTarget,
+ FileSystemTestHelper.createFile(fsTarget,
targetFilePath, 10, 1024);
Path viewFilePath = new Path("/data/largeFile");
Assert.assertTrue("Created File should be type File",
- fsView.isFile(viewFilePath));
+ fsView.getFileStatus(viewFilePath).isFile());
Review Comment:
avoid - not related
##########
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
Review Comment:
Replace the same for other places "mp" is used.
##########
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());
Review Comment:
avoid this change - not related?
##########
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
Review Comment:
nit: mp -> mount point.
##########
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
Review Comment:
nit: "create file" -> "created file"
##########
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:
you seem to have replaced a bunch of deprecated calls here - don't mix that
up with this PR. You can have a separate PR to clean this up. It confuses the
reader as to how the changes are related to the current change.
##########
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:
javadoc
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java:
##########
@@ -167,8 +169,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);
Review Comment:
Can you create a new test case in this class for these? It would be good to
keep the nested mount points logic separate
##########
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:
avoid - not related
##########
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:
avoid - not related
##########
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:
Add a test for listStatus?
##########
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:
Thanks for adding the comments for this.
##########
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:
You are basically saying isLink() is true only for leaf links and is always
false for internal inodes even if they are INodeDirLinks
Issue Time Tracking
-------------------
Worklog Id: (was: 767029)
Time Spent: 3h 10m (was: 3h)
> 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 10m
> 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]
