Skip to content

Conversation

@roshannaik
Copy link
Contributor

For a simple Speed of light topology with 1 spout (generating random nos) and 1 bolt (which just multiplies the number by 2), Ackers=0 and Event Loggers=0 ... this change showed approx 15% improvement in emits/sec.

[component-id message-id (System/currentTimeMillis) values]))))

(defmethod mk-threads :spout [executor-data task-datas initial-credentials]
(defmethod mk-threads :spout [executor-data task-datas initial-credentials throttle-on]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is logically wrong. Throttle-on flag is a boolean that you have to obtain dynamically from the executor-data in run time, not that you can pass in as a constant when making the spout thread at the begining.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this change will effectively disable backpressure.

@roshannaik
We still need to look up that value in spout async loop.
One sketching idea for workaround is that we may not need to look up that value for each iteration, which is based on precondition that it may also not hurt when spout notices about throttle slightly later.

Btw, I guess when nextTuple() takes some amount of time (say, about to 1 ms, not speed of light but still not slow), looking up that value may be not hurt overall performance. When we don't have data for emit, we sleep 1ms in loop for default configuration.

topology.spout.wait.strategy: "org.apache.storm.spout.SleepSpoutWaitStrategy"
topology.sleep.spout.wait.strategy.time.ms: 1

@zhuoliu
Copy link

zhuoliu commented Mar 22, 2016

I would suggest testing a special topology with backpressure enabled. E.g., this one., in which you can intermittently see a throttle-on flag (like "6703") under /backpressure/wc2-topo/ directory of Zookeeper.

@knusbaum
Copy link
Contributor

knusbaum commented Oct 4, 2016

This hasn't been updated for ~7 months, and I'm -1 on the concept anyhow, so I'm going to close it.

If you'd like to discuss, feel free to reopen.

@asfgit asfgit closed this in ad0f9ae Oct 4, 2016
d2r pushed a commit to d2r/storm that referenced this pull request Oct 16, 2018
YSTORM-5241 update ystorm contrib to pickup metrics flush on shutdown
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants