xinglin commented on code in PR #4869:
URL: https://github.com/apache/hadoop/pull/4869#discussion_r970195382
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Trash.java:
##########
@@ -94,6 +96,34 @@ public static boolean moveToAppropriateTrash(FileSystem fs,
Path p,
LOG.warn("Failed to get server trash configuration", e);
throw new IOException("Failed to get server trash configuration", e);
}
+
+ /*
+ * In HADOOP-18144, we fixed the logical path vs. target path bug of
getTrashRoot() in ViewFileSystem.
+ * moveToTrash works for ViewFileSystem now. ViewFileSystem will do path
resolution internally by itself.
+ *
+ * When localized trash flag is enabled:
+ * 1). if fs is a ViewFileSystem, we can initialize Trash() with this
ViewFileSystem object;
+ * 2). When fs is not a ViewFileSystem, the only place we would need to
resolve a path is for symbolic links.
+ * However, symlink is not enabled in Hadoop due to the complexity
to support it (HADOOP-10019).
+ */
+ if (conf.getBoolean(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT,
+ CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT_DEFAULT)) {
+ // Save the original config in savedValue for localized trash config.
+ String savedValue =
fs.getConf().get(CONFIG_VIEWFS_TRASH_FORCE_INSIDE_MOUNT_POINT);
Review Comment:
Hi Owen,
This is mainly to address a case we encountered during our hive-cli testing.
We are trying to make sure the flag is set in viewfs.getConf() object. Here is
what we observed.
1. we started a hive-cli. it will initialize a viewfs object, without
localized trash flag set to true.
2. then, we use the "set fs.viewfs.enforce-inside-trash" to set this flag.
3. we do some operations which leads to moveToTrash(fs, path, conf) to be
called.
When that happens, the fs object we gets in moveToTrash does not have the
localized trash flag set. Even when we use FileSystem.get(conf) with the new
conf object inside moveToTrash(), the fs we get doesn't have the localized
trash flag set when _fs.cache_ is enabled: we get the existing fs object from
the cache, which is using the old conf object.
We just want to make sure when the flag is set in Conf, we need to turn it
on for fs.getConf() as well.
--
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]
