STORM-1272: port backtype.storm.disruptor to java #1095
Conversation
|
Complete porting is pending on #1074 (util/async-loop is being referred by disruptor.clj) |
| "make a handler for the executor's receive disruptor queue to | ||
| check highWaterMark and lowWaterMark for backpressure" | ||
| (disruptor/disruptor-backpressure-handler | ||
| (DisruptorUtils/disruptorBackpressureHandler |
There was a problem hiding this comment.
Could we just inline the impl here? There isn't much point in having this translated to java when it is still so dependent on clojure.
There was a problem hiding this comment.
Removed DisruptorUtils.java and inlined everywhere.
|
I agree with @revans2. Inlining and getting rid of |
abhishekagarwal87
force-pushed
the
disruptor
branch
from
e64d78e to
c7e6689
Compare
|
I have removed DisruptorUtils.java and squashed the commits along with it. |
| (when (= true (storm-conf TOPOLOGY-DEBUG)) | ||
| (log-message "TRANSFERING tuple " val)) | ||
| (disruptor/publish batch-transfer->worker val)))) | ||
| (.publish ^DisruptorQueue batch-transfer->worker val) ))) |
There was a problem hiding this comment.
The indentation is wrong here. This is not under the when.
There was a problem hiding this comment.
will correct it.
|
The changes look good to me and the unit tests all pass. My only concern now is the performance, and the disruptor queue code is on the critical path. Have you run any performance tests? If not I can run some to do a quick comparison. |
|
I have not run performance tests as of now so you can run them |
|
I have fixed the indentation. |
|
The good news is that this patch doesn't seem to have made the performance on 2.x any worse. The bad news is that it is already 1/2 the throughput I can get on 1.x. So I am going to have to spend some time and track that down. |
|
That is quite a difference in performance. Are you using ThroughputVsLatency to get these numbers? Also while you are looking into performance degrade and #1074 has been merged, I will rebase my changes |
|
Yes, I used ThroughputvsLatency with a throughput of 20,000 sentences per second that I know 0.10.x can handle. Having a single reflection on the critical path can cause it, but fixing it really just comes down to hooking up a profiler and finding it. Not a big deal, just want to stay on top of it. |
|
I have merged my changes with util.clj ported code. disruptor.clj has been deleted in entirety. |
|
Looks good to me +1 pending Travis |
|
Travis check has passed. Ready for merge now. |
4 participants
No description provided.