Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3848

A Bug in recoverLeaseInternal method of FSNameSystem class

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.1
    • Fix Version/s: 2.5.0
    • Component/s: namenode
    • Labels:
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      This is a bug in logic of the method recoverLeaseInternal. In line 1322 it checks if the owner of the file is trying to recreate the file. The condition of the if statement is

      (leaseFile != null && leaseFile.equals(lease)) || lease.getHolder().equals(holder)

      As it can be seen, there are two operands (conditions) connected with an "or" operator. The first operand is straight and will be true only if the holder of the file is the new holder. But the problem is the second operand which will be always true since the "lease" object is the one found by the "holder" by calling "Lease lease = leaseManager.getLease(holder);" in line 1315.

      To fix this I think the if statement only should contain the following the condition:
      (leaseFile != null && leaseFile.getHolder().equals(holder))

      1. HDFS-3848-1.patch
        1.0 kB
        Hooman Peiro Sajjad
      2. HDFS-3848-v2.patch
        3 kB
        Chen He

        Activity

        Hooman Peiro Sajjad created issue -
        Hide
        Kihwal Lee added a comment -

        Good find!

        I think we can just get rid of lease.getHolder().equals(holder). The first condition, (leaseFile != null && leaseFile.equals(lease)), is sufficient, because Lease#equals() checks whether the holders are equal.

        Show
        Kihwal Lee added a comment - Good find! I think we can just get rid of lease.getHolder().equals(holder) . The first condition, (leaseFile != null && leaseFile.equals(lease)) , is sufficient, because Lease#equals() checks whether the holders are equal.
        Hide
        Hooman Peiro Sajjad added a comment -

        Hi Kihwal, I think that's enough.

        Show
        Hooman Peiro Sajjad added a comment - Hi Kihwal, I think that's enough.
        Suresh Srinivas made changes -
        Field Original Value New Value
        Priority Major [ 3 ] Minor [ 4 ]
        Hide
        Suresh Srinivas added a comment -

        Hooman, could you post a patch for this? I will review and commit it.

        Show
        Suresh Srinivas added a comment - Hooman, could you post a patch for this? I will review and commit it.
        Hide
        Hooman Peiro Sajjad added a comment -

        Hi Suresh, no problem. I will post a patch soon.

        Show
        Hooman Peiro Sajjad added a comment - Hi Suresh, no problem. I will post a patch soon.
        Hooman Peiro Sajjad made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hooman Peiro Sajjad made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Hooman Peiro Sajjad added a comment -

        Please review this patch.

        Show
        Hooman Peiro Sajjad added a comment - Please review this patch.
        Hooman Peiro Sajjad made changes -
        Attachment HDFS-3848-1.patch [ 12545608 ]
        Hooman Peiro Sajjad made changes -
        Attachment HDFS-3848-1.patch [ 12545608 ]
        Hide
        Hooman Peiro Sajjad added a comment -

        Please review this.

        Show
        Hooman Peiro Sajjad added a comment - Please review this.
        Hooman Peiro Sajjad made changes -
        Attachment HDFS-3848-1.patch [ 12545609 ]
        Hooman Peiro Sajjad 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/12545609/HDFS-3848-1.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. The javadoc tool did not generate any 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.TestDatanodeBlockScanner

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3203//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3203//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/12545609/HDFS-3848-1.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. The javadoc tool did not generate any 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.TestDatanodeBlockScanner +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3203//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3203//console This message is automatically generated.
        Kihwal Lee made changes -
        Target Version/s 2.5.0 [ 12326264 ]
        Priority Minor [ 4 ] Major [ 3 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12545609/HDFS-3848-1.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/7136//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7136//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/12545609/HDFS-3848-1.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/7136//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7136//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12545609/HDFS-3848-1.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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.server.datanode.TestBPOfferService

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7135//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7135//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/12545609/HDFS-3848-1.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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.datanode.TestBPOfferService +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7135//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7135//console This message is automatically generated.
        Hide
        Kihwal Lee added a comment -

        I kicked the precommit again. It looks like the patch is still good, but it will be nice if a test case is added.

        Show
        Kihwal Lee added a comment - I kicked the precommit again. It looks like the patch is still good, but it will be nice if a test case is added.
        Hide
        Chen He added a comment -

        I will add the test case.

        Show
        Chen He added a comment - I will add the test case.
        Hide
        Chen He added a comment -

        Unit test included.

        Show
        Chen He added a comment - Unit test included.
        Chen He made changes -
        Attachment HDFS-3848.patch [ 12651424 ]
        Chen He made changes -
        Attachment HDFS-3848.patch [ 12651424 ]
        Chen He made changes -
        Attachment HDFS-3848-v2.patch [ 12651425 ]
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 1 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 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/7176//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7176//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/12651425/HDFS-3848-v2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 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 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/7176//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7176//console This message is automatically generated.
        Hide
        Chen He added a comment -

        Hi [~rike], If you are not working on this issue, I will take it.

        Show
        Chen He added a comment - Hi [~rike] , If you are not working on this issue, I will take it.
        Hide
        Kihwal Lee added a comment -

        +1 the patch looks good. Hooman has been inactive for about two years. I will assign it to you Chen, but will give credit to all.

        Show
        Kihwal Lee added a comment - +1 the patch looks good. Hooman has been inactive for about two years. I will assign it to you Chen, but will give credit to all.
        Kihwal Lee made changes -
        Assignee Chen He [ airbots ]
        Hide
        Kihwal Lee added a comment -

        I've committed this to trunk and branch-2. Thanks for working on this Hooman and Chen.

        Show
        Kihwal Lee added a comment - I've committed this to trunk and branch-2. Thanks for working on this Hooman and Chen.
        Kihwal Lee made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s 3.0.0 [ 12320356 ]
        Fix Version/s 2.5.0 [ 12326264 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #5737 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5737/)
        HDFS-3848. A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011)

        • /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/FSNamesystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5737 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5737/ ) HDFS-3848 . A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011 ) /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/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #589 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/589/)
        HDFS-3848. A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011)

        • /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/FSNamesystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #589 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/589/ ) HDFS-3848 . A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011 ) /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/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #1780 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1780/)
        HDFS-3848. A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011)

        • /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/FSNamesystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #1780 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1780/ ) HDFS-3848 . A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011 ) /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/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #1807 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1807/)
        HDFS-3848. A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011)

        • /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/FSNamesystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1807 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1807/ ) HDFS-3848 . A Bug in recoverLeaseInternal method of FSNameSystem class. Contributed by Hooman Peiro Sajjad and Chen He. (kihwal: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1604011 ) /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/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        Karthik Kambatla (Inactive) made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Allen Wittenauer made changes -
        Fix Version/s 3.0.0 [ 12320356 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        20s 1 Hooman Peiro Sajjad 18/Sep/12 20:54
        Open Open Patch Available Patch Available
        26d 3h 19m 2 Hooman Peiro Sajjad 18/Sep/12 21:02
        Patch Available Patch Available Resolved Resolved
        638d 23h 4m 1 Kihwal Lee 19/Jun/14 20:07
        Resolved Resolved Closed Closed
        56d 10h 33m 1 Karthik Kambatla (Inactive) 15/Aug/14 06:41

          People

          • Assignee:
            Chen He
            Reporter:
            Hooman Peiro Sajjad
          • Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1h
              1h
              Remaining:
              Remaining Estimate - 1h
              1h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development