diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java index 572cdce0cf6f..a554f967ab9a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java @@ -103,6 +103,30 @@ public TableOperationType getTableOperationType() { return TableOperationType.SNAPSHOT; } + @Override + protected LockState acquireLock(MasterProcedureEnv env) { + // AbstractStateMachineTableProcedure acquires exclusive table lock by default, + // but we may need to downgrade it to shared lock for some reasons: + // a. exclusive lock has a negative effect on assigning region. See HBASE-21480 for details. + // b. we want to support taking multiple different snapshots on same table on the same time. + if (env.getProcedureScheduler().waitTableSharedLock(this, getTableName())) { + return LockState.LOCK_EVENT_WAIT; + } + return LockState.LOCK_ACQUIRED; + } + + @Override + protected void releaseLock(MasterProcedureEnv env) { + env.getProcedureScheduler().wakeTableSharedLock(this, getTableName()); + } + + @Override + protected boolean holdLock(MasterProcedureEnv env) { + // In order to avoid enabling/disabling/modifying/deleting table during snapshot, + // we don't release lock during suspend + return true; + } + @Override protected Flow executeFromState(MasterProcedureEnv env, SnapshotState state) throws ProcedureSuspendedException, ProcedureYieldException, InterruptedException { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java index be66a28d275e..078dc8313863 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java @@ -52,7 +52,6 @@ static boolean requireTableExclusiveLock(TableProcedureInterface proc) { case CREATE: case DELETE: case DISABLE: - case SNAPSHOT: case ENABLE: return true; case EDIT: @@ -60,6 +59,7 @@ static boolean requireTableExclusiveLock(TableProcedureInterface proc) { return !proc.getTableName().equals(TableProcedureInterface.DUMMY_NAMESPACE_TABLE_NAME); case READ: case FLUSH: + case SNAPSHOT: return false; // region operations are using the shared-lock on the table // and then they will grab an xlock on the region. diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureConcurrently.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureConcurrently.java index 505bc4a49a9a..b4c62aa35dcd 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureConcurrently.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureConcurrently.java @@ -17,7 +17,6 @@ */ package org.apache.hadoop.hbase.master.procedure; -import static org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.EnableTableState.ENABLE_TABLE_MARK_REGIONS_ONLINE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; @@ -28,7 +27,6 @@ import java.util.List; import java.util.stream.Collectors; import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.SnapshotType; import org.apache.hadoop.hbase.master.snapshot.SnapshotManager; @@ -138,59 +136,4 @@ public void run() { SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotProto, TABLE_NAME, CF); SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotOnSameTableProto, TABLE_NAME, CF); } - - @Test - public void testItFailsIfTableIsNotDisabledOrEnabled() throws Exception { - ProcedureExecutor executor = master.getMasterProcedureExecutor(); - MasterProcedureEnv env = executor.getEnvironment(); - TEST_UTIL.getAdmin().disableTable(TABLE_NAME); - - TestEnableTableProcedure enableTable = new TestEnableTableProcedure( - master.getMasterProcedureExecutor().getEnvironment(), TABLE_NAME); - long enableProcId = executor.submitProcedure(enableTable); - TEST_UTIL.waitFor(60000, () -> { - Procedure proc = executor.getProcedure(enableProcId); - if (proc == null) { - return false; - } - return ((TestEnableTableProcedure) proc).getProcedureState() - == ENABLE_TABLE_MARK_REGIONS_ONLINE; - }); - - // Using a delayed spy ensures we hit the problem state while the table enable procedure - // is waiting to run - SnapshotProcedure snapshotProc = new SnapshotProcedure(env, snapshotProto); - long snapshotProcId = executor.submitProcedure(snapshotProc); - TEST_UTIL.waitTableEnabled(TABLE_NAME); - // Wait for procedure to run and finish - TEST_UTIL.waitFor(60000, () -> executor.getProcedure(snapshotProcId) != null); - TEST_UTIL.waitFor(60000, () -> executor.getProcedure(snapshotProcId) == null); - - SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotProto, TABLE_NAME, CF); - } - - // Needs to be publicly accessible for Procedure validation - public static class TestEnableTableProcedure extends EnableTableProcedure { - // Necessary for Procedure validation - public TestEnableTableProcedure() { - } - - public TestEnableTableProcedure(MasterProcedureEnv env, TableName tableName) { - super(env, tableName); - } - - public MasterProcedureProtos.EnableTableState getProcedureState() { - return getState(stateCount); - } - - @Override - protected Flow executeFromState(MasterProcedureEnv env, - MasterProcedureProtos.EnableTableState state) throws InterruptedException { - if (state == ENABLE_TABLE_MARK_REGIONS_ONLINE) { - Thread.sleep(10000); - } - - return super.executeFromState(env, state); - } - } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureRIT.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureRIT.java index e7cfeae324e7..60e96f7b80dc 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureRIT.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureRIT.java @@ -53,7 +53,8 @@ public void testTableInMergeWhileTakingSnapshot() throws Exception { () -> procExec.getProcedure(mergeProcId).getState() == ProcedureState.RUNNABLE); SnapshotProcedure sp = new SnapshotProcedure(procExec.getEnvironment(), snapshotProto); long snapshotProcId = procExec.submitProcedure(sp); - TEST_UTIL.waitFor(2000, 1000, () -> procExec.getProcedure(snapshotProcId) != null); + TEST_UTIL.waitFor(2000, 1000, () -> procExec.getProcedure(snapshotProcId) != null + && procExec.getProcedure(snapshotProcId).getState() == ProcedureState.WAITING_TIMEOUT); ProcedureTestingUtility.waitProcedure(procExec, snapshotProcId); SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotProto, TABLE_NAME, CF); }