ayushtkn commented on a change in pull request #2088:
URL: https://github.com/apache/hadoop/pull/2088#discussion_r443999571
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1258,63 +1261,72 @@ public FileStatus getFileStatus(Path f) throws
IOException {
FileStatus status =
((ChRootedFileSystem)link.getTargetFileSystem())
.getMyFs().getFileStatus(new Path(linkedPath));
- result[i++] = new FileStatus(status.getLen(), status.isDirectory(),
- status.getReplication(), status.getBlockSize(),
- status.getModificationTime(), status.getAccessTime(),
- status.getPermission(), status.getOwner(), status.getGroup(),
- null, path);
+ linkStatuses.add(
+ new FileStatus(status.getLen(), status.isDirectory(),
+ status.getReplication(), status.getBlockSize(),
+ status.getModificationTime(), status.getAccessTime(),
+ status.getPermission(), status.getOwner(),
+ status.getGroup(), null, path));
} catch (FileNotFoundException ex) {
LOG.warn("Cannot get one of the children's(" + path
+ ") target path(" + link.getTargetFileSystem().getUri()
+ ") file status.", ex);
throw ex;
}
} else {
- result[i++] =
+ internalDirStatuses.add(
new FileStatus(0, true, 0, 0, creationTime, creationTime,
PERMISSION_555, ugi.getShortUserName(),
- ugi.getPrimaryGroupName(), path);
+ ugi.getPrimaryGroupName(), path));
}
}
+ FileStatus[] internalDirStatusesMergedWithFallBack = internalDirStatuses
+ .toArray(new FileStatus[internalDirStatuses.size()]);
if (fallbackStatuses.length > 0) {
- return consolidateFileStatuses(fallbackStatuses, result);
- } else {
- return result;
+ internalDirStatusesMergedWithFallBack =
+ merge(fallbackStatuses, internalDirStatusesMergedWithFallBack);
}
+ // Links will always have precedence than internalDir or fallback paths.
+ return merge(linkStatuses.toArray(new FileStatus[linkStatuses.size()]),
+ internalDirStatusesMergedWithFallBack);
}
- private FileStatus[] consolidateFileStatuses(FileStatus[] fallbackStatuses,
- FileStatus[] mountPointStatuses) {
+ private FileStatus[] merge(FileStatus[] toStatuses,
+ FileStatus[] fromStatuses) {
ArrayList<FileStatus> result = new ArrayList<>();
Set<String> pathSet = new HashSet<>();
- for (FileStatus status : mountPointStatuses) {
+ for (FileStatus status : toStatuses) {
result.add(status);
pathSet.add(status.getPath().getName());
}
- for (FileStatus status : fallbackStatuses) {
+ for (FileStatus status : fromStatuses) {
if (!pathSet.contains(status.getPath().getName())) {
result.add(status);
}
}
- return result.toArray(new FileStatus[0]);
+ return result.toArray(new FileStatus[result.size()]);
}
private FileStatus[] listStatusForFallbackLink() throws IOException {
- if (theInternalDir.isRoot() &&
- theInternalDir.getFallbackLink() != null) {
- FileSystem linkedFs =
- theInternalDir.getFallbackLink().getTargetFileSystem();
- // Fallback link is only applicable for root
- FileStatus[] statuses = linkedFs.listStatus(new Path("/"));
- for (FileStatus status : statuses) {
- // Fix the path back to viewfs scheme
- status.setPath(
- new Path(myUri.toString(), status.getPath().getName()));
+ if (this.fsState.getRootFallbackLink() != null) {
+ FileSystem linkedFallbackFs =
+ this.fsState.getRootFallbackLink().getTargetFileSystem();
+ Path p = Path.getPathWithoutSchemeAndAuthority(
+ new Path(theInternalDir.fullPath));
+ if (theInternalDir.isRoot() || linkedFallbackFs.exists(p)) {
+ // Fallback link is only applicable for root
Review comment:
This comment line can be removed now? No it isn't just root
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemLinkFallback.java
##########
@@ -359,4 +361,249 @@ public void
testListingWithFallbackLinkWithSameMountDirectories()
assertTrue(vfs.getFileStatus(childDir).isDirectory());
}
}
+
+ /**
+ * Tests ListStatus on non-link parent with fallback configured.
+ *
=============================Example.======================================
+ * ===== Fallback path tree =============== Mount Path Tree
==================
+ *
===========================================================================
+ * * / ***** /
*****************
+ * * / ***** /
*****************
+ * * user1 ***** user1
*****************
+ * * / ***** /
*****************
+ * * hive ***** hive
*****************
+ * * / \ ***** /
*****************
+ * * warehouse warehouse1 ***** warehouse
*****************
+ * * (-rwxr--r--) ***** (-r-xr--r--)
*****************
+ * * / ***** /
*****************
+ * * partition-0 ***** partition-0
*****************
+ *
===========================================================================
+ *
===========================================================================
+ * *** ls /user1/hive
*********
+ * *** viewfs://default/user1/hive/warehouse (-rwxr--r--)
*********
+ * *** viewfs://default/user1/hive/warehouse1
*********
+ *
===========================================================================
+ */
+ @Test
+ public void testListingWithFallbackLinkWithSameMountDirectoryTree()
+ throws Exception {
+ Configuration conf = new Configuration();
+ conf.setBoolean(Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS, false);
+ ConfigUtil.addLink(conf, "/user1/hive/warehouse/partition-0",
+ new Path(targetTestRoot.toString()).toUri());
+ // Creating multiple directories path under the fallback directory.
+ // "/user1/hive/warehouse/partition-0" directory already exists as
+ // configured mount point.
+ Path dir1 = new Path(targetTestRoot,
+ "fallbackDir/user1/hive/warehouse/partition-0");
+ Path dir2 = new Path(targetTestRoot, "fallbackDir/user1/hive/warehouse1");
+ fsTarget.mkdirs(dir1);
+ fsTarget.mkdirs(dir2);
+ fsTarget.setPermission(new Path(targetTestRoot, "fallbackDir/user1/hive/"),
+ FsPermission.valueOf("-rwxr--r--"));
+ URI viewFsUri = new URI(FsConstants.VIEWFS_SCHEME,
+ Constants.CONFIG_VIEWFS_DEFAULT_MOUNT_TABLE, "/", null, null);
+
+ HashSet<Path> beforeFallback = new HashSet<>();
+ try (FileSystem vfs = FileSystem.get(viewFsUri, conf)) {
+ for (FileStatus stat : vfs
+ .listStatus(new Path(viewFsUri.toString(), "/user1/hive/"))) {
+ beforeFallback.add(stat.getPath());
+ }
+ }
+ ConfigUtil
+ .addLinkFallback(conf, new Path(targetTestRoot,
"fallbackDir").toUri());
+
+ try (FileSystem vfs = FileSystem.get(viewFsUri, conf)) {
+ HashSet<Path> afterFallback = new HashSet<>();
+ for (FileStatus stat : vfs
+ .listStatus(new Path(viewFsUri.toString(), "/user1/hive/"))) {
+ afterFallback.add(stat.getPath());
+ if (dir1.getName().equals(stat.getPath().getName())) {
+ // make sure fallback dir listed out with correct permissions, but
not
+ // with link permissions.
+ assertEquals(FsPermission.valueOf("-rwxr--r--"),
+ stat.getPermission());
+ }
+ }
+ //
+ //viewfs://default/user1/hive/warehouse
+ afterFallback.removeAll(beforeFallback);
+ assertTrue("The same directory name in fallback link should be shaded",
Review comment:
nit: can be assertEqual()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
