[
https://issues.apache.org/jira/browse/HDFS-17046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17742261#comment-17742261
]
ASF GitHub Bot commented on HDFS-17046:
---------------------------------------
Hexiaoqiao commented on code in PR #5744:
URL: https://github.com/apache/hadoop/pull/5744#discussion_r1260563388
##########
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 {
+ Configuration conf = new Configuration();
+ // Trash with 12 second deletes and 6 seconds checkpoints
+ conf.set(FS_TRASH_INTERVAL_KEY, "0.2"); // 12 seconds
+ conf.setClass("fs.file.impl", TestLFS.class, FileSystem.class);
+ conf.set(FS_TRASH_CHECKPOINT_INTERVAL_KEY, "0.1"); // 6 seconds
+ conf.setBoolean(FS_TRASH_CLEAN_TRASHROOT_ENABLE_KEY, true);
+ FileSystem fs = FileSystem.getLocal(conf);
+ conf.set("fs.default.name", fs.getUri().toString());
+
+ Trash trash = new Trash(conf);
+
+ // Start Emptier in background
+ Runnable emptier = trash.getEmptier();
+ Thread emptierThread = new Thread(emptier);
+ emptierThread.start();
+
+ FsShell shell = new FsShell();
+ shell.setConf(conf);
+ shell.init();
+
+ // First create a new directory with mkdirs
+ Path myPath = new Path(TEST_DIR, "test/mkdirs");
+ mkdir(fs, myPath);
+ // Make sure the .Trash dir existed.
+ mkdir(fs, shell.getCurrentTrashDir());
+ assertTrue(fs.exists(shell.getCurrentTrashDir()));
+
+ int fileIndex = 0;
+ int loopCount = 10;
+ while (fileIndex < loopCount) {
+ // Create a file with a new name
+ Path myFile = new Path(TEST_DIR, "test/mkdirs/myFile" + fileIndex++);
+ writeFile(fs, myFile, 10);
+
+ // Move the file to trash root.
+ String[] args = new String[3];
+ args[0] = "-mv";
+ args[1] = myFile.toString();
+ args[2] = shell.getCurrentTrashDir().getParent().toString();
+ int val = -1;
+ try {
+ val = shell.run(args);
+ } catch (Exception e) {
+ System.err.println("Exception raised from Trash.run " +
+ e.getLocalizedMessage());
+ }
+ assertTrue(val == 0);
+ }
+ Thread.sleep(18000);
Review Comment:
Please try to use `GenericTestUtils.waitFor` rather than `Thread.sleep`.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestTrash.java:
##########
@@ -579,6 +579,7 @@ public void testExistingFileTrash() throws IOException {
conf.setClass("fs.file.impl", TestLFS.class, FileSystem.class);
FileSystem fs = FileSystem.getLocal(conf);
conf.set("fs.defaultFS", fs.getUri().toString());
+ conf.setBoolean(FS_TRASH_CLEAN_TRASHROOT_ENABLE_KEY, true);
Review Comment:
I do not think we need to update this configuration item here.
(L582,L639,L649, etc)
##########
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) {
+ fs.delete(path, true);
+ LOG.warn("Unexpected item in trash: " + dir + ". Force moving to
trash.");
Review Comment:
Force moving to trash. -> Force to delete it.
##########
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:
For this unit test, IMO, we should enable this config and then mkdir one
path under Trash home directly, then wait to clean by Emptier. JFYI.
> 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]
