[
https://issues.apache.org/jira/browse/HADOOP-18144?focusedWorklogId=736900&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-736900
]
ASF GitHub Bot logged work on HADOOP-18144:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 04/Mar/22 21:02
Start Date: 04/Mar/22 21:02
Worklog Time Spent: 10m
Work Description: ibuenros commented on a change in pull request #4034:
URL: https://github.com/apache/hadoop/pull/4034#discussion_r819904428
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java
##########
@@ -191,8 +191,8 @@ public boolean moveToTrash(Path path) throws IOException {
cause = e;
}
}
- throw (IOException)
- new IOException("Failed to move to trash: " + path).initCause(cause);
+ throw (IOException) new IOException(
Review comment:
nit: Is this change intentional? i.e. removing the line break?
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java
##########
@@ -134,8 +134,10 @@
HCFSMountTableConfigLoader.class;
/**
- * Enable ViewFileSystem to return a trashRoot which is local to mount point.
+ * Enable ViewFileSystem to return a trashRoot which is in the root dir of a
+ * mount point.
*/
- String CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH =
"fs.viewfs.mount.point.local.trash";
- boolean CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT = false;
+ String CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT =
+ "fs.viewfs.trash.root.under.mount.point.root";
Review comment:
Why are we changing the configuration? Also @omalley what's Hadoop's
policy on using dots as spaces as opposed to dots as hierarchical separators?
i.e. should this be fs.viewfs.trash.rootUnderMountPointRoot?
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java
##########
@@ -1141,60 +1143,52 @@ public void testTrashRootLocalizedTrash() throws
IOException {
fsView2.getTrashRoot(nonExistentPath);
} catch (NotInMountpointException ignored) {
}
-
- // Case 5: path p is in the same mount point as targetFS.getTrashRoot().
- // Return targetFS.getTrashRoot()
- // Use a new Configuration object, so that we can start with an empty
- // mount table. This would avoid a conflict between the /user link in
- // setupMountPoints() and homeDir we will need to setup for this test.
- // default homeDir for hdfs is /user/.
- Configuration conf3 = ViewFileSystemTestSetup.createConfig();
- Path homeDir = fsTarget.getHomeDirectory();
- String homeParentDir = homeDir.getParent().toUri().getPath();
- conf3.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true);
- ConfigUtil.addLink(conf3, homeParentDir,
- new Path(targetTestRoot, homeParentDir).toUri());
- Path homeTestPath = new Path(homeDir.toUri().getPath(), "testuser/file");
- FileSystem fsView3 = FileSystem.get(FsConstants.VIEWFS_URI, conf3);
- Assert.assertEquals(userTrashRoot, fsView3.getTrashRoot(homeTestPath));
}
/**
- * A mocked FileSystem which returns a deep trash dir.
+ * A mocked FileSystem which returns overwrites getFileStatus() to always
+ * return an encrypted FileStatus.
*/
static class MockTrashRootFS extends MockFileSystem {
Review comment:
nit: make the name reflect the behavior, e.g.
`AllPathsAreEncryptionRootsFS`.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 736900)
Time Spent: 0.5h (was: 20m)
> getTrashRoot/s in ViewFileSystem should return viewFS path, not targetFS path
> -----------------------------------------------------------------------------
>
> Key: HADOOP-18144
> URL: https://issues.apache.org/jira/browse/HADOOP-18144
> Project: Hadoop Common
> Issue Type: Improvement
> Components: common
> Reporter: Xing Lin
> Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> It is probably incorrect that we return a targetFS path from getTrashRoot()
> in ViewFileSystem, as that path will be used later on by ViewFileSystem in
> other operations, such as rename. ViewFileSystem is assuming the path that it
> receives is a viewFS path, but not a target FS path. For example, rename() in
> ViewFileSystem will call getUriPath() for src/dst path, which will remove the
> scheme/authority and then try to resolve the path-only component. It thus
> sometimes leads to incorrect path resolution, as we are doing the path
> resolution again on a targetFS path.
>
> On the other hand, it is not always trivial/feasible to determine the correct
> viewFS path for a given trash root in targetFS path.
> Example:
> Assume we have a mount point for /user/foo -> abfs:/containerA
> User foo calls getTrashRoot("/a/b/c") and "/a/b/c" does not match any mount
> point. We fall back to the fallback hdfs, which by default returns
> hdfs://localhost/user/foo/.Trash. In this case, it is incorrect to return the
> trash root as viewfs:/user/foo, as it will be resolved to the abfs mount
> point, instead of the fallback hdfs.
>
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
