Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-6291

FSImage may be left unclosed in BootstrapStandby#doRun()

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: ha
    • Labels:
      None

      Description

      At around line 203:

            if (!checkLogsAvailableForRead(image, imageTxId, curTxId)) {
              return ERR_CODE_LOGS_UNAVAILABLE;
            }
      

      If we return following the above check, image is not closed.

      1. HDFS-6291.2.patch
        0.7 kB
        Sanghyun Yun
      2. HDFS-6291.patch
        0.6 kB
        Sanghyun Yun

        Activity

        Hide
        Hadoop QA added a comment -

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

        +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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.server.namenode.ha.TestDNFencingWithReplication

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8546//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8546//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/12677224/HDFS-6291.2.patch against trunk revision caecd9f. +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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestDNFencingWithReplication +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8546//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8546//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        lgtm

        Show
        Ted Yu added a comment - lgtm
        Hide
        Sanghyun Yun added a comment -

        Thanks for your review, Ted Yu.
        You are right. I removed image.close() in catch block.

        Show
        Sanghyun Yun added a comment - Thanks for your review, Ted Yu . You are right. I removed image.close() in catch block.
        Hide
        Ted Yu added a comment -

        With image.close() in finally block, the catch block doesn't need to call it, right ?

        Show
        Ted Yu added a comment - With image.close() in finally block, the catch block doesn't need to call it, right ?
        Hide
        Sanghyun Yun added a comment -

        Please review my patch, Vinayakumar B and Ted Yu.
        And can I assign to me this issue?

        Show
        Sanghyun Yun added a comment - Please review my patch, Vinayakumar B and Ted Yu . And can I assign to me this issue?
        Hide
        Hadoop QA added a comment -

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

        org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7684//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7684//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/12662718/HDFS-6291.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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7684//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7684//console This message is automatically generated.
        Hide
        Sanghyun Yun added a comment -

        I add image.close() in finally

        Show
        Sanghyun Yun added a comment - I add image.close() in finally
        Hide
        Vinayakumar B added a comment -

        Yes! you are right.
        image.close() should be finally clause.

        Show
        Vinayakumar B added a comment - Yes! you are right. image.close() should be finally clause.

          People

          • Assignee:
            Unassigned
            Reporter:
            Ted Yu
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:

              Development