goiri commented on code in PR #5802:
URL: https://github.com/apache/hadoop/pull/5802#discussion_r1251052837
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java:
##########
@@ -460,9 +465,13 @@ public PathLocation getDestinationForPath(final String
path)
this.getLocCacheAccess().increment();
}
if (isTrashPath(path)) {
+ boolean useMountPointCreateTrashPath = conf.getBoolean(
+ DFS_ROUTER_TRASH_PATH_CREATED_BY_MOUNT_POINT,
+ DFS_ROUTER_TRASH_PATH_CREATED_BY_MOUNT_POINT_DEFAULT);
List<RemoteLocation> remoteLocations = new ArrayList<>();
for (RemoteLocation remoteLocation : res.getDestinations()) {
- remoteLocations.add(new RemoteLocation(remoteLocation, path));
+ remoteLocations.add(new RemoteLocation(remoteLocation,
+ useMountPointCreateTrashPath ? path : getTrashRoot() +
"/Current" + remoteLocation.getDest()));
Review Comment:
There's no constant somewhere for Current?
There must be even a function to generate this.
It'd be cleaner to extract the path too.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java:
##########
@@ -460,9 +465,13 @@ public PathLocation getDestinationForPath(final String
path)
this.getLocCacheAccess().increment();
}
if (isTrashPath(path)) {
+ boolean useMountPointCreateTrashPath = conf.getBoolean(
Review Comment:
We usually want to expose all these things as configuration parameters but
this might be too detailed.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterTrash.java:
##########
@@ -276,6 +279,81 @@ public void testDeleteToTrashExistMountPoint() throws
IOException,
assertEquals(2, fileStatuses.length);
}
+ private void deleteToTrashExistMountPoint() throws IOException,
+ URISyntaxException, InterruptedException {
+ MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+ Collections.singletonMap(ns0, DST_PATH));
+ assertTrue(addMountTable(addEntry));
+
+ // current user client
+ DFSClient client = nnContext.getClient();
+ client.setOwner("/", TEST_USER, TEST_USER);
+ UserGroupInformation ugi = UserGroupInformation.
+ createRemoteUser(TEST_USER);
+ // test user client
+ client = nnContext.getClient(ugi);
+ client.mkdirs(DST_PATH, new FsPermission("777"), true);
+ assertTrue(client.exists(DST_PATH));
+ // create test file
+ client.create(DST_FILE, true);
+
+ Path filePath = new Path(FILE);
+ FileStatus[] fileStatuses = routerFs.listStatus(filePath);
+ assertEquals(1, fileStatuses.length);
+ assertEquals(TEST_USER, fileStatuses[0].getOwner());
+
+ // move to Trash.
+ Configuration routerConf = routerContext.getConf();
+ FileSystem fs =
+ DFSTestUtil.getFileSystemAs(ugi, routerConf);
Review Comment:
Spacing.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java:
##########
@@ -460,9 +465,13 @@ public PathLocation getDestinationForPath(final String
path)
this.getLocCacheAccess().increment();
}
if (isTrashPath(path)) {
+ boolean useMountPointCreateTrashPath = conf.getBoolean(
+ DFS_ROUTER_TRASH_PATH_CREATED_BY_MOUNT_POINT,
+ DFS_ROUTER_TRASH_PATH_CREATED_BY_MOUNT_POINT_DEFAULT);
Review Comment:
Spacing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
