You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by HeartSaVioR <gi...@git.apache.org> on 2015/04/01 00:04:08 UTC
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
GitHub user HeartSaVioR opened a pull request:
https://github.com/apache/storm/pull/497
STORM-742 Let ShellBolt treat all messages to update heartbeat
It's for avoiding design constraint being introduced from STORM-513.
ShellSpout is already applied.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HeartSaVioR/storm STORM-742
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/497.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #497
----
commit b815012509e15a68bdd72cc684ab5cc91dd53b86
Author: Jungtaek Lim <ka...@gmail.com>
Date: 2015-03-31T22:02:09Z
STORM-742 Let ShellBolt treat all messages to update heartbeat
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-121613384
Now I changed my mind to stick with current design constraint, this PR should be merged to solve heartbeat issue in specific scenarios.
Please take a look and comment. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-131097025
@ptgoetz @revans2 @harshach
Multi-lang users need this PR to overcome heartbeat design constraint.
Could you take a look? Thanks in advance!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-90296418
Thinking it once more, it cannot resolve @dashengju 's scenario from STORM-738 since users can skip acking with NO-ACK mode.
I'll find alternative way to resolve it.
But PR can resolve ACK mode with too busy bolt, so this PR is still valid.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-88301157
Yes, I've not modified about sending heartbeat tuple, so it doesn't make any issues. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/497
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by dashengju <gi...@git.apache.org>.
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-90392638
@HeartSaVioR
We have tested the patch. It solved most of the problem, but it still exist in some cases.
Currently, even with ACK, the subprocess handle one tuple, emit the result, and read all the tuples from stdin to get the id, read all the tuples from stdin needs long time, which cause exceed timeout.
So we limit the queue-size in ShellBolt to 100, just make the read all the tuples from stdin operation a short time can avoid the problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by dashengju <gi...@git.apache.org>.
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-88300765
I checked the code again, my consideration will not happen, it will send heartbeat tuple every second. please ignore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-131107320
OK I'll check it into both branches.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-121484496
@dashengju Could you share your patch (limiting queue size) if you don't mind?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-108116494
@ptgoetz @revans2
I'd like you to take a look. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-131131538
@revans2 Thanks for merging!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-88266644
@dashengju
It could be hard to make unit test, so could you apply this patch to your situation and report it works?
Thanks in advance!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-90348598
@dashengju
I filed new issue regarding very busy bolt with Non-ACK mode. https://issues.apache.org/jira/browse/STORM-758
Maybe you would be interested that instead of this, so please have a look at my idea in new issue.
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by dashengju <gi...@git.apache.org>.
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-88300244
thanks for submit this patch quickly.
we will test in our situation today.
But i have another consideration: does this increase too many heartbeat msg to subprocess? every emit from subprocess will put a heartbeat tuple.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-131107204
@revans2
Thanks for the review. :)
Yes, it would be better to include it to next release, since several users already reported such situation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by itaifrenkel <gi...@git.apache.org>.
Github user itaifrenkel commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-131598963
@HeartSaVioR I also recommend this fix forter@70f5689
Otherwise the bolt can fail when preparing in a busy build server (race condition between reader and heartbeat threads)
Also, I am not sure that anything other than fail/ack/sync really means the bolt is not in an endless loop.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-742 Let ShellBolt treat all messages to ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/497#issuecomment-131105849
@HeartSaVioR Sorry I have been way behind on looking at pull requests, among other things. It looks good to me +1. Do you want to pull this back into 0.10 too?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---