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.
Issue Links
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
 -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 | |
| 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.
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.
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.
| -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 | |
| 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.
Akira Ajisaka, can you please see see this patch, thanks.
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.
Thanks Akira Ajisaka for comments. Updating the latest patch and removing unnecessary code.
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); }
Good point! Akira Ajisaka, Update the latest patch, simplify the patch.
04 patch looks good to me. Hi Jing Zhao, would you please review this patch?
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.
+1, I'll commit it tomorrow if there are no additional comments.
Committed this to branch-2.7 and above. Thanks Yiqun Lin for the contribution!
Attaching patches used for branch-2/2.8 and branch-2.7.
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
Thanks Akira Ajisaka for commit!
Closing the JIRA as part of 2.7.3 release.
Attach a initial patch, kindly review.