You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/09/09 18:04:31 UTC
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4869: HADOOP-18444 Add Support for localized trash for ViewFileSystem in Trash.moveToAppropriateTrash
steveloughran commented on code in PR #4869:
URL: https://github.com/apache/hadoop/pull/4869#discussion_r967342541
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsTrash.java:
##########
@@ -65,5 +73,39 @@ public void testTrash() throws Exception {
TestTrash.trashShell(conf, fileSystemTestHelper.getTestRootPath(fsView),
fsView, new Path(fileSystemTestHelper.getTestRootPath(fsView), ".Trash/Current"));
}
-
+
+ @Test
+ public void testLocalizedTrashInMoveToAppropriateTrash() throws IOException {
+ Configuration conf2 = new Configuration(conf);
+
+ // Enable moveToTrash and add a mount point for /data
+ conf2.setLong(FS_TRASH_INTERVAL_KEY, 1);
+ ConfigUtil.addLink(conf2, "/data", new Path(fileSystemTestHelper.getAbsoluteTestRootPath(fsTarget), "data").toUri());
+
+ // Default case. file should be moved to fsTarget.getTrashRoot()/resolvedPath
+ conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, false);
+ FileSystem fsView2 = FileSystem.get(conf2);
+ Path f = new Path("/data/testfile.txt");
+ DataOutputStream out = fsView2.create(f);
+ out.writeBytes("testfile.txt:" + f);
Review Comment:
use FileSystemTestHelper helper methods
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsTrash.java:
##########
@@ -65,5 +73,39 @@ public void testTrash() throws Exception {
TestTrash.trashShell(conf, fileSystemTestHelper.getTestRootPath(fsView),
fsView, new Path(fileSystemTestHelper.getTestRootPath(fsView), ".Trash/Current"));
}
-
+
+ @Test
+ public void testLocalizedTrashInMoveToAppropriateTrash() throws IOException {
+ Configuration conf2 = new Configuration(conf);
+
+ // Enable moveToTrash and add a mount point for /data
+ conf2.setLong(FS_TRASH_INTERVAL_KEY, 1);
+ ConfigUtil.addLink(conf2, "/data", new Path(fileSystemTestHelper.getAbsoluteTestRootPath(fsTarget), "data").toUri());
+
+ // Default case. file should be moved to fsTarget.getTrashRoot()/resolvedPath
+ conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, false);
+ FileSystem fsView2 = FileSystem.get(conf2);
+ Path f = new Path("/data/testfile.txt");
+ DataOutputStream out = fsView2.create(f);
+ out.writeBytes("testfile.txt:" + f);
+ out.close();
+ Path resolvedFile = fsView2.resolvePath(f);
+
+ Trash.moveToAppropriateTrash(fsView2, f, conf2);
+ Trash trash = new Trash(fsTarget, conf2);
+ Path movedPath = Path.mergePaths(trash.getCurrentTrashDir(f), resolvedFile);
+ assertTrue("File not in trash", fsTarget.exists(movedPath));
+
+ // Turn on localized trash. File should be moved to viewfs:/data/.Trash/{user}/Current.
+ conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true);
+ fsView2 = FileSystem.get(conf2);
+ out = fsView2.create(f);
+ out.writeBytes("testfile.txt:" + f);
+ out.close();
+
+ Trash.moveToAppropriateTrash(fsView2, f, conf2);
+ trash = new Trash(fsView2, conf2);
+ movedPath = Path.mergePaths(trash.getCurrentTrashDir(f), f);
+ assertTrue("File not in localized trash", fsView2.exists(movedPath));
Review Comment:
prefer ContractTestUtils assertions as they do listings on failures
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Trash.java:
##########
@@ -94,6 +96,27 @@ public static boolean moveToAppropriateTrash(FileSystem fs, Path p,
LOG.warn("Failed to get server trash configuration", e);
throw new IOException("Failed to get server trash configuration", e);
}
+
+ // Directly use viewfs if localized trash is enabled
+ if (fs instanceof ViewFileSystem &&
Review Comment:
This is a messy way of passing a parameter down to the FS.
better to have a new getter/setter interface which viewfs can implement, and a custom trash policy which does the work of using it.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsTrash.java:
##########
@@ -65,5 +73,39 @@ public void testTrash() throws Exception {
TestTrash.trashShell(conf, fileSystemTestHelper.getTestRootPath(fsView),
fsView, new Path(fileSystemTestHelper.getTestRootPath(fsView), ".Trash/Current"));
}
-
+
+ @Test
+ public void testLocalizedTrashInMoveToAppropriateTrash() throws IOException {
+ Configuration conf2 = new Configuration(conf);
+
+ // Enable moveToTrash and add a mount point for /data
+ conf2.setLong(FS_TRASH_INTERVAL_KEY, 1);
+ ConfigUtil.addLink(conf2, "/data", new Path(fileSystemTestHelper.getAbsoluteTestRootPath(fsTarget), "data").toUri());
+
+ // Default case. file should be moved to fsTarget.getTrashRoot()/resolvedPath
+ conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, false);
+ FileSystem fsView2 = FileSystem.get(conf2);
+ Path f = new Path("/data/testfile.txt");
+ DataOutputStream out = fsView2.create(f);
+ out.writeBytes("testfile.txt:" + f);
+ out.close();
+ Path resolvedFile = fsView2.resolvePath(f);
+
+ Trash.moveToAppropriateTrash(fsView2, f, conf2);
+ Trash trash = new Trash(fsTarget, conf2);
+ Path movedPath = Path.mergePaths(trash.getCurrentTrashDir(f), resolvedFile);
+ assertTrue("File not in trash", fsTarget.exists(movedPath));
+
+ // Turn on localized trash. File should be moved to viewfs:/data/.Trash/{user}/Current.
+ conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true);
+ fsView2 = FileSystem.get(conf2);
Review Comment:
use a different try/with/resources lifecycle to ensure a different fs is used
--
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-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org