GitHub user HeartSaVioR opened a pull request:
https://github.com/apache/storm/pull/2293
STORM-2231 Fix multi-threads issue on executor send queue
[STORM-2231](https://issues.apache.org/jira/browse/STORM-2231) is an issue
to report broken thread-safety on output collector. We mostly considered about
grouper implementation, but there's a spot in other side as well.
In DisruptorQueue, each incoming threads have each ThreadLocalBatcher,
meaning that 'synchronized' in add() can synchronize incoming thread and
background flusher thread, but not between incoming threads.
Hopefully DisruptorQueue publish*() are implemented to respect
thread-safety, but it relies on RingBuffer producer option (Sequencer in
RingBuffer), and unfortunately we set producer to SINGLE, not MULTI for
executor batch queue. It seems one of optimization but we seemed to miss a
spot.
So there're two simple options to choose: just using same
ThreadLocalBatcher instance for all incoming thread (class name should be
changed) or just changing producer mode to MULTI. This patch addresses later
option, because it is just simple, and expected to be faster than former.
The change is expected to bring any kind of performance hit, so may also
need to do some performance tests against the patch.
@revans2 Could you take a look at? It is related to disruptor batching.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HeartSaVioR/storm STORM-2231
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2293.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 #2293
----
commit 2b15fc4fc7189f2f42a0fea13f2ca00a6675d6c7
Author: Jungtaek Lim <[email protected]>
Date: 2017-08-24T11:11:41Z
STORM-2231 Fix multi-threads issue on executor send queue
* we have use cases which launches threads and emit/ack concurrently
* launched threads will create ThreadLocalBatcher for each
* hence 'synchronized' in add() wouldn't help unlike background flushes
----
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
