[
https://issues.apache.org/jira/browse/HDFS-17046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17735129#comment-17735129
]
ASF GitHub Bot commented on HDFS-17046:
---------------------------------------
Hexiaoqiao commented on code in PR #5744:
URL: https://github.com/apache/hadoop/pull/5744#discussion_r1234732495
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicy.java:
##########
@@ -68,10 +68,11 @@ public void initialize(Configuration conf, FileSystem fs) {
/**
* Move a file or directory to the current trash directory.
* @param path the path.
+ * @param force Whether force moving or not.
* @return false if the item is already in the trash or trash is disabled
* @throws IOException raised on errors performing I/O.
- */
- public abstract boolean moveToTrash(Path path) throws IOException;
+ */
+ public abstract boolean moveToTrash(Path path, boolean force) throws
IOException;
Review Comment:
Sorry I don't get meaning of the new parameter `boolean force`.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestTrash.java:
##########
@@ -786,6 +793,69 @@ public void testTrashEmptier() throws Exception {
emptierThread.join();
}
+ /**
+ * Test trash emptier can whether delete non-checkpoint dir or not.
+ * @throws Exception
+ */
+ @Test
+ public void testTrashEmptierWithNonCheckpointDir() throws Exception {
Review Comment:
This new unit test seems not cover the case you try to fix, right? It could
pass with/without this improvement.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java:
##########
@@ -374,8 +382,14 @@ private void deleteCheckpoint(Path trashRoot, boolean
deleteImmediately)
try {
time = getTimeFromCheckpoint(name);
} catch (ParseException e) {
- LOG.warn("Unexpected item in trash: "+dir+". Ignoring.");
- continue;
+ if (cleanNonCheckpointUnderTrashRoot) {
+ this.moveToTrash(path, true);
Review Comment:
Why move to trash again since it has been already under Trash home AND has
configed to clean? IMO, it is safe to delete them directly.
> Delete path directly when it can not be parsed in trash
> -------------------------------------------------------
>
> Key: HDFS-17046
> URL: https://issues.apache.org/jira/browse/HDFS-17046
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: farmmamba
> Assignee: farmmamba
> Priority: Major
> Labels: pull-request-available
>
> If we move path to trash dir directly rather than use delete API or rm
> command, when
> invoke deleteCheckpoint method, it will catch ParseException and ignore
> deleting the path. It will never be deleted, so we should do something to
> prevent or monitor it.
> Some logs are listed below.
>
> {code:java}
> WARN org.apache.hadoop.fs.TrashPolicyDefault: Unexpected item in trash:
> /user/de_eight/.Trash/college_geek_job_recall_als_modelres_5_2_6.del.
> Ignoring.
> WARN org.apache.hadoop.fs.TrashPolicyDefault: Unexpected item in trash:
> /user/de_eight/.Trash/college_geek_addf_vector. Ignoring.
> {code}
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
