Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-6618

FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.5.0
    • Fix Version/s: 2.5.0
    • Component/s: None
    • Labels:
      None
    • Target Version/s:

      Description

      After HDFS-6527, we have not seen the edit log corruption for weeks on multiple clusters until yesterday. Previously, we would see it within 30 minutes on a cluster.

      But the same condition was reproduced even with HDFS-6527. The only explanation is that the RPC handler thread serving addBlock() was accessing stale parent value. Although nulling out parent is done inside the FSNamesystem and FSDirectory write lock, there is no memory barrier because there is no "synchronized" block involved in the process.

      I suggest making parent volatile.

      1. HDFS-6618.simpler.patch
        3 kB
        Kihwal Lee
      2. HDFS-6618.inodeRemover.v2.patch
        52 kB
        Kihwal Lee
      3. HDFS-6618.inodeRemover.patch
        52 kB
        Kihwal Lee
      4. HDFS-6618.AbstractList.patch
        4 kB
        Kihwal Lee
      5. HDFS-6618.patch
        2 kB
        Kihwal Lee

        Issue Links

          Activity

          Kihwal Lee created issue -
          Hide
          Kihwal Lee added a comment -

          The patch makes FileWithSnapshotFeature.isCurrentFileDeleted and INode.parent volatile.

          Show
          Kihwal Lee added a comment - The patch makes FileWithSnapshotFeature.isCurrentFileDeleted and INode.parent volatile.
          Kihwal Lee made changes -
          Field Original Value New Value
          Attachment HDFS-6618.patch [ 12653420 ]
          Kihwal Lee made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Jing Zhao added a comment -

          If the corruption is caused by the same issue, can we move the following code into the writelock of FSNamesystem in deleteInternal?

              dir.writeLock();
              try {
                dir.removeFromInodeMap(removedINodes);
              } finally {
                dir.writeUnlock();
              }
          
          Show
          Jing Zhao added a comment - If the corruption is caused by the same issue, can we move the following code into the writelock of FSNamesystem in deleteInternal? dir.writeLock(); try { dir.removeFromInodeMap(removedINodes); } finally { dir.writeUnlock(); }
          Hide
          Kihwal Lee added a comment - - edited

          can we move the following code into the writelock of FSNamesystem in deleteInternal?

          That can degrade performance when deleting a large tree. I am also worried about similar kind of race during rename and others.

          Show
          Kihwal Lee added a comment - - edited can we move the following code into the writelock of FSNamesystem in deleteInternal? That can degrade performance when deleting a large tree. I am also worried about similar kind of race during rename and others.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12653420/HDFS-6618.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7263//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7263//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12653420/HDFS-6618.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7263//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7263//console This message is automatically generated.
          Hide
          Kihwal Lee added a comment -

          I guess we can move it inside the first lock, since it is already holding the directory write lock. Not many types of ops will go through anyway. But if we remove them as we unlink inodes, instead of building up potentially huge data structure and do it at once, it may be faster & cheaper.

          Is there a clean way to remove each inode from the inode map from destroyAndCollectBlocks() of INodeFile and INodeDirectory?

          Show
          Kihwal Lee added a comment - I guess we can move it inside the first lock, since it is already holding the directory write lock. Not many types of ops will go through anyway. But if we remove them as we unlink inodes, instead of building up potentially huge data structure and do it at once, it may be faster & cheaper. Is there a clean way to remove each inode from the inode map from destroyAndCollectBlocks() of INodeFile and INodeDirectory ?
          Kihwal Lee made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Kihwal Lee added a comment -

          The new patch implements an idea for minimal change. This is just an idea. We could make more invasive changes to make it "right".

          Show
          Kihwal Lee added a comment - The new patch implements an idea for minimal change. This is just an idea. We could make more invasive changes to make it "right".
          Kihwal Lee made changes -
          Attachment HDFS-6681.AbstractList.patch [ 12653485 ]
          Kihwal Lee made changes -
          Attachment HDFS-6681.AbstractList.patch [ 12653485 ]
          Kihwal Lee made changes -
          Attachment HDFS-6618.AbstractList.patch [ 12653487 ]
          Hide
          Kihwal Lee added a comment -

          Here is a new patch that implements INodeRemover. It replaces replaces removedINodes list and remove() is called instead of add(). There is no longer deferred removal of inodes from inodeMap.

          Show
          Kihwal Lee added a comment - Here is a new patch that implements INodeRemover . It replaces replaces removedINodes list and remove() is called instead of add() . There is no longer deferred removal of inodes from inodeMap .
          Kihwal Lee made changes -
          Attachment HDFS-6618.inodeRemover.patch [ 12653531 ]
          Kihwal Lee made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12653531/HDFS-6618.inodeRemover.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.
          See https://builds.apache.org/job/PreCommit-HDFS-Build/7270//artifact/trunk/patchprocess/diffJavadocWarnings.txt for details.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.balancer.TestBalancer

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7270//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7270//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12653531/HDFS-6618.inodeRemover.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. See https://builds.apache.org/job/PreCommit-HDFS-Build/7270//artifact/trunk/patchprocess/diffJavadocWarnings.txt for details. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.balancer.TestBalancer +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7270//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7270//console This message is automatically generated.
          Hide
          Kihwal Lee added a comment -

          Fixed the javadoc warning in v2.

          Show
          Kihwal Lee added a comment - Fixed the javadoc warning in v2.
          Kihwal Lee made changes -
          Attachment HDFS-6618.inodeRemover.v2.patch [ 12653582 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12653582/HDFS-6618.inodeRemover.v2.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFS

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7272//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7272//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12653582/HDFS-6618.inodeRemover.v2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFS +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7272//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7272//console This message is automatically generated.
          Kihwal Lee made changes -
          Summary Edit log corruption may still happen even after HDFS-6527 Remove deleted INodes from INodeMap right away
          Hide
          Kihwal Lee added a comment -

          Unit test failure is unrelated to the patch.
          Changed the subject to reflect the nature of change. This was actually proposed in HDFS-6527.

          Show
          Kihwal Lee added a comment - Unit test failure is unrelated to the patch. Changed the subject to reflect the nature of change. This was actually proposed in HDFS-6527 .
          Hide
          Colin Patrick McCabe added a comment -

          Ugh, I wrote a long comment and then closed the window.

          One thing that I'm a bit afraid of here is that previously, if we threw an exception while building up the List<INode> removedINodes, we'd avoid modifying the INodeMap. But now, we may leave the INodeMap in an inconsistent state.

          A lot of these functions you're passing the remover to are declared to throw checked exceptions. Like this one:

            public Quota.Counts cleanSubtree(final int snapshotId, int priorSnapshotId,
                final BlocksMapUpdateInfo collectedBlocks,
                final INodeRemover inodeRemover, final boolean countDiffChange)
                throws QuotaExceededException {
          

          What happens if a QuotaExceededException is thrown here after we modified the INodeMap, but before we invalidated the blocks (which are still collected in collectedBlocks)? Seems like we'll be in an inconsistent state.

          Maybe I'm missing something obvious, but can't we just move the dir.removeFromInodeMap line into the first writeLock block? Then we could keep the "add inodes to a list" approach (deferred approach) rather than directly mutating the inodeMap.

              writeLock();
              try {
                checkOperation(OperationCategory.WRITE);
                checkNameNodeSafeMode("Cannot delete " + src);
                src = FSDirectory.resolvePath(src, pathComponents, dir);
                if (!recursive && dir.isNonEmptyDirectory(src)) {
                  throw new PathIsNotEmptyDirectoryException(src + " is non empty");
                }
                if (enforcePermission && isPermissionEnabled) {
                  checkPermission(pc, src, false, null, FsAction.WRITE, null,
                      FsAction.ALL, true, false);
                }
                long mtime = now();
                // Unlink the target directory from directory tree
                long filesRemoved = dir.delete(src, collectedBlocks, removedINodes,
                        mtime);
                if (filesRemoved < 0) {
                  return false;
                }
                getEditLog().logDelete(src, mtime, logRetryCache);
                incrDeletedFileCount(filesRemoved);
                // Blocks/INodes will be handled later
                removePathAndBlocks(src, null, null);
                ret = true;
              } finally {
                writeUnlock();
              }
              getEditLog().logSync(); 
              removeBlocks(collectedBlocks); // Incremental deletion of blocks
              collectedBlocks.clear();
          
              dir.writeLock();
              try {
                dir.removeFromInodeMap(removedINodes);  <=== move this up?
              } finally {
                dir.writeUnlock();
              }
              removedINodes.clear();
          

          I apologize if I'm missing an obvious locking or other issue that prevents this.

          Show
          Colin Patrick McCabe added a comment - Ugh, I wrote a long comment and then closed the window. One thing that I'm a bit afraid of here is that previously, if we threw an exception while building up the List<INode> removedINodes , we'd avoid modifying the INodeMap. But now, we may leave the INodeMap in an inconsistent state. A lot of these functions you're passing the remover to are declared to throw checked exceptions. Like this one: public Quota.Counts cleanSubtree( final int snapshotId, int priorSnapshotId, final BlocksMapUpdateInfo collectedBlocks, final INodeRemover inodeRemover, final boolean countDiffChange) throws QuotaExceededException { What happens if a QuotaExceededException is thrown here after we modified the INodeMap, but before we invalidated the blocks (which are still collected in collectedBlocks)? Seems like we'll be in an inconsistent state. Maybe I'm missing something obvious, but can't we just move the dir.removeFromInodeMap line into the first writeLock block? Then we could keep the "add inodes to a list" approach (deferred approach) rather than directly mutating the inodeMap. writeLock(); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode( "Cannot delete " + src); src = FSDirectory.resolvePath(src, pathComponents, dir); if (!recursive && dir.isNonEmptyDirectory(src)) { throw new PathIsNotEmptyDirectoryException(src + " is non empty" ); } if (enforcePermission && isPermissionEnabled) { checkPermission(pc, src, false , null , FsAction.WRITE, null , FsAction.ALL, true , false ); } long mtime = now(); // Unlink the target directory from directory tree long filesRemoved = dir.delete(src, collectedBlocks, removedINodes, mtime); if (filesRemoved < 0) { return false ; } getEditLog().logDelete(src, mtime, logRetryCache); incrDeletedFileCount(filesRemoved); // Blocks/INodes will be handled later removePathAndBlocks(src, null , null ); ret = true ; } finally { writeUnlock(); } getEditLog().logSync(); removeBlocks(collectedBlocks); // Incremental deletion of blocks collectedBlocks.clear(); dir.writeLock(); try { dir.removeFromInodeMap(removedINodes); <=== move this up? } finally { dir.writeUnlock(); } removedINodes.clear(); I apologize if I'm missing an obvious locking or other issue that prevents this.
          Kihwal Lee made changes -
          Assignee Kihwal Lee [ kihwal ]
          Hide
          Kihwal Lee added a comment - - edited

          Colin Patrick McCabe, thanks for the review.

          What happens if a QuotaExceededException is thrown here .....

          This is indeed problematic, but is also the case for existing code and what you are suggesting. If an exception is thrown in the middle of deleting, the partial delete is not undone. The inode at the top of the tree being deleted and potentially more will have already unlinked and the rest will remain linked, but unreachable. If inodes are removed altogether at the end, none of inodes will get removed from inodeMap, when an exception is thrown. This will cause inodes and blocks to leak. If we remove inodes as we go, at leaset some inodes will get removed in the same situation. Either way things will leak, but to a lesser degree in the latter case. But I wouldn't say the latter is superior because of this difference. I am just saying it's no worse.

          One of the key motivation of removing inodes inline was to avoid overhead of building up large data structure when deleting a large tree. Although now it's backed by ChunkedArrayList, there will be lots of realloc and quite a bit of memory consumption. All or part of them may be promoted and remain in the heap until the next old gen collection. This may be acceptable if we are doing deferred removal outside the lock. But since we are trying to do it inside both FSNamesystem and FSDirectory lock, building the list is just a waste.

          About leaking inodes and blocks:

          • inodes were removed from inodeMap but blocks weren't. This includes adding a block after the inode is deleted due to the delete-addBlock race. Since the block is not removed from blocksMap, but the block still has reference to the block collection (i.e. inode), the block will look valid to BlockManager. This will cause memory leak, which will disappear when namenode is restarted.
          • unlinked/deleted inodes were not deleted from inodeMap. The deleted inodes will remain in memory. If blocks were also not removed from blocksMap, they will remain in memory. If blocks were collected, but not removed from blocksMap, they will disappear after restart. When saving fsimage, the orphaned inodes will be saved in the inode section. The way it saves INodeDirectorySection also causes all leaked (still linked) children and blocks to be saved. When loading the fsimage, the leak will be recreated in memory.

          I am a bit depressed after writing this. Let's fix things one at time...

          Show
          Kihwal Lee added a comment - - edited Colin Patrick McCabe , thanks for the review. What happens if a QuotaExceededException is thrown here ..... This is indeed problematic, but is also the case for existing code and what you are suggesting. If an exception is thrown in the middle of deleting, the partial delete is not undone. The inode at the top of the tree being deleted and potentially more will have already unlinked and the rest will remain linked, but unreachable. If inodes are removed altogether at the end, none of inodes will get removed from inodeMap, when an exception is thrown. This will cause inodes and blocks to leak. If we remove inodes as we go, at leaset some inodes will get removed in the same situation. Either way things will leak, but to a lesser degree in the latter case. But I wouldn't say the latter is superior because of this difference. I am just saying it's no worse. One of the key motivation of removing inodes inline was to avoid overhead of building up large data structure when deleting a large tree. Although now it's backed by ChunkedArrayList , there will be lots of realloc and quite a bit of memory consumption. All or part of them may be promoted and remain in the heap until the next old gen collection. This may be acceptable if we are doing deferred removal outside the lock. But since we are trying to do it inside both FSNamesystem and FSDirectory lock, building the list is just a waste. About leaking inodes and blocks: inodes were removed from inodeMap but blocks weren't. This includes adding a block after the inode is deleted due to the delete-addBlock race. Since the block is not removed from blocksMap, but the block still has reference to the block collection (i.e. inode), the block will look valid to BlockManager . This will cause memory leak, which will disappear when namenode is restarted. unlinked/deleted inodes were not deleted from inodeMap. The deleted inodes will remain in memory. If blocks were also not removed from blocksMap, they will remain in memory. If blocks were collected, but not removed from blocksMap, they will disappear after restart. When saving fsimage, the orphaned inodes will be saved in the inode section. The way it saves INodeDirectorySection also causes all leaked (still linked) children and blocks to be saved. When loading the fsimage, the leak will be recreated in memory. I am a bit depressed after writing this. Let's fix things one at time...
          Hide
          Colin Patrick McCabe added a comment -

          Thanks for that explanation. If we're just trying to make a minimal change to put out the fire without fixing the (existing) leak issue, why not just move the call to dir.removeFromInodeMap(removedINodes) up inside the first try... catch block? Then we can file a JIRA for some refactoring which fixes the leak issue (we'll need to discuss how)

          Show
          Colin Patrick McCabe added a comment - Thanks for that explanation. If we're just trying to make a minimal change to put out the fire without fixing the (existing) leak issue, why not just move the call to dir.removeFromInodeMap(removedINodes) up inside the first try... catch block? Then we can file a JIRA for some refactoring which fixes the leak issue (we'll need to discuss how)
          Hide
          Jing Zhao added a comment -

          If we're just trying to make a minimal change to put out the fire without fixing the (existing) leak issue, why not just move the call to dir.removeFromInodeMap(removedINodes) up inside the first try... catch block? Then we can file a JIRA for some refactoring which fixes the leak issue

          I agree with Colin. Maybe we can first fix the bug here by moving the removeFromInodeMap call into the FSNamesystem lock, and fix the remaining issues in a separate jira(s).

          Since the block is not removed from blocksMap, but the block still has reference to the block collection (i.e. inode), the block will look valid to BlockManager. This will cause memory leak, which will disappear when namenode is restarted.

          The block deletion has to be after the logSync call here thus I guess it's hard to avoid this kind of leak (or we have to change the blocksMap structure etc.). Since the memory leak can go away after restarting, maybe we do not need to worry about this part too much right now and focus on the leak on the inodeMap.

          Show
          Jing Zhao added a comment - If we're just trying to make a minimal change to put out the fire without fixing the (existing) leak issue, why not just move the call to dir.removeFromInodeMap(removedINodes) up inside the first try... catch block? Then we can file a JIRA for some refactoring which fixes the leak issue I agree with Colin. Maybe we can first fix the bug here by moving the removeFromInodeMap call into the FSNamesystem lock, and fix the remaining issues in a separate jira(s). Since the block is not removed from blocksMap, but the block still has reference to the block collection (i.e. inode), the block will look valid to BlockManager. This will cause memory leak, which will disappear when namenode is restarted. The block deletion has to be after the logSync call here thus I guess it's hard to avoid this kind of leak (or we have to change the blocksMap structure etc.). Since the memory leak can go away after restarting, maybe we do not need to worry about this part too much right now and focus on the leak on the inodeMap.
          Hide
          Kihwal Lee added a comment -

          Ok, I will take the simple approach.

          Show
          Kihwal Lee added a comment - Ok, I will take the simple approach.
          Kihwal Lee made changes -
          Attachment HDFS-6618.simpler.patch [ 12654834 ]
          Hide
          Kihwal Lee added a comment -

          With the new patch removedINodes is passed to FSNamesystem#removePathAndBlocks() while in the write lock. The method was modified to conditionally acquire the directory lock. I didn't move the removal to FSDirectory, since we may want to do something with the inodes in FSNamesystem later as a part of failure handling in a separate jira.

          Show
          Kihwal Lee added a comment - With the new patch removedINodes is passed to FSNamesystem#removePathAndBlocks() while in the write lock. The method was modified to conditionally acquire the directory lock. I didn't move the removal to FSDirectory , since we may want to do something with the inodes in FSNamesystem later as a part of failure handling in a separate jira.
          Hide
          Jing Zhao added a comment -

          Thanks Kihwal Lee. The patch looks good to me. +1 pending Jenkins.

          Show
          Jing Zhao added a comment - Thanks Kihwal Lee . The patch looks good to me. +1 pending Jenkins.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12654834/HDFS-6618.simpler.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7306//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12654834/HDFS-6618.simpler.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7306//console This message is automatically generated.
          Hide
          Kihwal Lee added a comment -

          Filed HDFS-6651 for the leak problem.

          Show
          Kihwal Lee added a comment - Filed HDFS-6651 for the leak problem.
          Hide
          Kihwal Lee added a comment -

          The build failed because of this.

          [exec] CMake Error at /usr/share/cmake-2.8/Modules/FindPackageHandleStandardArgs.cmake:108 (message):
          [exec] Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE)
          [exec] Call Stack (most recent call first):
          [exec] /usr/share/cmake-2.8/Modules/FindPackageHandleStandardArgs.cmake:315 (_FPHSA_FAILURE_MESSAGE)
          [exec] /usr/share/cmake-2.8/Modules/FindPkgConfig.cmake:106 (find_package_handle_standard_args)
          [exec] main/native/fuse-dfs/CMakeLists.txt:23 (find_package)

          Show
          Kihwal Lee added a comment - The build failed because of this. [exec] CMake Error at /usr/share/cmake-2.8/Modules/FindPackageHandleStandardArgs.cmake:108 (message): [exec] Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE) [exec] Call Stack (most recent call first): [exec] /usr/share/cmake-2.8/Modules/FindPackageHandleStandardArgs.cmake:315 (_FPHSA_FAILURE_MESSAGE) [exec] /usr/share/cmake-2.8/Modules/FindPkgConfig.cmake:106 (find_package_handle_standard_args) [exec] main/native/fuse-dfs/CMakeLists.txt:23 (find_package)
          Hide
          Colin Patrick McCabe added a comment -

          It looks like someone is updating the build slaves, and somehow pkg-config got uninstalled? I will kick the build again.

          Show
          Colin Patrick McCabe added a comment - It looks like someone is updating the build slaves, and somehow pkg-config got uninstalled? I will kick the build again.
          Hide
          Colin Patrick McCabe added a comment -

          +1 for the patch.

          Just one small note... I'd prefer to see lock... unlock blocks around removePathAndBlocks when appropriate, rather than a boolean "lock me" passed in, but we can address that in the refactoring, I guess

          Show
          Colin Patrick McCabe added a comment - +1 for the patch. Just one small note... I'd prefer to see lock... unlock blocks around removePathAndBlocks when appropriate, rather than a boolean "lock me" passed in, but we can address that in the refactoring, I guess
          Kihwal Lee made changes -
          Link This issue is depended upon by HDFS-6647 [ HDFS-6647 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12654834/HDFS-6618.simpler.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7309//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7309//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12654834/HDFS-6618.simpler.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7309//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7309//console This message is automatically generated.
          Colin Patrick McCabe made changes -
          Summary Remove deleted INodes from INodeMap right away FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #5852 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5852/)
          HDFS-6618. FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #5852 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5852/ ) HDFS-6618 . FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Colin Patrick McCabe made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 2.5.0 [ 12326264 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #609 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/609/)
          HDFS-6618. FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #609 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/609/ ) HDFS-6618 . FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #1800 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1800/)
          HDFS-6618. FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #1800 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1800/ ) HDFS-6618 . FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1827 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1827/)
          HDFS-6618. FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1827 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1827/ ) HDFS-6618 . FSNamesystem#delete drops the FSN lock between removing INodes from the tree and deleting them from the inode map (kihwal via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1609380 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          Karthik Kambatla (Inactive) made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Kihwal Lee
              Reporter:
              Kihwal Lee
            • Votes:
              0 Vote for this issue
              Watchers:
              18 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development