diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java index 19a7a693ef8f..84b571476d99 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java @@ -407,6 +407,15 @@ private boolean checkAndDeleteFiles(List files) { return deleteFiles(filesToDelete) == files.size(); } + /** + * Check if the directory can be deleted + * @param dir The path of a directory + * @return True if the directory can be deleted, otherwise false + */ + boolean shouldDeleteDirectory(Path dir) { + return true; + } + /** * Delete the given files * @param filesToDelete files to delete @@ -515,8 +524,8 @@ protected Boolean compute() { boolean result = allFilesDeleted && allSubdirsDeleted; // if and only if files and subdirs under current dir are deleted successfully, and - // it is not the root dir, then task will try to delete it. - if (result && !root) { + // it is not the root dir, and the directory can be deleted, then task will try to delete it. + if (result && !root && shouldDeleteDirectory(dir)) { result &= deleteAction(() -> fs.delete(dir, false), "dir"); } return result; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java index 7ad6177764c2..cf579546e934 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java @@ -33,8 +33,8 @@ import org.apache.hadoop.hbase.Stoppable; import org.apache.hadoop.hbase.io.HFileLink; import org.apache.hadoop.hbase.master.HMaster; -import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.regionserver.StoreFileInfo; +import org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclHelper; import org.apache.hadoop.hbase.util.StealJobQueue; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; @@ -46,7 +46,7 @@ */ @InterfaceAudience.Private public class HFileCleaner extends CleanerChore { - private MasterServices master; + private HMaster master; public static final String MASTER_HFILE_CLEANER_PLUGINS = "hbase.master.hfilecleaner.plugins"; @@ -103,6 +103,7 @@ public HFileCleaner(final int period, final Stoppable stopper, Configuration con private long cleanerThreadCheckIntervalMsec; private List threads = new ArrayList(); private boolean running; + private boolean userScanSnapshotEnabled; private AtomicLong deletedLargeFiles = new AtomicLong(); private AtomicLong deletedSmallFiles = new AtomicLong(); @@ -136,6 +137,10 @@ public HFileCleaner(final int period, final Stoppable stopper, Configuration con cleanerThreadCheckIntervalMsec = conf.getLong(HFILE_DELETE_THREAD_CHECK_INTERVAL_MSEC, DEFAULT_HFILE_DELETE_THREAD_CHECK_INTERVAL_MSEC); + userScanSnapshotEnabled = SnapshotScannerHDFSAclHelper.isUserScanSnapshotEnabled(conf); + if (userScanSnapshotEnabled) { + init(params); + } startHFileDeleteThreads(); } @@ -502,7 +507,21 @@ public synchronized void cancel(boolean mayInterruptIfRunning) { public void init(Map params) { if (params != null && params.containsKey(HMaster.MASTER)) { - this.master = (MasterServices) params.get(HMaster.MASTER); + this.master = (HMaster) params.get(HMaster.MASTER); } } + + @Override + boolean shouldDeleteDirectory(Path dir) { + /* + * If user scan snapshot feature is enabled(see HBASE-21995), then when namespace or table + * exists, the archive namespace or table directories should not be deleted because the HDFS + * acls are set at these directories; the archive data directory should not be deleted because + * the HDFS acls of global permission is set at this directory. + */ + /*if (userScanSnapshotEnabled) { + return SnapshotScannerHDFSAclHelper.shouldDeleteArchiveDirectory(dir, master); + }*/ + return true; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java index 2a62ff0ab69d..2d2981d7d7b4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java @@ -47,17 +47,21 @@ import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hbase.AuthUtil; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.mob.MobUtils; import org.apache.hadoop.hbase.util.Bytes; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; import org.apache.hbase.thirdparty.com.google.common.collect.Sets; import org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -515,6 +519,84 @@ private static AclEntry aclEntry(AclEntryScope scope, String name) { .setPermission(READ_EXECUTE).build(); } + public static boolean isUserScanSnapshotEnabled(Configuration conf) { + String masterCoprocessors = conf.get(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY); + if (conf.getBoolean(USER_SCAN_SNAPSHOT_ENABLE, false) + && masterCoprocessors.contains(SnapshotScannerHDFSAclController.class.getName()) + && masterCoprocessors.contains(AccessController.class.getName())) { + return true; + } + return false; + } + + /** + * If user scan snapshot feature is enabled, then when namespace or table exists, the archive + * namespace or table directories should not be deleted because the HDFS acls are set at these + * directories. The archive data directory should not be deleted because the HDFS acls of global + * permission is set at this directory. + */ + public static boolean shouldDeleteArchiveDirectory(Path dir, HMaster master) { + if (isArchiveDataDir(dir)) { + return false; + } else if (isArchiveNamespaceDir(dir) && namespaceExists(dir.getName(), master)) { + return false; + } else if (isArchiveTableDir(dir)) { + String name = dir.getName(); + String namespace = dir.getParent().getName(); + if (tableExists(TableName.valueOf(namespace, name), master)) { + return false; + } + } + return true; + } + + @VisibleForTesting + static boolean isArchiveDataDir(Path path) { + if (path != null && path.getName().equals(HConstants.BASE_NAMESPACE_DIR)) { + Path parent = path.getParent(); + return parent != null && parent.getName().equals(HConstants.HFILE_ARCHIVE_DIRECTORY); + } + return false; + } + + @VisibleForTesting + static boolean isArchiveNamespaceDir(Path path) { + return path == null ? false : isArchiveDataDir(path.getParent()); + } + + @VisibleForTesting + static boolean isArchiveTableDir(Path path) { + return path == null ? false : isArchiveNamespaceDir(path.getParent()); + } + + private static boolean namespaceExists(String namespace, HMaster master) { + try { + if (master != null) { + return master.listNamespaces().contains(namespace); + } else { + LOG.warn("Master is null when check if namespace {} exists", namespace); + return false; + } + } catch (IOException e) { + LOG.warn("Check if namespace {} exists error", namespace, e); + return true; + } + } + + private static boolean tableExists(TableName tableName, HMaster master) { + try { + if (master != null) { + return MetaTableAccessor.tableExists(master.getConnection(), tableName); + } else { + LOG.warn("Master is null when check if table {} exists", tableName); + return false; + } + } catch (IOException e) { + LOG.error("Check if table {} exists error", tableName, e); + return true; + } + } + /** * Inner class used to describe modify or remove what type of acl entries(ACCESS, DEFAULT, * ACCESS_AND_DEFAULT) for files or directories(and child files). diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java index e35e3a1b1d99..47fc2ecca13b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java @@ -21,6 +21,8 @@ import static org.apache.hadoop.hbase.security.access.Permission.Action.READ; import static org.apache.hadoop.hbase.security.access.Permission.Action.WRITE; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.security.PrivilegedExceptionAction; @@ -46,11 +48,14 @@ import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.client.TableSnapshotScanner; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.master.cleaner.HFileCleaner; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.testclassification.SecurityTests; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.HFileArchiveUtil; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Rule; @@ -546,6 +551,39 @@ public void testDeleteNamespace() throws Exception { } } + @Test + public void testCleanArchiveTableDir() throws Exception { + final String grantUserName = name.getMethodName(); + User grantUser = User.createUserForTesting(conf, grantUserName, new String[] {}); + String namespace = name.getMethodName(); + TableName table = TableName.valueOf(namespace, "t1"); + String snapshot = namespace + "t1"; + + TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table); + admin.snapshot(snapshot, table); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table, READ); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot, 6); + + // HFileCleaner will not delete archive table directory even if it's a empty directory + Path archiveTableDir = HFileArchiveUtil.getTableArchivePath(rootDir, table); + HFileCleaner cleaner = TEST_UTIL.getHBaseCluster().getMaster().getHFileCleaner(); + cleaner.choreForTesting(); + assertTrue(fs.exists(archiveTableDir)); + + // delete table and grant user can scan snapshot + admin.disableTable(table); + admin.deleteTable(table); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot, 6); + + // Check SnapshotScannerHDFSAclHelper method + assertTrue(SnapshotScannerHDFSAclHelper.isArchiveTableDir(archiveTableDir)); + assertTrue(SnapshotScannerHDFSAclHelper.isArchiveNamespaceDir(archiveTableDir.getParent())); + assertTrue( + SnapshotScannerHDFSAclHelper.isArchiveDataDir(archiveTableDir.getParent().getParent())); + assertFalse(SnapshotScannerHDFSAclHelper + .isArchiveDataDir(archiveTableDir.getParent().getParent().getParent())); + } + @Test public void testGrantMobTable() throws Exception { final String grantUserName = name.getMethodName();