[
https://issues.apache.org/jira/browse/STORM-442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14191868#comment-14191868
]
ASF GitHub Bot commented on STORM-442:
--------------------------------------
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/305#issuecomment-61269957
If we're narrowing issue to just handle parent process's Exception hang,
looks good to me.
(We can make it better with extracting logic that reads from stderr to
method, since it's duplicated from logErrorStream() and getErrorsString(). )
This could be off-topic, but is there no chance to occur that subprocess
writes stderr for some reason but parent process runs well without Exception?
If it can possible, is parent process responsible to read stderr and do
something (like logging, raise Exception, etc.)?
> 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)
