From fea90ca71e0459b959e3a4d77813882db70380db Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Tue, 22 Feb 2022 15:52:03 -0800 Subject: [PATCH 01/26] Change to check for EZ/snapshots --- .../apache/hadoop/fs/TrashPolicyDefault.java | 5 ++-- .../hadoop/fs/viewfs/ViewFileSystem.java | 20 +++++++++---- .../fs/viewfs/ViewFileSystemBaseTest.java | 30 +++++++------------ 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java index 99467f5633625..f94936ccb8ac1 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java @@ -132,6 +132,7 @@ public boolean moveToTrash(Path path) throws IOException { String qpath = fs.makeQualified(path).toString(); Path trashRoot = fs.getTrashRoot(path); + LOG.info("trashRoot: " + trashRoot); Path trashCurrent = new Path(trashRoot, CURRENT); if (qpath.startsWith(trashRoot.toString())) { return false; // already in trash @@ -191,8 +192,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( + "Failed to move " + path + " to trash " + trashPath).initCause(cause); } @SuppressWarnings("deprecation") diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 8c3cdb82d7447..e50258118d388 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1128,6 +1128,17 @@ public Collection getAllStoragePolicies() return allPolicies; } + // Test whether a path is inside a snapshot or encrypted. + boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException { + try { + FileStatus status = getFileStatus(p); + return status.isSnapshotEnabled() || status.isEncrypted(); + } catch (FileNotFoundException ignored) { + // return false if path p does not exist yet. + return false; + } + } + /** * Get the trash root directory for current user when the path * specified is deleted. @@ -1146,6 +1157,7 @@ public Collection getAllStoragePolicies() */ @Override public Path getTrashRoot(Path path) { + LOG.info("new getTrashRoot for ViewFileSystem"); boolean useMountPointLocalTrash = config.getBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT); @@ -1155,15 +1167,13 @@ public Path getTrashRoot(Path path) { fsState.resolve(getUriPath(path), true); Path trashRoot = res.targetFileSystem.getTrashRoot(res.remainingPath); - if (!useMountPointLocalTrash) { + if (!useMountPointLocalTrash || isSnapshotEnabledOrEncrypted(path)) { return trashRoot; } else { // Path p is either in a mount point or in the fallback FS - if (ROOT_PATH.equals(new Path(res.resolvedPath)) - || trashRoot.toUri().getPath().startsWith(res.resolvedPath)) { - // Path p is in the fallback FS or targetFileSystem.trashRoot is in - // the same mount point as Path p + if (ROOT_PATH.equals(new Path(res.resolvedPath))) { + // Path p is in the fallback FS return trashRoot; } else { // targetFileSystem.trashRoot is in a different mount point from diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 91a90751e873e..1d0231192ed9a 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1141,26 +1141,11 @@ 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 { public static final Path TRASH = @@ -1170,13 +1155,20 @@ static class MockTrashRootFS extends MockFileSystem { public Path getTrashRoot(Path path) { return TRASH; } + + @Override + public FileStatus getFileStatus(Path path) throws IOException { + FileStatus status = super.getFileStatus(path); + return new FileStatus(0, false, 3, 1024, 0, 0, null, "test", "testg", + path, path, false, true, false); + } } /** - * Test a trash root that is inside a mount point for getTrashRoot + * Test a trash root that is encrypted for getTrashRoot */ @Test - public void testTrashRootDeepTrashDir() throws IOException { + public void testTrashRootEncryptedTrashDir() throws IOException { Configuration conf2 = ViewFileSystemTestSetup.createConfig(); conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true); From f9963d2ff24faa53a18c1b9fab1be4d3cff0184c Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Thu, 24 Feb 2022 10:30:35 -0800 Subject: [PATCH 02/26] Change getTrashRoot to return viewFS path, instead of targetFS path --- .../hadoop/fs/viewfs/ViewFileSystem.java | 24 +++++++------- .../fs/viewfs/ViewFileSystemBaseTest.java | 31 +++++++++++-------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index e50258118d388..c739839201694 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1144,20 +1144,19 @@ boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException { * specified is deleted. * * If CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is not set, return - * the default trash root from targetFS. + * a trash root based on the trash root returned by targetFS. * * When CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is set to true, - * 1) If path p is in fallback FS or from the same mount point as the default - * trash root for targetFS, return the default trash root for targetFS. - * 2) else, return a trash root in the mounted targetFS - * (/mntpoint/.Trash/{user}) + * 1) If path p is in a snapshot or encryption zone, or when it is in the + * fallback FS, return a trash root based on the trash root returned + * by targetFS (/mntpoint/trashRootInTargetFS). + * 2) else, return a trash root in the mount point (/mntpoint/.Trash/{user}). * * @param path the trash root of the path to be determined. * @return the trash root path. */ @Override public Path getTrashRoot(Path path) { - LOG.info("new getTrashRoot for ViewFileSystem"); boolean useMountPointLocalTrash = config.getBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT); @@ -1166,7 +1165,10 @@ public Path getTrashRoot(Path path) { InodeTree.ResolveResult res = fsState.resolve(getUriPath(path), true); - Path trashRoot = res.targetFileSystem.getTrashRoot(res.remainingPath); + Path targetFSTrashRoot = + res.targetFileSystem.getTrashRoot(res.remainingPath); + Path trashRoot = + new Path(res.resolvedPath + targetFSTrashRoot.toUri().getPath()); if (!useMountPointLocalTrash || isSnapshotEnabledOrEncrypted(path)) { return trashRoot; } else { @@ -1174,13 +1176,11 @@ public Path getTrashRoot(Path path) { if (ROOT_PATH.equals(new Path(res.resolvedPath))) { // Path p is in the fallback FS + trashRoot = new Path(targetFSTrashRoot.toUri().getPath()); return trashRoot; } else { - // targetFileSystem.trashRoot is in a different mount point from - // Path p. Return the trash root for the mount point. - Path mountPointRoot = - res.targetFileSystem.getFileStatus(new Path("/")).getPath(); - return new Path(mountPointRoot, + // Return the trash root for the mount point. + return new Path(res.resolvedPath, TRASH_PREFIX + "/" + ugi.getShortUserName()); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 1d0231192ed9a..f7485b12e765e 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1056,9 +1056,9 @@ public void testTrashRoot() throws IOException { // Verify if Trash roots from ViewFileSystem matches that of the ones // from the target mounted FileSystem. assertEquals(mountDataRootTrashPath.toUri().getPath(), - fsTargetRootTrashRoot.toUri().getPath()); + "/data" + fsTargetRootTrashRoot.toUri().getPath()); assertEquals(mountDataFileTrashPath.toUri().getPath(), - fsTargetFileTrashPath.toUri().getPath()); + "/data" + fsTargetFileTrashPath.toUri().getPath()); assertEquals(mountDataRootTrashPath.toUri().getPath(), mountDataFileTrashPath.toUri().getPath()); @@ -1105,34 +1105,38 @@ public void testTrashRoot() throws IOException { } /** - * Test the localized trash root for getTrashRoot. + * Test trash root at the root of mount points */ @Test - public void testTrashRootLocalizedTrash() throws IOException { + public void testTrashRootRootOfMountPoint() throws IOException { UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); Configuration conf2 = new Configuration(conf); conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true); ConfigUtil.addLinkFallback(conf2, targetTestRoot.toUri()); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - // Case 1: path p not in the default FS. + // Case 1: path p in the /data mount point. // Return a trash root within the mount point. Path dataTestPath = new Path("/data/dir/file"); - Path dataTrashRoot = new Path(targetTestRoot, - "data/" + TRASH_PREFIX + "/" + ugi.getShortUserName()); + Path dataTrashRoot = + new Path("/data/" + TRASH_PREFIX + "/" + ugi.getShortUserName()); Assert.assertEquals(dataTrashRoot, fsView2.getTrashRoot(dataTestPath)); // Case 2: path p not found in mount table, fall back to the default FS // Return a trash root in user home dir Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); - Path userTrashRoot = new Path(fsTarget.getHomeDirectory(), TRASH_PREFIX); + Path userTrashRoot = + new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX); Assert.assertEquals(userTrashRoot, fsView2.getTrashRoot(nonExistentPath)); // Case 3: turn off the CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH flag. - // Return a trash root in user home dir. + // Return a trash root in user home dir in the mount point. conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Assert.assertEquals(userTrashRoot, fsView2.getTrashRoot(dataTestPath)); + Path dataUserTrashRoot = new Path( + "/data" + fsTarget.getHomeDirectory().toUri().getPath() + "/" + + TRASH_PREFIX); + Assert.assertEquals(dataUserTrashRoot, fsView2.getTrashRoot(dataTestPath)); // Case 4: viewFS without fallback. Expect exception for a nonExistent path conf2 = new Configuration(conf); @@ -1149,7 +1153,7 @@ public void testTrashRootLocalizedTrash() throws IOException { */ static class MockTrashRootFS extends MockFileSystem { public static final Path TRASH = - new Path("/mnt/very/deep/deep/trash/dir/.Trash"); + new Path("/very/deep/deep/trash/dir/.Trash"); @Override public Path getTrashRoot(Path path) { @@ -1175,9 +1179,10 @@ public void testTrashRootEncryptedTrashDir() throws IOException { conf2.setClass("fs.mocktrashfs.impl", MockTrashRootFS.class, FileSystem.class); ConfigUtil.addLink(conf2, "/mnt", URI.create("mocktrashfs://mnt/path")); - Path testPath = new Path(MockTrashRootFS.TRASH, "projs/proj"); + Path testPath = new Path("/mnt/projs/proj"); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Assert.assertEquals(MockTrashRootFS.TRASH, fsView2.getTrashRoot(testPath)); + Assert.assertEquals("/mnt" + MockTrashRootFS.TRASH, + fsView2.getTrashRoot(testPath).toUri().getPath()); } /** From dc300202b6a38a2fddf0c45d6bf5ee0f8ccdcafe Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Thu, 24 Feb 2022 14:24:19 -0800 Subject: [PATCH 03/26] Rename MOUNT_POINT_LOCAL_TRASH macro --- .../java/org/apache/hadoop/fs/viewfs/Constants.java | 8 +++++--- .../org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 12 ++++++------ .../hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 12 ++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java index 94b07973ac824..fe59d26fbf451 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java @@ -134,8 +134,10 @@ public interface Constants { HCFSMountTableConfigLoader.class; /** - * Enable ViewFileSystem to return a trashRoot which is local to mount point. + * Enable ViewFileSystem to return a trashRoot which 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"; + boolean CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT = false; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index c739839201694..bd0439cf63381 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -25,8 +25,8 @@ import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT; import java.util.function.Function; import java.io.FileNotFoundException; @@ -1158,8 +1158,8 @@ boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException { @Override public Path getTrashRoot(Path path) { boolean useMountPointLocalTrash = - config.getBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, - CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT); + config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, + CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT); try { InodeTree.ResolveResult res = @@ -1204,8 +1204,8 @@ public Collection getTrashRoots(boolean allUsers) { // Add trash dirs for each mount point boolean useMountPointLocalTrash = - config.getBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, - CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT); + config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, + CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT); if (useMountPointLocalTrash) { Set currentTrashPaths = new HashSet<>(); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index f7485b12e765e..7083e5e364127 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -69,7 +69,7 @@ import static org.apache.hadoop.fs.FileSystemTestHelper.*; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_ENABLE_INNER_CACHE; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT; import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; import org.junit.After; @@ -1111,7 +1111,7 @@ public void testTrashRoot() throws IOException { public void testTrashRootRootOfMountPoint() throws IOException { UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); ConfigUtil.addLinkFallback(conf2, targetTestRoot.toUri()); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); @@ -1131,7 +1131,7 @@ public void testTrashRootRootOfMountPoint() throws IOException { // Case 3: turn off the CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH flag. // Return a trash root in user home dir in the mount point. - conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, false); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); Path dataUserTrashRoot = new Path( "/data" + fsTarget.getHomeDirectory().toUri().getPath() + "/" @@ -1175,7 +1175,7 @@ public FileStatus getFileStatus(Path path) throws IOException { public void testTrashRootEncryptedTrashDir() throws IOException { Configuration conf2 = ViewFileSystemTestSetup.createConfig(); - conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); conf2.setClass("fs.mocktrashfs.impl", MockTrashRootFS.class, FileSystem.class); ConfigUtil.addLink(conf2, "/mnt", URI.create("mocktrashfs://mnt/path")); @@ -1191,7 +1191,7 @@ public void testTrashRootEncryptedTrashDir() throws IOException { @Test public void testTrashRootsAllUsers() throws IOException { Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); // Case 1: verify correct trash roots from fsView and fsView2 @@ -1237,7 +1237,7 @@ public void testTrashRootsCurrentUser() throws IOException { String currentUser = UserGroupInformation.getCurrentUser().getShortUserName(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); int beforeTrashRootNum = fsView.getTrashRoots(false).size(); From eb7e04876c3cebb9d7f84bb28d5472c903b62afc Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 25 Feb 2022 15:48:06 -0800 Subject: [PATCH 04/26] Modified getTrashRoot/s to return hybrid paths. getTrashRoot: if flag is off or when the path is in a snapshot/encryption zone or when it is in the fallback FS, return a targetFS path. otherwise, return a viewFS path. getTrashRoots: return targetFS paths from targetFS.getTrashRoots() and viewFS paths for each mount point. --- .../hadoop/fs/viewfs/ViewFileSystem.java | 117 +++++++++--------- .../fs/viewfs/ViewFileSystemBaseTest.java | 22 ++-- 2 files changed, 70 insertions(+), 69 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index bd0439cf63381..82dc3963d9cad 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1157,7 +1157,7 @@ boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException { */ @Override public Path getTrashRoot(Path path) { - boolean useMountPointLocalTrash = + boolean trashRootUnderMountPointRoot = config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT); @@ -1167,22 +1167,19 @@ public Path getTrashRoot(Path path) { Path targetFSTrashRoot = res.targetFileSystem.getTrashRoot(res.remainingPath); - Path trashRoot = - new Path(res.resolvedPath + targetFSTrashRoot.toUri().getPath()); - if (!useMountPointLocalTrash || isSnapshotEnabledOrEncrypted(path)) { - return trashRoot; - } else { - // Path p is either in a mount point or in the fallback FS - if (ROOT_PATH.equals(new Path(res.resolvedPath))) { - // Path p is in the fallback FS - trashRoot = new Path(targetFSTrashRoot.toUri().getPath()); - return trashRoot; - } else { - // Return the trash root for the mount point. - return new Path(res.resolvedPath, - TRASH_PREFIX + "/" + ugi.getShortUserName()); - } + if (!trashRootUnderMountPointRoot) { + return targetFSTrashRoot; + } + + // New trash root policy + if (isSnapshotEnabledOrEncrypted(path) || ROOT_PATH.equals( + new Path(res.resolvedPath))) { + return targetFSTrashRoot; + } else { + // Return the trash root for the mount point. + return new Path(res.resolvedPath, + TRASH_PREFIX + "/" + ugi.getShortUserName()); } } catch (IOException | IllegalArgumentException e) { throw new NotInMountpointException(path, "getTrashRoot"); @@ -1202,59 +1199,67 @@ public Collection getTrashRoots(boolean allUsers) { trashRoots.addAll(fs.getTrashRoots(allUsers)); } - // Add trash dirs for each mount point - boolean useMountPointLocalTrash = + boolean trashRootUnderMountPointRoot = config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT); - if (useMountPointLocalTrash) { + // Return trashRoots if CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is + // disabled. + if (!trashRootUnderMountPointRoot) { + return trashRoots; + } - Set currentTrashPaths = new HashSet<>(); - for (FileStatus file : trashRoots) { - currentTrashPaths.add(file.getPath()); - } + // We use targetFS Path to check for duplicate paths. + Set uniqueTrashRoots = new HashSet<>(); + for (FileStatus file : trashRoots) { + uniqueTrashRoots.add(file.getPath()); + } - MountPoint[] mountPoints = getMountPoints(); - try { - for (int i = 0; i < mountPoints.length; i++) { - Path trashRoot = makeQualified( - new Path(mountPoints[i].mountedOnPath + "/" + TRASH_PREFIX)); + List> mountPoints = + fsState.getMountPoints(); + try { + for (InodeTree.MountPoint mountPoint : mountPoints) { - // Continue if trashRoot does not exist for this filesystem - if (!exists(trashRoot)) { - continue; - } + Path trashRoot = new Path(mountPoint.src + "/" + TRASH_PREFIX); - InodeTree.ResolveResult res = - fsState.resolve(getUriPath(trashRoot), true); + // Continue if trashRoot does not exist for this mount point + if (!exists(trashRoot)) { + continue; + } - if (!allUsers) { - Path userTrash = + if (!allUsers) { + Path userTrashRoot = new Path(trashRoot, ugi.getShortUserName()); + if (exists(userTrashRoot)) { + Path targetFsTrashPath = new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName()); - try { - FileStatus file = res.targetFileSystem.getFileStatus(userTrash); - if (!currentTrashPaths.contains(file.getPath())) { - trashRoots.add(file); - currentTrashPaths.add(file.getPath()); - } - } catch (FileNotFoundException ignored) { + FileStatus targetFsTrash = mountPoint.target.getTargetFileSystem() + .getFileStatus(targetFsTrashPath); + if (!uniqueTrashRoots.contains(targetFsTrash.getPath())) { + FileStatus trash = getFileStatus(userTrashRoot); + trashRoots.add(trash); + uniqueTrashRoots.add(targetFsTrash.getPath()); } - } else { - FileStatus[] targetFsTrashRoots = - res.targetFileSystem.listStatus(new Path("/" + TRASH_PREFIX)); - for (FileStatus file : targetFsTrashRoots) { - // skip if we already include it in currentTrashPaths - if (currentTrashPaths.contains(file.getPath())) { - continue; - } - - trashRoots.add(file); - currentTrashPaths.add(file.getPath()); + } + } else { + FileStatus[] mountPointTrashRoots = listStatus(trashRoot); + for (FileStatus trash : mountPointTrashRoots) { + // Remove the mountPoint to get the targetFS path + Path targetFsTrashPath = + new Path(trash.getPath().toUri().getPath() + .substring(mountPoint.src.length())); + FileStatus targetFsTrash = mountPoint.target.getTargetFileSystem() + .getFileStatus(targetFsTrashPath); + // skip if we already include it in uniqueTrashRoots + if (uniqueTrashRoots.contains(targetFsTrash.getPath())) { + continue; } + + trashRoots.add(trash); + uniqueTrashRoots.add(targetFsTrash.getPath()); } } - } catch (IOException e) { - LOG.warn("Exception in get all trash roots", e); } + } catch (IOException e) { + LOG.warn("Exception in get all trash roots", e); } return trashRoots; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 7083e5e364127..bb63ddf9fea63 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1056,13 +1056,12 @@ public void testTrashRoot() throws IOException { // Verify if Trash roots from ViewFileSystem matches that of the ones // from the target mounted FileSystem. assertEquals(mountDataRootTrashPath.toUri().getPath(), - "/data" + fsTargetRootTrashRoot.toUri().getPath()); + fsTargetRootTrashRoot.toUri().getPath()); assertEquals(mountDataFileTrashPath.toUri().getPath(), - "/data" + fsTargetFileTrashPath.toUri().getPath()); + fsTargetFileTrashPath.toUri().getPath()); assertEquals(mountDataRootTrashPath.toUri().getPath(), mountDataFileTrashPath.toUri().getPath()); - // Verify trash root for an non-existing file but on a valid mountpoint. Path trashRoot = fsView.getTrashRoot(mountDataNonExistingFilePath); assertEquals(mountDataRootTrashPath.toUri().getPath(), @@ -1125,18 +1124,16 @@ public void testTrashRootRootOfMountPoint() throws IOException { // Case 2: path p not found in mount table, fall back to the default FS // Return a trash root in user home dir Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); - Path userTrashRoot = - new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX); - Assert.assertEquals(userTrashRoot, fsView2.getTrashRoot(nonExistentPath)); + Path targetFsTrashRoot = fsTarget.makeQualified( + new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX)); + Assert.assertEquals(targetFsTrashRoot, + fsView2.getTrashRoot(nonExistentPath)); // Case 3: turn off the CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH flag. - // Return a trash root in user home dir in the mount point. + // Return a trash root in user home dir. conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Path dataUserTrashRoot = new Path( - "/data" + fsTarget.getHomeDirectory().toUri().getPath() + "/" - + TRASH_PREFIX); - Assert.assertEquals(dataUserTrashRoot, fsView2.getTrashRoot(dataTestPath)); + Assert.assertEquals(targetFsTrashRoot, fsView2.getTrashRoot(dataTestPath)); // Case 4: viewFS without fallback. Expect exception for a nonExistent path conf2 = new Configuration(conf); @@ -1181,8 +1178,7 @@ public void testTrashRootEncryptedTrashDir() throws IOException { ConfigUtil.addLink(conf2, "/mnt", URI.create("mocktrashfs://mnt/path")); Path testPath = new Path("/mnt/projs/proj"); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Assert.assertEquals("/mnt" + MockTrashRootFS.TRASH, - fsView2.getTrashRoot(testPath).toUri().getPath()); + Assert.assertEquals(MockTrashRootFS.TRASH, fsView2.getTrashRoot(testPath)); } /** From 12bc904a069a4150e7cdfb45cab0ffba5456ec05 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 25 Feb 2022 16:19:20 -0800 Subject: [PATCH 05/26] Minor edits --- .../org/apache/hadoop/fs/TrashPolicyDefault.java | 1 - .../org/apache/hadoop/fs/viewfs/Constants.java | 2 +- .../apache/hadoop/fs/viewfs/ViewFileSystem.java | 16 ++++++++-------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java index f94936ccb8ac1..ec08abfbdcb28 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java @@ -132,7 +132,6 @@ public boolean moveToTrash(Path path) throws IOException { String qpath = fs.makeQualified(path).toString(); Path trashRoot = fs.getTrashRoot(path); - LOG.info("trashRoot: " + trashRoot); Path trashCurrent = new Path(trashRoot, CURRENT); if (qpath.startsWith(trashRoot.toString())) { return false; // already in trash diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java index fe59d26fbf451..a5ea8316b7d14 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java @@ -134,7 +134,7 @@ public interface Constants { HCFSMountTableConfigLoader.class; /** - * Enable ViewFileSystem to return a trashRoot which in the root dir of a + * Enable ViewFileSystem to return a trashRoot which is in the root dir of a * mount point. */ String CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT = diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 82dc3963d9cad..0ee76f73f4490 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1143,14 +1143,14 @@ boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException { * Get the trash root directory for current user when the path * specified is deleted. * - * If CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is not set, return - * a trash root based on the trash root returned by targetFS. + * If CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is not set, return + * the default trash root from targetFS. * - * When CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is set to true, + * When CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is set to true, * 1) If path p is in a snapshot or encryption zone, or when it is in the - * fallback FS, return a trash root based on the trash root returned - * by targetFS (/mntpoint/trashRootInTargetFS). - * 2) else, return a trash root in the mount point (/mntpoint/.Trash/{user}). + * fallback FS, return the default trash root from targetFS. + * 2) else, return a viewFS path for the trash root under the root of the + * mount point (/mntpoint/.Trash/{user}). * * @param path the trash root of the path to be determined. * @return the trash root path. @@ -1173,8 +1173,8 @@ public Path getTrashRoot(Path path) { } // New trash root policy - if (isSnapshotEnabledOrEncrypted(path) || ROOT_PATH.equals( - new Path(res.resolvedPath))) { + if (isSnapshotEnabledOrEncrypted(path) || + ROOT_PATH.equals(new Path(res.resolvedPath))) { return targetFSTrashRoot; } else { // Return the trash root for the mount point. From d605fed5933894de84706ab7372c36a3b1a0e39c Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 25 Feb 2022 16:28:14 -0800 Subject: [PATCH 06/26] More minor edits --- .../main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 4 ++++ .../org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 1 + 2 files changed, 5 insertions(+) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 0ee76f73f4490..d2ff54a227ea3 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1189,6 +1189,10 @@ public Path getTrashRoot(Path path) { /** * Get all the trash roots for current user or all users. * + * When CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is set to true, we + * also return trash roots under the root of each mount point, with their + * viewFS paths. + * * @param allUsers return trash roots for all users if true. * @return all Trash root directories. */ diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index bb63ddf9fea63..5b2c5f48d75b2 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1062,6 +1062,7 @@ public void testTrashRoot() throws IOException { assertEquals(mountDataRootTrashPath.toUri().getPath(), mountDataFileTrashPath.toUri().getPath()); + // Verify trash root for an non-existing file but on a valid mountpoint. Path trashRoot = fsView.getTrashRoot(mountDataNonExistingFilePath); assertEquals(mountDataRootTrashPath.toUri().getPath(), From 26cb868b7acfd20853da172b79ad02658e33978e Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 25 Feb 2022 16:51:21 -0800 Subject: [PATCH 07/26] HADOOP-18144. getTrashRoot/s in ViewFileSystem should return viewFS path, not targetFS path --- .../org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 5b2c5f48d75b2..ef50700f84d6e 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1183,7 +1183,7 @@ public void testTrashRootEncryptedTrashDir() throws IOException { } /** - * Test localized trash roots in getTrashRoots() for all users. + * Test getTrashRoots() for all users. */ @Test public void testTrashRootsAllUsers() throws IOException { @@ -1227,7 +1227,7 @@ public void testTrashRootsAllUsers() throws IOException { } /** - * Test localized trash roots in getTrashRoots() for current user. + * Test getTrashRoots() for current user. */ @Test public void testTrashRootsCurrentUser() throws IOException { From 62f44d30bc3749349f96987fddcb4fa378fd6d66 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 4 Mar 2022 19:43:23 -0800 Subject: [PATCH 08/26] check trashRoot from targetFS with mountpoint.targetPath. Remove check for snapshots/ez. --- .../hadoop/fs/viewfs/ViewFileSystem.java | 52 ++++++++++++------- .../fs/viewfs/ViewFileSystemBaseTest.java | 35 ++++++------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index d2ff54a227ea3..cd20dbf03fbb5 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1128,17 +1128,6 @@ public Collection getAllStoragePolicies() return allPolicies; } - // Test whether a path is inside a snapshot or encrypted. - boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException { - try { - FileStatus status = getFileStatus(p); - return status.isSnapshotEnabled() || status.isEncrypted(); - } catch (FileNotFoundException ignored) { - // return false if path p does not exist yet. - return false; - } - } - /** * Get the trash root directory for current user when the path * specified is deleted. @@ -1147,10 +1136,12 @@ boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException { * the default trash root from targetFS. * * When CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is set to true, - * 1) If path p is in a snapshot or encryption zone, or when it is in the - * fallback FS, return the default trash root from targetFS. - * 2) else, return a viewFS path for the trash root under the root of the - * mount point (/mntpoint/.Trash/{user}). + * 1) If path p in fallback FS, return trash root from fallback FS. Print a + * warning if there is a mount point for trash root. + * 2) If trash root for path p is in the same mount point as path p, get + * the corresponding viewFS path for the trash root and return it. + * 3) else, return the trash root under the root of the mount point + * (/mntpoint/.Trash/{user}). * * @param path the trash root of the path to be determined. * @return the trash root path. @@ -1167,15 +1158,40 @@ public Path getTrashRoot(Path path) { Path targetFSTrashRoot = res.targetFileSystem.getTrashRoot(res.remainingPath); + String targetFSTrashRootPath = targetFSTrashRoot.toUri().getPath(); + String mountTargetPath = res.targetFileSystem.getUri().getPath(); if (!trashRootUnderMountPointRoot) { return targetFSTrashRoot; } // New trash root policy - if (isSnapshotEnabledOrEncrypted(path) || - ROOT_PATH.equals(new Path(res.resolvedPath))) { - return targetFSTrashRoot; + if (ROOT_PATH.equals(new Path(res.resolvedPath))) { + // Path path is in fallback FS. Return targetFSTrashRootPath; + + // Check and print a warning if a mountpoint exists for + // targetFSTrashRootPath. The trash root from fallback FS should be in + // the fallback FS, not in a different mount point. + InodeTree.ResolveResult res2 = + fsState.resolve(targetFSTrashRootPath, true); + if (!ROOT_PATH.equals(new Path(res2.resolvedPath))) { + LOG.warn(String.format("A mount point %s exists for trash root %s " + + "returned by fallback FS for path %s. Rename between %s and " + + "trash root %s will fail, as the trash root is in a mount" + + " point while %s is in the fallback FS", + res2.resolvedPath, targetFSTrashRootPath, path, path, + targetFSTrashRootPath, path)); + } + return new Path(targetFSTrashRootPath); + } else if (targetFSTrashRootPath.startsWith(mountTargetPath)) { + // targetFSTrash is inside mountPoint.targetPath + String relativeTrashRoot = + targetFSTrashRootPath.substring(mountTargetPath.length()); + // remove the starting '/' if it exists + if (relativeTrashRoot.startsWith("/")) { + relativeTrashRoot = relativeTrashRoot.substring(1); + } + return new Path(res.resolvedPath, relativeTrashRoot); } else { // Return the trash root for the mount point. return new Path(res.resolvedPath, diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index ef50700f84d6e..cf3265883ce71 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1125,16 +1125,18 @@ public void testTrashRootRootOfMountPoint() throws IOException { // Case 2: path p not found in mount table, fall back to the default FS // Return a trash root in user home dir Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); - Path targetFsTrashRoot = fsTarget.makeQualified( - new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX)); - Assert.assertEquals(targetFsTrashRoot, + Path userHomeTrashRoot = + new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX); + Assert.assertEquals(userHomeTrashRoot, fsView2.getTrashRoot(nonExistentPath)); // Case 3: turn off the CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH flag. // Return a trash root in user home dir. conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Assert.assertEquals(targetFsTrashRoot, fsView2.getTrashRoot(dataTestPath)); + Path targetFSUserHomeTrashRoot = fsTarget.makeQualified(userHomeTrashRoot); + Assert.assertEquals(targetFSUserHomeTrashRoot, + fsView2.getTrashRoot(dataTestPath)); // Case 4: viewFS without fallback. Expect exception for a nonExistent path conf2 = new Configuration(conf); @@ -1146,40 +1148,35 @@ public void testTrashRootRootOfMountPoint() throws IOException { } /** - * A mocked FileSystem which returns overwrites getFileStatus() to always - * return an encrypted FileStatus. + * A mocked FileSystem which returns a deep trash dir. */ static class MockTrashRootFS extends MockFileSystem { public static final Path TRASH = - new Path("/very/deep/deep/trash/dir/.Trash"); + new Path("/vol/very/deep/deep/trash/dir/.Trash"); @Override public Path getTrashRoot(Path path) { return TRASH; } - - @Override - public FileStatus getFileStatus(Path path) throws IOException { - FileStatus status = super.getFileStatus(path); - return new FileStatus(0, false, 3, 1024, 0, 0, null, "test", "testg", - path, path, false, true, false); - } } /** - * Test a trash root that is encrypted for getTrashRoot + * Test a trash root that is inside a mount point for getTrashRoot */ @Test - public void testTrashRootEncryptedTrashDir() throws IOException { + public void testTrashRootDeepTrashDir() throws IOException { Configuration conf2 = ViewFileSystemTestSetup.createConfig(); conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); conf2.setClass("fs.mocktrashfs.impl", MockTrashRootFS.class, FileSystem.class); - ConfigUtil.addLink(conf2, "/mnt", URI.create("mocktrashfs://mnt/path")); - Path testPath = new Path("/mnt/projs/proj"); + ConfigUtil.addLink(conf2, "/mnt/datavol1", + URI.create("mocktrashfs://localhost/vol")); + Path testPath = new Path("/mnt/datavol1/projs/proj"); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Assert.assertEquals(MockTrashRootFS.TRASH, fsView2.getTrashRoot(testPath)); + Path expectedTrash = + new Path("/mnt/datavol1/very/deep/deep/trash/dir/.Trash"); + Assert.assertEquals(expectedTrash, fsView2.getTrashRoot(testPath)); } /** From a278cbccb6585cad126cc6793391a4484f5f3719 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 4 Mar 2022 20:16:38 -0800 Subject: [PATCH 09/26] Rename MACRO for our flag --- .../org/apache/hadoop/fs/viewfs/Constants.java | 6 +++--- .../apache/hadoop/fs/viewfs/ViewFileSystem.java | 12 ++++++------ .../hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 16 ++++++++-------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java index a5ea8316b7d14..de5f2b814f20e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java @@ -137,7 +137,7 @@ public interface Constants { * Enable ViewFileSystem to return a trashRoot which is in the root dir of a * mount point. */ - String CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT = - "fs.viewfs.trash.root.under.mount.point.root"; - boolean CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT = false; + String CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT = + "fs.viewfs.trash.force-inside-mount-point"; + boolean CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT = true; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index cd20dbf03fbb5..05acd3fad2559 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -25,8 +25,8 @@ import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT; import java.util.function.Function; import java.io.FileNotFoundException; @@ -1149,8 +1149,8 @@ public Collection getAllStoragePolicies() @Override public Path getTrashRoot(Path path) { boolean trashRootUnderMountPointRoot = - config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, - CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT); + config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, + CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); try { InodeTree.ResolveResult res = @@ -1220,8 +1220,8 @@ public Collection getTrashRoots(boolean allUsers) { } boolean trashRootUnderMountPointRoot = - config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, - CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT); + config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, + CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); // Return trashRoots if CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is // disabled. if (!trashRootUnderMountPointRoot) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index cf3265883ce71..7e4de56e2fa73 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -69,7 +69,7 @@ import static org.apache.hadoop.fs.FileSystemTestHelper.*; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_ENABLE_INNER_CACHE; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT; import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; import org.junit.After; @@ -1111,7 +1111,7 @@ public void testTrashRoot() throws IOException { public void testTrashRootRootOfMountPoint() throws IOException { UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); ConfigUtil.addLinkFallback(conf2, targetTestRoot.toUri()); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); @@ -1132,7 +1132,7 @@ public void testTrashRootRootOfMountPoint() throws IOException { // Case 3: turn off the CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH flag. // Return a trash root in user home dir. - conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, false); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); Path targetFSUserHomeTrashRoot = fsTarget.makeQualified(userHomeTrashRoot); Assert.assertEquals(targetFSUserHomeTrashRoot, @@ -1150,7 +1150,7 @@ public void testTrashRootRootOfMountPoint() throws IOException { /** * A mocked FileSystem which returns a deep trash dir. */ - static class MockTrashRootFS extends MockFileSystem { + static class DeepTrashRootMockFS extends MockFileSystem { public static final Path TRASH = new Path("/vol/very/deep/deep/trash/dir/.Trash"); @@ -1167,8 +1167,8 @@ public Path getTrashRoot(Path path) { public void testTrashRootDeepTrashDir() throws IOException { Configuration conf2 = ViewFileSystemTestSetup.createConfig(); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); - conf2.setClass("fs.mocktrashfs.impl", MockTrashRootFS.class, + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); + conf2.setClass("fs.mocktrashfs.impl", DeepTrashRootMockFS.class, FileSystem.class); ConfigUtil.addLink(conf2, "/mnt/datavol1", URI.create("mocktrashfs://localhost/vol")); @@ -1185,7 +1185,7 @@ public void testTrashRootDeepTrashDir() throws IOException { @Test public void testTrashRootsAllUsers() throws IOException { Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); // Case 1: verify correct trash roots from fsView and fsView2 @@ -1231,7 +1231,7 @@ public void testTrashRootsCurrentUser() throws IOException { String currentUser = UserGroupInformation.getCurrentUser().getShortUserName(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); int beforeTrashRootNum = fsView.getTrashRoots(false).size(); From 87f2a5de3eff355e98daed1618c660b2cbe23357 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 4 Mar 2022 20:40:19 -0800 Subject: [PATCH 10/26] Renamed flag to CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH --- .../apache/hadoop/fs/viewfs/Constants.java | 9 +++--- .../hadoop/fs/viewfs/ViewFileSystem.java | 30 +++++++++---------- .../fs/viewfs/ViewFileSystemBaseTest.java | 16 +++++----- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java index de5f2b814f20e..bbc7794aa14ba 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java @@ -134,10 +134,9 @@ public interface Constants { HCFSMountTableConfigLoader.class; /** - * Enable ViewFileSystem to return a trashRoot which is in the root dir of a - * mount point. + * Enable ViewFileSystem to return the viewFS path for the trashRoot */ - String CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT = - "fs.viewfs.trash.force-inside-mount-point"; - boolean CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT = true; + String CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH = + "fs.viewfs.trash.use-viewfs-path"; + boolean CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT = true; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 05acd3fad2559..8887ee4c1a02c 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -25,8 +25,8 @@ import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT; import java.util.function.Function; import java.io.FileNotFoundException; @@ -1132,14 +1132,15 @@ public Collection getAllStoragePolicies() * Get the trash root directory for current user when the path * specified is deleted. * - * If CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is not set, return - * the default trash root from targetFS. + * If TRASH_USE_VIEWFS_PATH flag is not set, return the default trash root + * from targetFS. * - * When CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is set to true, + * When TRASH_USE_VIEWFS_PATH is set to true, * 1) If path p in fallback FS, return trash root from fallback FS. Print a - * warning if there is a mount point for trash root. - * 2) If trash root for path p is in the same mount point as path p, get - * the corresponding viewFS path for the trash root and return it. + * warning if there is a mount point for the trash root. + * 2) If the trash root for path p is in the same mount point as path p, + * when trashRoot.startWith(mountPoint.destPath) == true, + * get the corresponding viewFS path for the trash root and return it. * 3) else, return the trash root under the root of the mount point * (/mntpoint/.Trash/{user}). * @@ -1149,8 +1150,8 @@ public Collection getAllStoragePolicies() @Override public Path getTrashRoot(Path path) { boolean trashRootUnderMountPointRoot = - config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, - CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); + config.getBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, + CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT); try { InodeTree.ResolveResult res = @@ -1205,9 +1206,8 @@ public Path getTrashRoot(Path path) { /** * Get all the trash roots for current user or all users. * - * When CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is set to true, we - * also return trash roots under the root of each mount point, with their - * viewFS paths. + * When TRASH_USE_VIEWFS_PATH is set to true, we also return trash roots + * under the root of each mount point, with their viewFS paths. * * @param allUsers return trash roots for all users if true. * @return all Trash root directories. @@ -1220,8 +1220,8 @@ public Collection getTrashRoots(boolean allUsers) { } boolean trashRootUnderMountPointRoot = - config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, - CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); + config.getBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, + CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT); // Return trashRoots if CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is // disabled. if (!trashRootUnderMountPointRoot) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 7e4de56e2fa73..06fdf2209d45c 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -69,7 +69,7 @@ import static org.apache.hadoop.fs.FileSystemTestHelper.*; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_ENABLE_INNER_CACHE; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH; import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; import org.junit.After; @@ -1105,13 +1105,13 @@ public void testTrashRoot() throws IOException { } /** - * Test trash root at the root of mount points + * Test TRASH_USE_VIEWFS_PATH feature for getTrashRoot */ @Test public void testTrashRootRootOfMountPoint() throws IOException { UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); ConfigUtil.addLinkFallback(conf2, targetTestRoot.toUri()); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); @@ -1130,9 +1130,9 @@ public void testTrashRootRootOfMountPoint() throws IOException { Assert.assertEquals(userHomeTrashRoot, fsView2.getTrashRoot(nonExistentPath)); - // Case 3: turn off the CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH flag. + // Case 3: turn off the CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH flag. // Return a trash root in user home dir. - conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, false); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); Path targetFSUserHomeTrashRoot = fsTarget.makeQualified(userHomeTrashRoot); Assert.assertEquals(targetFSUserHomeTrashRoot, @@ -1167,7 +1167,7 @@ public Path getTrashRoot(Path path) { public void testTrashRootDeepTrashDir() throws IOException { Configuration conf2 = ViewFileSystemTestSetup.createConfig(); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); conf2.setClass("fs.mocktrashfs.impl", DeepTrashRootMockFS.class, FileSystem.class); ConfigUtil.addLink(conf2, "/mnt/datavol1", @@ -1185,7 +1185,7 @@ public void testTrashRootDeepTrashDir() throws IOException { @Test public void testTrashRootsAllUsers() throws IOException { Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); // Case 1: verify correct trash roots from fsView and fsView2 @@ -1231,7 +1231,7 @@ public void testTrashRootsCurrentUser() throws IOException { String currentUser = UserGroupInformation.getCurrentUser().getShortUserName(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); int beforeTrashRootNum = fsView.getTrashRoots(false).size(); From d0ae15eecde9f3c08b861cabfb8c582487a98bc2 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 4 Mar 2022 20:53:53 -0800 Subject: [PATCH 11/26] Alright, renamed flag back to CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT inside the same targetFS is different from inside the same mount point. --- .../apache/hadoop/fs/viewfs/Constants.java | 8 +++--- .../hadoop/fs/viewfs/ViewFileSystem.java | 25 +++++++++---------- .../fs/viewfs/ViewFileSystemBaseTest.java | 16 ++++++------ 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java index bbc7794aa14ba..2bb6a1229da70 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java @@ -134,9 +134,9 @@ public interface Constants { HCFSMountTableConfigLoader.class; /** - * Enable ViewFileSystem to return the viewFS path for the trashRoot + * Force ViewFileSystem to return a trashRoot that is inside a mount point. */ - String CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH = - "fs.viewfs.trash.use-viewfs-path"; - boolean CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT = true; + String CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT = + "fs.viewfs.trash.force-inside-mount-point"; + boolean CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT = true; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 8887ee4c1a02c..cac4344f0e8f4 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -25,8 +25,8 @@ import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT; import java.util.function.Function; import java.io.FileNotFoundException; @@ -1149,9 +1149,9 @@ public Collection getAllStoragePolicies() */ @Override public Path getTrashRoot(Path path) { - boolean trashRootUnderMountPointRoot = - config.getBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, - CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT); + boolean trashForceInsideMountPoint = + config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, + CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); try { InodeTree.ResolveResult res = @@ -1162,7 +1162,7 @@ public Path getTrashRoot(Path path) { String targetFSTrashRootPath = targetFSTrashRoot.toUri().getPath(); String mountTargetPath = res.targetFileSystem.getUri().getPath(); - if (!trashRootUnderMountPointRoot) { + if (!trashForceInsideMountPoint) { return targetFSTrashRoot; } @@ -1206,7 +1206,7 @@ public Path getTrashRoot(Path path) { /** * Get all the trash roots for current user or all users. * - * When TRASH_USE_VIEWFS_PATH is set to true, we also return trash roots + * When FORCE_INSIDE_MOUNT_POINT is set to true, we also return trash roots * under the root of each mount point, with their viewFS paths. * * @param allUsers return trash roots for all users if true. @@ -1219,12 +1219,11 @@ public Collection getTrashRoots(boolean allUsers) { trashRoots.addAll(fs.getTrashRoots(allUsers)); } - boolean trashRootUnderMountPointRoot = - config.getBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, - CONFIG_VIEWFS_TRASH_VIEWFS_PATH_DEFAULT); - // Return trashRoots if CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is - // disabled. - if (!trashRootUnderMountPointRoot) { + boolean trashForceInsideMountPoint = + config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, + CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); + // Return trashRoots if FORCE_INSIDE_MOUNT_POINT is disabled. + if (!trashForceInsideMountPoint) { return trashRoots; } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 06fdf2209d45c..4447256fabc5b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -69,7 +69,7 @@ import static org.apache.hadoop.fs.FileSystemTestHelper.*; import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_ENABLE_INNER_CACHE; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; -import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT; import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; import org.junit.After; @@ -1105,13 +1105,13 @@ public void testTrashRoot() throws IOException { } /** - * Test TRASH_USE_VIEWFS_PATH feature for getTrashRoot + * Test TRASH_FORCE_INSIDE_MOUNT_POINT feature for getTrashRoot */ @Test public void testTrashRootRootOfMountPoint() throws IOException { UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); ConfigUtil.addLinkFallback(conf2, targetTestRoot.toUri()); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); @@ -1130,9 +1130,9 @@ public void testTrashRootRootOfMountPoint() throws IOException { Assert.assertEquals(userHomeTrashRoot, fsView2.getTrashRoot(nonExistentPath)); - // Case 3: turn off the CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH flag. + // Case 3: turn off the CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT flag. // Return a trash root in user home dir. - conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, false); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); Path targetFSUserHomeTrashRoot = fsTarget.makeQualified(userHomeTrashRoot); Assert.assertEquals(targetFSUserHomeTrashRoot, @@ -1167,7 +1167,7 @@ public Path getTrashRoot(Path path) { public void testTrashRootDeepTrashDir() throws IOException { Configuration conf2 = ViewFileSystemTestSetup.createConfig(); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); conf2.setClass("fs.mocktrashfs.impl", DeepTrashRootMockFS.class, FileSystem.class); ConfigUtil.addLink(conf2, "/mnt/datavol1", @@ -1185,7 +1185,7 @@ public void testTrashRootDeepTrashDir() throws IOException { @Test public void testTrashRootsAllUsers() throws IOException { Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); // Case 1: verify correct trash roots from fsView and fsView2 @@ -1231,7 +1231,7 @@ public void testTrashRootsCurrentUser() throws IOException { String currentUser = UserGroupInformation.getCurrentUser().getShortUserName(); Configuration conf2 = new Configuration(conf); - conf2.setBoolean(CONFIG_VIEWFS_TRASH_USE_VIEWFS_PATH, true); + conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); int beforeTrashRootNum = fsView.getTrashRoots(false).size(); From 7a8d790cdea335bb3b46ac8429f33e6f0df1b59e Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 4 Mar 2022 21:04:38 -0800 Subject: [PATCH 12/26] minor fix fo the warning --- .../java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index cac4344f0e8f4..d1346deb128ff 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1177,9 +1177,9 @@ public Path getTrashRoot(Path path) { fsState.resolve(targetFSTrashRootPath, true); if (!ROOT_PATH.equals(new Path(res2.resolvedPath))) { LOG.warn(String.format("A mount point %s exists for trash root %s " - + "returned by fallback FS for path %s. Rename between %s and " - + "trash root %s will fail, as the trash root is in a mount" - + " point while %s is in the fallback FS", + + "returned by fallback FS for path %s. Rename between path %s " + + "and trash root %s will fail, as the trash root is in a " + + "mount point while path %s is in the fallback FS", res2.resolvedPath, targetFSTrashRootPath, path, path, targetFSTrashRootPath, path)); } From 36b5e3d4de905ec1bf9a747802be8753818818b9 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 4 Mar 2022 21:10:08 -0800 Subject: [PATCH 13/26] Fixed a unit test function name --- .../org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 4447256fabc5b..87a06a51b99d1 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1108,7 +1108,7 @@ public void testTrashRoot() throws IOException { * Test TRASH_FORCE_INSIDE_MOUNT_POINT feature for getTrashRoot */ @Test - public void testTrashRootRootOfMountPoint() throws IOException { + public void testTrashRootForceInsideMountPoint() throws IOException { UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); Configuration conf2 = new Configuration(conf); conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, true); From 83e6be7125c6885411d6ff74cb80bdaeb331fc97 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Mon, 7 Mar 2022 09:48:49 -0800 Subject: [PATCH 14/26] changed CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT to false --- .../src/main/java/org/apache/hadoop/fs/viewfs/Constants.java | 2 +- .../org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java index 2bb6a1229da70..21f4d99f891c2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java @@ -138,5 +138,5 @@ public interface Constants { */ String CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT = "fs.viewfs.trash.force-inside-mount-point"; - boolean CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT = true; + boolean CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT = false; } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 87a06a51b99d1..1c8da2c2f5845 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1161,7 +1161,7 @@ public Path getTrashRoot(Path path) { } /** - * Test a trash root that is inside a mount point for getTrashRoot + * Test getTrashRoot that is very deep inside a mount point. */ @Test public void testTrashRootDeepTrashDir() throws IOException { From 10f1436fed70a73c71a641a6c1d8b14a2e1af4fc Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Mon, 7 Mar 2022 09:51:28 -0800 Subject: [PATCH 15/26] Fixed checkstyle --- .../org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 1c8da2c2f5845..2911b763ca01c 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1105,7 +1105,7 @@ public void testTrashRoot() throws IOException { } /** - * Test TRASH_FORCE_INSIDE_MOUNT_POINT feature for getTrashRoot + * Test TRASH_FORCE_INSIDE_MOUNT_POINT feature for getTrashRoot. */ @Test public void testTrashRootForceInsideMountPoint() throws IOException { From 28bff6a3ab3b05229a2650da53434e3a81e45593 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Mon, 7 Mar 2022 09:58:37 -0800 Subject: [PATCH 16/26] Fixed out-of-date flag name in comments --- .../main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index d1346deb128ff..a781d3a772b78 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1132,10 +1132,10 @@ public Collection getAllStoragePolicies() * Get the trash root directory for current user when the path * specified is deleted. * - * If TRASH_USE_VIEWFS_PATH flag is not set, return the default trash root + * If FORCE_INSIDE_MOUNT_POINT flag is not set, return the default trash root * from targetFS. * - * When TRASH_USE_VIEWFS_PATH is set to true, + * When FORCE_INSIDE_MOUNT_POINT is set to true, * 1) If path p in fallback FS, return trash root from fallback FS. Print a * warning if there is a mount point for the trash root. * 2) If the trash root for path p is in the same mount point as path p, From 4d27c7593cc3c5a98152da54e484ec5908397bce Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Tue, 8 Mar 2022 10:47:35 -0800 Subject: [PATCH 17/26] Addressed Owen's comment and add a check for default trash when mount target is null or /. --- .../apache/hadoop/fs/TrashPolicyDefault.java | 2 +- .../hadoop/fs/viewfs/ViewFileSystem.java | 108 ++++++++++-------- .../fs/viewfs/ViewFileSystemBaseTest.java | 11 +- 3 files changed, 67 insertions(+), 54 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java index ec08abfbdcb28..b86d1050b35f9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java @@ -192,7 +192,7 @@ public boolean moveToTrash(Path path) throws IOException { } } throw (IOException) new IOException( - "Failed to move " + path + " to trash " + trashPath).initCause(cause); + "Failed to move " + path + " to trash " + trashPath, cause); } @SuppressWarnings("deprecation") diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index a781d3a772b78..e9dbd8de74eb4 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1149,24 +1149,23 @@ public Collection getAllStoragePolicies() */ @Override public Path getTrashRoot(Path path) { - boolean trashForceInsideMountPoint = - config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, - CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); try { InodeTree.ResolveResult res = fsState.resolve(getUriPath(path), true); - Path targetFSTrashRoot = res.targetFileSystem.getTrashRoot(res.remainingPath); - String targetFSTrashRootPath = targetFSTrashRoot.toUri().getPath(); - String mountTargetPath = res.targetFileSystem.getUri().getPath(); + boolean trashForceInsideMountPoint = + config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, + CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); if (!trashForceInsideMountPoint) { return targetFSTrashRoot; } // New trash root policy + String targetFSTrashRootPath = targetFSTrashRoot.toUri().getPath(); + String mountTargetPath = res.targetFileSystem.getUri().getPath(); if (ROOT_PATH.equals(new Path(res.resolvedPath))) { // Path path is in fallback FS. Return targetFSTrashRootPath; @@ -1177,26 +1176,38 @@ public Path getTrashRoot(Path path) { fsState.resolve(targetFSTrashRootPath, true); if (!ROOT_PATH.equals(new Path(res2.resolvedPath))) { LOG.warn(String.format("A mount point %s exists for trash root %s " - + "returned by fallback FS for path %s. Rename between path %s " - + "and trash root %s will fail, as the trash root is in a " + + "returned by fallback FS for path %s. Rename between path " + + "%s and trash root %s will fail, as the trash root is in a " + "mount point while path %s is in the fallback FS", res2.resolvedPath, targetFSTrashRootPath, path, path, targetFSTrashRootPath, path)); } - return new Path(targetFSTrashRootPath); + return makeQualified(new Path(targetFSTrashRootPath)); } else if (targetFSTrashRootPath.startsWith(mountTargetPath)) { // targetFSTrash is inside mountPoint.targetPath - String relativeTrashRoot = - targetFSTrashRootPath.substring(mountTargetPath.length()); - // remove the starting '/' if it exists - if (relativeTrashRoot.startsWith("/")) { - relativeTrashRoot = relativeTrashRoot.substring(1); + + // Check whether mountTargetPath is null or '/' and whether + // targetFSTrashRootPath is the default trash root based on user home + // dir. + Path defaultUserHome = res.targetFileSystem.getHomeDirectory(); + if ((mountTargetPath == null || ROOT_PATH.equals(mountTargetPath)) + && defaultUserHome != null && targetFSTrashRootPath.equals( + new Path(defaultUserHome, TRASH_PREFIX))) { + return makeQualified(new Path(res.resolvedPath, + TRASH_PREFIX + "/" + ugi.getShortUserName())); + } else { + String relativeTrashRoot = + targetFSTrashRootPath.substring(mountTargetPath.length()); + // remove the starting '/' if it exists + if (relativeTrashRoot.startsWith("/")) { + relativeTrashRoot = relativeTrashRoot.substring(1); + } + return makeQualified(new Path(res.resolvedPath, relativeTrashRoot)); } - return new Path(res.resolvedPath, relativeTrashRoot); } else { // Return the trash root for the mount point. - return new Path(res.resolvedPath, - TRASH_PREFIX + "/" + ugi.getShortUserName()); + return makeQualified(new Path(res.resolvedPath, + TRASH_PREFIX + "/" + ugi.getShortUserName())); } } catch (IOException | IllegalArgumentException e) { throw new NotInMountpointException(path, "getTrashRoot"); @@ -1214,9 +1225,17 @@ public Path getTrashRoot(Path path) { */ @Override public Collection getTrashRoots(boolean allUsers) { - List trashRoots = new ArrayList<>(); + // A map from targetFSPath -> FileStatus. + // FileStatus can be from targetFS or viewFS. + + // TargetFS path is used to check and remove duplicate trash roots + // between childFS.getTrashRoots() and trash roots in TRASH_PREFIX dir for + // each mount point. + HashMap trashRoots = new HashMap<>(); for (FileSystem fs : getChildFileSystems()) { - trashRoots.addAll(fs.getTrashRoots(allUsers)); + for (FileStatus trash : fs.getTrashRoots(allUsers)) { + trashRoots.put(trash.getPath(), trash); + } } boolean trashForceInsideMountPoint = @@ -1224,56 +1243,49 @@ public Collection getTrashRoots(boolean allUsers) { CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); // Return trashRoots if FORCE_INSIDE_MOUNT_POINT is disabled. if (!trashForceInsideMountPoint) { - return trashRoots; - } - - // We use targetFS Path to check for duplicate paths. - Set uniqueTrashRoots = new HashSet<>(); - for (FileStatus file : trashRoots) { - uniqueTrashRoots.add(file.getPath()); + return trashRoots.values(); } + // Get trash roots in TRASH_PREFIX dir inside mount points. List> mountPoints = fsState.getMountPoints(); try { for (InodeTree.MountPoint mountPoint : mountPoints) { - Path trashRoot = new Path(mountPoint.src + "/" + TRASH_PREFIX); + Path trashRoot = + makeQualified(new Path(mountPoint.src + "/" + TRASH_PREFIX)); // Continue if trashRoot does not exist for this mount point if (!exists(trashRoot)) { continue; } + FileSystem targetFS = mountPoint.target.getTargetFileSystem(); if (!allUsers) { Path userTrashRoot = new Path(trashRoot, ugi.getShortUserName()); if (exists(userTrashRoot)) { - Path targetFsTrashPath = - new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName()); - FileStatus targetFsTrash = mountPoint.target.getTargetFileSystem() - .getFileStatus(targetFsTrashPath); - if (!uniqueTrashRoots.contains(targetFsTrash.getPath())) { - FileStatus trash = getFileStatus(userTrashRoot); - trashRoots.add(trash); - uniqueTrashRoots.add(targetFsTrash.getPath()); - } + Path targetFsTrashPath = targetFS.getFileStatus( + new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName())) + .getPath(); + // Set or replace FileStatus for targetFsTrashPath with the + // FileStatus from viewFS + trashRoots.put(targetFsTrashPath, getFileStatus(userTrashRoot)); } } else { FileStatus[] mountPointTrashRoots = listStatus(trashRoot); for (FileStatus trash : mountPointTrashRoots) { - // Remove the mountPoint to get the targetFS path + // Remove the mountPoint to get the targetFS path and use + String targetFsTrash = trash.getPath().toUri().getPath() + .substring(mountPoint.src.length()); + + // Use getFileStatus to get the full path. + // Do not use targetFS.makeQualified(new Path(targetFsTrash)). Path targetFsTrashPath = - new Path(trash.getPath().toUri().getPath() - .substring(mountPoint.src.length())); - FileStatus targetFsTrash = mountPoint.target.getTargetFileSystem() - .getFileStatus(targetFsTrashPath); - // skip if we already include it in uniqueTrashRoots - if (uniqueTrashRoots.contains(targetFsTrash.getPath())) { - continue; - } + targetFS.getFileStatus(new Path(targetFsTrash)).getPath(); - trashRoots.add(trash); - uniqueTrashRoots.add(targetFsTrash.getPath()); + // Set or replace FileStatus for targetFsTrashPath with the + // FileStatus from viewFS + trashRoots.put(targetFsTrashPath, trash); } } } @@ -1281,7 +1293,7 @@ public Collection getTrashRoots(boolean allUsers) { LOG.warn("Exception in get all trash roots", e); } - return trashRoots; + return trashRoots.values(); } @Override diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 2911b763ca01c..9bedd9684fc01 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1118,8 +1118,8 @@ public void testTrashRootForceInsideMountPoint() throws IOException { // Case 1: path p in the /data mount point. // Return a trash root within the mount point. Path dataTestPath = new Path("/data/dir/file"); - Path dataTrashRoot = - new Path("/data/" + TRASH_PREFIX + "/" + ugi.getShortUserName()); + Path dataTrashRoot = fsView2.makeQualified( + new Path("/data/" + TRASH_PREFIX + "/" + ugi.getShortUserName())); Assert.assertEquals(dataTrashRoot, fsView2.getTrashRoot(dataTestPath)); // Case 2: path p not found in mount table, fall back to the default FS @@ -1127,7 +1127,8 @@ public void testTrashRootForceInsideMountPoint() throws IOException { Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); Path userHomeTrashRoot = new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX); - Assert.assertEquals(userHomeTrashRoot, + Path viewFSUserHomeTrashRoot = fsView2.makeQualified(userHomeTrashRoot); + Assert.assertEquals(viewFSUserHomeTrashRoot, fsView2.getTrashRoot(nonExistentPath)); // Case 3: turn off the CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT flag. @@ -1174,8 +1175,8 @@ public void testTrashRootDeepTrashDir() throws IOException { URI.create("mocktrashfs://localhost/vol")); Path testPath = new Path("/mnt/datavol1/projs/proj"); FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Path expectedTrash = - new Path("/mnt/datavol1/very/deep/deep/trash/dir/.Trash"); + Path expectedTrash = fsView2.makeQualified( + new Path("/mnt/datavol1/very/deep/deep/trash/dir/.Trash")); Assert.assertEquals(expectedTrash, fsView2.getTrashRoot(testPath)); } From f77abb0e5c17bf75e999f978e7192071318f2152 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Tue, 8 Mar 2022 10:52:36 -0800 Subject: [PATCH 18/26] fixed line break --- .../main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index e9dbd8de74eb4..0c297bf84de53 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1274,9 +1274,9 @@ public Collection getTrashRoots(boolean allUsers) { } else { FileStatus[] mountPointTrashRoots = listStatus(trashRoot); for (FileStatus trash : mountPointTrashRoots) { - // Remove the mountPoint to get the targetFS path and use + // Remove the mountPoint to get the targetFsTrash path String targetFsTrash = trash.getPath().toUri().getPath() - .substring(mountPoint.src.length()); + .substring(mountPoint.src.length()); // Use getFileStatus to get the full path. // Do not use targetFS.makeQualified(new Path(targetFsTrash)). From af3071503d9353b1be18008f4daec7093128b2aa Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Tue, 8 Mar 2022 14:20:23 -0800 Subject: [PATCH 19/26] Fixed the path comparison --- .../java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 0c297bf84de53..d7ab7c4b46ee5 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1190,9 +1190,10 @@ public Path getTrashRoot(Path path) { // targetFSTrashRootPath is the default trash root based on user home // dir. Path defaultUserHome = res.targetFileSystem.getHomeDirectory(); - if ((mountTargetPath == null || ROOT_PATH.equals(mountTargetPath)) - && defaultUserHome != null && targetFSTrashRootPath.equals( - new Path(defaultUserHome, TRASH_PREFIX))) { + if ((mountTargetPath == null || mountTargetPath.isEmpty() + || ROOT_PATH.equals(mountTargetPath)) && defaultUserHome != null + && targetFSTrashRootPath.equals( + defaultUserHome.toUri().getPath() + "/" + TRASH_PREFIX)) { return makeQualified(new Path(res.resolvedPath, TRASH_PREFIX + "/" + ugi.getShortUserName())); } else { From 808749d4b5a469dc1096e364b7146e3b377da23a Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Tue, 8 Mar 2022 15:45:16 -0800 Subject: [PATCH 20/26] fix spotbugs --- .../java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index d7ab7c4b46ee5..ca81f3682bdd0 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1164,6 +1164,8 @@ public Path getTrashRoot(Path path) { } // New trash root policy + // Uri.getPath() will return an empty string if path is not defined in + // the URI. String targetFSTrashRootPath = targetFSTrashRoot.toUri().getPath(); String mountTargetPath = res.targetFileSystem.getUri().getPath(); if (ROOT_PATH.equals(new Path(res.resolvedPath))) { @@ -1190,9 +1192,9 @@ public Path getTrashRoot(Path path) { // targetFSTrashRootPath is the default trash root based on user home // dir. Path defaultUserHome = res.targetFileSystem.getHomeDirectory(); - if ((mountTargetPath == null || mountTargetPath.isEmpty() - || ROOT_PATH.equals(mountTargetPath)) && defaultUserHome != null - && targetFSTrashRootPath.equals( + if ((mountTargetPath.isEmpty() || + ROOT_PATH.equals(new Path(mountTargetPath))) + && defaultUserHome != null && targetFSTrashRootPath.equals( defaultUserHome.toUri().getPath() + "/" + TRASH_PREFIX)) { return makeQualified(new Path(res.resolvedPath, TRASH_PREFIX + "/" + ugi.getShortUserName())); From b18af88b1e291f959feae6506ca4dba9d625cdd5 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 11 Mar 2022 15:48:05 -0800 Subject: [PATCH 21/26] Another pass to incorporate Owen's changes. --- .../apache/hadoop/fs/TrashPolicyDefault.java | 4 +- .../hadoop/fs/viewfs/ViewFileSystem.java | 136 ++++++++---------- .../fs/viewfs/ViewFileSystemBaseTest.java | 13 +- 3 files changed, 66 insertions(+), 87 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java index b86d1050b35f9..f4228dea69f49 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java +++ b/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 " + path + " to trash " + trashPath, cause); + throw new IOException("Failed to move " + path + " to trash " + trashPath, + cause); } @SuppressWarnings("deprecation") diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index ca81f3682bdd0..35b6151b770e8 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1136,13 +1136,33 @@ public Collection getAllStoragePolicies() * from targetFS. * * When FORCE_INSIDE_MOUNT_POINT is set to true, - * 1) If path p in fallback FS, return trash root from fallback FS. Print a - * warning if there is a mount point for the trash root. - * 2) If the trash root for path p is in the same mount point as path p, - * when trashRoot.startWith(mountPoint.destPath) == true, - * get the corresponding viewFS path for the trash root and return it. - * 3) else, return the trash root under the root of the mount point - * (/mntpoint/.Trash/{user}). + *
    + *
  1. + * If the trash root for path p is in the same mount point as path p, + * and one of: + *
      + *
    1. The mount point isn't at the top of the target fs.
      2. + *
    2. The resolved path of path is root (in fallback FS).
      3. + *
    3. The trash isn't in user's target fs home directory + * get the corresponding viewFS path for the trash root and return + * it. + *
      4. + *
    + *
    2. + *
  2. + * else, return the trash root under the root of the mount point + * (/{mntpoint}/.Trash/{user}). + *
    3. + *
+ * + * Condition 1 handles several different important cases: + *
    + *
  • File systems may need to have more local trash roots, such as + * encryption zones or snapshot roots.
    • + *
  • The fallback mount should use the user's home directory.
    • + *
  • Cloud storage systems should not use the user's home directory, + * if the mount points to the top of the container or bucket.
    • + *
* * @param path the trash root of the path to be determined. * @return the trash root path. @@ -1156,57 +1176,29 @@ public Path getTrashRoot(Path path) { Path targetFSTrashRoot = res.targetFileSystem.getTrashRoot(res.remainingPath); - boolean trashForceInsideMountPoint = - config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, - CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); - if (!trashForceInsideMountPoint) { + // Allow clients to use old behavior of delegating to target fs. + if (!config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, + CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT)) { return targetFSTrashRoot; } - // New trash root policy - // Uri.getPath() will return an empty string if path is not defined in - // the URI. + // The trash root path from the target fs String targetFSTrashRootPath = targetFSTrashRoot.toUri().getPath(); + // The mount point path in the target fs String mountTargetPath = res.targetFileSystem.getUri().getPath(); - if (ROOT_PATH.equals(new Path(res.resolvedPath))) { - // Path path is in fallback FS. Return targetFSTrashRootPath; - - // Check and print a warning if a mountpoint exists for - // targetFSTrashRootPath. The trash root from fallback FS should be in - // the fallback FS, not in a different mount point. - InodeTree.ResolveResult res2 = - fsState.resolve(targetFSTrashRootPath, true); - if (!ROOT_PATH.equals(new Path(res2.resolvedPath))) { - LOG.warn(String.format("A mount point %s exists for trash root %s " - + "returned by fallback FS for path %s. Rename between path " - + "%s and trash root %s will fail, as the trash root is in a " - + "mount point while path %s is in the fallback FS", - res2.resolvedPath, targetFSTrashRootPath, path, path, - targetFSTrashRootPath, path)); - } - return makeQualified(new Path(targetFSTrashRootPath)); - } else if (targetFSTrashRootPath.startsWith(mountTargetPath)) { - // targetFSTrash is inside mountPoint.targetPath - - // Check whether mountTargetPath is null or '/' and whether - // targetFSTrashRootPath is the default trash root based on user home - // dir. - Path defaultUserHome = res.targetFileSystem.getHomeDirectory(); - if ((mountTargetPath.isEmpty() || - ROOT_PATH.equals(new Path(mountTargetPath))) - && defaultUserHome != null && targetFSTrashRootPath.equals( - defaultUserHome.toUri().getPath() + "/" + TRASH_PREFIX)) { - return makeQualified(new Path(res.resolvedPath, - TRASH_PREFIX + "/" + ugi.getShortUserName())); - } else { - String relativeTrashRoot = - targetFSTrashRootPath.substring(mountTargetPath.length()); - // remove the starting '/' if it exists - if (relativeTrashRoot.startsWith("/")) { - relativeTrashRoot = relativeTrashRoot.substring(1); - } - return makeQualified(new Path(res.resolvedPath, relativeTrashRoot)); - } + if (!mountTargetPath.endsWith("/")) { + mountTargetPath = mountTargetPath + "/"; + } + + Path targetFsUserHome = res.targetFileSystem.getHomeDirectory(); + if (targetFSTrashRootPath.startsWith(mountTargetPath) && !( + mountTargetPath.equals(ROOT_PATH.toString()) + && !res.resolvedPath.equals(ROOT_PATH.toString()) && ( + targetFsUserHome != null && targetFSTrashRootPath.startsWith( + targetFsUserHome.toUri().getPath())))) { + String relativeTrashRoot = + targetFSTrashRootPath.substring(mountTargetPath.length()); + return makeQualified(new Path(res.resolvedPath, relativeTrashRoot)); } else { // Return the trash root for the mount point. return makeQualified(new Path(res.resolvedPath, @@ -1231,9 +1223,6 @@ public Collection getTrashRoots(boolean allUsers) { // A map from targetFSPath -> FileStatus. // FileStatus can be from targetFS or viewFS. - // TargetFS path is used to check and remove duplicate trash roots - // between childFS.getTrashRoots() and trash roots in TRASH_PREFIX dir for - // each mount point. HashMap trashRoots = new HashMap<>(); for (FileSystem fs : getChildFileSystems()) { for (FileStatus trash : fs.getTrashRoots(allUsers)) { @@ -1241,19 +1230,15 @@ public Collection getTrashRoots(boolean allUsers) { } } - boolean trashForceInsideMountPoint = - config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, - CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT); // Return trashRoots if FORCE_INSIDE_MOUNT_POINT is disabled. - if (!trashForceInsideMountPoint) { + if (!config.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, + CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT)) { return trashRoots.values(); } // Get trash roots in TRASH_PREFIX dir inside mount points. - List> mountPoints = - fsState.getMountPoints(); try { - for (InodeTree.MountPoint mountPoint : mountPoints) { + for (InodeTree.MountPoint mountPoint : fsState.getMountPoints()) { Path trashRoot = makeQualified(new Path(mountPoint.src + "/" + TRASH_PREFIX)); @@ -1267,27 +1252,20 @@ public Collection getTrashRoots(boolean allUsers) { if (!allUsers) { Path userTrashRoot = new Path(trashRoot, ugi.getShortUserName()); if (exists(userTrashRoot)) { - Path targetFsTrashPath = targetFS.getFileStatus( - new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName())) - .getPath(); - // Set or replace FileStatus for targetFsTrashPath with the - // FileStatus from viewFS - trashRoots.put(targetFsTrashPath, getFileStatus(userTrashRoot)); + Path targetFSUserTrashRoot = targetFS.makeQualified( + new Path(targetFS.getUri().getPath(), + TRASH_PREFIX + "/" + ugi.getShortUserName())); + trashRoots.put(targetFSUserTrashRoot, getFileStatus(userTrashRoot)); } } else { FileStatus[] mountPointTrashRoots = listStatus(trashRoot); for (FileStatus trash : mountPointTrashRoots) { - // Remove the mountPoint to get the targetFsTrash path + // Remove the mountPoint and the leading '/' to get the + // relative targetFsTrash path String targetFsTrash = trash.getPath().toUri().getPath() - .substring(mountPoint.src.length()); - - // Use getFileStatus to get the full path. - // Do not use targetFS.makeQualified(new Path(targetFsTrash)). - Path targetFsTrashPath = - targetFS.getFileStatus(new Path(targetFsTrash)).getPath(); - - // Set or replace FileStatus for targetFsTrashPath with the - // FileStatus from viewFS + .substring(mountPoint.src.length() + 1); + Path targetFsTrashPath = targetFS.makeQualified( + new Path(targetFS.getUri().getPath(), targetFsTrash)); trashRoots.put(targetFsTrashPath, trash); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 9bedd9684fc01..2d355cf6d5a69 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1116,18 +1116,18 @@ public void testTrashRootForceInsideMountPoint() throws IOException { FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); // Case 1: path p in the /data mount point. - // Return a trash root within the mount point. + // Return a trash root within the /data mount point. Path dataTestPath = new Path("/data/dir/file"); Path dataTrashRoot = fsView2.makeQualified( new Path("/data/" + TRASH_PREFIX + "/" + ugi.getShortUserName())); Assert.assertEquals(dataTrashRoot, fsView2.getTrashRoot(dataTestPath)); // Case 2: path p not found in mount table, fall back to the default FS - // Return a trash root in user home dir + // Return a trash root in mount point "/" Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); - Path userHomeTrashRoot = - new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX); - Path viewFSUserHomeTrashRoot = fsView2.makeQualified(userHomeTrashRoot); + Path rootTrashRoot = fsView2.makeQualified( + new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName())); + Path viewFSUserHomeTrashRoot = fsView2.makeQualified(rootTrashRoot); Assert.assertEquals(viewFSUserHomeTrashRoot, fsView2.getTrashRoot(nonExistentPath)); @@ -1135,7 +1135,8 @@ public void testTrashRootForceInsideMountPoint() throws IOException { // Return a trash root in user home dir. conf2.setBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT, false); fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2); - Path targetFSUserHomeTrashRoot = fsTarget.makeQualified(userHomeTrashRoot); + Path targetFSUserHomeTrashRoot = fsTarget.makeQualified( + new Path(fsTarget.getHomeDirectory(), TRASH_PREFIX)); Assert.assertEquals(targetFSUserHomeTrashRoot, fsView2.getTrashRoot(dataTestPath)); From 78b66f7f398ad2c55e4c7bdd23f228959f4e6238 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 11 Mar 2022 21:45:18 -0800 Subject: [PATCH 22/26] checkpoint of use if/else to decide expectedTrash. --- .../fs/viewfs/ViewFileSystemBaseTest.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 2d355cf6d5a69..4c4f95d3c860b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1104,6 +1104,9 @@ public void testTrashRoot() throws IOException { Assert.assertTrue("", fsView.getTrashRoots(true).size() > 0); } + private String getTrashRootForPathInFallBackFS() { + return fsTarget.getHomeDirectory().toUri().getPath() + "/" + TRASH_PREFIX; + } /** * Test TRASH_FORCE_INSIDE_MOUNT_POINT feature for getTrashRoot. */ @@ -1125,11 +1128,17 @@ public void testTrashRootForceInsideMountPoint() throws IOException { // Case 2: path p not found in mount table, fall back to the default FS // Return a trash root in mount point "/" Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); - Path rootTrashRoot = fsView2.makeQualified( - new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName())); - Path viewFSUserHomeTrashRoot = fsView2.makeQualified(rootTrashRoot); - Assert.assertEquals(viewFSUserHomeTrashRoot, - fsView2.getTrashRoot(nonExistentPath)); + Path expectedTrash; + if (targetTestRoot.toUri().getPath().equals("/")) { + expectedTrash = + fsView2.makeQualified(new Path(fsTarget.getHomeDirectory().toUri().getPath(), + TRASH_PREFIX)); + } else { + expectedTrash = + fsView2.makeQualified(new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName())); + } + + Assert.assertEquals(expectedTrash, fsView2.getTrashRoot(nonExistentPath)); // Case 3: turn off the CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT flag. // Return a trash root in user home dir. From 095daccfaf08414fb39d10f54a90322b6ee58ced Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 11 Mar 2022 22:16:39 -0800 Subject: [PATCH 23/26] Added and overwrite getTrashRootInFallBackFS() --- .../TestViewFileSystemLocalFileSystem.java | 9 +++++++++ ...ileSystemWithAuthorityLocalFileSystem.java | 10 ++++++++++ .../fs/viewfs/ViewFileSystemBaseTest.java | 19 ++++++++----------- .../fs/viewfs/TestViewFileSystemHdfs.java | 8 ++++++++ 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemLocalFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemLocalFileSystem.java index 808d8b06c35ba..adc5db87e7725 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemLocalFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemLocalFileSystem.java @@ -33,6 +33,8 @@ import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; +import org.apache.hadoop.security.UserGroupInformation; import org.junit.After; import org.junit.Before; @@ -61,6 +63,13 @@ public void setUp() throws Exception { } + @Override + Path getTrashRootInFallBackFS() throws IOException { + return new Path( + "/" + TRASH_PREFIX + "/" + UserGroupInformation.getCurrentUser() + .getShortUserName()); + } + @Test public void testNflyWriteSimple() throws IOException { LOG.info("Starting testNflyWriteSimple"); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemWithAuthorityLocalFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemWithAuthorityLocalFileSystem.java index 877c2228c1eea..9223338f34bf5 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemWithAuthorityLocalFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemWithAuthorityLocalFileSystem.java @@ -25,6 +25,9 @@ import org.apache.hadoop.fs.FileSystemTestHelper; import org.apache.hadoop.fs.FsConstants; import org.apache.hadoop.fs.Path; +import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; +import org.apache.hadoop.security.UserGroupInformation; +import java.io.IOException; import org.junit.After; import org.junit.Assert; @@ -63,6 +66,13 @@ public void tearDown() throws Exception { super.tearDown(); } + @Override + Path getTrashRootInFallBackFS() throws IOException { + return new Path( + "/" + TRASH_PREFIX + "/" + UserGroupInformation.getCurrentUser() + .getShortUserName()); + } + @Override @Test public void testBasicPaths() { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 4c4f95d3c860b..369b0f6e9401d 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1104,9 +1104,13 @@ public void testTrashRoot() throws IOException { Assert.assertTrue("", fsView.getTrashRoots(true).size() > 0); } - private String getTrashRootForPathInFallBackFS() { - return fsTarget.getHomeDirectory().toUri().getPath() + "/" + TRASH_PREFIX; + // Default implementation of getTrashRoot for a fallback FS mounted at root: + // e.g., fallbackFS.uri.getPath = '/' + Path getTrashRootInFallBackFS() throws IOException { + return new Path(fsTarget.getHomeDirectory().toUri().getPath(), + TRASH_PREFIX); } + /** * Test TRASH_FORCE_INSIDE_MOUNT_POINT feature for getTrashRoot. */ @@ -1128,15 +1132,8 @@ public void testTrashRootForceInsideMountPoint() throws IOException { // Case 2: path p not found in mount table, fall back to the default FS // Return a trash root in mount point "/" Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); - Path expectedTrash; - if (targetTestRoot.toUri().getPath().equals("/")) { - expectedTrash = - fsView2.makeQualified(new Path(fsTarget.getHomeDirectory().toUri().getPath(), - TRASH_PREFIX)); - } else { - expectedTrash = - fsView2.makeQualified(new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName())); - } + Path expectedTrash = + fsView2.makeQualified(getTrashRootInFallBackFS()); Assert.assertEquals(expectedTrash, fsView2.getTrashRoot(nonExistentPath)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java index fdc746464f4e5..62dc3076d00a6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java @@ -55,6 +55,7 @@ import org.apache.hadoop.test.GenericTestUtils; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.IPC_CLIENT_CONNECT_MAX_RETRIES_KEY; +import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; import org.junit.After; import org.junit.AfterClass; @@ -181,6 +182,13 @@ int getExpectedDelegationTokenCountWithCredentials() { return 2; } + @Override + Path getTrashRootInFallBackFS() throws IOException { + return new Path( + "/" + TRASH_PREFIX + "/" + UserGroupInformation.getCurrentUser() + .getShortUserName()); + } + @Test public void testTrashRootsAfterEncryptionZoneDeletion() throws Exception { final Path zone = new Path("/EZ"); From a78bada06ec8e1f65b753a13ccc6d89f4447d13a Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 11 Mar 2022 22:38:23 -0800 Subject: [PATCH 24/26] Fix new lines --- .../java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 35b6151b770e8..096c23252fd87 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1191,10 +1191,10 @@ public Path getTrashRoot(Path path) { } Path targetFsUserHome = res.targetFileSystem.getHomeDirectory(); - if (targetFSTrashRootPath.startsWith(mountTargetPath) && !( - mountTargetPath.equals(ROOT_PATH.toString()) - && !res.resolvedPath.equals(ROOT_PATH.toString()) && ( - targetFsUserHome != null && targetFSTrashRootPath.startsWith( + if (targetFSTrashRootPath.startsWith(mountTargetPath) && + !(mountTargetPath.equals(ROOT_PATH.toString()) && + !res.resolvedPath.equals(ROOT_PATH.toString()) && + (targetFsUserHome != null && targetFSTrashRootPath.startsWith( targetFsUserHome.toUri().getPath())))) { String relativeTrashRoot = targetFSTrashRootPath.substring(mountTargetPath.length()); From 485a650c1dd8115e3e0a56476b669791eeb192ef Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Fri, 11 Mar 2022 22:45:04 -0800 Subject: [PATCH 25/26] minor fix to comments. --- .../java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 1 - .../org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 096c23252fd87..e26a31c10debc 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1222,7 +1222,6 @@ public Path getTrashRoot(Path path) { public Collection getTrashRoots(boolean allUsers) { // A map from targetFSPath -> FileStatus. // FileStatus can be from targetFS or viewFS. - HashMap trashRoots = new HashMap<>(); for (FileSystem fs : getChildFileSystems()) { for (FileStatus trash : fs.getTrashRoots(allUsers)) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 369b0f6e9401d..eab89f13377bf 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1129,12 +1129,11 @@ public void testTrashRootForceInsideMountPoint() throws IOException { new Path("/data/" + TRASH_PREFIX + "/" + ugi.getShortUserName())); Assert.assertEquals(dataTrashRoot, fsView2.getTrashRoot(dataTestPath)); - // Case 2: path p not found in mount table, fall back to the default FS - // Return a trash root in mount point "/" + // Case 2: path p not found in mount table. + // Return a trash root in fallback FS. Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile"); Path expectedTrash = fsView2.makeQualified(getTrashRootInFallBackFS()); - Assert.assertEquals(expectedTrash, fsView2.getTrashRoot(nonExistentPath)); // Case 3: turn off the CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT flag. From 15a6dcc00772fca320e0dab9322dfdafb2e3fa67 Mon Sep 17 00:00:00 2001 From: Xing Lin Date: Sun, 13 Mar 2022 22:49:38 -0700 Subject: [PATCH 26/26] GetTrashRoots(): include trash roots in fallback FS as well. --- .../hadoop/fs/viewfs/ViewFileSystem.java | 16 +++++++++++++--- .../fs/viewfs/ViewFileSystemBaseTest.java | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index e26a31c10debc..b1ec55ee4f483 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -1235,9 +1235,19 @@ public Collection getTrashRoots(boolean allUsers) { return trashRoots.values(); } - // Get trash roots in TRASH_PREFIX dir inside mount points. + // Get trash roots in TRASH_PREFIX dir inside mount points and fallback FS. + List> mountPoints = + fsState.getMountPoints(); + // If we have a fallback FS, add a mount point for it as <"", fallback FS>. + // The source path of a mount point shall not end with '/', thus for + // fallback fs, we set its mount point src as "". + if (fsState.getRootFallbackLink() != null) { + mountPoints.add(new InodeTree.MountPoint<>("", + fsState.getRootFallbackLink())); + } + try { - for (InodeTree.MountPoint mountPoint : fsState.getMountPoints()) { + for (InodeTree.MountPoint mountPoint : mountPoints) { Path trashRoot = makeQualified(new Path(mountPoint.src + "/" + TRASH_PREFIX)); @@ -1270,7 +1280,7 @@ public Collection getTrashRoots(boolean allUsers) { } } } catch (IOException e) { - LOG.warn("Exception in get all trash roots", e); + LOG.warn("Exception in get all trash roots for mount points", e); } return trashRoots.values(); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index eab89f13377bf..90ffa06bfce30 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1228,6 +1228,16 @@ public void testTrashRootsAllUsers() throws IOException { FileSystem fsView4 = FileSystem.get(FsConstants.VIEWFS_URI, conf4); int trashRootsNum4 = fsView4.getTrashRoots(true).size(); Assert.assertEquals(afterTrashRootsNum2 + 2, trashRootsNum4); + + // Case 4: test trash roots in fallback FS + fsTarget.mkdirs(new Path(targetTestRoot, ".Trash/user10")); + fsTarget.mkdirs(new Path(targetTestRoot, ".Trash/user11")); + fsTarget.mkdirs(new Path(targetTestRoot, ".Trash/user12")); + Configuration conf5 = new Configuration(conf2); + ConfigUtil.addLinkFallback(conf5, targetTestRoot.toUri()); + FileSystem fsView5 = FileSystem.get(FsConstants.VIEWFS_URI, conf5); + int trashRootsNum5 = fsView5.getTrashRoots(true).size(); + Assert.assertEquals(afterTrashRootsNum2 + 3, trashRootsNum5); } /** @@ -1254,6 +1264,14 @@ public void testTrashRootsCurrentUser() throws IOException { int afterTrashRootsNum2 = fsView2.getTrashRoots(false).size(); Assert.assertEquals(beforeTrashRootNum, afterTrashRootsNum); Assert.assertEquals(beforeTrashRootNum2 + 2, afterTrashRootsNum2); + + // Test trash roots in fallback FS + Configuration conf3 = new Configuration(conf2); + fsTarget.mkdirs(new Path(targetTestRoot, TRASH_PREFIX + "/" + currentUser)); + ConfigUtil.addLinkFallback(conf3, targetTestRoot.toUri()); + FileSystem fsView3 = FileSystem.get(FsConstants.VIEWFS_URI, conf3); + int trashRootsNum3 = fsView3.getTrashRoots(false).size(); + Assert.assertEquals(afterTrashRootsNum2 + 1, trashRootsNum3); } @Test(expected = NotInMountpointException.class)