You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by vj...@apache.org on 2023/03/18 07:07:23 UTC
[hbase] branch branch-2 updated: HBASE-27671 Client should not be able to restore/clone a snapshot after it has TTL expired it's TTL has expired (#5117) ( #5109)
This is an automated email from the ASF dual-hosted git repository.
vjasani pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new b1a00be1bc9 HBASE-27671 Client should not be able to restore/clone a snapshot after it has TTL expired it's TTL has expired (#5117) ( #5109)
b1a00be1bc9 is described below
commit b1a00be1bc921ca478e97133132fc5e6e9e65bdc
Author: Nihal Jain <ni...@gmail.com>
AuthorDate: Sat Mar 18 12:37:16 2023 +0530
HBASE-27671 Client should not be able to restore/clone a snapshot after it has TTL expired it's TTL has expired (#5117) ( #5109)
Signed-off-by: Viraj Jasani <vj...@apache.org>
---
.../snapshot/SnapshotTTLExpiredException.java | 46 ++++
.../hbase/master/cleaner/SnapshotCleanerChore.java | 22 +-
.../master/procedure/CloneSnapshotProcedure.java | 10 +
.../master/procedure/RestoreSnapshotProcedure.java | 10 +
.../hbase/snapshot/SnapshotDescriptionUtils.java | 14 ++
.../client/TestSnapshotWithTTLFromClient.java | 240 +++++++++++++++++++++
.../hbase/snapshot/SnapshotTestingUtils.java | 12 +-
7 files changed, 338 insertions(+), 16 deletions(-)
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotTTLExpiredException.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotTTLExpiredException.java
new file mode 100644
index 00000000000..9fabc13a77b
--- /dev/null
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotTTLExpiredException.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.snapshot;
+
+import org.apache.hadoop.hbase.client.SnapshotDescription;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * Thrown when a snapshot could not be restored/cloned because the ttl for snapshot has already
+ * expired
+ */
+@SuppressWarnings("serial")
+@InterfaceAudience.Public
+public class SnapshotTTLExpiredException extends HBaseSnapshotException {
+ /**
+ * Failure when the ttl for snapshot has already expired.
+ * @param message the full description of the failure
+ */
+ public SnapshotTTLExpiredException(String message) {
+ super(message);
+ }
+
+ /**
+ * Failure when the ttl for snapshot has already expired.
+ * @param snapshotDescription snapshot that was attempted
+ */
+ public SnapshotTTLExpiredException(SnapshotDescription snapshotDescription) {
+ super("TTL for snapshot '" + snapshotDescription.getName() + "' has already expired.",
+ snapshotDescription);
+ }
+}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/SnapshotCleanerChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/SnapshotCleanerChore.java
index ab0481ace1a..67533b3b504 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/SnapshotCleanerChore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/SnapshotCleanerChore.java
@@ -19,11 +19,11 @@ package org.apache.hadoop.hbase.master.cleaner;
import java.io.IOException;
import java.util.List;
-import java.util.concurrent.TimeUnit;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.ScheduledChore;
import org.apache.hadoop.hbase.Stoppable;
import org.apache.hadoop.hbase.master.snapshot.SnapshotManager;
+import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
@@ -72,22 +72,14 @@ public class SnapshotCleanerChore extends ScheduledChore {
for (SnapshotProtos.SnapshotDescription snapshotDescription : completedSnapshotsList) {
long snapshotCreatedTime = snapshotDescription.getCreationTime();
long snapshotTtl = snapshotDescription.getTtl();
- /*
- * Backward compatibility after the patch deployment on HMaster Any snapshot with ttl 0 is
- * to be considered as snapshot to keep FOREVER Default ttl value specified by
- * {@HConstants.DEFAULT_SNAPSHOT_TTL}
- */
+ long currentTime = EnvironmentEdgeManager.currentTime();
if (
- snapshotCreatedTime > 0 && snapshotTtl > 0
- && snapshotTtl < TimeUnit.MILLISECONDS.toSeconds(Long.MAX_VALUE)
+ SnapshotDescriptionUtils.isExpiredSnapshot(snapshotTtl, snapshotCreatedTime, currentTime)
) {
- long currentTime = EnvironmentEdgeManager.currentTime();
- if ((snapshotCreatedTime + TimeUnit.SECONDS.toMillis(snapshotTtl)) < currentTime) {
- LOG.info("Event: {} Name: {}, CreatedTime: {}, TTL: {}, currentTime: {}",
- DELETE_SNAPSHOT_EVENT, snapshotDescription.getName(), snapshotCreatedTime,
- snapshotTtl, currentTime);
- deleteExpiredSnapshot(snapshotDescription);
- }
+ LOG.info("Event: {} Name: {}, CreatedTime: {}, TTL: {}, currentTime: {}",
+ DELETE_SNAPSHOT_EVENT, snapshotDescription.getName(), snapshotCreatedTime, snapshotTtl,
+ currentTime);
+ deleteExpiredSnapshot(snapshotDescription);
}
}
} catch (IOException e) {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
index 6394f1d6ce6..1a9c93b82c0 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
@@ -52,7 +52,9 @@ import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
import org.apache.hadoop.hbase.snapshot.RestoreSnapshotHelper;
import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
import org.apache.hadoop.hbase.snapshot.SnapshotManifest;
+import org.apache.hadoop.hbase.snapshot.SnapshotTTLExpiredException;
import org.apache.hadoop.hbase.util.CommonFSUtils;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
import org.apache.hadoop.hbase.util.FSTableDescriptors;
import org.apache.hadoop.hbase.util.Pair;
import org.apache.yetus.audience.InterfaceAudience;
@@ -380,6 +382,14 @@ public class CloneSnapshotProcedure extends AbstractStateMachineTableProcedure<C
throw new TableExistsException(tableName);
}
+ // check whether ttl has expired for this snapshot
+ if (
+ SnapshotDescriptionUtils.isExpiredSnapshot(snapshot.getTtl(), snapshot.getCreationTime(),
+ EnvironmentEdgeManager.currentTime())
+ ) {
+ throw new SnapshotTTLExpiredException(ProtobufUtil.createSnapshotDesc(snapshot));
+ }
+
validateSFT();
}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
index fa22dcd4ce8..cdc5f5ab3a2 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
@@ -50,6 +50,8 @@ import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils;
import org.apache.hadoop.hbase.snapshot.RestoreSnapshotHelper;
import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
import org.apache.hadoop.hbase.snapshot.SnapshotManifest;
+import org.apache.hadoop.hbase.snapshot.SnapshotTTLExpiredException;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
import org.apache.hadoop.hbase.util.Pair;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
@@ -328,6 +330,14 @@ public class RestoreSnapshotProcedure
throw new TableNotFoundException(tableName);
}
+ // check whether ttl has expired for this snapshot
+ if (
+ SnapshotDescriptionUtils.isExpiredSnapshot(snapshot.getTtl(), snapshot.getCreationTime(),
+ EnvironmentEdgeManager.currentTime())
+ ) {
+ throw new SnapshotTTLExpiredException(ProtobufUtil.createSnapshotDesc(snapshot));
+ }
+
// Check whether table is disabled.
env.getMasterServices().checkTableModifiable(tableName);
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
index 114e989ca04..6b61a2eab06 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
@@ -481,4 +481,18 @@ public final class SnapshotDescriptionUtils {
return snapshot.toBuilder()
.setUsersAndPermissions(ShadedAccessControlUtil.toUserTablePermissions(perms)).build();
}
+
+ /**
+ * Method to check whether TTL has expired for specified snapshot creation time and snapshot ttl.
+ * NOTE: For backward compatibility (after the patch deployment on HMaster), any snapshot with ttl
+ * 0 is to be considered as snapshot to keep FOREVER. Default ttl value specified by
+ * {@link HConstants#DEFAULT_SNAPSHOT_TTL}
+ * @return true if ttl has expired, or, false, otherwise
+ */
+ public static boolean isExpiredSnapshot(long snapshotTtl, long snapshotCreatedTime,
+ long currentTime) {
+ return snapshotCreatedTime > 0 && snapshotTtl > HConstants.DEFAULT_SNAPSHOT_TTL
+ && snapshotTtl < TimeUnit.MILLISECONDS.toSeconds(Long.MAX_VALUE)
+ && (snapshotCreatedTime + TimeUnit.SECONDS.toMillis(snapshotTtl)) < currentTime;
+ }
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithTTLFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithTTLFromClient.java
new file mode 100644
index 00000000000..4309b922b8f
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithTTLFromClient.java
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.client;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.master.snapshot.SnapshotManager;
+import org.apache.hadoop.hbase.snapshot.SnapshotTTLExpiredException;
+import org.apache.hadoop.hbase.snapshot.SnapshotTestingUtils;
+import org.apache.hadoop.hbase.testclassification.ClientTests;
+import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.Threads;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Test restore/clone snapshots with TTL from the client
+ */
+@Category({ LargeTests.class, ClientTests.class })
+public class TestSnapshotWithTTLFromClient {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestSnapshotWithTTLFromClient.class);
+
+ private static final Logger LOG = LoggerFactory.getLogger(TestSnapshotWithTTLFromClient.class);
+
+ private static final HBaseTestingUtility UTIL = new HBaseTestingUtility();
+ private static final int NUM_RS = 2;
+ private static final String STRING_TABLE_NAME = "test";
+ private static final byte[] TEST_FAM = Bytes.toBytes("fam");
+ private static final TableName TABLE_NAME = TableName.valueOf(STRING_TABLE_NAME);
+ private static final TableName CLONED_TABLE_NAME = TableName.valueOf("clonedTable");
+ private static final String TTL_KEY = "TTL";
+ private static final int CHORE_INTERVAL_SECS = 30;
+
+ /**
+ * Setup the config for the cluster
+ * @throws Exception on failure
+ */
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ setupConf(UTIL.getConfiguration());
+ UTIL.startMiniCluster(NUM_RS);
+ }
+
+ protected static void setupConf(Configuration conf) {
+ // Enable snapshot
+ conf.setBoolean(SnapshotManager.HBASE_SNAPSHOT_ENABLED, true);
+
+ // Set this to high value so that cleaner chore is not triggered
+ conf.setInt("hbase.master.cleaner.snapshot.interval", CHORE_INTERVAL_SECS * 60 * 1000);
+ }
+
+ @Before
+ public void setup() throws Exception {
+ createTable();
+ }
+
+ protected void createTable() throws Exception {
+ UTIL.createTable(TABLE_NAME, new byte[][] { TEST_FAM });
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ UTIL.deleteTableIfAny(TABLE_NAME);
+ UTIL.deleteTableIfAny(CLONED_TABLE_NAME);
+ SnapshotTestingUtils.deleteAllSnapshots(UTIL.getAdmin());
+ SnapshotTestingUtils.deleteArchiveDirectory(UTIL);
+ }
+
+ @AfterClass
+ public static void cleanupTest() throws Exception {
+ try {
+ UTIL.shutdownMiniCluster();
+ } catch (Exception e) {
+ LOG.warn("failure shutting down cluster", e);
+ }
+ }
+
+ @Test
+ public void testRestoreSnapshotWithTTLSuccess() throws Exception {
+ String snapshotName = "nonExpiredTTLRestoreSnapshotTest";
+
+ // table should exist
+ assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
+
+ // create snapshot fo given table with specified ttl
+ createSnapshotWithTTL(TABLE_NAME, snapshotName, CHORE_INTERVAL_SECS * 2);
+ Admin admin = UTIL.getAdmin();
+
+ // Disable and drop table
+ admin.disableTable(TABLE_NAME);
+ admin.deleteTable(TABLE_NAME);
+ assertFalse(UTIL.getAdmin().tableExists(TABLE_NAME));
+
+ // restore snapshot
+ admin.restoreSnapshot(snapshotName);
+
+ // table should be created
+ assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
+ }
+
+ @Test
+ public void testRestoreSnapshotFailsDueToTTLExpired() throws Exception {
+ String snapshotName = "expiredTTLRestoreSnapshotTest";
+
+ // table should exist
+ assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
+
+ // create snapshot fo given table with specified ttl
+ createSnapshotWithTTL(TABLE_NAME, snapshotName, 1);
+ Admin admin = UTIL.getAdmin();
+
+ // Disable and drop table
+ admin.disableTable(TABLE_NAME);
+ admin.deleteTable(TABLE_NAME);
+ assertFalse(UTIL.getAdmin().tableExists(TABLE_NAME));
+
+ // Sleep so that TTL may expire
+ Threads.sleep(2000);
+
+ // restore snapshot which has expired
+ try {
+ admin.restoreSnapshot(snapshotName);
+ fail("Restore snapshot succeeded even though TTL has expired.");
+ } catch (SnapshotTTLExpiredException e) {
+ LOG.info("Correctly failed to restore a TTL expired snapshot table:" + e.getMessage());
+ }
+
+ // table should not be created
+ assertFalse(UTIL.getAdmin().tableExists(TABLE_NAME));
+ }
+
+ @Test
+ public void testCloneSnapshotWithTTLSuccess() throws Exception {
+ String snapshotName = "nonExpiredTTLCloneSnapshotTest";
+
+ // table should exist
+ assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
+
+ // create snapshot fo given table with specified ttl
+ createSnapshotWithTTL(TABLE_NAME, snapshotName, CHORE_INTERVAL_SECS * 2);
+ Admin admin = UTIL.getAdmin();
+
+ // restore snapshot
+ admin.cloneSnapshot(snapshotName, CLONED_TABLE_NAME);
+
+ // table should be created
+ assertTrue(UTIL.getAdmin().tableExists(CLONED_TABLE_NAME));
+ }
+
+ @Test
+ public void testCloneSnapshotFailsDueToTTLExpired() throws Exception {
+ String snapshotName = "expiredTTLCloneSnapshotTest";
+
+ // table should exist
+ assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
+
+ // create snapshot fo given table with specified ttl
+ createSnapshotWithTTL(TABLE_NAME, snapshotName, 1);
+ Admin admin = UTIL.getAdmin();
+
+ assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
+
+ // Sleep so that TTL may expire
+ Threads.sleep(2000);
+
+ // clone snapshot which has expired
+ try {
+ admin.cloneSnapshot(snapshotName, CLONED_TABLE_NAME);
+ fail("Clone snapshot succeeded even though TTL has expired.");
+ } catch (SnapshotTTLExpiredException e) {
+ LOG.info("Correctly failed to clone a TTL expired snapshot table:" + e.getMessage());
+ }
+
+ // table should not be created
+ assertFalse(UTIL.getAdmin().tableExists(CLONED_TABLE_NAME));
+ }
+
+ private void createSnapshotWithTTL(TableName tableName, final String snapshotName,
+ final int snapshotTTL) throws IOException {
+ Admin admin = UTIL.getAdmin();
+
+ // make sure we don't fail on listing snapshots
+ SnapshotTestingUtils.assertNoSnapshots(admin);
+
+ // put some stuff in the table
+ Table table = UTIL.getConnection().getTable(tableName);
+ UTIL.loadTable(table, TEST_FAM);
+
+ Map<String, Object> props = new HashMap<>();
+ props.put(TTL_KEY, snapshotTTL);
+
+ // take a snapshot of the table
+ SnapshotTestingUtils.snapshot(UTIL.getAdmin(), snapshotName, tableName, SnapshotType.FLUSH, 3,
+ props);
+ LOG.debug("Snapshot completed.");
+
+ // make sure we have the snapshot with expectd TTL
+ List<SnapshotDescription> snapshots =
+ SnapshotTestingUtils.assertOneSnapshotThatMatches(admin, snapshotName, tableName);
+ assertEquals(1, snapshots.size());
+ assertEquals(snapshotTTL, snapshots.get(0).getTtl());
+ }
+}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/SnapshotTestingUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/SnapshotTestingUtils.java
index f74ed244678..e9041570046 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/SnapshotTestingUtils.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/SnapshotTestingUtils.java
@@ -270,11 +270,21 @@ public final class SnapshotTestingUtils {
*/
public static void snapshot(Admin admin, final String snapshotName, final TableName tableName,
final SnapshotType type, final int numTries) throws IOException {
+ snapshot(admin, snapshotName, tableName, type, numTries, null);
+ }
+
+ /*
+ * Take snapshot having snapshot properties with maximum of numTries attempts, ignoring
+ * CorruptedSnapshotException except for the last CorruptedSnapshotException
+ */
+ public static void snapshot(Admin admin, final String snapshotName, final TableName tableName,
+ final SnapshotType type, final int numTries, Map<String, Object> snapshotProps)
+ throws IOException {
int tries = 0;
CorruptedSnapshotException lastEx = null;
while (tries++ < numTries) {
try {
- admin.snapshot(snapshotName, tableName, type);
+ admin.snapshot(snapshotName, tableName, type, snapshotProps);
return;
} catch (CorruptedSnapshotException cse) {
LOG.warn("Got CorruptedSnapshotException", cse);