Browse Source

HDFS-6618. FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2.5@1609382 13f79535-47bb-0310-9956-ffa450edef68
Colin McCabe 11 years ago
parent
commit
97d22609c0

+ 3 - 0
hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -587,6 +587,9 @@ Release 2.5.0 - UNRELEASED
     HDFS-6312. WebHdfs HA failover is broken on secure clusters. 
     HDFS-6312. WebHdfs HA failover is broken on secure clusters. 
     (daryn via tucu)
     (daryn via tucu)
 
 
+    HDFS-6618. FSNamesystem#delete drops the FSN lock between removing INodes
+    from the tree and deleting them from the inode map (kihwal via cmccabe)
+
 Release 2.4.1 - 2014-06-23 
 Release 2.4.1 - 2014-06-23 
 
 
   INCOMPATIBLE CHANGES
   INCOMPATIBLE CHANGES

+ 2 - 2
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java

@@ -656,7 +656,7 @@ public class FSDirectory implements Closeable {
                 dstIIP.getLatestSnapshotId(), collectedBlocks, removedINodes,
                 dstIIP.getLatestSnapshotId(), collectedBlocks, removedINodes,
                 true).get(Quota.NAMESPACE);
                 true).get(Quota.NAMESPACE);
             getFSNamesystem().removePathAndBlocks(src, collectedBlocks,
             getFSNamesystem().removePathAndBlocks(src, collectedBlocks,
-                removedINodes);
+                removedINodes, false);
           }
           }
         }
         }
 
 
@@ -1194,7 +1194,7 @@ public class FSDirectory implements Closeable {
 
 
     if (filesRemoved >= 0) {
     if (filesRemoved >= 0) {
       getFSNamesystem().removePathAndBlocks(src, collectedBlocks, 
       getFSNamesystem().removePathAndBlocks(src, collectedBlocks, 
-          removedINodes);
+          removedINodes, false);
     }
     }
   }
   }
   
   

+ 13 - 10
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -3525,7 +3525,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
       getEditLog().logDelete(src, mtime, logRetryCache);
       getEditLog().logDelete(src, mtime, logRetryCache);
       incrDeletedFileCount(filesRemoved);
       incrDeletedFileCount(filesRemoved);
       // Blocks/INodes will be handled later
       // Blocks/INodes will be handled later
-      removePathAndBlocks(src, null, null);
+      removePathAndBlocks(src, null, removedINodes, true);
       ret = true;
       ret = true;
     } finally {
     } finally {
       writeUnlock();
       writeUnlock();
@@ -3534,13 +3534,6 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     removeBlocks(collectedBlocks); // Incremental deletion of blocks
     removeBlocks(collectedBlocks); // Incremental deletion of blocks
     collectedBlocks.clear();
     collectedBlocks.clear();
 
 
-    dir.writeLock();
-    try {
-      dir.removeFromInodeMap(removedINodes);
-    } finally {
-      dir.writeUnlock();
-    }
-    removedINodes.clear();
     if (NameNode.stateChangeLog.isDebugEnabled()) {
     if (NameNode.stateChangeLog.isDebugEnabled()) {
       NameNode.stateChangeLog.debug("DIR* Namesystem.delete: "
       NameNode.stateChangeLog.debug("DIR* Namesystem.delete: "
         + src +" is removed");
         + src +" is removed");
@@ -3578,14 +3571,24 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
    * @param blocks Containing the list of blocks to be deleted from blocksMap
    * @param blocks Containing the list of blocks to be deleted from blocksMap
    * @param removedINodes Containing the list of inodes to be removed from 
    * @param removedINodes Containing the list of inodes to be removed from 
    *                      inodesMap
    *                      inodesMap
+   * @param acquireINodeMapLock Whether to acquire the lock for inode removal
    */
    */
   void removePathAndBlocks(String src, BlocksMapUpdateInfo blocks,
   void removePathAndBlocks(String src, BlocksMapUpdateInfo blocks,
-      List<INode> removedINodes) {
+      List<INode> removedINodes, final boolean acquireINodeMapLock) {
     assert hasWriteLock();
     assert hasWriteLock();
     leaseManager.removeLeaseWithPrefixPath(src);
     leaseManager.removeLeaseWithPrefixPath(src);
     // remove inodes from inodesMap
     // remove inodes from inodesMap
     if (removedINodes != null) {
     if (removedINodes != null) {
-      dir.removeFromInodeMap(removedINodes);
+      if (acquireINodeMapLock) {
+        dir.writeLock();
+      }
+      try {
+        dir.removeFromInodeMap(removedINodes);
+      } finally {
+        if (acquireINodeMapLock) {
+          dir.writeUnlock();
+        }
+      }
       removedINodes.clear();
       removedINodes.clear();
     }
     }
     if (blocks == null) {
     if (blocks == null) {