Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 17A6C200BFB for ; Wed, 28 Dec 2016 01:17:55 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 16369160B3D; Wed, 28 Dec 2016 00:17:55 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 3701E160B31 for ; Wed, 28 Dec 2016 01:17:54 +0100 (CET) Received: (qmail 41414 invoked by uid 500); 28 Dec 2016 00:17:53 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 41405 invoked by uid 99); 28 Dec 2016 00:17:53 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 28 Dec 2016 00:17:53 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 32046DFC15; Wed, 28 Dec 2016 00:17:53 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: stack@apache.org To: commits@hbase.apache.org Message-Id: <8d9c8d56448c4030a9c68544317297c4@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: hbase git commit: HBASE-17068 Procedure v2 - inherit region locks (Matteo Bertozzi) Date: Wed, 28 Dec 2016 00:17:53 +0000 (UTC) archived-at: Wed, 28 Dec 2016 00:17:55 -0000 Repository: hbase Updated Branches: refs/heads/master 319ecd867 -> 306ef83c9 HBASE-17068 Procedure v2 - inherit region locks (Matteo Bertozzi) Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/306ef83c Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/306ef83c Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/306ef83c Branch: refs/heads/master Commit: 306ef83c9cde9730ae2268db3814d59b936de4c1 Parents: 319ecd8 Author: Michael Stack Authored: Tue Dec 27 16:17:45 2016 -0800 Committer: Michael Stack Committed: Tue Dec 27 16:17:45 2016 -0800 ---------------------------------------------------------------------- .../procedure/MasterProcedureScheduler.java | 67 ++++++++++++-------- .../procedure/TestMasterProcedureScheduler.java | 43 +++++++++++++ 2 files changed, 85 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/306ef83c/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 691442c..3f588ff 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 @@ -412,15 +412,27 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { return exclusiveLockProcIdOwner == procId; } - public boolean tryExclusiveLock(final long procIdOwner) { - assert procIdOwner != Long.MIN_VALUE; - if (hasExclusiveLock() && !isLockOwner(procIdOwner)) return false; - exclusiveLockProcIdOwner = procIdOwner; + public boolean hasParentLock(final Procedure proc) { + return proc.hasParent() && + (isLockOwner(proc.getParentProcId()) || isLockOwner(proc.getRootProcId())); + } + + public boolean hasLockAccess(final Procedure proc) { + return isLockOwner(proc.getProcId()) || hasParentLock(proc); + } + + public boolean tryExclusiveLock(final Procedure proc) { + if (hasExclusiveLock()) return hasLockAccess(proc); + exclusiveLockProcIdOwner = proc.getProcId(); return true; } - private void releaseExclusiveLock() { - exclusiveLockProcIdOwner = Long.MIN_VALUE; + public boolean releaseExclusiveLock(final Procedure proc) { + if (isLockOwner(proc.getProcId())) { + exclusiveLockProcIdOwner = Long.MIN_VALUE; + return true; + } + return false; } public HRegionInfo getRegionInfo() { @@ -443,7 +455,7 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { public static class TableQueue extends QueueImpl { private final NamespaceQueue namespaceQueue; - private HashMap regionEventMap; + private HashMap regionEventMap; private TableLock tableLock = null; public TableQueue(TableName tableName, NamespaceQueue namespaceQueue, int priority) { @@ -476,18 +488,18 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { public synchronized RegionEvent getRegionEvent(final HRegionInfo regionInfo) { if (regionEventMap == null) { - regionEventMap = new HashMap(); + regionEventMap = new HashMap(); } - RegionEvent event = regionEventMap.get(regionInfo); + RegionEvent event = regionEventMap.get(regionInfo.getEncodedName()); if (event == null) { event = new RegionEvent(regionInfo); - regionEventMap.put(regionInfo, event); + regionEventMap.put(regionInfo.getEncodedName(), event); } return event; } public synchronized void removeRegionEvent(final RegionEvent event) { - regionEventMap.remove(event.getRegionInfo()); + regionEventMap.remove(event.getRegionInfo().getEncodedName()); if (regionEventMap.isEmpty()) { regionEventMap = null; } @@ -675,7 +687,7 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { hasXLock = queue.tryZkExclusiveLock(lockManager, procedure.toString()); if (!hasXLock) { schedLock(); - if (!hasParentLock) queue.releaseExclusiveLock(); + if (!hasParentLock) queue.releaseExclusiveLock(procedure); queue.getNamespaceQueue().releaseSharedLock(); addToRunQueue(tableRunQueue, queue); schedUnlock(); @@ -699,7 +711,7 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { } schedLock(); - if (!hasParentLock) queue.releaseExclusiveLock(); + if (!hasParentLock) queue.releaseExclusiveLock(procedure); queue.getNamespaceQueue().releaseSharedLock(); addToRunQueue(tableRunQueue, queue); schedUnlock(); @@ -846,11 +858,11 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { assert i == 0 || regionInfo[i] != regionInfo[i-1] : "duplicate region: " + regionInfo[i]; event[i] = queue.getRegionEvent(regionInfo[i]); - if (!event[i].tryExclusiveLock(procedure.getProcId())) { + if (!event[i].tryExclusiveLock(procedure)) { suspendProcedure(event[i], procedure); hasLock = false; while (i-- > 0) { - event[i].releaseExclusiveLock(); + event[i].releaseExclusiveLock(procedure); } break; } @@ -892,12 +904,13 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { assert i == 0 || regionInfo[i] != regionInfo[i-1] : "duplicate region: " + regionInfo[i]; RegionEvent event = queue.getRegionEvent(regionInfo[i]); - event.releaseExclusiveLock(); - if (event.hasWaitingProcedures()) { - // release one procedure at the time since regions has an xlock - nextProcs[numProcs++] = event.popWaitingProcedure(true); - } else { - queue.removeRegionEvent(event); + if (event.releaseExclusiveLock(procedure)) { + if (event.hasWaitingProcedures()) { + // release one procedure at the time since regions has an xlock + nextProcs[numProcs++] = event.popWaitingProcedure(true); + } else { + queue.removeRegionEvent(event); + } } } } @@ -960,7 +973,7 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { final TableQueue tableQueue = getTableQueue(TableName.NAMESPACE_TABLE_NAME); final NamespaceQueue queue = getNamespaceQueue(nsName); - queue.releaseExclusiveLock(); + queue.releaseExclusiveLock(procedure); if (tableQueue.releaseSharedLock()) { addToRunQueue(tableRunQueue, tableQueue); } @@ -1005,7 +1018,7 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { schedLock(); try { ServerQueue queue = getServerQueue(serverName); - queue.releaseExclusiveLock(); + queue.releaseExclusiveLock(procedure); addToRunQueue(serverRunQueue, queue); } finally { schedUnlock(); @@ -1135,8 +1148,12 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { return true; } - public synchronized void releaseExclusiveLock() { - exclusiveLockProcIdOwner = Long.MIN_VALUE; + public synchronized boolean releaseExclusiveLock(final Procedure proc) { + if (isLockOwner(proc.getProcId())) { + exclusiveLockProcIdOwner = Long.MIN_VALUE; + return true; + } + return false; } // This should go away when we have the new AM and its events http://git-wip-us.apache.org/repos/asf/hbase/blob/306ef83c/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 776416f..7397168 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 @@ -543,6 +543,49 @@ public class TestMasterProcedureScheduler { } @Test + public void testInheritedRegionXLock() { + final TableName tableName = TableName.valueOf("testInheritedRegionXLock"); + final HRegionInfo region = new HRegionInfo(tableName, Bytes.toBytes("a"), Bytes.toBytes("b")); + + queue.addBack(new TestRegionProcedure(1, tableName, + TableProcedureInterface.TableOperationType.SPLIT, region)); + queue.addBack(new TestRegionProcedure(1, 2, tableName, + TableProcedureInterface.TableOperationType.UNASSIGN, region)); + queue.addBack(new TestRegionProcedure(3, tableName, + TableProcedureInterface.TableOperationType.REGION_EDIT, region)); + + // fetch the root proc and take the lock on the region + Procedure rootProc = queue.poll(); + assertEquals(1, rootProc.getProcId()); + assertEquals(false, queue.waitRegion(rootProc, region)); + + // fetch the sub-proc and take the lock on the region (inherited lock) + Procedure childProc = queue.poll(); + assertEquals(2, childProc.getProcId()); + assertEquals(false, queue.waitRegion(childProc, region)); + + // proc-3 will be fetched but it can't take the lock + Procedure proc = queue.poll(); + assertEquals(3, proc.getProcId()); + assertEquals(true, queue.waitRegion(proc, region)); + + // release the child lock + queue.wakeRegion(childProc, region); + + // nothing in the queue (proc-3 is suspended) + assertEquals(null, queue.poll(0)); + + // release the root lock + queue.wakeRegion(rootProc, region); + + // proc-3 should be now available + proc = queue.poll(); + assertEquals(3, proc.getProcId()); + assertEquals(false, queue.waitRegion(proc, region)); + queue.wakeRegion(proc, region); + } + + @Test public void testSuspendedProcedure() throws Exception { final TableName tableName = TableName.valueOf("testSuspendedProcedure");