ayushtkn commented on a change in pull request #2092:
URL: https://github.com/apache/hadoop/pull/2092#discussion_r445862114
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java
##########
@@ -1139,6 +1139,27 @@ public void mkdir(final Path dir, final FsPermission
permission,
if (theInternalDir.isRoot() && dir == null) {
throw new FileAlreadyExistsException("/ already exits");
}
+
+ if (this.fsState.getRootFallbackLink() != null) {
+ AbstractFileSystem linkedFallbackFs =
+ this.fsState.getRootFallbackLink().getTargetFileSystem();
+ Path p = Path.getPathWithoutSchemeAndAuthority(
+ new Path(theInternalDir.fullPath));
+ String child = (InodeTree.SlashPath.equals(dir)) ?
+ InodeTree.SlashPath.toString() :
+ dir.getName();
+ Path dirToCreate = new Path(p, child);
+ try {
+ linkedFallbackFs.mkdir(dirToCreate, permission, createParent);
+ } catch (IOException e) {
+ if (LOG.isDebugEnabled()) {
+ StringBuilder msg = new StringBuilder("Failed to create {}")
+ .append(" at fallback fs : {}");
+ LOG.debug(msg.toString(), dirToCreate, linkedFallbackFs.getUri());
+ }
+ }
+ }
+
Review comment:
Thanx @umamaheswararao for the work here,
* Are the changes here in viewFs.java covered anywhere in test? If not will
be good to cover this change as well
* Regarding the IOE, we are logging the IOE and then throwing
`readOnlyMountTable("mkdir", dir);` instead of the actual exception. Is it
intended? It actually suppress the actual reason, There is a difference in
behavior between the implementation here and at ViewFileSystem, in case of
exception there we handle it and don't throw `readOnlyMountTable("mkdir",
dir);` rather a response, though mkdirs and mkdir behave differently and
returning false there still might work, but here a person can not fix the issue
based on the end exception he receives.
* I think the `createParent` needs to be tackled, when the parent is there
in the mount table, in that case we have to explicitly change it to true. I
tried tweaking one of your test give a check :
`
@ Test
public void
testMkdirsOfDeepTreeWithFallbackLinkAndMountPathMatchingDirExist()
throws Exception {
Configuration conf = new Configuration();
conf.setBoolean(Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS, false);
ConfigUtil.addLink(conf, "/user1/hive",
new Path(targetTestRoot.toString() + "/").toUri());
Path fallbackTarget = new Path(targetTestRoot, "fallbackDir");
fsTarget.mkdirs(fallbackTarget);
ConfigUtil.addLinkFallback(conf, fallbackTarget.toUri());
AbstractFileSystem vfs = AbstractFileSystem.get(viewFsDefaultClusterUri,
conf);
//user1 does not exist in fallback
Path multipleLevelToInternalDir = new Path("/user1/test");
Path test = Path.mergePaths(fallbackTarget,
multipleLevelToInternalDir);
assertFalse(fsTarget.exists(test));
// Creating /user1/test
// Parent /user1 Exists.
assertNotNull(vfs.getFileStatus(new Path("/user1")));
// Creating /user1/test should be success, with createParent false, as
// parent /user1 exists.
vfs.mkdir(multipleLevelToInternalDir,
FsPermission.getDirDefault(),false); // This throws Exception...
assertTrue(fsTarget.exists(test));
}
`
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
