HBASE-16865 Procedure v2 - Inherit lock from root proc

hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/ProcedureTestingUtility.java

@@ -349,11 +349,18 @@ public class ProcedureTestingUtility {
     }
 
     public TestProcedure(long procId, long parentId, byte[] data) {
+      this(procId, parentId, parentId, data);
+    }
+
+    public TestProcedure(long procId, long parentId, long rootId, byte[] data) {
       setData(data);
       setProcId(procId);
       if (parentId > 0) {
         setParentProcId(parentId);
       }
+      if (rootId > 0 || parentId > 0) {
+        setRootProcId(rootId);
+      }
     }
 
     public void addStackId(final int index) {

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java

@@ -1121,7 +1121,8 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler {
     }
 
     public synchronized boolean hasParentLock(final Procedure proc) {
-      return proc.hasParent() && isLockOwner(proc.getParentProcId());
+      return proc.hasParent() &&
+        (isLockOwner(proc.getParentProcId()) || isLockOwner(proc.getRootProcId()));
     }
 
     public synchronized boolean hasLockAccess(final Procedure proc) {
@@ -1159,7 +1160,9 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler {
     @Override
     public String toString() {
       return String.format("%s(%s, suspended=%s xlock=%s sharedLock=%s size=%s)",
-        getClass().getSimpleName(), key, isSuspended(), hasExclusiveLock(), sharedLock, size());
+          getClass().getSimpleName(), key, isSuspended(),
+          hasExclusiveLock() ? "true (" + exclusiveLockProcIdOwner + ")" : "false",
+          sharedLock, size());
     }
   }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java

@@ -570,27 +570,57 @@ public class TestMasterProcedureScheduler {
     assertEquals(null, queue.poll(0));
   }
 
-  @Test
+  private static HRegionInfo[] generateRegionInfo(final TableName tableName) {
-  public void testParentXLockAndChildrenSharedLock() throws Exception {
+    return new HRegionInfo[] {
-    final TableName tableName = TableName.valueOf("testParentXLockAndChildrenSharedLock");
-    final HRegionInfo[] regions = new HRegionInfo[] {
       new HRegionInfo(tableName, Bytes.toBytes("a"), Bytes.toBytes("b")),
       new HRegionInfo(tableName, Bytes.toBytes("b"), Bytes.toBytes("c")),
       new HRegionInfo(tableName, Bytes.toBytes("c"), Bytes.toBytes("d")),
     };
+  }
 
-    queue.addBack(new TestTableProcedure(1, tableName,
+  @Test
-        TableProcedureInterface.TableOperationType.CREATE));
+  public void testParentXLockAndChildrenSharedLock() throws Exception {
+    final TableName tableName = TableName.valueOf("testParentXLockAndChildrenSharedLock");
+    final HRegionInfo[] regions = generateRegionInfo(tableName);
+    final TestRegionProcedure[] childProcs = new TestRegionProcedure[regions.length];
+    for (int i = 0; i < regions.length; ++i) {
+      childProcs[i] = new TestRegionProcedure(1, 2 + i, tableName,
+          TableProcedureInterface.TableOperationType.ASSIGN, regions[i]);
+    }
+    testInheritedXLockAndChildrenSharedLock(tableName,
+      new TestTableProcedure(1, tableName, TableProcedureInterface.TableOperationType.CREATE),
+      childProcs
+    );
+  }
+
+  @Test
+  public void testRootXLockAndChildrenSharedLock() throws Exception {
+    final TableName tableName = TableName.valueOf("testRootXLockAndChildrenSharedLock");
+    final HRegionInfo[] regions = generateRegionInfo(tableName);
+    final TestRegionProcedure[] childProcs = new TestRegionProcedure[regions.length];
+    for (int i = 0; i < regions.length; ++i) {
+      childProcs[i] = new TestRegionProcedure(1, 2, 3 + i, tableName,
+          TableProcedureInterface.TableOperationType.ASSIGN, regions[i]);
+    }
+    testInheritedXLockAndChildrenSharedLock(tableName,
+      new TestTableProcedure(1, tableName, TableProcedureInterface.TableOperationType.CREATE),
+      childProcs
+    );
+  }
+
+  private void testInheritedXLockAndChildrenSharedLock(final TableName tableName,
+      final TestTableProcedure rootProc, final TestRegionProcedure[] childProcs)
+      throws Exception {
+    queue.addBack(rootProc);
 
     // fetch and acquire first xlock proc
     Procedure parentProc = queue.poll();
-    assertEquals(1, parentProc.getProcId());
+    assertEquals(rootProc, parentProc);
     assertTrue(queue.tryAcquireTableExclusiveLock(parentProc, tableName));
 
     // add child procedure
-    for (int i = 0; i < regions.length; ++i) {
+    for (int i = 0; i < childProcs.length; ++i) {
-      queue.addFront(new TestRegionProcedure(1, 1 + i, tableName,
+      queue.addFront(childProcs[i]);
-          TableProcedureInterface.TableOperationType.ASSIGN, regions[i]));
     }
 
     // add another xlock procedure (no parent)
@@ -598,13 +628,11 @@ public class TestMasterProcedureScheduler {
         TableProcedureInterface.TableOperationType.EDIT));
 
     // fetch and execute child
-    for (int i = 0; i < regions.length; ++i) {
+    for (int i = 0; i < childProcs.length; ++i) {
-      final int regionIdx = regions.length - i - 1;
+      TestRegionProcedure childProc = (TestRegionProcedure)queue.poll();
-      Procedure childProc = queue.poll();
       LOG.debug("fetch children " + childProc);
-      assertEquals(1 + regionIdx, childProc.getProcId());
+      assertEquals(false, queue.waitRegions(childProc, tableName, childProc.getRegionInfo()));
-      assertEquals(false, queue.waitRegion(childProc, regions[regionIdx]));
+      queue.wakeRegions(childProc, tableName, childProc.getRegionInfo());
-      queue.wakeRegion(childProc, regions[regionIdx]);
     }
 
     // nothing available, until xlock release
@@ -623,22 +651,37 @@ public class TestMasterProcedureScheduler {
   @Test
   public void testParentXLockAndChildrenXLock() throws Exception {
     final TableName tableName = TableName.valueOf("testParentXLockAndChildrenXLock");
+    testInheritedXLockAndChildrenXLock(tableName,
+      new TestTableProcedure(1, tableName, TableProcedureInterface.TableOperationType.EDIT),
+      new TestTableProcedure(1, 2, tableName, TableProcedureInterface.TableOperationType.EDIT)
+    );
+  }
 
-    queue.addBack(new TestTableProcedure(1, tableName,
+  @Test
-        TableProcedureInterface.TableOperationType.EDIT));
+  public void testRootXLockAndChildrenXLock() throws Exception {
+    final TableName tableName = TableName.valueOf("testRootXLockAndChildrenXLock");
+    // simulate 3 procedures: 1 (root), (2) child of root, (3) child of proc-2
+    testInheritedXLockAndChildrenXLock(tableName,
+      new TestTableProcedure(1, tableName, TableProcedureInterface.TableOperationType.EDIT),
+      new TestTableProcedure(1, 2, 3, tableName, TableProcedureInterface.TableOperationType.EDIT)
+    );
+  }
+
+  private void testInheritedXLockAndChildrenXLock(final TableName tableName,
+      final TestTableProcedure rootProc, final TestTableProcedure childProc) throws Exception {
+    queue.addBack(rootProc);
 
     // fetch and acquire first xlock proc
     Procedure parentProc = queue.poll();
-    assertEquals(1, parentProc.getProcId());
+    assertEquals(rootProc, parentProc);
     assertTrue(queue.tryAcquireTableExclusiveLock(parentProc, tableName));
 
     // add child procedure
-    queue.addFront(new TestTableProcedure(1, 2, tableName,
+    queue.addFront(childProc);
-      TableProcedureInterface.TableOperationType.EDIT));
 
     // fetch the other xlock proc
     Procedure proc = queue.poll();
-    assertEquals(2, proc.getProcId());
+    assertEquals(childProc, proc);
     assertTrue(queue.tryAcquireTableExclusiveLock(proc, tableName));
     queue.releaseTableExclusiveLock(proc, tableName);
 
@@ -734,7 +777,12 @@ public class TestMasterProcedureScheduler {
 
     public TestTableProcedure(long parentProcId, long procId, TableName tableName,
         TableOperationType opType) {
-      super(procId, parentProcId);
+      this(-1, parentProcId, procId, tableName, opType);
+    }
+
+    public TestTableProcedure(long rootProcId, long parentProcId, long procId, TableName tableName,
+        TableOperationType opType) {
+      super(procId, parentProcId, rootProcId, null);
       this.tableName = tableName;
       this.opType = opType;
     }
@@ -784,7 +832,12 @@ public class TestMasterProcedureScheduler {
 
     public TestRegionProcedure(long parentProcId, long procId, TableName tableName,
         TableOperationType opType, HRegionInfo... regionInfo) {
-      super(parentProcId, procId, tableName, opType);
+      this(-1, parentProcId, procId, tableName, opType, regionInfo);
+    }
+
+    public TestRegionProcedure(long rootProcId, long parentProcId, long procId, TableName tableName,
+        TableOperationType opType, HRegionInfo... regionInfo) {
+      super(rootProcId, parentProcId, procId, tableName, opType);
       this.regionInfo = regionInfo;
     }