----------------------------------------------------------- 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. Summary (updated) ----------------- QPID-5421: HA replication error in stand-alone replication Repository: qpid Description (updated) ------- 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 (updated) ----- /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
