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/11/19 03:57:33 UTC
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/
-----------------------------------------------------------
Review request for qpid and Gordon Sim.
Repository: qpid
Description
-------
My tests that roll back are failing, messages acquired during tx are not being put back on queues. I can see from logs that Transaction::discharge is being called and is doing the pn_delivery_update() calls, and that there is a driver wakeup after that, but there are no -> @dispositions leaving from the client. I can see dispositions being sent by the client when the messages are acknowledged within the transaction, and the code looks similar so I'm not sure why they are not being sent at discharge.
(The locking between ConnectionContext and Transaction is screwed up, I know that & will fix)
Summary of changes:
Added descriptor list to Variant and support in Encoder. Variant changes are source and binary compatible.
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.
QPID-4710: [AMQP 1.0] Support for transactions in qpid.messaging C++ client.
- interop_tests.py: transaction tests that can be run against any broker (env QPID_INTEROP_URL)
- messaging/amqp/Transaction.h,cpp: transaction logic
- communicate with coordinator, send declare/dischange messages.
- add tx state info to transfers and acknowledgements.
- Misc. improvements to logging
Diffs
-----
trunk/qpid/cpp/include/qpid/types/Variant.h 1640198
trunk/qpid/cpp/src/amqp.cmake 1640198
trunk/qpid/cpp/src/qpid/amqp/CharSequence.cpp 1640198
trunk/qpid/cpp/src/qpid/amqp/Descriptor.h 1640198
trunk/qpid/cpp/src/qpid/amqp/Descriptor.cpp 1640198
trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1640198
trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1640198
trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1640198
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1640198
trunk/qpid/cpp/src/qpid/broker/TxDequeue.cpp 1640198
trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1640198
trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1640198
trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.h 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.cpp 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.h 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/SenderHandle.cpp 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.h 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.cpp 1640198
trunk/qpid/cpp/src/qpid/messaging/amqp/SessionHandle.cpp 1640198
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 1640198
trunk/qpid/cpp/src/qpid/types/encodings.h 1640198
trunk/qpid/cpp/src/tests/Variant.cpp 1640198
trunk/qpid/cpp/src/tests/brokertest.py 1640198
trunk/qpid/cpp/src/tests/ha_tests.py 1640198
trunk/qpid/cpp/src/tests/interop_tests.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 Gordon Sim <gs...@redhat.com>.
> On Nov. 27, 2014, 5:10 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/include/qpid/types/Variant.h, line 194
> > <https://reviews.apache.org/r/28209/diff/2/?file=777786#file777786line194>
> >
> > Can you have multiple descriptors for a given value (as opposed to a decribed value where the value is itself a described value)?
>
> Alan Conway wrote:
> You can have multiple descriptors: getDescriptors() returns a modifiable Variant::List of descriptors. However this is the _same thing_ as a described value where the value is itself described. I did initially fiddle with the notion of recursive Variants for nested described values but it was messy, and when you look at the encoding of a described value who's value is is a described value it is simply:
> <described>Descriptor1 <described>Descriptor2 ... <described>DescriptorN <value>
> So representing it as a Variant containing <value> with a list of descriptors seems to be perfectly adequate.
yes, I guess that is true
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28209/#review63239
-----------------------------------------------------------
On Nov. 27, 2014, 4:17 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28209/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2014, 4:17 p.m.)
>
>
> Review request for qpid and Gordon Sim.
>
>
> Repository: qpid
>
>
> Description
> -------
>
> Notes:
> - the basic interop_tests are passing on qpidd, not on active or java broker.
> - this branch requires the proton examples branch, won't work on proton master till all the blocking proton JIRAS for QPID-4710 are fixed.
> - several tests are still failing including ha_tests, swig_python_tests, interlink_tests
>
> QPID-4710: 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
>
> QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.
>
> Implements the "transactoinal retire and settle immediately" option for
> transactions as specified in AMQP 1.0 in the qpid::messaging C++ client.
>
> 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.
>
> Added interop_tests.py: transaction tests that can be run against any broker (env QPID_INTEROP_URL)
>
> Fixes to existing client code:
> - Fix use of pn_drain API in C++ client to work with C++ and Java brokers.
> - amqp::Exception derive from qpid::Exception
>
> 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.
>
> QPID-4710: Additional transaction tests
>
> Enable transaction tests in ha_tests over AMQP 1.0.
>
> Minor fixes
> - Only run interop tests if AMQP built.
> - Skip interop tests if no URL.
> - 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.
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/types/Variant.h 1641910
> trunk/qpid/cpp/src/amqp.cmake 1641910
> trunk/qpid/cpp/src/qpid/amqp/CharSequence.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/Descriptor.h 1641910
> trunk/qpid/cpp/src/qpid/amqp/Descriptor.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1641910
> trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1641910
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Exception.h 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderHandle.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionHandle.cpp 1641910
> 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 1641910
> trunk/qpid/cpp/src/qpid/types/encodings.h 1641910
> trunk/qpid/cpp/src/tests/CMakeLists.txt 1641910
> trunk/qpid/cpp/src/tests/Variant.cpp 1641910
> trunk/qpid/cpp/src/tests/brokertest.py 1641910
> trunk/qpid/cpp/src/tests/ha_test.py 1641910
> trunk/qpid/cpp/src/tests/ha_tests.py 1641910
> trunk/qpid/cpp/src/tests/interop_tests.py PRE-CREATION
> trunk/qpid/cpp/src/tests/qpid-receive.cpp 1641910
> trunk/qpid/cpp/src/tests/test_store.cpp 1641910
> trunk/qpid/tests/src/py/qpid_tests/broker_1_0/__init__.py 1641910
> 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>.
> On Nov. 27, 2014, 5:10 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/include/qpid/types/Variant.h, line 194
> > <https://reviews.apache.org/r/28209/diff/2/?file=777786#file777786line194>
> >
> > Can you have multiple descriptors for a given value (as opposed to a decribed value where the value is itself a described value)?
You can have multiple descriptors: getDescriptors() returns a modifiable Variant::List of descriptors. However this is the _same thing_ as a described value where the value is itself described. I did initially fiddle with the notion of recursive Variants for nested described values but it was messy, and when you look at the encoding of a described value who's value is is a described value it is simply:
<described>Descriptor1 <described>Descriptor2 ... <described>DescriptorN <value>
So representing it as a Variant containing <value> with a list of descriptors seems to be perfectly adequate.
> On Nov. 27, 2014, 5:10 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp, line 157
> > <https://reviews.apache.org/r/28209/diff/2/?file=777802#file777802line157>
> >
> > Name is a bt confusing here... syncUnlocked() is called with the lock held. I think the lock needs to be held when calling that function since it will/may wait on that lock.
> >
> > Assuming what we want here is just a way to call the logic form a context where we already hold the lock, perhaps syncLH(const &Montior::ScopedLock) would be clearer?
Yep, will do. Not sure why I didn't in the first place.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28209/#review63239
-----------------------------------------------------------
On Nov. 27, 2014, 4:17 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28209/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2014, 4:17 p.m.)
>
>
> Review request for qpid and Gordon Sim.
>
>
> Repository: qpid
>
>
> Description
> -------
>
> Notes:
> - the basic interop_tests are passing on qpidd, not on active or java broker.
> - this branch requires the proton examples branch, won't work on proton master till all the blocking proton JIRAS for QPID-4710 are fixed.
> - several tests are still failing including ha_tests, swig_python_tests, interlink_tests
>
> QPID-4710: 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
>
> QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.
>
> Implements the "transactoinal retire and settle immediately" option for
> transactions as specified in AMQP 1.0 in the qpid::messaging C++ client.
>
> 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.
>
> Added interop_tests.py: transaction tests that can be run against any broker (env QPID_INTEROP_URL)
>
> Fixes to existing client code:
> - Fix use of pn_drain API in C++ client to work with C++ and Java brokers.
> - amqp::Exception derive from qpid::Exception
>
> 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.
>
> QPID-4710: Additional transaction tests
>
> Enable transaction tests in ha_tests over AMQP 1.0.
>
> Minor fixes
> - Only run interop tests if AMQP built.
> - Skip interop tests if no URL.
> - 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.
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/types/Variant.h 1641910
> trunk/qpid/cpp/src/amqp.cmake 1641910
> trunk/qpid/cpp/src/qpid/amqp/CharSequence.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/Descriptor.h 1641910
> trunk/qpid/cpp/src/qpid/amqp/Descriptor.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1641910
> trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1641910
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Exception.h 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderHandle.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionHandle.cpp 1641910
> 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 1641910
> trunk/qpid/cpp/src/qpid/types/encodings.h 1641910
> trunk/qpid/cpp/src/tests/CMakeLists.txt 1641910
> trunk/qpid/cpp/src/tests/Variant.cpp 1641910
> trunk/qpid/cpp/src/tests/brokertest.py 1641910
> trunk/qpid/cpp/src/tests/ha_test.py 1641910
> trunk/qpid/cpp/src/tests/ha_tests.py 1641910
> trunk/qpid/cpp/src/tests/interop_tests.py PRE-CREATION
> trunk/qpid/cpp/src/tests/qpid-receive.cpp 1641910
> trunk/qpid/cpp/src/tests/test_store.cpp 1641910
> trunk/qpid/tests/src/py/qpid_tests/broker_1_0/__init__.py 1641910
> 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 Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28209/#review63239
-----------------------------------------------------------
trunk/qpid/cpp/include/qpid/types/Variant.h
<https://reviews.apache.org/r/28209/#comment105416>
Can you have multiple descriptors for a given value (as opposed to a decribed value where the value is itself a described value)?
trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp
<https://reviews.apache.org/r/28209/#comment105419>
I think this is wrong. The ulong is in fact the code of a descriptor, so you need the enter/next.
trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp
<https://reviews.apache.org/r/28209/#comment105420>
Name is a bt confusing here... syncUnlocked() is called with the lock held. I think the lock needs to be held when calling that function since it will/may wait on that lock.
Assuming what we want here is just a way to call the logic form a context where we already hold the lock, perhaps syncLH(const &Montior::ScopedLock) would be clearer?
trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp
<https://reviews.apache.org/r/28209/#comment105421>
Same comment on naming here.
- Gordon Sim
On Nov. 27, 2014, 4:17 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28209/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2014, 4:17 p.m.)
>
>
> Review request for qpid and Gordon Sim.
>
>
> Repository: qpid
>
>
> Description
> -------
>
> Notes:
> - the basic interop_tests are passing on qpidd, not on active or java broker.
> - this branch requires the proton examples branch, won't work on proton master till all the blocking proton JIRAS for QPID-4710 are fixed.
> - several tests are still failing including ha_tests, swig_python_tests, interlink_tests
>
> QPID-4710: 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
>
> QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.
>
> Implements the "transactoinal retire and settle immediately" option for
> transactions as specified in AMQP 1.0 in the qpid::messaging C++ client.
>
> 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.
>
> Added interop_tests.py: transaction tests that can be run against any broker (env QPID_INTEROP_URL)
>
> Fixes to existing client code:
> - Fix use of pn_drain API in C++ client to work with C++ and Java brokers.
> - amqp::Exception derive from qpid::Exception
>
> 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.
>
> QPID-4710: Additional transaction tests
>
> Enable transaction tests in ha_tests over AMQP 1.0.
>
> Minor fixes
> - Only run interop tests if AMQP built.
> - Skip interop tests if no URL.
> - 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.
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/types/Variant.h 1641910
> trunk/qpid/cpp/src/amqp.cmake 1641910
> trunk/qpid/cpp/src/qpid/amqp/CharSequence.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/Descriptor.h 1641910
> trunk/qpid/cpp/src/qpid/amqp/Descriptor.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1641910
> trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1641910
> trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1641910
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Exception.h 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1641910
> trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SenderHandle.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.h 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.cpp 1641910
> trunk/qpid/cpp/src/qpid/messaging/amqp/SessionHandle.cpp 1641910
> 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 1641910
> trunk/qpid/cpp/src/qpid/types/encodings.h 1641910
> trunk/qpid/cpp/src/tests/CMakeLists.txt 1641910
> trunk/qpid/cpp/src/tests/Variant.cpp 1641910
> trunk/qpid/cpp/src/tests/brokertest.py 1641910
> trunk/qpid/cpp/src/tests/ha_test.py 1641910
> trunk/qpid/cpp/src/tests/ha_tests.py 1641910
> trunk/qpid/cpp/src/tests/interop_tests.py PRE-CREATION
> trunk/qpid/cpp/src/tests/qpid-receive.cpp 1641910
> trunk/qpid/cpp/src/tests/test_store.cpp 1641910
> trunk/qpid/tests/src/py/qpid_tests/broker_1_0/__init__.py 1641910
> 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
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: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 Nov. 27, 2014, 4:17 p.m.)
Review request for qpid and Gordon Sim.
Summary (updated)
-----------------
QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.
Repository: qpid
Description (updated)
-------
Notes:
- the basic interop_tests are passing on qpidd, not on active or java broker.
- this branch requires the proton examples branch, won't work on proton master till all the blocking proton JIRAS for QPID-4710 are fixed.
- several tests are still failing including ha_tests, swig_python_tests, interlink_tests
QPID-4710: 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
QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.
Implements the "transactoinal retire and settle immediately" option for
transactions as specified in AMQP 1.0 in the qpid::messaging C++ client.
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.
Added interop_tests.py: transaction tests that can be run against any broker (env QPID_INTEROP_URL)
Fixes to existing client code:
- Fix use of pn_drain API in C++ client to work with C++ and Java brokers.
- amqp::Exception derive from qpid::Exception
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.
QPID-4710: Additional transaction tests
Enable transaction tests in ha_tests over AMQP 1.0.
Minor fixes
- Only run interop tests if AMQP built.
- Skip interop tests if no URL.
- 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.
Diffs (updated)
-----
trunk/qpid/cpp/include/qpid/types/Variant.h 1641910
trunk/qpid/cpp/src/amqp.cmake 1641910
trunk/qpid/cpp/src/qpid/amqp/CharSequence.cpp 1641910
trunk/qpid/cpp/src/qpid/amqp/Descriptor.h 1641910
trunk/qpid/cpp/src/qpid/amqp/Descriptor.cpp 1641910
trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1641910
trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1641910
trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1641910
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1641910
trunk/qpid/cpp/src/qpid/broker/amqp/Exception.h 1641910
trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1641910
trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1641910
trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1641910
trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.h 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/PnData.cpp 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.h 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/SenderHandle.cpp 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.h 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/SessionContext.cpp 1641910
trunk/qpid/cpp/src/qpid/messaging/amqp/SessionHandle.cpp 1641910
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 1641910
trunk/qpid/cpp/src/qpid/types/encodings.h 1641910
trunk/qpid/cpp/src/tests/CMakeLists.txt 1641910
trunk/qpid/cpp/src/tests/Variant.cpp 1641910
trunk/qpid/cpp/src/tests/brokertest.py 1641910
trunk/qpid/cpp/src/tests/ha_test.py 1641910
trunk/qpid/cpp/src/tests/ha_tests.py 1641910
trunk/qpid/cpp/src/tests/interop_tests.py PRE-CREATION
trunk/qpid/cpp/src/tests/qpid-receive.cpp 1641910
trunk/qpid/cpp/src/tests/test_store.cpp 1641910
trunk/qpid/tests/src/py/qpid_tests/broker_1_0/__init__.py 1641910
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