Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-9812

Streamer threads leak if failure happens when closing DFSOutputStream

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.1
    • Fix Version/s: 2.8.0, 2.7.3, 3.0.0-alpha1
    • Component/s: hdfs-client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      In HDFS-9794, it has solved problem of that stream thread leak if failure happens when closing the striped outputstream. And in DFSOutputStream, it also exists the same problem in DFSOutputStream#closeImpl. If failures happen when flushing data blocks, the streamer threads will also not be closed.

      1. HDFS.001.patch
        2 kB
        Yiqun Lin
      2. HDFS-9812.002.patch
        12 kB
        Yiqun Lin
      3. HDFS-9812.003.patch
        4 kB
        Yiqun Lin
      4. HDFS-9812.004.patch
        3 kB
        Yiqun Lin
      5. HDFS-9812.branch-2.patch
        1 kB
        Akira Ajisaka
      6. HDFS-9812-branch-2.7.patch
        2 kB
        Akira Ajisaka

        Issue Links

          Activity

          Hide
          linyiqun Yiqun Lin added a comment -

          Attach a initial patch, kindly review.

          Show
          linyiqun Yiqun Lin added a comment - Attach a initial patch, kindly review.
          Hide
          linyiqun Yiqun Lin added a comment -

          Hi, Jing Zhao, is this the similar problem with HDFS-9794?

          Show
          linyiqun Yiqun Lin added a comment - Hi, Jing Zhao , is this the similar problem with HDFS-9794 ?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 9m 21s trunk passed
          +1 compile 0m 45s trunk passed with JDK v1.8.0_72
          +1 compile 0m 40s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 47s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 2m 14s trunk passed
          +1 javadoc 0m 34s trunk passed with JDK v1.8.0_72
          +1 javadoc 0m 34s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 43s the patch passed
          +1 compile 0m 42s the patch passed with JDK v1.8.0_72
          +1 javac 0m 42s the patch passed
          +1 compile 0m 37s the patch passed with JDK v1.7.0_95
          +1 javac 0m 37s the patch passed
          +1 checkstyle 0m 18s the patch passed
          +1 mvnsite 0m 44s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 22s the patch passed
          +1 javadoc 0m 30s the patch passed with JDK v1.8.0_72
          +1 javadoc 0m 32s the patch passed with JDK v1.7.0_95
          +1 unit 1m 22s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72.
          +1 unit 1m 15s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 24s Patch does not generate ASF License warnings.
          28m 26s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787885/HDFS.001.patch
          JIRA Issue HDFS-9812
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 54a5323e79ee 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 1c48e50
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14494/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14494/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 9m 21s trunk passed +1 compile 0m 45s trunk passed with JDK v1.8.0_72 +1 compile 0m 40s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 47s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 2m 14s trunk passed +1 javadoc 0m 34s trunk passed with JDK v1.8.0_72 +1 javadoc 0m 34s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 43s the patch passed +1 compile 0m 42s the patch passed with JDK v1.8.0_72 +1 javac 0m 42s the patch passed +1 compile 0m 37s the patch passed with JDK v1.7.0_95 +1 javac 0m 37s the patch passed +1 checkstyle 0m 18s the patch passed +1 mvnsite 0m 44s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 22s the patch passed +1 javadoc 0m 30s the patch passed with JDK v1.8.0_72 +1 javadoc 0m 32s the patch passed with JDK v1.7.0_95 +1 unit 1m 22s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72. +1 unit 1m 15s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 24s Patch does not generate ASF License warnings. 28m 26s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787885/HDFS.001.patch JIRA Issue HDFS-9812 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 54a5323e79ee 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1c48e50 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14494/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14494/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          linyiqun Yiqun Lin added a comment -

          All the test are passed, the fix looks fine to the current code. I did a code review to find similar issues in DfsOutputStream. The flushing data method DfsOutputStream#flushInternal is only used in DfsOutputStream#closeImpl, it seems that the case of this is the only place that will lead to streamer thread leak.

          Show
          linyiqun Yiqun Lin added a comment - All the test are passed, the fix looks fine to the current code. I did a code review to find similar issues in DfsOutputStream . The flushing data method DfsOutputStream#flushInternal is only used in DfsOutputStream#closeImpl , it seems that the case of this is the only place that will lead to streamer thread leak.
          Hide
          linyiqun Yiqun Lin added a comment -

          Update the latest patch. There are many places doesn't judging LOG.isDebugEnabled() when logging debug info in DFSOutputStream and DataStreamer. The latest patch fix this issues.

          Show
          linyiqun Yiqun Lin added a comment - Update the latest patch. There are many places doesn't judging LOG.isDebugEnabled() when logging debug info in DFSOutputStream and DataStreamer . The latest patch fix this issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 9s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 6m 29s trunk passed
          +1 compile 0m 26s trunk passed with JDK v1.8.0_72
          +1 compile 0m 27s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 33s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 36s trunk passed
          +1 javadoc 0m 19s trunk passed with JDK v1.8.0_72
          +1 javadoc 0m 24s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 24s the patch passed with JDK v1.8.0_72
          +1 javac 0m 24s the patch passed
          +1 compile 0m 25s the patch passed with JDK v1.7.0_95
          +1 javac 0m 25s the patch passed
          -1 checkstyle 0m 15s hadoop-hdfs-project/hadoop-hdfs-client: patch generated 1 new + 102 unchanged - 1 fixed = 103 total (was 103)
          +1 mvnsite 0m 31s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 48s the patch passed
          +1 javadoc 0m 16s the patch passed with JDK v1.8.0_72
          +1 javadoc 0m 22s the patch passed with JDK v1.7.0_95
          +1 unit 0m 48s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72.
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 17s Patch does not generate ASF License warnings.
          19m 25s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788202/HDFS-9812.002.patch
          JIRA Issue HDFS-9812
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 308ccb44844d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 453e7e0
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14512/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14512/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14512/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 9s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 6m 29s trunk passed +1 compile 0m 26s trunk passed with JDK v1.8.0_72 +1 compile 0m 27s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 33s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 36s trunk passed +1 javadoc 0m 19s trunk passed with JDK v1.8.0_72 +1 javadoc 0m 24s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 30s the patch passed +1 compile 0m 24s the patch passed with JDK v1.8.0_72 +1 javac 0m 24s the patch passed +1 compile 0m 25s the patch passed with JDK v1.7.0_95 +1 javac 0m 25s the patch passed -1 checkstyle 0m 15s hadoop-hdfs-project/hadoop-hdfs-client: patch generated 1 new + 102 unchanged - 1 fixed = 103 total (was 103) +1 mvnsite 0m 31s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 16s the patch passed with JDK v1.8.0_72 +1 javadoc 0m 22s the patch passed with JDK v1.7.0_95 +1 unit 0m 48s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72. +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 17s Patch does not generate ASF License warnings. 19m 25s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788202/HDFS-9812.002.patch JIRA Issue HDFS-9812 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 308ccb44844d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 453e7e0 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14512/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14512/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14512/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          linyiqun Yiqun Lin added a comment -

          Akira Ajisaka, can you please see see this patch, thanks.

          Show
          linyiqun Yiqun Lin added a comment - Akira Ajisaka , can you please see see this patch, thanks.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Thanks Yiqun Lin for contributing to this issue.

          There are many places doesn't judging LOG.isDebugEnabled() when logging debug info in DFSOutputStream and DataStreamer.

          There is no need to use LOG.isDebugEnabled() when the logging is the following format (e.g. "+" is not used):

            LOG.debug("Connecting to datanode {}", dnAddr);
          

          Please see the Logging section of https://github.com/steveloughran/formality/blob/master/styleguide/styleguide.md for the detail.

          Show
          ajisakaa Akira Ajisaka added a comment - Thanks Yiqun Lin for contributing to this issue. There are many places doesn't judging LOG.isDebugEnabled() when logging debug info in DFSOutputStream and DataStreamer. There is no need to use LOG.isDebugEnabled() when the logging is the following format (e.g. "+" is not used): LOG.debug( "Connecting to datanode {}" , dnAddr); Please see the Logging section of https://github.com/steveloughran/formality/blob/master/styleguide/styleguide.md for the detail.
          Hide
          linyiqun Yiqun Lin added a comment -

          Thanks Akira Ajisaka for comments. Updating the latest patch and removing unnecessary code.

          Show
          linyiqun Yiqun Lin added a comment - Thanks Akira Ajisaka for comments. Updating the latest patch and removing unnecessary code.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Thanks Yiqun Lin for updating the patch. I'm thinking we don't need to use nested try-catch-finally clause. The code can be replaced by one try-catch-finally clause as follows:

          try {
            // call some methods to flush buffer.
            // don't need to call closeThreads(false) because the threads are closed by the finally block.
          } catch (ClosedChannelException ignored) {
          } finally {
            // don't need to call setClosed() because closeThreads(true) calls setClosed() in the finally block.
            closeThreads(true);
          }
          
          Show
          ajisakaa Akira Ajisaka added a comment - Thanks Yiqun Lin for updating the patch. I'm thinking we don't need to use nested try-catch-finally clause. The code can be replaced by one try-catch-finally clause as follows: try { // call some methods to flush buffer. // don't need to call closeThreads( false ) because the threads are closed by the finally block. } catch (ClosedChannelException ignored) { } finally { // don't need to call setClosed() because closeThreads( true ) calls setClosed() in the finally block. closeThreads( true ); }
          Hide
          linyiqun Yiqun Lin added a comment -

          Good point! Akira Ajisaka, Update the latest patch, simplify the patch.

          Show
          linyiqun Yiqun Lin added a comment - Good point! Akira Ajisaka , Update the latest patch, simplify the patch.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          04 patch looks good to me. Hi Jing Zhao, would you please review this patch?

          Show
          ajisakaa Akira Ajisaka added a comment - 04 patch looks good to me. Hi Jing Zhao , would you please review this patch?
          Hide
          linyiqun Yiqun Lin added a comment -

          HI, Akira Ajisaka, could you help commit to trunk if there are no comments recent time, it seems that this jira has not been updated long time.

          Show
          linyiqun Yiqun Lin added a comment - HI, Akira Ajisaka , could you help commit to trunk if there are no comments recent time, it seems that this jira has not been updated long time.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          +1, I'll commit it tomorrow if there are no additional comments.

          Show
          ajisakaa Akira Ajisaka added a comment - +1, I'll commit it tomorrow if there are no additional comments.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Committed this to branch-2.7 and above. Thanks Yiqun Lin for the contribution!

          Show
          ajisakaa Akira Ajisaka added a comment - Committed this to branch-2.7 and above. Thanks Yiqun Lin for the contribution!
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Attaching patches used for branch-2/2.8 and branch-2.7.

          Show
          ajisakaa Akira Ajisaka added a comment - Attaching patches used for branch-2/2.8 and branch-2.7.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9440 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9440/)
          HDFS-9812. Streamer threads leak if failure happens when closing (aajisaka: rev 352d299cf8ebe330d24117df98d1e6a64ae38c26)

          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9440 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9440/ ) HDFS-9812 . Streamer threads leak if failure happens when closing (aajisaka: rev 352d299cf8ebe330d24117df98d1e6a64ae38c26) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java
          Hide
          linyiqun Yiqun Lin added a comment -

          Thanks Akira Ajisaka for commit!

          Show
          linyiqun Yiqun Lin added a comment - Thanks Akira Ajisaka for commit!
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Closing the JIRA as part of 2.7.3 release.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Closing the JIRA as part of 2.7.3 release.

            People

            • Assignee:
              linyiqun Yiqun Lin
              Reporter:
              linyiqun Yiqun Lin
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development