[
https://issues.apache.org/jira/browse/STORM-442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14153809#comment-14153809
]
ASF GitHub Bot commented on STORM-442:
--------------------------------------
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/218#discussion_r18249411
--- Diff: storm-core/src/jvm/backtype/storm/spout/ShellSpout.java ---
@@ -152,8 +152,9 @@ private void querySubprocess() {
}
}
} catch (Exception e) {
- String processInfo = _process.getProcessInfoString() +
_process.getProcessTerminationInfoString();
- throw new RuntimeException(processInfo, e);
+ LOG.error("Halting process: ShellSpout died.", e);
--- End diff --
I think we should change
[ShellProcess#getErrorsStream](https://github.com/apache/storm/blob/24b5eefb1553ae37951807502a433f24d3e95ec1/storm-core/src/jvm/backtype/storm/utils/ShellProcess.java#L135-L145)
so that it checks that there is actually something to read.
This would fix a couple of other places where a hang looks possible:
*
https://github.com/apache/storm/blob/24b5eefb1553ae37951807502a433f24d3e95ec1/storm-core/src/jvm/backtype/storm/utils/ShellProcess.java#L68
*
https://github.com/apache/storm/blob/24b5eefb1553ae37951807502a433f24d3e95ec1/storm-core/src/jvm/backtype/storm/utils/ShellProcess.java#L70
*
https://github.com/apache/storm/blob/24b5eefb1553ae37951807502a433f24d3e95ec1/storm-core/src/jvm/backtype/storm/utils/ShellProcess.java#L101
ShellProcess#logErrorStream [does
this](https://github.com/apache/storm/blob/24b5eefb1553ae37951807502a433f24d3e95ec1/storm-core/src/jvm/backtype/storm/utils/ShellProcess.java#L123-L133).
> multilang ShellBolt/ShellSpout die() can be hang when Exception happened
> ------------------------------------------------------------------------
>
> Key: STORM-442
> URL: https://issues.apache.org/jira/browse/STORM-442
> Project: Apache Storm
> Issue Type: Bug
> Affects Versions: 0.9.3
> Reporter: DashengJu
>
> In ShellBolt, the _readerThread read command from python/shell process, and
> handle like this:
> try {
> ShellMsg shellMsg = _process.readShellMsg();
> ...
> } catch (InterruptedException e) {
> } catch (Throwable t) {
> die(t);
> }
> And in the die function, getProcessTerminationInfoString will read
> getErrorsString() from processErrorStream.
> private void die(Throwable exception) {
>
> String processInfo = _process.getProcessInfoString() +
> _process.getProcessTerminationInfoString();
>
> _exception = new RuntimeException(processInfo, exception);
>
> }
> so when ShellBolt got exception(for example, readShellMsg() throw NPE ) ,
> but it is not an error from sub process, then
> getProcessTerminationInfoString will be hang because processErrorStream have
> no data to read.
> On the other hand, as [~xiaokang] says ShellBolt should fail fast on
> exception ( https://github.com/apache/incubator-storm/pull/46 ) , I think it
> is not a good idea to read error info from stream.
> Because [~xiaokang] 's PR is based old version, so I will move his code to
> this PR, and modify some other place in ShellSpout.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
