Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-28658 The failsafe snapshot should be deleted after rollback successfully (for branch-2) #6019

Merged
merged 1 commit into from
Jun 22, 2024
Merged

HBASE-28658 The failsafe snapshot should be deleted after rollback successfully (for branch-2) #6019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2816,6 +2816,13 @@ public void restoreSnapshot(final String snapshotName, final boolean takeFailSaf
String msg = "Restore snapshot=" + snapshotName + " failed. Rollback to snapshot="
+ failSafeSnapshotSnapshotName + " succeeded.";
LOG.error(msg, e);
try {
LOG.info("Deleting restore-failsafe snapshot: {}", failSafeSnapshotSnapshotName);
deleteSnapshot(failSafeSnapshotSnapshotName);
} catch (IOException deleteSnapshotException) {
LOG.error("Unable to remove the failsafe snapshot: {}", failSafeSnapshotSnapshotName,
deleteSnapshotException);
}
throw new RestoreSnapshotException(msg, e);
} catch (IOException ex) {
String msg = "Failed to restore and rollback to snapshot=" + failSafeSnapshotSnapshotName;
Expand Down
19 changes: 15 additions & 4 deletions hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2143,10 +2143,21 @@ private CompletableFuture<Void> 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 {
Expand Down
50 changes: 43 additions & 7 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/client/SnapshotWithAclTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.HBaseCommonTestingUtility;
import org.apache.hadoop.hbase.HBaseTestingUtility;
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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -163,7 +174,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);
}
}

Expand All @@ -172,7 +183,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 {
Expand Down Expand Up @@ -215,7 +227,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);
Expand All @@ -224,12 +236,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 {
Expand Down Expand Up @@ -257,8 +293,8 @@ public void testDeleteSnapshot() throws Exception {
USER_RO, USER_RW, USER_NONE);
List<SnapshotDescription> 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()) {
Expand All @@ -271,6 +307,6 @@ public void testDeleteSnapshot() throws Exception {

List<SnapshotDescription> snapshotsAfterDelete =
TEST_UTIL.getAdmin().listSnapshots(Pattern.compile(testSnapshotName));
Assert.assertEquals(0, snapshotsAfterDelete.size());
assertEquals(0, snapshotsAfterDelete.size());
}
}
5 changes: 3 additions & 2 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAcl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
5 changes: 3 additions & 2 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAclAsyncAdmin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}