----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12289/#review22810 -----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/TransactionObserver.h <https://reviews.apache.org/r/12289/#comment46520> It's really the enqueue and dequeue operations that are transactional. It might be better to define this interface in terms of those. It would also seem - on the surface at least - a little more consistent to have a set of TransactionObservers at the broker level (along side those for queues and connections), rather than a factory that is then used to attach different observer instances to transaction buffers. E.g. class TransactionObserver { void enqueued(TransactionContext&, const Message&, const boost::shared_ptr<Queue>) = 0; void dequeued(TransactionContext&, const Message&, const boost::shared_ptr<Queue>) = 0; void started(TransactionContext&) = 0; void prepared(TransactionContext&) = 0; void committed(TransactionContext&) = 0; void rolledback(TransactionContext&) = 0; } Perhaps adding a 'std::string getId()' method to TransactionContext if needed. Though 1.0 doesn't yet support transactions, DeliveryRecords is an 0-10 specific class so the interface you have here would need to change for 1.0 anyway I suspect. - Gordon Sim On July 5, 2013, 9:34 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12289/ > ----------------------------------------------------------- > > (Updated July 5, 2013, 9:34 p.m.) > > > Review request for qpid, Andrew Stitcher and Gordon Sim. > > > Repository: qpid > > > Description > ------- > > QPID-4327: TransactionObserver interface. > > Plugin can set TransactionObserverFactory to create TransactionObservers. > TransactionObserver interface is called at each point in a transactions > lifecycle. > Currently only allows a single TransactionObserverFactory per broker. > > This is a bit ugly, any ideas to make it neater would be much appreciated. > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/Broker.h 1500107 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1500107 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1500107 > /trunk/qpid/cpp/src/qpid/broker/TransactionObserver.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/TxBuffer.h 1500107 > /trunk/qpid/cpp/src/tests/brokertest.py 1500107 > /trunk/qpid/cpp/src/tests/ha_tests.py 1500107 > > Diff: https://reviews.apache.org/r/12289/diff/ > > > Testing > ------- > > It compiles > > > Thanks, > > Alan Conway > >
