----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16245/#review30350 -----------------------------------------------------------
Ship it! Ship It! - Gordon Sim On Dec. 13, 2013, 5:11 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16245/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2013, 5:11 p.m.) > > > Review request for qpid and Gordon Sim. > > > Repository: qpid > > > Description > ------- > > This is the reason why I did the observer refactor. > > A while back you pointed out that it would be simpler to eliminate IdSetter > and have the Queue set the replication IDs on messages. However there's a > problem with that: On the primary you want the queue setting the IDs, but on > the backup you don't - you want the QueueReplicator setting the IDs based on > what it's getting from the ReplicatingSubscription. But then when that backup > is promoted, you want the queue to start setting the IDs again. Using an > observer makes it easy to switch the behaviour on and off. Can you think of a > neater way? > > > QPID-5421: HA replication error in stand-alone replication > > There were replication errors because with stand-alone replication an IdSetter > was not set on the original queue until queue replication was set up. Any > messages on the queue *before* replication was setup had 0 replication IDs. > When > one of those messages was dequeued on the source queue, an incorrect message > was > dequeued on the replica queue. > > The fix is to add an IdSetter to every queue when replication is enabled. > > The unit test ha_tests.ReplicationTests.test_standalone_queue_replica has > been > updated to test for this issue. > > This commit also has some general tidy-up work around IdSetter and > QueueSnapshot. > > QPID-5421: Refactor: clean up QueueObservers. > > Refactor of queue observers to use the broker::Observers base class. > Simplifies Queue code > and makes it more consistent with other observers (BrokerObservers, > ConnectionObservers.) > > Modified Observers base class to allow identical locking behaviour to previous > impementation. > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/BrokerObservers.h 1550190 > /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1550190 > /trunk/qpid/cpp/src/qpid/broker/Observers.h 1550190 > /trunk/qpid/cpp/src/qpid/broker/Queue.h 1550190 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1550190 > /trunk/qpid/cpp/src/qpid/broker/QueueFactory.cpp 1550190 > /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1550190 > /trunk/qpid/cpp/src/qpid/broker/QueueObservers.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.cpp 1550190 > /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1550190 > /trunk/qpid/cpp/src/qpid/ha/HaBroker.h 1550190 > /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1550190 > /trunk/qpid/cpp/src/qpid/ha/IdSetter.h 1550190 > /trunk/qpid/cpp/src/qpid/ha/Primary.h 1550190 > /trunk/qpid/cpp/src/qpid/ha/Primary.cpp 1550190 > /trunk/qpid/cpp/src/qpid/ha/QueueGuard.cpp 1550190 > /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1550190 > /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1550190 > /trunk/qpid/cpp/src/qpid/ha/QueueSnapshot.h 1550190 > /trunk/qpid/cpp/src/qpid/ha/QueueSnapshots.h 1550190 > /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.cpp 1550190 > /trunk/qpid/cpp/src/tests/ha_tests.py 1550190 > > Diff: https://reviews.apache.org/r/16245/diff/ > > > Testing > ------- > > Full ctest > > > Thanks, > > Alan Conway > >
