diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java index 634f54faa63d..232aa3e8b635 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java @@ -2181,10 +2181,21 @@ private CompletableFuture restoreSnapshot(String snapshotName, TableName t if (err3 != null) { future.completeExceptionally(err3); } else { - String msg = - "Restore snapshot=" + snapshotName + " failed. Rollback to snapshot=" - + failSafeSnapshotSnapshotName + " succeeded."; - future.completeExceptionally(new RestoreSnapshotException(msg, err2)); + // If fail to restore snapshot but rollback successfully, delete the + // restore-failsafe snapshot. + LOG.info( + "Deleting restore-failsafe snapshot: " + failSafeSnapshotSnapshotName); + addListener(deleteSnapshot(failSafeSnapshotSnapshotName), (ret4, err4) -> { + if (err4 != null) { + LOG.error("Unable to remove the failsafe snapshot: {}", + failSafeSnapshotSnapshotName, err4); + } + String msg = + "Restore snapshot=" + snapshotName + " failed, Rollback to snapshot=" + + failSafeSnapshotSnapshotName + " succeeded."; + LOG.error(msg); + future.completeExceptionally(new RestoreSnapshotException(msg, err2)); + }); } }); } else { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/SnapshotWithAclTestBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/SnapshotWithAclTestBase.java index 752a9ba92f99..5e37901840b4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/SnapshotWithAclTestBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/SnapshotWithAclTestBase.java @@ -17,13 +17,20 @@ */ package org.apache.hadoop.hbase.client; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; + import java.io.IOException; import java.util.List; import java.util.regex.Pattern; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.HBaseCommonTestingUtil; import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.master.MasterCoprocessorHost; @@ -33,6 +40,8 @@ import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.PermissionStorage; import org.apache.hadoop.hbase.security.access.SecureTestUtil; +import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils; +import org.apache.hadoop.hbase.snapshot.SnapshotManifest; import org.apache.hadoop.hbase.util.Bytes; import org.junit.AfterClass; import org.junit.Assert; @@ -110,6 +119,8 @@ public static void setupBeforeClass() throws Exception { verifyConfiguration(conf); // Enable EXEC permission checking conf.setBoolean(AccessControlConstants.EXEC_PERMISSION_CHECKS_KEY, true); + TEST_UTIL.getConfiguration().set(HConstants.SNAPSHOT_RESTORE_FAILSAFE_NAME, + "hbase-failsafe-{snapshot.name}-{restore.timestamp}"); TEST_UTIL.startMiniCluster(); TEST_UTIL.waitUntilAllRegionsAssigned(PermissionStorage.ACL_TABLE_NAME); MasterCoprocessorHost cpHost = @@ -168,7 +179,7 @@ private void verifyRows(TableName tableName) throws IOException { byte[] value = result.getValue(TEST_FAMILY, TEST_QUALIFIER); Assert.assertArrayEquals(value, Bytes.toBytes(rowCount++)); } - Assert.assertEquals(ROW_COUNT, rowCount); + assertEquals(ROW_COUNT, rowCount); } } @@ -177,7 +188,8 @@ private void verifyRows(TableName tableName) throws IOException { protected abstract void cloneSnapshot(String snapshotName, TableName tableName, boolean restoreAcl) throws Exception; - protected abstract void restoreSnapshot(String snapshotName, boolean restoreAcl) throws Exception; + protected abstract void restoreSnapshot(String snapshotName, boolean takeFailSafeSnapshot, + boolean restoreAcl) throws Exception; @Test public void testRestoreSnapshot() throws Exception { @@ -220,7 +232,7 @@ public void testRestoreSnapshot() throws Exception { // restore snapshot with restoreAcl false. TEST_UTIL.getAdmin().disableTable(TEST_TABLE); - restoreSnapshot(snapshotName1, false); + restoreSnapshot(snapshotName1, false, false); TEST_UTIL.getAdmin().enableTable(TEST_TABLE); verifyAllowed(new AccessReadAction(TEST_TABLE), USER_OWNER, USER_RW); verifyDenied(new AccessReadAction(TEST_TABLE), USER_RO, USER_NONE); @@ -229,12 +241,36 @@ public void testRestoreSnapshot() throws Exception { // restore snapshot with restoreAcl true. TEST_UTIL.getAdmin().disableTable(TEST_TABLE); - restoreSnapshot(snapshotName1, true); + restoreSnapshot(snapshotName1, false, true); TEST_UTIL.getAdmin().enableTable(TEST_TABLE); verifyAllowed(new AccessReadAction(TEST_TABLE), USER_OWNER, USER_RO, USER_RW); verifyDenied(new AccessReadAction(TEST_TABLE), USER_NONE); verifyAllowed(new AccessWriteAction(TEST_TABLE), USER_OWNER, USER_RW); verifyDenied(new AccessWriteAction(TEST_TABLE), USER_RO, USER_NONE); + + // Delete data.manifest of the snapshot to simulate an invalid snapshot. + Configuration configuration = TEST_UTIL.getConfiguration(); + Path rootDir = new Path(configuration.get(HConstants.HBASE_DIR)); + Path snapshotDir = SnapshotDescriptionUtils.getCompletedSnapshotDir(snapshotName1, rootDir); + FileSystem fileSystem = FileSystem.get(rootDir.toUri(), configuration); + Path maniFestPath = new Path(snapshotDir, SnapshotManifest.DATA_MANIFEST_NAME); + fileSystem.delete(maniFestPath, false); + assertFalse(fileSystem.exists(maniFestPath)); + assertEquals(1, TEST_UTIL.getAdmin().listSnapshots(Pattern.compile(snapshotName1)).size()); + // There is no failsafe snapshot before restoring. + int failsafeSnapshotCount = TEST_UTIL.getAdmin() + .listSnapshots(Pattern.compile("hbase-failsafe-" + snapshotName1 + ".*")).size(); + assertEquals(0, failsafeSnapshotCount); + TEST_UTIL.getAdmin().disableTable(TEST_TABLE); + // We would get Exception when restoring data by this an invalid snapshot. + assertThrows(Exception.class, () -> restoreSnapshot(snapshotName1, true, true)); + TEST_UTIL.getAdmin().enableTable(TEST_TABLE); + verifyRows(TEST_TABLE); + // Fail to store snapshot but rollback successfully, so there is no failsafe snapshot after + // restoring. + failsafeSnapshotCount = TEST_UTIL.getAdmin() + .listSnapshots(Pattern.compile("hbase-failsafe-" + snapshotName1 + ".*")).size(); + assertEquals(0, failsafeSnapshotCount); } final class AccessSnapshotAction implements AccessTestAction { @@ -262,8 +298,8 @@ public void testDeleteSnapshot() throws Exception { USER_RO, USER_RW, USER_NONE); List snapshotDescriptions = TEST_UTIL.getAdmin().listSnapshots(Pattern.compile(testSnapshotName)); - Assert.assertEquals(1, snapshotDescriptions.size()); - Assert.assertEquals(USER_OWNER.getShortName(), snapshotDescriptions.get(0).getOwner()); + assertEquals(1, snapshotDescriptions.size()); + assertEquals(USER_OWNER.getShortName(), snapshotDescriptions.get(0).getOwner()); AccessTestAction deleteSnapshotAction = () -> { try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()); Admin admin = conn.getAdmin()) { @@ -276,6 +312,6 @@ public void testDeleteSnapshot() throws Exception { List snapshotsAfterDelete = TEST_UTIL.getAdmin().listSnapshots(Pattern.compile(testSnapshotName)); - Assert.assertEquals(0, snapshotsAfterDelete.size()); + assertEquals(0, snapshotsAfterDelete.size()); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAcl.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAcl.java index f8d0a7950628..411076dcf047 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAcl.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAcl.java @@ -43,7 +43,8 @@ protected void cloneSnapshot(String snapshotName, TableName tableName, boolean r } @Override - protected void restoreSnapshot(String snapshotName, boolean restoreAcl) throws Exception { - TEST_UTIL.getAdmin().restoreSnapshot(snapshotName, false, restoreAcl); + protected void restoreSnapshot(String snapshotName, boolean takeFailSafeSnapshot, + boolean restoreAcl) throws Exception { + TEST_UTIL.getAdmin().restoreSnapshot(snapshotName, takeFailSafeSnapshot, restoreAcl); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAclAsyncAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAclAsyncAdmin.java index d20b73540c9f..7c84f645ea73 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAclAsyncAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAclAsyncAdmin.java @@ -49,10 +49,11 @@ protected void cloneSnapshot(String snapshotName, TableName tableName, boolean r } @Override - protected void restoreSnapshot(String snapshotName, boolean restoreAcl) throws Exception { + protected void restoreSnapshot(String snapshotName, boolean takeFailSafeSnapshot, + boolean restoreAcl) throws Exception { try (AsyncConnection conn = ConnectionFactory.createAsyncConnection(TEST_UTIL.getConfiguration()).get()) { - conn.getAdmin().restoreSnapshot(snapshotName, false, restoreAcl).get(); + conn.getAdmin().restoreSnapshot(snapshotName, takeFailSafeSnapshot, restoreAcl).get(); } } }