You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Alan Conway <ac...@redhat.com> on 2013/07/24 21:51:46 UTC

Re: Review Request 12289: QPID-4327: HA TxReplicator, basic replication of transactions.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12289/
-----------------------------------------------------------

(Updated July 24, 2013, 7:51 p.m.)


Review request for qpid, Andrew Stitcher and Gordon Sim.


Summary (updated)
-----------------

QPID-4327: HA TxReplicator, basic replication of transactions.


Bugs: QPID-4327
    https://issues.apache.org/jira/browse/QPID-4327


Repository: qpid


Description (updated)
-------

Basic tx replication is working but there are a bunch of questions marked in the code as to whether we're doing things correctly or not.

TxBuffer on the primary calls PrimaryTxObserver with tx events. The observer puts events onto a special tx-queue. The backup TxReplicator is QueueReplicator subclass that constructs a corresponding TxBuffer on the backup. Handling dequeues is complicated by the way we collect acks on the session up to prepare/commit and then process them. TxReplicator does something similar but this is the most dubious area of the code.

There are tests in ha_tests.py but the dequeue tests also pass due to normal queue replication, so hard to verify the tx stuff is really working.
Used temporary log statements to prove it was but need a better test. Ideas?

Here is the commit log:

QPID-4327: HA TxReplicator, basic replication of transactions.

TxReplicator is a specialized QueueReplicator to hand tx-queues.
- PrimaryTxObserver puts transaction events on the tx-queue.
- TxReplicator creates a replica TxBuffer on the backup.

QPID-4327: Add PrimaryTransactionObserver and tests.

- PrimaryTransactionObserver observes tx evens and populates tx-queue
- ClusterFixture for unit tests, manages ports for brokers.

QPID-4327: Refactor events & dispatching

- added ha/Event.h for encoding event messages
- hashmap dispatching for QueueReplicator

QPID-4327: Added TransactionObserver interface.

Added TransactionObserver interface, called at each point in a transaction's
lifecycle. Currently only a single observer can be associated with a
transaction.

Added startTx, startDtx to BrokerObserver so plugins can observe transactions
starting and set a TransactionObserver.

QPID-4327: Renamed ConfigurationObserver as BrokerObserver.

This class really was intended as a observer for broker-level events which
includes configuration but may in future include other non-configuration events
such as transactions.

QPID-4327: Refactor to simplify TxAccept.

Removed un-necessary RangeOps layers.

QPID-4327: Optimize brokertest.ready() to improve test runtimes.


Diffs (updated)
-----

  /trunk/qpid/cpp/include/qpid/framing/Buffer.h 1506613 
  /trunk/qpid/cpp/include/qpid/framing/BufferTypes.h PRE-CREATION 
  /trunk/qpid/cpp/src/CMakeLists.txt 1506613 
  /trunk/qpid/cpp/src/Makefile.am 1506613 
  /trunk/qpid/cpp/src/ha.mk 1506613 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/broker/BrokerObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/BrokerObservers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/DtxAck.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/broker/Queue.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/broker/RecoveredDequeue.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/RecoveredEnqueue.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/broker/TransactionObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/TxAccept.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/TxAccept.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/broker/TxBuffer.h 1506613 
  /trunk/qpid/cpp/src/qpid/broker/TxBuffer.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/broker/TxOp.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/Backup.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/Event.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/Event.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/FailoverExchange.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/HaBroker.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/Primary.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/Primary.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/QueueSnapshots.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/TxReplicator.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/makeMessage.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/makeMessage.cpp 1506613 
  /trunk/qpid/cpp/src/qpid/ha/types.h 1506613 
  /trunk/qpid/cpp/src/qpid/ha/types.cpp 1506613 
  /trunk/qpid/cpp/src/tests/BrokerFixture.h 1506613 
  /trunk/qpid/cpp/src/tests/CMakeLists.txt 1506613 
  /trunk/qpid/cpp/src/tests/ClusterFixture.h PRE-CREATION 
  /trunk/qpid/cpp/src/tests/ClusterFixture.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/tests/ForkedBroker.h PRE-CREATION 
  /trunk/qpid/cpp/src/tests/ForkedBroker.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/tests/Makefile.am 1506613 
  /trunk/qpid/cpp/src/tests/MessagingFixture.h 1506613 
  /trunk/qpid/cpp/src/tests/TransactionObserverTest.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/tests/TxMocks.h 1506613 
  /trunk/qpid/cpp/src/tests/brokertest.py 1506613 
  /trunk/qpid/cpp/src/tests/ha_cpp_tests.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/tests/ha_test.py 1506613 
  /trunk/qpid/cpp/src/tests/ha_tests.py 1506613 
  /trunk/qpid/cpp/src/tests/test_env.sh.in 1506613 
  /trunk/qpid/cpp/src/tests/test_tools.h 1506613 

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


Testing (updated)
-------

Basic tx replication appears to be working, used temporary log statements to prove the messages were being processed by the TxBuffer on the backup and not just being sent by the normal means.


Thanks,

Alan Conway


Re: Review Request 12289: QPID-4327: HA TX transactions: basic replication.

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12289/
-----------------------------------------------------------

(Updated July 31, 2013, 1:34 p.m.)


Review request for qpid, Andrew Stitcher and Gordon Sim.


Changes
-------

Closer to final code:
- added dummy store to verify actions on store.
- de-duplication of transactional messages

Unfinished:
- Primary does not wait for backups to prepare() before committing.
- All connected backups are assumed to be in the transaction, there are race
  conditions around brokers joining/leavinv where this assumption is invalid.
- Need more tests.


Summary (updated)
-----------------

QPID-4327: HA TX transactions: basic replication.


Bugs: QPID-4327
    https://issues.apache.org/jira/browse/QPID-4327


Repository: qpid


Description (updated)
-------

QPID-4327: HA TX transactions: basic replication.

On primary a PrimaryTxObserver observes a transaction's TxBuffer and generates
transaction events on a tx-replication-queue. On the backup a TxReplicator
receives the events and constructs a TxBuffer equivalent to the one in the
primary.

Unfinished:
- Primary does not wait for backups to prepare() before committing.
- All connected backups are assumed to be in the transaction, there are race
  conditions around brokers joining/leavinv where this assumption is invalid.
- Need more tests.

QPID-4327: HA get rid of Primary::get() singleton.


QPID-4327: Added TransactionObserver interface.

Added TransactionObserver interface, called at each point in a transaction's
lifecycle. Currently only a single observer can be associated with a
transaction.

Added startTx, startDtx to BrokerObserver so plugins can observe transactions
starting and set a TransactionObserver.

QPID-4327: Renamed ConfigurationObserver as BrokerObserver.

This class really was intended as a observer for broker-level events which
includes configuration but may in future include other non-configuration events
such as transactions.

QPID-4327: Refactor to simplify TxAccept.

Removed un-necessary RangeOps layers.

QPID-4327: Optimize brokertest.ready() to improve test runtimes.


Diffs (updated)
-----

  /trunk/qpid/cpp/include/qpid/framing/BufferTypes.h PRE-CREATION 
  /trunk/qpid/cpp/src/CMakeLists.txt 1508523 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/BrokerObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/BrokerObservers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/DtxAck.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/MessageStoreModule.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/Queue.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/RecoveredDequeue.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/RecoveredEnqueue.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/TransactionObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/TxAccept.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/TxAccept.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/TxBuffer.h 1508523 
  /trunk/qpid/cpp/src/qpid/broker/TxBuffer.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/broker/TxOp.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/Backup.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/Event.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/Event.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/FailoverExchange.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/HaBroker.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/Membership.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/Membership.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/Primary.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/Primary.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/QueueSnapshots.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/StatusCheck.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/TxReplicator.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/ha/makeMessage.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/makeMessage.cpp 1508523 
  /trunk/qpid/cpp/src/qpid/ha/types.h 1508523 
  /trunk/qpid/cpp/src/qpid/ha/types.cpp 1508523 
  /trunk/qpid/cpp/src/tests/BrokerFixture.h 1508523 
  /trunk/qpid/cpp/src/tests/CMakeLists.txt 1508523 
  /trunk/qpid/cpp/src/tests/MessagingFixture.h 1508523 
  /trunk/qpid/cpp/src/tests/TransactionObserverTest.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/tests/TxMocks.h 1508523 
  /trunk/qpid/cpp/src/tests/brokertest.py 1508523 
  /trunk/qpid/cpp/src/tests/cluster_test.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/tests/ha_test.py 1508523 
  /trunk/qpid/cpp/src/tests/ha_tests.py 1508523 
  /trunk/qpid/cpp/src/tests/test_env.sh.in 1508523 
  /trunk/qpid/cpp/src/tests/test_store.cpp 1508523 
  /trunk/qpid/cpp/src/tests/test_tools.h 1508523 

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


Testing
-------

Basic tx replication appears to be working, used temporary log statements to prove the messages were being processed by the TxBuffer on the backup and not just being sent by the normal means.


Thanks,

Alan Conway