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
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
