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-v2.patch
        3 kB
        Chen He
      2. HDFS-3848-1.patch
        1.0 kB
        Hooman Peiro Sajjad

        Activity

        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.
        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.
        Hide
        Hooman Peiro Sajjad added a comment -

        Please review this patch.

        Show
        Hooman Peiro Sajjad added a comment - Please review this patch.
        Hide
        Hooman Peiro Sajjad added a comment -

        Please review this.

        Show
        Hooman Peiro Sajjad added a comment - Please review this.
        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.
        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.
        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.
        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.
        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

          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