[GitHub] [hbase] wchevreuil commented on a change in pull request #348: HBASE-22643 : Delete region without archiving only if regiondir is pr…

[GitBox <gi...@apache.org>]

wchevreuil commented on a change in pull request #348: HBASE-22643 : Delete region without archiving only if regiondir is pr…
URL: https://github.com/apache/hbase/pull/348#discussion_r302199583
 
 

 ##########
 File path: hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java
 ##########
 @@ -526,6 +526,51 @@ public void testCleaningRace() throws Exception {
     }
   }
 
+  @Test
+  public void testUnDeletedRegionWithoutArchive() throws IOException {
+    Path regionDir = new Path(FSUtils.getTableDir(new Path("./"),
+            TableName.valueOf(name.getMethodName())), "abcdef");
+    Path familyDir = new Path(regionDir, "cf");
+    Path rootDir = UTIL.getDataTestDirOnTestFS("testCleaningRace");
+    Path file = new Path(familyDir, "0");
+    Path sourceFile = new Path(rootDir, file);
+    FileSystem fileSystem = UTIL.getTestFileSystem();
+    fileSystem.createNewFile(sourceFile);
+    try {
+      // Try to archive the file but with null regionDir, can't delete sourceFile
+      HFileArchiver.archiveRegion(fileSystem, null, null, null);
+    } catch (IOException e) {
+      assertTrue(fileSystem.exists(sourceFile));
+    } finally {
+      fileSystem.delete(sourceFile, false);
+      fileSystem.delete(rootDir, true);
+    }
+    assertFalse(fileSystem.exists(sourceFile));
+  }
+
+  @Test
+  public void testDeleteRegionWithoutArchive() throws IOException {
+    Path regionDir = new Path(FSUtils.getTableDir(new Path("./"),
+            TableName.valueOf(name.getMethodName())), "xyzabc");
+    Path familyDir = new Path(regionDir, "rd");
+    Path rootDir = UTIL.getDataTestDirOnTestFS("testCleaningRace");
+    Path file = new Path(familyDir, "1");
+    Path sourceFile = new Path(rootDir, file);
+    FileSystem fileSystem = UTIL.getTestFileSystem();
+    fileSystem.createNewFile(sourceFile);
+    Path sourceRegionDir = new Path(rootDir, regionDir);
+    fileSystem.mkdirs(sourceRegionDir);
+    try {
+      // Try to archive the file
+      HFileArchiver.archiveRegion(fileSystem, rootDir, null, sourceRegionDir);
+    } catch (IOException e) {
+      fileSystem.delete(sourceFile, false);
+    } finally {
+      fileSystem.delete(rootDir, true);
+    }
+    assertFalse(fileSystem.exists(sourceFile));
 
 Review comment:
   Again, why are you expecting an IOE in this test, that's not the correct expected behaviour. Also, you are forcing this test to always pass, by manually doing a recursive delete of _rootDir_ in line #569. The assert check from line #571 is correct, but we should not do the manual deletes from lines #567 and #569, after all, the file should had been deleted by the tested method: _HFileArchiver.archiveRegion_

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services