From fd067f9a0f0134a8f37043d1ff69bbce19d43571 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Mon, 1 Jul 2019 00:08:56 +0530 Subject: [PATCH 1/4] HBASE-22643 : Delete region without archiving only if regiondir is present --- .../hadoop/hbase/backup/HFileArchiver.java | 4 +- .../hbase/backup/TestHFileArchiving.java | 45 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java index 4fb9f4f08d..53d2dbb065 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java @@ -120,7 +120,9 @@ public class HFileArchiver { if (tableDir == null || regionDir == null) { LOG.error("No archive directory could be found because tabledir (" + tableDir + ") or regiondir (" + regionDir + "was null. Deleting files instead."); - deleteRegionWithoutArchiving(fs, regionDir); + if (regionDir != null) { + deleteRegionWithoutArchiving(fs, regionDir); + } // we should have archived, but failed to. Doesn't matter if we deleted // the archived files correctly or not. return false; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java index eee0ac3d3c..d2c351ba29 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java @@ -526,6 +526,51 @@ public class TestHFileArchiving { } } + @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)); + } + // Avoid passing a null master to CleanerChore, see HBASE-21175 private HFileCleaner getHFileCleaner(Stoppable stoppable, Configuration conf, FileSystem fs, Path archiveDir) throws IOException { -- 2.17.2 (Apple Git-113) From cd136ee61d9dac50f5012955314efacaf55cbb94 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Thu, 11 Jul 2019 00:59:12 +0530 Subject: [PATCH 2/4] updating test case verify step based on review comments --- .../hbase/backup/TestHFileArchiving.java | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java index d2c351ba29..3890348344 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java @@ -536,16 +536,8 @@ public class TestHFileArchiving { 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)); + // Try to archive the file but with null regionDir, can't delete sourceFile + assertFalse(HFileArchiver.archiveRegion(fileSystem, null, null, null)); } @Test @@ -560,15 +552,8 @@ public class TestHFileArchiving { 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)); + // Try to archive the file + assertFalse(HFileArchiver.archiveRegion(fileSystem, rootDir, null, sourceRegionDir)); } // Avoid passing a null master to CleanerChore, see HBASE-21175 -- 2.17.2 (Apple Git-113) From 196c15c1cca1bf093e218a30922a0f9311d829ae Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Thu, 11 Jul 2019 20:28:33 +0530 Subject: [PATCH 3/4] minor changes based on review comments --- .../hbase/backup/TestHFileArchiving.java | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java index 3890348344..c8bd687d49 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java @@ -527,7 +527,7 @@ public class TestHFileArchiving { } @Test - public void testUnDeletedRegionWithoutArchive() throws IOException { + public void testArchiveRegionTableAndRegionDirsNull() throws IOException { Path regionDir = new Path(FSUtils.getTableDir(new Path("./"), TableName.valueOf(name.getMethodName())), "abcdef"); Path familyDir = new Path(regionDir, "cf"); @@ -537,11 +537,11 @@ public class TestHFileArchiving { FileSystem fileSystem = UTIL.getTestFileSystem(); fileSystem.createNewFile(sourceFile); // Try to archive the file but with null regionDir, can't delete sourceFile - assertFalse(HFileArchiver.archiveRegion(fileSystem, null, null, null)); + assertFalse(HFileArchiver.archiveRegion(fileSystem, rootDir, null, null)); } @Test - public void testDeleteRegionWithoutArchive() throws IOException { + public void testArchiveRegionWithTableDirNull() throws IOException { Path regionDir = new Path(FSUtils.getTableDir(new Path("./"), TableName.valueOf(name.getMethodName())), "xyzabc"); Path familyDir = new Path(regionDir, "rd"); @@ -554,6 +554,26 @@ public class TestHFileArchiving { fileSystem.mkdirs(sourceRegionDir); // Try to archive the file assertFalse(HFileArchiver.archiveRegion(fileSystem, rootDir, null, sourceRegionDir)); + assertFalse(fileSystem.exists(sourceRegionDir)); + } + + @Test + public void testArchiveRegionWithRegionDirNull() throws IOException { + Path regionDir = new Path(FSUtils.getTableDir(new Path("./"), + TableName.valueOf(name.getMethodName())), "elgn4nf"); + Path familyDir = new Path(regionDir, "rdar"); + Path rootDir = UTIL.getDataTestDirOnTestFS("testCleaningRace"); + Path file = new Path(familyDir, "2"); + Path sourceFile = new Path(rootDir, file); + FileSystem fileSystem = UTIL.getTestFileSystem(); + fileSystem.createNewFile(sourceFile); + Path sourceRegionDir = new Path(rootDir, regionDir); + fileSystem.mkdirs(sourceRegionDir); + // Try to archive the file but with null regionDir, can't delete sourceFile + assertFalse(HFileArchiver.archiveRegion(fileSystem, rootDir, sourceRegionDir.getParent(), + null)); + assertTrue(fileSystem.exists(sourceRegionDir)); + fileSystem.delete(sourceRegionDir, true); } // Avoid passing a null master to CleanerChore, see HBASE-21175 -- 2.17.2 (Apple Git-113) From 44ec55f7fc07f8dd41f24a3a3d46e06bd17adc91 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Sun, 14 Jul 2019 20:20:08 +0530 Subject: [PATCH 4/4] minor changes --- .../org/apache/hadoop/hbase/backup/TestHFileArchiving.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java index c8bd687d49..144cdfd5b3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java @@ -528,14 +528,8 @@ public class TestHFileArchiving { @Test public void testArchiveRegionTableAndRegionDirsNull() 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 to archive the file but with null regionDir, can't delete sourceFile assertFalse(HFileArchiver.archiveRegion(fileSystem, rootDir, null, null)); } -- 2.17.2 (Apple Git-113)