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.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
