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 2014/12/01 15:15:35 UTC

Re: Review Request 28209: QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.

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

(Updated Dec. 1, 2014, 2:15 p.m.)


Review request for qpid and Gordon Sim.


Repository: qpid


Description (updated)
-------

NOTE: This patch WILL NOT WORK with the proton 0.8 release or current master
branch at 0985a11.  It requires all the proton issues listed as blockers in
QPID-4710 to be fixed.  It will work against a build of the proton examples
branch (afc52a0) which has workarounds for those issues.

Implements the "transactional retire and settle immediately" option for
transactions as specified in AMQP 1.0 in the qpid::messaging C++ client.

1. Added descriptor list to Variant with support in Encoder and PnData.

Required to support transactions, need to be able to create described lists.
Variant changes are source and binary compatible.

A Variant now has a Variant::List of descripors which can be numeric or string.
Nested descriptors are implemented by putting multiple descriptors in the list.

Other minor changes:
- Variant refactor: don't delete impl on every assignment.
- Add Variant constructors that take a string encoding.
  (new constructors, not defaulted arguments, so the change is binary and source compatible.)
- Growable buffer support for Encoder.
- Printing described Variant prints descriptors in form @descriptor value

2. Added transaction support to AMQP 1.0 client code

Added messaging/amqp/Transaction.h,cpp: transaction logic
- communicate with coordinator, send declare/dischange messages.
- add tx state info to transfers and acknowledgements.
- Sync session after discharge.
- A transactional session automatically acks any message retrieved by fetch/get
  to bring them into the transaction. This is consistent the 0-10 client.

Minor fixes to existing client code:
- Fix use of pn_drain API in C++ client to work with C++ and Java brokers.
- Make amqp::Exception derive from qpid::Exception

3. Fixes to existing broker code:

- Incoming.cpp fix: start async completion before processing message.
- Delay accept of dischage message till commit is complete.
- newSession - handle failover during session creation.

4. Added tests

interop_tests.py: transaction tests that can run against an external broker, see comments.

ha_tests.py: Enable transaction tests over AMQP 1.0.

Minor test fixes:
- brokertest.py don't set default logging if QPID_LOG env vars set.
- brokertest.py Pass kwargs to broker() create function.
- qpid-receive: capacity should never be larger than message count.
- Accept user:pass as well as user/pass in Url.
- brokertest.py: Always do a ready() check on all brokers.


Diffs (updated)
-----

  trunk/qpid/cpp/include/qpid/types/Variant.h 1642681 
  trunk/qpid/cpp/src/amqp.cmake 1642681 
  trunk/qpid/cpp/src/qpid/Url.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/CharSequence.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Descriptor.h 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Descriptor.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1642681 
  trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Exception.h 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1642681 
  trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/ReceiverContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SenderHandle.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SessionHandle.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/Transaction.h PRE-CREATION 
  trunk/qpid/cpp/src/qpid/messaging/amqp/Transaction.cpp PRE-CREATION 
  trunk/qpid/cpp/src/qpid/types/Variant.cpp 1642681 
  trunk/qpid/cpp/src/qpid/types/encodings.h 1642681 
  trunk/qpid/cpp/src/tests/CMakeLists.txt 1642681 
  trunk/qpid/cpp/src/tests/Variant.cpp 1642681 
  trunk/qpid/cpp/src/tests/brokertest.py 1642681 
  trunk/qpid/cpp/src/tests/ha_test.py 1642681 
  trunk/qpid/cpp/src/tests/ha_tests.py 1642681 
  trunk/qpid/cpp/src/tests/interop_tests.py PRE-CREATION 
  trunk/qpid/cpp/src/tests/qpid-receive.cpp 1642681 
  trunk/qpid/cpp/src/tests/qpid-send.cpp 1642681 
  trunk/qpid/cpp/src/tests/qpid-txtest2.cpp 1642681 
  trunk/qpid/cpp/src/tests/swig_python_tests 1642681 
  trunk/qpid/cpp/src/tests/test_env.sh.in 1642681 
  trunk/qpid/cpp/src/tests/test_store.cpp 1642681 
  trunk/qpid/tests/src/py/qpid_tests/broker_1_0/__init__.py 1642681 
  trunk/qpid/tests/src/py/qpid_tests/broker_1_0/tx.py PRE-CREATION 

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


Testing
-------


Thanks,

Alan Conway


Re: Review Request 28209: QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.

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

(Updated Dec. 1, 2014, 2:24 p.m.)


Review request for qpid and Gordon Sim.


Repository: qpid


Description (updated)
-------

NOTE: This patch WILL NOT WORK with the proton 0.8 release or current master
branch at (proton 0985a11).  It requires all the proton issues listed as blockers in
QPID-4710 to be fixed.  It will work against a build of the proton examples
branch (proton afc52a0) which has workarounds for those issues.
This patch is based on qpid r1642681.

Implements the "transactional retire and settle immediately" option for
transactions as specified in AMQP 1.0 in the qpid::messaging C++ client.

1. Added descriptor list to Variant with support in Encoder and PnData.

Required to support transactions, need to be able to create described lists.
Variant changes are source and binary compatible.

A Variant now has a Variant::List of descripors which can be numeric or string.
Nested descriptors are implemented by putting multiple descriptors in the list.

Other minor changes:
- Variant refactor: don't delete impl on every assignment.
- Add Variant constructors that take a string encoding.
  (new constructors, not defaulted arguments, so the change is binary and source compatible.)
- Growable buffer support for Encoder.
- Printing described Variant prints descriptors in form @descriptor value

2. Added transaction support to AMQP 1.0 client code

Added messaging/amqp/Transaction.h,cpp: transaction logic
- communicate with coordinator, send declare/dischange messages.
- add tx state info to transfers and acknowledgements.
- Sync session after discharge.
- A transactional session automatically acks any message retrieved by fetch/get
  to bring them into the transaction. This is consistent the 0-10 client.

Minor fixes to existing client code:
- Fix use of pn_drain API in C++ client to work with C++ and Java brokers.
- Make amqp::Exception derive from qpid::Exception

3. Fixes to existing broker code:

- Incoming.cpp fix: start async completion before processing message.
- Delay accept of dischage message till commit is complete.
- newSession - handle failover during session creation.

4. Added tests

interop_tests.py: transaction tests that can run against an external broker, see comments.

ha_tests.py: Enable transaction tests over AMQP 1.0.

Minor test fixes:
- brokertest.py don't set default logging if QPID_LOG env vars set.
- brokertest.py Pass kwargs to broker() create function.
- qpid-receive: capacity should never be larger than message count.
- Accept user:pass as well as user/pass in Url.
- brokertest.py: Always do a ready() check on all brokers.


Diffs
-----

  trunk/qpid/cpp/include/qpid/types/Variant.h 1642681 
  trunk/qpid/cpp/src/amqp.cmake 1642681 
  trunk/qpid/cpp/src/qpid/Url.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/CharSequence.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Descriptor.h 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Descriptor.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1642681 
  trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1642681 
  trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1642681 
  trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Exception.h 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1642681 
  trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1642681 
  trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/ReceiverContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SenderHandle.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.h 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/SessionHandle.cpp 1642681 
  trunk/qpid/cpp/src/qpid/messaging/amqp/Transaction.h PRE-CREATION 
  trunk/qpid/cpp/src/qpid/messaging/amqp/Transaction.cpp PRE-CREATION 
  trunk/qpid/cpp/src/qpid/types/Variant.cpp 1642681 
  trunk/qpid/cpp/src/qpid/types/encodings.h 1642681 
  trunk/qpid/cpp/src/tests/CMakeLists.txt 1642681 
  trunk/qpid/cpp/src/tests/Variant.cpp 1642681 
  trunk/qpid/cpp/src/tests/brokertest.py 1642681 
  trunk/qpid/cpp/src/tests/ha_test.py 1642681 
  trunk/qpid/cpp/src/tests/ha_tests.py 1642681 
  trunk/qpid/cpp/src/tests/interop_tests.py PRE-CREATION 
  trunk/qpid/cpp/src/tests/qpid-receive.cpp 1642681 
  trunk/qpid/cpp/src/tests/qpid-send.cpp 1642681 
  trunk/qpid/cpp/src/tests/qpid-txtest2.cpp 1642681 
  trunk/qpid/cpp/src/tests/swig_python_tests 1642681 
  trunk/qpid/cpp/src/tests/test_env.sh.in 1642681 
  trunk/qpid/cpp/src/tests/test_store.cpp 1642681 
  trunk/qpid/tests/src/py/qpid_tests/broker_1_0/__init__.py 1642681 
  trunk/qpid/tests/src/py/qpid_tests/broker_1_0/tx.py PRE-CREATION 

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


Testing
-------


Thanks,

Alan Conway