From d5c86243e10abd8f9ea67d3aaa824bffa38aeaa4 Mon Sep 17 00:00:00 2001 From: Nihal Jain Date: Sat, 18 Mar 2023 12:36:28 +0530 Subject: [PATCH] =?UTF-8?q?HBASE-27671=20Client=20should=20not=20be=20able?= =?UTF-8?q?=20to=20restore/clone=20a=20snapshot=20aft=E2=80=A6=20(#5109)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Viraj Jasani --- .../snapshot/SnapshotTTLExpiredException.java | 46 ++++ .../master/cleaner/SnapshotCleanerChore.java | 22 +- .../procedure/CloneSnapshotProcedure.java | 10 + .../procedure/RestoreSnapshotProcedure.java | 10 + .../snapshot/SnapshotDescriptionUtils.java | 14 + .../client/TestSnapshotWithTTLFromClient.java | 240 ++++++++++++++++++ .../hbase/snapshot/SnapshotTestingUtils.java | 12 +- 7 files changed, 338 insertions(+), 16 deletions(-) create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotTTLExpiredException.java create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithTTLFromClient.java 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 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..d5ae1774675 --- /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.HBaseTestingUtil; +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 HBaseTestingUtil UTIL = new HBaseTestingUtil(); + 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 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 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 5d71486e3c0..980b0322bce 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 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);