> On July 11, 2013, 10:04 a.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/TransactionObserver.h, line 59 > > <https://reviews.apache.org/r/12289/diff/4/?file=320029#file320029line59> > > > > Is there a reason not to have QueuePtr and Message as the two arguments > > for dequeue as well as enqueue (and I do prefer those names as I think they > > are clearer in the transactional context). > > > > The Message would allow access to either sequence or replication id (or > > any other aspect that may be necessary).
Yes, I'm driving this from the TxBuffer and TxOps. The TxAccept op doesn't have message pointers, just DeliveryRecords. We can get the queue position and replication ID from those. We could use that to look up the message in the queue but in the case of HA that would be wasted effort since HA only needs to know the replication ID. The names follow the same pattern as the TxOps, but I've no objection to changing them. - Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12289/#review23009 ----------------------------------------------------------- On July 10, 2013, 10:28 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12289/ > ----------------------------------------------------------- > > (Updated July 10, 2013, 10:28 p.m.) > > > Review request for qpid, Andrew Stitcher and Gordon Sim. > > > Repository: qpid > > > Description > ------- > > QPID-4327: HA added PrimaryTransactionObserver. > > Initial implementation only logs some trace messages. > > QPID-4327: 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. > > > Diffs > ----- > > /trunk/qpid/cpp/src/CMakeLists.txt 1501768 > /trunk/qpid/cpp/src/Makefile.am 1501768 > /trunk/qpid/cpp/src/ha.mk 1501768 > /trunk/qpid/cpp/src/qpid/broker/Broker.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1501768 > /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 1501768 > /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/DtxAck.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1501768 > /trunk/qpid/cpp/src/qpid/broker/Queue.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1501768 > /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1501768 > /trunk/qpid/cpp/src/qpid/broker/RecoveredDequeue.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/RecoveredEnqueue.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1501768 > /trunk/qpid/cpp/src/qpid/broker/TransactionObserver.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/TxAccept.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/TxAccept.cpp 1501768 > /trunk/qpid/cpp/src/qpid/broker/TxBuffer.h 1501768 > /trunk/qpid/cpp/src/qpid/broker/TxBuffer.cpp 1501768 > /trunk/qpid/cpp/src/qpid/broker/TxOp.h 1501768 > /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1501768 > /trunk/qpid/cpp/src/qpid/ha/Primary.h 1501768 > /trunk/qpid/cpp/src/qpid/ha/Primary.cpp 1501768 > /trunk/qpid/cpp/src/qpid/ha/PrimaryTransactionObserver.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/ha/PrimaryTransactionObserver.cpp PRE-CREATION > /trunk/qpid/cpp/src/qpid/ha/QueueSnapshots.h 1501768 > /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.h 1501768 > /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.cpp 1501768 > /trunk/qpid/cpp/src/tests/CMakeLists.txt 1501768 > /trunk/qpid/cpp/src/tests/Makefile.am 1501768 > /trunk/qpid/cpp/src/tests/TransactionObserverTest.cpp PRE-CREATION > /trunk/qpid/cpp/src/tests/TxMocks.h 1501768 > /trunk/qpid/cpp/src/tests/brokertest.py 1501768 > /trunk/qpid/cpp/src/tests/ha_tests.py 1501768 > /trunk/qpid/cpp/src/tests/test_env.sh.in 1501768 > /trunk/qpid/cpp/src/tests/test_tools.h 1501768 > > Diff: https://reviews.apache.org/r/12289/diff/ > > > Testing > ------- > > It compiles > > > Thanks, > > Alan Conway > >