Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20173 )
Change subject: IMPALA-12268: Fix possible blocking when use java.lang.Process. ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/20173/3/fe/src/test/java/org/apache/impala/common/InputStreamGobbler.java File fe/src/test/java/org/apache/impala/common/InputStreamGobbler.java: http://gerrit.cloudera.org:8080/#/c/20173/3/fe/src/test/java/org/apache/impala/common/InputStreamGobbler.java@77 PS3, Line 77: } > I think this won't be caught. Should we log the error instead? Done http://gerrit.cloudera.org:8080/#/c/20173/3/fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java File fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java: http://gerrit.cloudera.org:8080/#/c/20173/3/fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java@65 PS3, Line 65: // Collect the stdout (which has the resultsets). > Should we wait until the stderrGobbler thread finish? Hi quanlong, I think it's no need to make it more complex, because the subprocess has been finished when 'process.waitFor()' returned, the'stderrBuf' can finished almost the same time. even if the 'stderrBuf' is incomplete, the assert statement will throw and we can find the problem easily. http://gerrit.cloudera.org:8080/#/c/20173/3/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java File fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java: http://gerrit.cloudera.org:8080/#/c/20173/3/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java@42 PS3, Line 42: InputStreamGobbler stdoutGobbler = > Why do we need these? The original code doesn't collect any outputs. As described in this jira, if we don't collect the outputs, they can block the subprocess. -- To view, visit http://gerrit.cloudera.org:8080/20173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I06b728e6134fa14d3970ea7db4d64a962bb2d694 Gerrit-Change-Number: 20173 Gerrit-PatchSet: 4 Gerrit-Owner: Xiang Yang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Xiang Yang <[email protected]> Gerrit-Comment-Date: Fri, 21 Jul 2023 02:07:21 +0000 Gerrit-HasComments: Yes
