Repository: hbase Updated Branches: refs/heads/master 243fe287b -> 28d8609e5
HBASE-16233 Procedure V2: Support acquire/release shared table lock concurrently (Stephen Yuan Jiang) Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/28d8609e Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/28d8609e Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/28d8609e Branch: refs/heads/master Commit: 28d8609e5de62de78b6ab7570c7fea2c8f78a856 Parents: 243fe28 Author: Stephen Yuan Jiang <syuanjiang...@gmail.com> Authored: Thu Jul 14 21:32:59 2016 -0700 Committer: Stephen Yuan Jiang <syuanjiang...@gmail.com> Committed: Thu Jul 14 21:32:59 2016 -0700 ---------------------------------------------------------------------- .../procedure/MasterProcedureScheduler.java | 38 +++++++++++------- .../procedure/TestMasterProcedureScheduler.java | 42 ++++++++++++++++++-- 2 files changed, 62 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/28d8609e/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java index d4791fe..d368a82 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java @@ -794,21 +794,28 @@ public class MasterProcedureScheduler implements ProcedureRunnableSet { private synchronized boolean tryZkSharedLock(final TableLockManager lockManager, final String purpose) { - // Take zk-read-lock - TableName tableName = getKey(); - tableLock = lockManager.readLock(tableName, purpose); - try { - tableLock.acquire(); - } catch (IOException e) { - LOG.error("failed acquire read lock on " + tableName, e); - tableLock = null; - return false; + // Since we only have one lock resource. We should only acquire zk lock if the znode + // does not exist. + // + if (isSingleSharedLock()) { + // Take zk-read-lock + TableName tableName = getKey(); + tableLock = lockManager.readLock(tableName, purpose); + try { + tableLock.acquire(); + } catch (IOException e) { + LOG.error("failed acquire read lock on " + tableName, e); + tableLock = null; + return false; + } } return true; } private synchronized void releaseZkSharedLock(final TableLockManager lockManager) { - releaseTableLock(lockManager, isSingleSharedLock()); + if (isSingleSharedLock()) { + releaseTableLock(lockManager, true); + } } private synchronized boolean tryZkExclusiveLock(final TableLockManager lockManager, @@ -969,16 +976,17 @@ public class MasterProcedureScheduler implements ProcedureRunnableSet { return null; } - schedLock.unlock(); - - // Zk lock is expensive... + // TODO: Zk lock is expensive and it would be perf bottleneck. Long term solution is + // to remove it. if (!queue.tryZkSharedLock(lockManager, procedure.toString())) { - schedLock.lock(); queue.releaseSharedLock(); queue.getNamespaceQueue().releaseSharedLock(); schedLock.unlock(); return null; } + + schedLock.unlock(); + return queue; } @@ -990,10 +998,10 @@ public class MasterProcedureScheduler implements ProcedureRunnableSet { public void releaseTableSharedLock(final Procedure procedure, final TableName table) { final TableQueue queue = getTableQueueWithLock(table); + schedLock.lock(); // Zk lock is expensive... queue.releaseZkSharedLock(lockManager); - schedLock.lock(); queue.releaseSharedLock(); queue.getNamespaceQueue().releaseSharedLock(); schedLock.unlock(); http://git-wip-us.apache.org/repos/asf/hbase/blob/28d8609e/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java index 9c37404..3de6d36 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.master.procedure; +import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.HashSet; @@ -28,16 +29,19 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HRegionInfo; +import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.master.procedure.MasterProcedureScheduler.ProcedureEvent; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility.TestProcedure; -import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.util.Bytes; - +import org.apache.hadoop.hbase.zookeeper.MiniZooKeeperCluster; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -47,7 +51,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -@Category({MasterTests.class, SmallTests.class}) +@Category({MasterTests.class, MediumTests.class}) public class TestMasterProcedureScheduler { private static final Log LOG = LogFactory.getLog(TestMasterProcedureScheduler.class); @@ -350,6 +354,38 @@ public class TestMasterProcedureScheduler { } @Test + public void testSharedZkLock() throws Exception { + final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + final String dir = TEST_UTIL.getDataTestDir("TestSharedZkLock").toString(); + MiniZooKeeperCluster zkCluster = new MiniZooKeeperCluster(conf); + int zkPort = zkCluster.startup(new File(dir)); + + try { + conf.set("hbase.zookeeper.quorum", "localhost:" + zkPort); + + ZooKeeperWatcher zkw = new ZooKeeperWatcher(conf, "testSchedWithZkLock", null, false); + ServerName mockName = ServerName.valueOf("localhost", 60000, 1); + MasterProcedureScheduler procQueue = new MasterProcedureScheduler( + conf, + TableLockManager.createTableLockManager(conf, zkw, mockName)); + + final TableName tableName = TableName.valueOf("testtb"); + TestTableProcedure procA = + new TestTableProcedure(1, tableName, TableProcedureInterface.TableOperationType.READ); + TestTableProcedure procB = + new TestTableProcedure(2, tableName, TableProcedureInterface.TableOperationType.READ); + + assertTrue(procQueue.tryAcquireTableSharedLock(procA, tableName)); + assertTrue(procQueue.tryAcquireTableSharedLock(procB, tableName)); + + procQueue.releaseTableSharedLock(procA, tableName); + procQueue.releaseTableSharedLock(procB, tableName); + } finally { + zkCluster.shutdown(); + } + } + + @Test public void testVerifyRegionLocks() throws Exception { final TableName tableName = TableName.valueOf("testtb"); final HRegionInfo regionA = new HRegionInfo(tableName, Bytes.toBytes("a"), Bytes.toBytes("b"));
