Tim Bain created AMQ-6226:
-----------------------------
Summary: Eliminate AbstractRegion.destinationsLock
Key: AMQ-6226
URL: https://issues.apache.org/jira/browse/AMQ-6226
Project: ActiveMQ
Issue Type: Bug
Affects Versions: 5.2.0
Reporter: Tim Bain
This mailing list thread
(http://activemq.2283324.n4.nabble.com/ActiveMQ-with-KahaDB-as-persistent-store-becomes-very-slow-almost-unresponsive-after-creating-large-s-td4709985.html)
described awful performance problems when creating large numbers of
destinations in parallel, and provided thread dumps that pointed to lots of
threads contending for AbstractRegion.destinationsLock.
AbstractRegion has lots of places where we do something like this:
455 destinationsLock.readLock().lock();
456 try {
457 dest = destinations.get(destination);
458 } finally {
459 destinationsLock.readLock().unlock();
460 }
destinations is a ConcurrentHashMap, so it's already thread-safe. Why are we
using a single external lock around a ConcurrentHashMap that would be
concurrent if we weren't making it single-locked???
I suspect that the primary goal is to make sure that addDestination() and
receiveDestination() don't create or remove the same destination twice, but
there are other ways to do that (for example, having the map be
ConcurrentHashMap<ActiveMQDestination, AtomicReference<Destination>> and
putting the AtomicReference into the map immediately and then later populating
the reference once the object is constructed).
We need to eliminate this singleton lock and use the thread-safe
ConcurrentHashMap's own locking, possibly coupled with thread-safe objects
(AtomicReference, etc.) and/or explicit per-key locking, to implement the same
algorithm in a way that allows reasonable parallelism under heavy load.
NOTE: This might also help eliminate AMQ-5901.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
