You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by caofangkun <gi...@git.apache.org> on 2014/10/16 04:49:25 UTC
[GitHub] storm pull request: STORM-378,SleepSpoutWaitStrategy.emptyEmit sho...
GitHub user caofangkun opened a pull request:
https://github.com/apache/storm/pull/295
STORM-378,SleepSpoutWaitStrategy.emptyEmit should use the variable strea...
...k
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/caofangkun/apache-storm storm-378
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/295.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 #295
----
commit 154c54d53399756a66a8ad51d93e9e7f58117c7d
Author: caofangkun <ca...@gmail.com>
Date: 2014-10-16T02:41:25Z
STORM-378,SleepSpoutWaitStrategy.emptyEmit should use the variable streak
----
---
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-378,SleepSpoutWaitStrategy.emptyEmit sho...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/295#issuecomment-61767078
To make it clear, PR points to "increase/decrease" sleep time with streak.
Fixed sleep time is configurable so it doesn't matter how long it is, optimal value should be vary for workload.
I think we can make new ISpoutWaitStrategy implementation that play with streak if we really need it.
We definitely need wait strategy to always sleep same time (ex. 1ms), so it isn't a good idea to change existing class's behavior.
---
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-378,SleepSpoutWaitStrategy.emptyEmit sho...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/295#issuecomment-60460916
@caofangkun
It's mk-threads in executor.clj.
```
(if (and (= curr-count (.get emitted-count)) active?)
(do (.increment empty-emit-streak)
(.emptyEmit spout-wait-strategy (.get empty-emit-streak)))
(.set empty-emit-streak 0)
))
```
You can find that streak gets increased by 1, so I think it's for alternative implementation of ISpoutWaitStrategy, not SleepSpoutWaitStrategy.
(@nathanmarz Could you please confirm it?)
If it is for, just adding it to sleepMillis barely affects sleep time.
Streak should be multiplied by 10 or something bigger, maybe we can apply exponential value of already multiplied streak.
---
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-378,SleepSpoutWaitStrategy.emptyEmit sho...
Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on the pull request:
https://github.com/apache/storm/pull/295#issuecomment-61634029
To make the whole topology responsive, the spout need to stay active to pull data frequently from acker or system tick.
When setting "topology.sleep.spout.wait.strategy.time.ms" to 1 ms, it should be good enough, the system load is relatively small.
What is the motivation to make it increasing?
---
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-378,SleepSpoutWaitStrategy.emptyEmit sho...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/295#issuecomment-61860848
-1
I agree with @HeartSaVioR that if we want an implementation of `ISpoutWaitStrategy` that takes into account the `streak` parameter, it should be a separate implementation and `SleepSpoutWaitStrategy` should be left as-is.
---
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-378,SleepSpoutWaitStrategy.emptyEmit sho...
Posted by itaifrenkel <gi...@git.apache.org>.
Github user itaifrenkel commented on the pull request:
https://github.com/apache/storm/pull/295#issuecomment-61766182
On the one hand - when the spout is a multilang bolt, 1ms drains ~10-5% of a CPU core on aws c3.large, so we increase it to 10ms.
On the other hand, increasing the sleep adds jitter to the spout latency when maxSpoutPending is enabled. Is there a configurable maximum value for the streak?
---
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-378,SleepSpoutWaitStrategy.emptyEmit sho...
Posted by caofangkun <gi...@git.apache.org>.
Github user caofangkun closed the pull request at:
https://github.com/apache/storm/pull/295
---
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.
---