Skip to content

Commit 1ecc0e4

Browse files
guluo2016Apache9
guluo2016
authored and
Apache9
committed
HBASE-28658 The failsafe snapshot should be deleted after rollback successfully (#5984)
Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit e0ada25)
1 parent 014fc51 commit 1ecc0e4

File tree

4 files changed

+64
-15
lines changed

4 files changed

+64
-15
lines changed

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2181,10 +2181,21 @@ private CompletableFuture<Void> restoreSnapshot(String snapshotName, TableName t
21812181
if (err3 != null) {
21822182
future.completeExceptionally(err3);
21832183
} else {
2184-
String msg =
2185-
"Restore snapshot=" + snapshotName + " failed. Rollback to snapshot="
2186-
+ failSafeSnapshotSnapshotName + " succeeded.";
2187-
future.completeExceptionally(new RestoreSnapshotException(msg, err2));
2184+
// If fail to restore snapshot but rollback successfully, delete the
2185+
// restore-failsafe snapshot.
2186+
LOG.info(
2187+
"Deleting restore-failsafe snapshot: " + failSafeSnapshotSnapshotName);
2188+
addListener(deleteSnapshot(failSafeSnapshotSnapshotName), (ret4, err4) -> {
2189+
if (err4 != null) {
2190+
LOG.error("Unable to remove the failsafe snapshot: {}",
2191+
failSafeSnapshotSnapshotName, err4);
2192+
}
2193+
String msg =
2194+
"Restore snapshot=" + snapshotName + " failed, Rollback to snapshot="
2195+
+ failSafeSnapshotSnapshotName + " succeeded.";
2196+
LOG.error(msg);
2197+
future.completeExceptionally(new RestoreSnapshotException(msg, err2));
2198+
});
21882199
}
21892200
});
21902201
} else {

‎hbase-server/src/test/java/org/apache/hadoop/hbase/client/SnapshotWithAclTestBase.java‎

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,20 @@
1717
*/
1818
package org.apache.hadoop.hbase.client;
1919

20+
import static org.junit.Assert.assertEquals;
21+
import static org.junit.Assert.assertFalse;
22+
import static org.junit.Assert.assertThrows;
23+
2024
import java.io.IOException;
2125
import java.util.List;
2226
import java.util.regex.Pattern;
2327
import org.apache.hadoop.conf.Configuration;
28+
import org.apache.hadoop.fs.FileSystem;
29+
import org.apache.hadoop.fs.Path;
2430
import org.apache.hadoop.hbase.Coprocessor;
2531
import org.apache.hadoop.hbase.HBaseCommonTestingUtil;
2632
import org.apache.hadoop.hbase.HBaseTestingUtil;
33+
import org.apache.hadoop.hbase.HConstants;
2734
import org.apache.hadoop.hbase.TableName;
2835
import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
2936
import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
@@ -33,6 +40,8 @@
3340
import org.apache.hadoop.hbase.security.access.Permission;
3441
import org.apache.hadoop.hbase.security.access.PermissionStorage;
3542
import org.apache.hadoop.hbase.security.access.SecureTestUtil;
43+
import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
44+
import org.apache.hadoop.hbase.snapshot.SnapshotManifest;
3645
import org.apache.hadoop.hbase.util.Bytes;
3746
import org.junit.AfterClass;
3847
import org.junit.Assert;
@@ -110,6 +119,8 @@ public static void setupBeforeClass() throws Exception {
110119
verifyConfiguration(conf);
111120
// Enable EXEC permission checking
112121
conf.setBoolean(AccessControlConstants.EXEC_PERMISSION_CHECKS_KEY, true);
122+
TEST_UTIL.getConfiguration().set(HConstants.SNAPSHOT_RESTORE_FAILSAFE_NAME,
123+
"hbase-failsafe-{snapshot.name}-{restore.timestamp}");
113124
TEST_UTIL.startMiniCluster();
114125
TEST_UTIL.waitUntilAllRegionsAssigned(PermissionStorage.ACL_TABLE_NAME);
115126
MasterCoprocessorHost cpHost =
@@ -168,7 +179,7 @@ private void verifyRows(TableName tableName) throws IOException {
168179
byte[] value = result.getValue(TEST_FAMILY, TEST_QUALIFIER);
169180
Assert.assertArrayEquals(value, Bytes.toBytes(rowCount++));
170181
}
171-
Assert.assertEquals(ROW_COUNT, rowCount);
182+
assertEquals(ROW_COUNT, rowCount);
172183
}
173184
}
174185

@@ -177,7 +188,8 @@ private void verifyRows(TableName tableName) throws IOException {
177188
protected abstract void cloneSnapshot(String snapshotName, TableName tableName,
178189
boolean restoreAcl) throws Exception;
179190

180-
protected abstract void restoreSnapshot(String snapshotName, boolean restoreAcl) throws Exception;
191+
protected abstract void restoreSnapshot(String snapshotName, boolean takeFailSafeSnapshot,
192+
boolean restoreAcl) throws Exception;
181193

182194
@Test
183195
public void testRestoreSnapshot() throws Exception {
@@ -220,7 +232,7 @@ public void testRestoreSnapshot() throws Exception {
220232

221233
// restore snapshot with restoreAcl false.
222234
TEST_UTIL.getAdmin().disableTable(TEST_TABLE);
223-
restoreSnapshot(snapshotName1, false);
235+
restoreSnapshot(snapshotName1, false, false);
224236
TEST_UTIL.getAdmin().enableTable(TEST_TABLE);
225237
verifyAllowed(new AccessReadAction(TEST_TABLE), USER_OWNER, USER_RW);
226238
verifyDenied(new AccessReadAction(TEST_TABLE), USER_RO, USER_NONE);
@@ -229,12 +241,36 @@ public void testRestoreSnapshot() throws Exception {
229241

230242
// restore snapshot with restoreAcl true.
231243
TEST_UTIL.getAdmin().disableTable(TEST_TABLE);
232-
restoreSnapshot(snapshotName1, true);
244+
restoreSnapshot(snapshotName1, false, true);
233245
TEST_UTIL.getAdmin().enableTable(TEST_TABLE);
234246
verifyAllowed(new AccessReadAction(TEST_TABLE), USER_OWNER, USER_RO, USER_RW);
235247
verifyDenied(new AccessReadAction(TEST_TABLE), USER_NONE);
236248
verifyAllowed(new AccessWriteAction(TEST_TABLE), USER_OWNER, USER_RW);
237249
verifyDenied(new AccessWriteAction(TEST_TABLE), USER_RO, USER_NONE);
250+
251+
// Delete data.manifest of the snapshot to simulate an invalid snapshot.
252+
Configuration configuration = TEST_UTIL.getConfiguration();
253+
Path rootDir = new Path(configuration.get(HConstants.HBASE_DIR));
254+
Path snapshotDir = SnapshotDescriptionUtils.getCompletedSnapshotDir(snapshotName1, rootDir);
255+
FileSystem fileSystem = FileSystem.get(rootDir.toUri(), configuration);
256+
Path maniFestPath = new Path(snapshotDir, SnapshotManifest.DATA_MANIFEST_NAME);
257+
fileSystem.delete(maniFestPath, false);
258+
assertFalse(fileSystem.exists(maniFestPath));
259+
assertEquals(1, TEST_UTIL.getAdmin().listSnapshots(Pattern.compile(snapshotName1)).size());
260+
// There is no failsafe snapshot before restoring.
261+
int failsafeSnapshotCount = TEST_UTIL.getAdmin()
262+
.listSnapshots(Pattern.compile("hbase-failsafe-" + snapshotName1 + ".*")).size();
263+
assertEquals(0, failsafeSnapshotCount);
264+
TEST_UTIL.getAdmin().disableTable(TEST_TABLE);
265+
// We would get Exception when restoring data by this an invalid snapshot.
266+
assertThrows(Exception.class, () -> restoreSnapshot(snapshotName1, true, true));
267+
TEST_UTIL.getAdmin().enableTable(TEST_TABLE);
268+
verifyRows(TEST_TABLE);
269+
// Fail to store snapshot but rollback successfully, so there is no failsafe snapshot after
270+
// restoring.
271+
failsafeSnapshotCount = TEST_UTIL.getAdmin()
272+
.listSnapshots(Pattern.compile("hbase-failsafe-" + snapshotName1 + ".*")).size();
273+
assertEquals(0, failsafeSnapshotCount);
238274
}
239275

240276
final class AccessSnapshotAction implements AccessTestAction {
@@ -262,8 +298,8 @@ public void testDeleteSnapshot() throws Exception {
262298
USER_RO, USER_RW, USER_NONE);
263299
List<SnapshotDescription> snapshotDescriptions =
264300
TEST_UTIL.getAdmin().listSnapshots(Pattern.compile(testSnapshotName));
265-
Assert.assertEquals(1, snapshotDescriptions.size());
266-
Assert.assertEquals(USER_OWNER.getShortName(), snapshotDescriptions.get(0).getOwner());
301+
assertEquals(1, snapshotDescriptions.size());
302+
assertEquals(USER_OWNER.getShortName(), snapshotDescriptions.get(0).getOwner());
267303
AccessTestAction deleteSnapshotAction = () -> {
268304
try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration());
269305
Admin admin = conn.getAdmin()) {
@@ -276,6 +312,6 @@ public void testDeleteSnapshot() throws Exception {
276312

277313
List<SnapshotDescription> snapshotsAfterDelete =
278314
TEST_UTIL.getAdmin().listSnapshots(Pattern.compile(testSnapshotName));
279-
Assert.assertEquals(0, snapshotsAfterDelete.size());
315+
assertEquals(0, snapshotsAfterDelete.size());
280316
}
281317
}

‎hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAcl.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ protected void cloneSnapshot(String snapshotName, TableName tableName, boolean r
4343
}
4444

4545
@Override
46-
protected void restoreSnapshot(String snapshotName, boolean restoreAcl) throws Exception {
47-
TEST_UTIL.getAdmin().restoreSnapshot(snapshotName, false, restoreAcl);
46+
protected void restoreSnapshot(String snapshotName, boolean takeFailSafeSnapshot,
47+
boolean restoreAcl) throws Exception {
48+
TEST_UTIL.getAdmin().restoreSnapshot(snapshotName, takeFailSafeSnapshot, restoreAcl);
4849
}
4950
}

‎hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithAclAsyncAdmin.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,11 @@ protected void cloneSnapshot(String snapshotName, TableName tableName, boolean r
4949
}
5050

5151
@Override
52-
protected void restoreSnapshot(String snapshotName, boolean restoreAcl) throws Exception {
52+
protected void restoreSnapshot(String snapshotName, boolean takeFailSafeSnapshot,
53+
boolean restoreAcl) throws Exception {
5354
try (AsyncConnection conn =
5455
ConnectionFactory.createAsyncConnection(TEST_UTIL.getConfiguration()).get()) {
55-
conn.getAdmin().restoreSnapshot(snapshotName, false, restoreAcl).get();
56+
conn.getAdmin().restoreSnapshot(snapshotName, takeFailSafeSnapshot, restoreAcl).get();
5657
}
5758
}
5859
}

0 commit comments

Comments
 (0)