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_r302196217
##########
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));
Review comment:
This does not look correct to me, as far I can see, if regionDir is null,
_HFileArchiver.archiveRegion_, with this PR change, expected behaviour is to
return false, not an IOE. And because you are always manually deleting
_sourceFile_ on line #545, the assertion check on line #548 will always pass.
AFAICT, we really just need to assert that
_HFileArchiver.archiveRegion(fileSystem, null, null, null)_ returns false, no
need to create/delete extra files that would never be touched by the tested
method itself.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
