diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java index e8fba43ca7d..46e74d4bb0e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java @@ -18,15 +18,15 @@ package org.apache.hadoop.hbase.master.cleaner; import java.io.IOException; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseInterfaceAudience; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.io.HFileLink; import org.apache.hadoop.hbase.util.FSUtils; @@ -57,7 +57,14 @@ public class HFileLinkCleaner extends BaseHFileCleanerDelegate { if (HFileLink.isBackReferencesDir(parentDir)) { Path hfilePath = null; try { - hfilePath = HFileLink.getHFileFromBackReference(getConf(), filePath); + // Also check if the HFile is in the HBASE_TEMP_DIRECTORY; this is where the referenced + // file gets created when cloning a snapshot. + hfilePath = HFileLink.getHFileFromBackReference( + new Path(FSUtils.getRootDir(getConf()), HConstants.HBASE_TEMP_DIRECTORY), filePath); + if (fs.exists(hfilePath)) { + return false; + } + hfilePath = HFileLink.getHFileFromBackReference(FSUtils.getRootDir(getConf()), filePath); return !fs.exists(hfilePath); } catch (IOException e) { if (LOG.isDebugEnabled()) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotCloneIndependence.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotCloneIndependence.java index 9efe0b14bbf..6b15e77c462 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotCloneIndependence.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotCloneIndependence.java @@ -58,6 +58,7 @@ public class TestSnapshotCloneIndependence { private static final String TEST_FAM_STR = "fam"; private static final byte[] TEST_FAM = Bytes.toBytes(TEST_FAM_STR); private static final TableName TABLE_NAME = TableName.valueOf(STRING_TABLE_NAME); + private static final int CLEANER_INTERVAL = 10; /** * Setup the config for the cluster and start it @@ -87,6 +88,13 @@ public class TestSnapshotCloneIndependence { // Avoid potentially aggressive splitting which would cause snapshot to fail conf.set(HConstants.HBASE_REGION_SPLIT_POLICY_KEY, ConstantSizeRegionSplitPolicy.class.getName()); + // Execute cleaner frequently to induce failures + conf.setInt("hbase.master.cleaner.interval", CLEANER_INTERVAL); + conf.setInt("hbase.master.hfilecleaner.plugins.snapshot.period", CLEANER_INTERVAL); + // Effectively disable TimeToLiveHFileCleaner. Don't want to fully disable it because that + // will even trigger races between creating the directory containing back references and + // the back reference itself. + conf.setInt("hbase.master.hfilecleaner.ttl", CLEANER_INTERVAL); } @Before @@ -164,6 +172,16 @@ public class TestSnapshotCloneIndependence { runTestRegionOperationsIndependent(true); } + @Test (timeout=300000) + public void testOfflineSnapshotDeleteIndependent() throws Exception { + runTestSnapshotDeleteIndependent(false); + } + + @Test (timeout=300000) + public void testOnlineSnapshotDeleteIndependent() throws Exception { + runTestSnapshotDeleteIndependent(true); + } + private static void waitOnSplit(final HTable t, int originalCount) throws Exception { for (int i = 0; i < 200; i++) { try { @@ -358,4 +376,54 @@ public class TestSnapshotCloneIndependence { Assert.assertTrue("The new family was not found. ", !clonedTableDescriptor.hasFamily(TEST_FAM_2)); } + + /* + * Take a snapshot of a table, add data, and verify that deleting the snapshot does not affect + * either table. + * @param online - Whether the table is online or not during the snapshot + */ + private void runTestSnapshotDeleteIndependent(boolean online) throws Exception { + FileSystem fs = UTIL.getHBaseCluster().getMaster().getMasterFileSystem().getFileSystem(); + Path rootDir = UTIL.getHBaseCluster().getMaster().getMasterFileSystem().getRootDir(); + + final Admin admin = UTIL.getHBaseAdmin(); + final long startTime = System.currentTimeMillis(); + final TableName localTableName = + TableName.valueOf(STRING_TABLE_NAME + startTime); + + try (Table original = UTIL.createTable(localTableName, TEST_FAM)) { + UTIL.loadTable(original, TEST_FAM); + } + + // Take a snapshot + final String snapshotNameAsString = "snapshot_" + localTableName; + byte[] snapshotName = Bytes.toBytes(snapshotNameAsString); + + SnapshotTestingUtils.createSnapshotAndValidate(admin, localTableName, TEST_FAM_STR, + snapshotNameAsString, rootDir, fs, online); + + if (!online) { + admin.enableTable(localTableName); + } + TableName cloneTableName = TableName.valueOf("test-clone-" + localTableName); + admin.cloneSnapshot(snapshotName, cloneTableName); + + // Ensure the original table does not reference the HFiles anymore + admin.majorCompact(localTableName); + + // Deleting the snapshot used to break the cloned table by deleting in-use HFiles + admin.deleteSnapshot(snapshotName); + + // Wait for cleaner run and DFS heartbeats so that anything that is deletable is fully deleted + Thread.sleep(10000); + + try (Table original = UTIL.getConnection().getTable(localTableName)) { + try (Table clonedTable = UTIL.getConnection().getTable(cloneTableName)) { + // Verify that all regions of both tables are readable + final int origTableRowCount = UTIL.countRows(original); + final int clonedTableRowCount = UTIL.countRows(clonedTable); + Assert.assertEquals(origTableRowCount, clonedTableRowCount); + } + } + } }