HDFS-15076. Fix tests that hold FSDirectory lock, without holding FSNamesystem lock. Contributed by Konstantin V Shvachko.

(cherry picked from commit b98ac2a3af)

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDiskspaceQuotaUpdate.java

@@ -380,14 +380,24 @@ public class TestDiskspaceQuotaUpdate {
     HashMap<String, Long> dsMap = new HashMap<String, Long>();
     scanDirsWithQuota(root, nsMap, dsMap, false);
 
+    updateCountForQuota(1);
+    scanDirsWithQuota(root, nsMap, dsMap, true);
+
+    updateCountForQuota(2);
+    scanDirsWithQuota(root, nsMap, dsMap, true);
+
+    updateCountForQuota(4);
+    scanDirsWithQuota(root, nsMap, dsMap, true);
+  }
+
+  private void updateCountForQuota(int i) {
+    FSNamesystem fsn = cluster.getNamesystem();
+    fsn.writeLock();
+    try {
       getFSDirectory().updateCountForQuota(1);
-    scanDirsWithQuota(root, nsMap, dsMap, true);
+    } finally {
-
+      fsn.writeUnlock();
-    getFSDirectory().updateCountForQuota(2);
+    }
-    scanDirsWithQuota(root, nsMap, dsMap, true);
-
-    getFSDirectory().updateCountForQuota(4);
-    scanDirsWithQuota(root, nsMap, dsMap, true);
   }
 
   private void scanDirsWithQuota(INodeDirectory dir,

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystem.java

@@ -89,7 +89,7 @@ public class TestFSNamesystem {
     LeaseManager leaseMan = fsn.getLeaseManager();
     leaseMan.addLease("client1", fsn.getFSDirectory().allocateNewInodeId());
     assertEquals(1, leaseMan.countLease());
-    fsn.clear();
+    clearNamesystem(fsn);
     leaseMan = fsn.getLeaseManager();
     assertEquals(0, leaseMan.countLease());
   }
@@ -184,8 +184,7 @@ public class TestFSNamesystem {
     FSNamesystem fsn = new FSNamesystem(conf, fsImage);
     fsn.imageLoadComplete();
     assertTrue(fsn.isImageLoaded());
-    fsn.clear();
+    clearNamesystem(fsn);
-    assertFalse(fsn.isImageLoaded());
     final INodeDirectory root = (INodeDirectory) fsn.getFSDirectory()
             .getINode("/");
     assertTrue(root.getChildrenList(Snapshot.CURRENT_STATE_ID).isEmpty());
@@ -193,6 +192,16 @@ public class TestFSNamesystem {
     assertTrue(fsn.isImageLoaded());
   }
 
+  private void clearNamesystem(FSNamesystem fsn) {
+    fsn.writeLock();
+    try {
+      fsn.clear();
+      assertFalse(fsn.isImageLoaded());
+    } finally {
+      fsn.writeUnlock();
+    }
+  }
+
   @Test
   public void testGetEffectiveLayoutVersion() {
     assertEquals(-63,

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGetBlockLocations.java

@@ -22,6 +22,7 @@ import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.fs.permission.PermissionStatus;
 import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo;
 import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp;
+import org.apache.hadoop.hdfs.server.namenode.NameNode.OperationCategory;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
@@ -61,22 +62,31 @@ public class TestGetBlockLocations {
 
   @Test(timeout = 30000)
   public void testGetBlockLocationsRacingWithDelete() throws IOException {
-    FSNamesystem fsn = spy(setupFileSystem());
+    final FSNamesystem fsn = spy(setupFileSystem());
     final FSDirectory fsd = fsn.getFSDirectory();
     FSEditLog editlog = fsn.getEditLog();
+    final boolean[] deleted = new boolean[]{false};
 
     doAnswer(new Answer<Void>() {
 
       @Override
       public Void answer(InvocationOnMock invocation) throws Throwable {
+        if(!deleted[0]) {
+          fsn.writeLock();
+          try {
             INodesInPath iip = fsd.getINodesInPath(FILE_PATH, DirOp.READ);
             FSDirDeleteOp.delete(fsd, iip, new INode.BlocksMapUpdateInfo(),
                                  new ArrayList<INode>(), new ArrayList<Long>(),
                                  now());
+          } finally {
+            fsn.writeUnlock();
+          }
+          deleted[0] = true;
+        }
         invocation.callRealMethod();
         return null;
       }
-    }).when(fsn).writeLock();
+    }).when(fsn).checkOperation(OperationCategory.WRITE);
     fsn.getBlockLocations("dummy", RESERVED_PATH, 0, 1024);
 
     verify(editlog, never()).logTimes(anyString(), anyLong(), anyLong());
@@ -85,26 +95,31 @@ public class TestGetBlockLocations {
 
   @Test(timeout = 30000)
   public void testGetBlockLocationsRacingWithRename() throws IOException {
-    FSNamesystem fsn = spy(setupFileSystem());
+    final FSNamesystem fsn = spy(setupFileSystem());
     final FSDirectory fsd = fsn.getFSDirectory();
     FSEditLog editlog = fsn.getEditLog();
     final String DST_PATH = "/bar";
-    final boolean[] renamed = new boolean[1];
+    final boolean[] renamed = new boolean[] {false};
 
     doAnswer(new Answer<Void>() {
 
       @Override
       public Void answer(InvocationOnMock invocation) throws Throwable {
-        invocation.callRealMethod();
         if (!renamed[0]) {
+          fsn.writeLock();
+          try {
             FSDirRenameOp.renameTo(fsd, fsd.getPermissionChecker(), FILE_PATH,
                                    DST_PATH, new INode.BlocksMapUpdateInfo(),
                                    false);
             renamed[0] = true;
+          } finally {
+            fsn.writeUnlock();
           }
+        }
+        invocation.callRealMethod();
         return null;
       }
-    }).when(fsn).writeLock();
+    }).when(fsn).checkOperation(OperationCategory.WRITE);
     fsn.getBlockLocations("dummy", RESERVED_PATH, 0, 1024);
 
     verify(editlog).logTimes(eq(DST_PATH), anyLong(), anyLong());
@@ -119,8 +134,6 @@ public class TestGetBlockLocations {
     when(image.getEditLog()).thenReturn(editlog);
     final FSNamesystem fsn = new FSNamesystem(conf, image, true);
 
-    final FSDirectory fsd = fsn.getFSDirectory();
-    INodesInPath iip = fsd.getINodesInPath("/", DirOp.READ);
     PermissionStatus perm = new PermissionStatus(
         "hdfs", "supergroup",
         FsPermission.createImmutable((short) 0x1ff));
@@ -128,7 +141,15 @@ public class TestGetBlockLocations {
         MOCK_INODE_ID, FILE_NAME.getBytes(StandardCharsets.UTF_8),
         perm, 1, 1, new BlockInfo[] {}, (short) 1,
         DFS_BLOCK_SIZE_DEFAULT);
-    fsn.getFSDirectory().addINode(iip, file);
+
+    fsn.writeLock();
+    try {
+      final FSDirectory fsd = fsn.getFSDirectory();
+      INodesInPath iip = fsd.getINodesInPath("/", DirOp.READ);
+      fsd.addINode(iip, file);
+    } finally {
+      fsn.writeUnlock();
+    }
     return fsn;
   }