qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Conway" <acon...@redhat.com>
Subject Re: Review Request 28209: QPID-4710: [AMQP 1.0] Support for transactions in qpid::messaging C++ client.
Date Thu, 27 Nov 2014 17:49:41 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message