qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chug Rolke" <cro...@redhat.com>
Subject Re: Review Request: C++ Broker normalize exchange locking and add RWlock for each
Date Fri, 08 Mar 2013 19:42:53 GMT

This is an automatically generated e-mail. To reply, visit:

(Updated March 8, 2013, 7:42 p.m.)

Review request for qpid, Andrew Stitcher, Alan Conway, Gordon Sim, and Ted Ross.


Here's an update-in-progress. It is overloaded with the mission code that the new locking
scheme is designed to protect. Although that code is not part of the locking, its integration
with CopyOnWriteArray is key to having the locks all work correctly.

Since this review was originally published a few simplifying assumptions are in play.

1. The desired locking on the exchange is on a per-binding basis. This is illustrated by the
new mission code rebind():
   bool rebind (boost::shared_ptr<Binding>, std::vector<boost::shared_ptr<Queue>

The effort is to remove the binding (that has a queue) and replace it with one or more identical
bindings that each has one of the queues from the vector.

2. It is better to work with CopyOnWriteArray by adding new functions to it to effect the
rebind() than it is to try locking around repeated calls into existing functions.

Keeping the route() functions with no exchange-level locks and using only the locks provided
by snapshot() keep things flowing quickly.


* Move Topic exchange RWlock up into the parent exchange class and call it 'bindingLock'.

* In Topic exchange take RWlock out of the derived class and use the parent's lock.
* In Direct and Headers exchanges change the semantics of the current lock to use the Topic
exchange pattern.
* In FanOut exchange add locks at the same places as in the other exchanges except only use
Rlock. FanOut receives the locking it needs from the CopyOnWriteArray and does not need anything
else. By adding only Rlocks threads will never block in the normal case. However, the Rlocks
are the hooks that the new application will need to freeze the exchange when that app takes
out the Wlock.

This addresses bug QPID-4616.

Diffs (updated)

  trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1454111 
  trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1454111 
  trunk/qpid/cpp/src/qpid/broker/Exchange.h 1454111 
  trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1454111 
  trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1454111 
  trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1454111 
  trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1454111 
  trunk/qpid/cpp/src/qpid/broker/Link.cpp 1454111 
  trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1454111 
  trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1454111 
  trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1454111 
  trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1454111 
  trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1454111 
  trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1454111 
  trunk/qpid/cpp/src/qpid/sys/CopyOnWriteArray.h 1454111 
  trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1454111 
  trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1454111 

Diff: https://reviews.apache.org/r/9698/diff/


Passes normal self tests


Chug Rolke

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message