You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Sam Joyce <sj...@redhat.com> on 2009/12/22 20:59:05 UTC
C++ usage of CopyOnWriteArray : background; adding federation for
the HeadersExchange
Hi Folks,
I'm fairly new to the list, about 3 working weeks, so if I get a few
terms wrong, please bear with me!
I'm doing work on adding federation support for the HeadersExchange and
the XMLExchange, Headers first, and then once that's complete, XML
Exchange will follow as its basically the same pattern.
There were basically several issues to address: (HeadersExchange)
1) adding support for dynamic bindings
2) updating the code to allow the qpid-route command to be used to
create routes between exchanges of type "headers" (support for fedOps
etc.) e.g. after creating a "link" the following cmd should work:
"qpid-route dynamic add localhost:5673 localhost:5672 amq.match"
3) updating the code to allow for the federated propagation of the
additional headers/args used in creating the binding, (x-match plus the
actual binding headers) so that messages are correctly propagated
between brokers. test case for this is producing messages to one broker,
and reading them from the other. I'd attach examples, but I understand
attachments get stripped on this list :( I do have a patch of WIP if
anyone's interested, that I can mail directly. Note: its heavy with
trace statements so definitively WIP.
4) updating the code to propagate unbind() operations
5) adding support for reOrigion when things get a bit flappy. (flappy is
a tech term :) )
I've had good success so far, and have successfully completed 1), 2) &
3) in that messages are now being federated across instances of header
exchanges in multiple brokers.
But I need to implement the unbind and reOrigion cases for completeness
before I can submit a proper patch. both of these cases require a lookup
of an existing binding and updating the details of their associated
fedBinding instances. In the headers exchange, unlike the
Direct/Topic/Fanout exchanges, there needs to be a 1:1 affinity between
a binding and a fedBinding. These other exchanges use a simple match
based on either a match pattern or a routingKey.
I've had some discussion with some folks on this already off list, but I
wanted to get some feedback from a wider audience, as I've only been in
this code for for a short time. So here goes:
The current headers Exchange uses a CopyOnWriteArray (def in
cpp/src/qpid/sys) There are several examples of using this class in the
exchange context; the DirectExchange. However, my case is slightly
different as I believe I need to be able to do a lookup for an existing
binding using more that just a simple bindingKey.
The HeadersExchange (like the XML exchange), for anyone who's not been
near that code recently, uses matching during a route() call based on
the args/headers rather than a simple key or match pattern string.
so: In the HeadersExachnge
1) during the bind() call, a bindingKey is passed in, is this key of any
value at all? i.e. is there always a 1:1 affinity between a bindingKey
and a successful binding. Can I use this key for a successful retrieval
of the associated binding. looking at the impl of route() this doesn't
appear to be the case; its not even used in that method.
2) I've not been able to find any examples of a usage of a CopyOnWrite
array where a lookup is done and an element is then updated. That is,
all the lookup's appear to be const; meaning I can't update them.
For the federated unbind case I need to be able to lookup up an element
of a CopyOnWriteArray, and update the contents of its fedBinding. So
having access to a const element isn't of much use. here's my BoundKey
and Bindings defns et. al.:
struct BoundKey
{
Binding::shared_ptr binding;
FedBinding fedBinding;
BoundKey(Binding::shared_ptr binding_) : binding(binding_) {}
};
typedef qpid::sys::CopyOnWriteArray<BoundKey> Bindings;
Bindings bindings;
So, has anyone used a CopyOnWriteArray to retrieve an element and then
update it? All the uses I've been able to find so far are read only, so
a const retval is ok in those cases.
I need to be able to do a lookup and get a non-const reference that I
can update.
I tried the following, but keep getting a bunch of const related
compiler errors:
HeadersExchange::BoundKey*
HeadersExchange::findIfExists(Queue::shared_ptr queue, const
std::string& key, const FieldTable* args)
{
BoundKey * bk = 0;
Bindings::ConstPtr p = bindings.snapshot();
if (p.get()){
for (std::vector<BoundKey>::iterator i = p->begin(); i !=
p->end(); ++i) {
if (((*i).binding->queue == queue) && ((*i).binding->key ==
key) && equal((*i).binding->args, *args)) {
bk = &(*i);
break;
}
}
}
return bk;
}
I also thought about having a fn() that return a bool and takes an inout
param of type BoundKey&. Tried that and still got const compiler issues.
If anyone can give me a hand, I'd really appreciate it!
best regards,
Sam.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: C++ usage of CopyOnWriteArray : background; adding federation
for the HeadersExchange
Posted by Sam Joyce <sj...@redhat.com>.
Thanks Alan,
I like your method name better! I'll give this usage pattern a spin and
see how it goes! For my case, I think it may be a bit too generic; I'm
still leaning towards a modify_if() or an update_if() type method. As I
said, I'll give it a spin and see if it will fly!
all the best,
Sam
Alan Conway wrote:
> On 12/23/2009 06:13 AM, Sam Joyce wrote:
>> Hi Alan,
>> thanks for this. I hadn't realized this was the use pattern; more
>> comments in code please folks.
>
> Perhaps you can add some while you still have the new-user
> perspective, that's the best time to write documentation :)
>
>> looking at the add() fn, I see what you mean, it takes a copy, does the
>> add and then replaces the main array element.
>> I think CopyOnWriteArray is the correct type to use in the
>> HeadersExchange. Personally I like it because I get to use things like
>> add_unless() etc. (less coding for me) and I like the idea of a wrapper
>> class like this that protects the coder from having to manage the
>> locking directly; leads to less race conds because of misplaced locks.
>> From what you've mentioned below Alan, I'd guess the best option would
>> be to add a new method to CopyOnWriteArray, something like
>> replace_or_add(T& t, F f). the match function being the one to identify
>> an existing element. I'm open to ideas for a better method name :) I
>> think it should work a bit like add_unless(), except doing a
>> copy/remove/replace
>> thoughts?
>> regards,
>> Sam.
>>
>
> How about a general-purpose modify function like this:
> // t takes a const vector<T>& , m takes a non-const vector<T>&
> template <class Test, class Modifier>
> bool modify_if(Test t, Modifier m) {
> Mutex::ScopedLock l(lock);
> if (array && t(*array)) {
> ArrayPtr copy(new std::vector<T>(*array));
> m(*copy);
> array = copy;
> return true;
> }
> return false;
> }
>
> That will let you do anything you like to the array - you could
> rewrite all the modifiers in terms of this one (although they'd be a
> little less efficient because the current ones use the results of the
> test part in the modify part.)
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: C++ usage of CopyOnWriteArray : background; adding federation
for the HeadersExchange
Posted by Alan Conway <ac...@redhat.com>.
On 12/23/2009 11:23 AM, Andrew Stitcher wrote:
> On Wed, 2009-12-23 at 09:14 -0500, Alan Conway wrote:
>> ...
>> How about a general-purpose modify function like this:
>> // t takes a const vector<T>& , m takes a non-const vector<T>&
>> template<class Test, class Modifier>
>> bool modify_if(Test t, Modifier m) {
>> Mutex::ScopedLock l(lock);
>> if (array&& t(*array)) {
>> ArrayPtr copy(new std::vector<T>(*array));
>> m(*copy);
>> array = copy;
>> return true;
>> }
>> return false;
>> }
>>
>> That will let you do anything you like to the array - you could rewrite all the
>> modifiers in terms of this one (although they'd be a little less efficient
>> because the current ones use the results of the test part in the modify part.)
>
> Just a cautionary note: I wouldn't necessarily think that you always
> know better than the optimiser (unless you customarily look at generated
> assembler! life is too short). I'm saying that if t and m can actually
> be expanded inline then the code wouldn't necessarily be worse. Except
> to understand of course, but then my brain always starts to glaze over
> when there is more than one template parameter and I didn't write the
> code!
>
Agreed in general, in this particular case though the existing methods look like
(T part) iterator i = find(...); ...
(M part) do_something_with(i)
But if you want to re-implement this using a generic modify_if you have no way
in the M part to refer to the iterator computed in the T part, so you'd have to
do the search again.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: C++ usage of CopyOnWriteArray : background; adding federation
for the HeadersExchange
Posted by Andrew Stitcher <as...@redhat.com>.
On Wed, 2009-12-23 at 09:14 -0500, Alan Conway wrote:
> ...
> How about a general-purpose modify function like this:
> // t takes a const vector<T>& , m takes a non-const vector<T>&
> template <class Test, class Modifier>
> bool modify_if(Test t, Modifier m) {
> Mutex::ScopedLock l(lock);
> if (array && t(*array)) {
> ArrayPtr copy(new std::vector<T>(*array));
> m(*copy);
> array = copy;
> return true;
> }
> return false;
> }
>
> That will let you do anything you like to the array - you could rewrite all the
> modifiers in terms of this one (although they'd be a little less efficient
> because the current ones use the results of the test part in the modify part.)
Just a cautionary note: I wouldn't necessarily think that you always
know better than the optimiser (unless you customarily look at generated
assembler! life is too short). I'm saying that if t and m can actually
be expanded inline then the code wouldn't necessarily be worse. Except
to understand of course, but then my brain always starts to glaze over
when there is more than one template parameter and I didn't write the
code!
Andrew
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: C++ usage of CopyOnWriteArray : background; adding federation
for the HeadersExchange
Posted by Alan Conway <ac...@redhat.com>.
On 12/23/2009 06:13 AM, Sam Joyce wrote:
> Hi Alan,
> thanks for this. I hadn't realized this was the use pattern; more
> comments in code please folks.
Perhaps you can add some while you still have the new-user perspective, that's
the best time to write documentation :)
> looking at the add() fn, I see what you mean, it takes a copy, does the
> add and then replaces the main array element.
> I think CopyOnWriteArray is the correct type to use in the
> HeadersExchange. Personally I like it because I get to use things like
> add_unless() etc. (less coding for me) and I like the idea of a wrapper
> class like this that protects the coder from having to manage the
> locking directly; leads to less race conds because of misplaced locks.
> From what you've mentioned below Alan, I'd guess the best option would
> be to add a new method to CopyOnWriteArray, something like
> replace_or_add(T& t, F f). the match function being the one to identify
> an existing element. I'm open to ideas for a better method name :) I
> think it should work a bit like add_unless(), except doing a
> copy/remove/replace
> thoughts?
> regards,
> Sam.
>
How about a general-purpose modify function like this:
// t takes a const vector<T>& , m takes a non-const vector<T>&
template <class Test, class Modifier>
bool modify_if(Test t, Modifier m) {
Mutex::ScopedLock l(lock);
if (array && t(*array)) {
ArrayPtr copy(new std::vector<T>(*array));
m(*copy);
array = copy;
return true;
}
return false;
}
That will let you do anything you like to the array - you could rewrite all the
modifiers in terms of this one (although they'd be a little less efficient
because the current ones use the results of the test part in the modify part.)
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: C++ usage of CopyOnWriteArray : background; adding federation
for the HeadersExchange
Posted by Sam Joyce <sj...@redhat.com>.
Hi Alan,
thanks for this. I hadn't realized this was the use pattern; more
comments in code please folks.
looking at the add() fn, I see what you mean, it takes a copy, does the
add and then replaces the main array element.
I think CopyOnWriteArray is the correct type to use in the
HeadersExchange. Personally I like it because I get to use things like
add_unless() etc. (less coding for me) and I like the idea of a wrapper
class like this that protects the coder from having to manage the
locking directly; leads to less race conds because of misplaced locks.
From what you've mentioned below Alan, I'd guess the best option would
be to add a new method to CopyOnWriteArray, something like
replace_or_add(T& t, F f). the match function being the one to identify
an existing element. I'm open to ideas for a better method name :) I
think it should work a bit like add_unless(), except doing a
copy/remove/replace
thoughts?
regards,
Sam.
Alan Conway wrote:
> On 12/22/2009 02:59 PM, Sam Joyce wrote:
>> Hi Folks,
>> I'm fairly new to the list, about 3 working weeks, so if I get a few
>> terms wrong, please bear with me!
>>
>> I'm doing work on adding federation support for the HeadersExchange and
>> the XMLExchange, Headers first, and then once that's complete, XML
>> Exchange will follow as its basically the same pattern.
>>
>> There were basically several issues to address: (HeadersExchange)
>> 1) adding support for dynamic bindings
>> 2) updating the code to allow the qpid-route command to be used to
>> create routes between exchanges of type "headers" (support for fedOps
>> etc.) e.g. after creating a "link" the following cmd should work:
>> "qpid-route dynamic add localhost:5673 localhost:5672 amq.match"
>> 3) updating the code to allow for the federated propagation of the
>> additional headers/args used in creating the binding, (x-match plus the
>> actual binding headers) so that messages are correctly propagated
>> between brokers. test case for this is producing messages to one broker,
>> and reading them from the other. I'd attach examples, but I understand
>> attachments get stripped on this list :( I do have a patch of WIP if
>> anyone's interested, that I can mail directly. Note: its heavy with
>> trace statements so definitively WIP.
>> 4) updating the code to propagate unbind() operations
>> 5) adding support for reOrigion when things get a bit flappy. (flappy is
>> a tech term :) )
>>
>> I've had good success so far, and have successfully completed 1), 2) &
>> 3) in that messages are now being federated across instances of header
>> exchanges in multiple brokers.
>>
>> But I need to implement the unbind and reOrigion cases for completeness
>> before I can submit a proper patch. both of these cases require a lookup
>> of an existing binding and updating the details of their associated
>> fedBinding instances. In the headers exchange, unlike the
>> Direct/Topic/Fanout exchanges, there needs to be a 1:1 affinity between
>> a binding and a fedBinding. These other exchanges use a simple match
>> based on either a match pattern or a routingKey.
>>
>> I've had some discussion with some folks on this already off list, but I
>> wanted to get some feedback from a wider audience, as I've only been in
>> this code for for a short time. So here goes:
>>
>> The current headers Exchange uses a CopyOnWriteArray (def in
>> cpp/src/qpid/sys) There are several examples of using this class in the
>> exchange context; the DirectExchange. However, my case is slightly
>> different as I believe I need to be able to do a lookup for an existing
>> binding using more that just a simple bindingKey.
>> The HeadersExchange (like the XML exchange), for anyone who's not been
>> near that code recently, uses matching during a route() call based on
>> the args/headers rather than a simple key or match pattern string.
>>
>> so: In the HeadersExachnge
>> 1) during the bind() call, a bindingKey is passed in, is this key of any
>> value at all? i.e. is there always a 1:1 affinity between a bindingKey
>> and a successful binding. Can I use this key for a successful retrieval
>> of the associated binding. looking at the impl of route() this doesn't
>> appear to be the case; its not even used in that method.
>> 2) I've not been able to find any examples of a usage of a CopyOnWrite
>> array where a lookup is done and an element is then updated. That is,
>> all the lookup's appear to be const; meaning I can't update them.
>>
>> For the federated unbind case I need to be able to lookup up an element
>> of a CopyOnWriteArray, and update the contents of its fedBinding. So
>> having access to a const element isn't of much use. here's my BoundKey
>> and Bindings defns et. al.:
>>
>> struct BoundKey
>> {
>> Binding::shared_ptr binding;
>> FedBinding fedBinding;
>> BoundKey(Binding::shared_ptr binding_) : binding(binding_) {}
>> };
>>
>> typedef qpid::sys::CopyOnWriteArray<BoundKey> Bindings;
>> Bindings bindings;
>>
>>
>> So, has anyone used a CopyOnWriteArray to retrieve an element and then
>> update it? All the uses I've been able to find so far are read only, so
>> a const retval is ok in those cases.
>> I need to be able to do a lookup and get a non-const reference that I
>> can update.
>>
>> I tried the following, but keep getting a bunch of const related
>> compiler errors:
>>
>> HeadersExchange::BoundKey*
>> HeadersExchange::findIfExists(Queue::shared_ptr queue, const
>> std::string& key, const FieldTable* args)
>> {
>> BoundKey * bk = 0;
>>
>> Bindings::ConstPtr p = bindings.snapshot();
>> if (p.get()){
>> for (std::vector<BoundKey>::iterator i = p->begin(); i != p->end();
>> ++i) {
>> if (((*i).binding->queue == queue) && ((*i).binding->key == key) &&
>> equal((*i).binding->args, *args)) {
>> bk = &(*i);
>> break;
>> }
>> }
>> }
>> return bk;
>> }
>>
>>
>> I also thought about having a fn() that return a bool and takes an inout
>> param of type BoundKey&. Tried that and still got const compiler issues.
>>
>> If anyone can give me a hand, I'd really appreciate it!
>>
>
> The goal of CopyOnWriteArray is to provide low-locking thread safety.
> The idea is that if you want to modify the array you actually create a
> new copy and modify that, leaving the old copy in place for any
> threads that are still using it. So you definitely don't want to get a
> non-const reference to an existing element and modify it in place -
> those compile errors are protcting you :)
>
> As it stands you could do
> if (array.remove_if(...)) array.add(...)
>
> If there's a performance issue with doing it in two steps like that
> you'd need to add a new function to copy and mutate the array, I'd
> have to think a bit as to the best signature for such a thing.
>
>
> Cheers,
> Alan.
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: C++ usage of CopyOnWriteArray : background; adding federation
for the HeadersExchange
Posted by Alan Conway <ac...@redhat.com>.
On 12/22/2009 02:59 PM, Sam Joyce wrote:
> Hi Folks,
> I'm fairly new to the list, about 3 working weeks, so if I get a few
> terms wrong, please bear with me!
>
> I'm doing work on adding federation support for the HeadersExchange and
> the XMLExchange, Headers first, and then once that's complete, XML
> Exchange will follow as its basically the same pattern.
>
> There were basically several issues to address: (HeadersExchange)
> 1) adding support for dynamic bindings
> 2) updating the code to allow the qpid-route command to be used to
> create routes between exchanges of type "headers" (support for fedOps
> etc.) e.g. after creating a "link" the following cmd should work:
> "qpid-route dynamic add localhost:5673 localhost:5672 amq.match"
> 3) updating the code to allow for the federated propagation of the
> additional headers/args used in creating the binding, (x-match plus the
> actual binding headers) so that messages are correctly propagated
> between brokers. test case for this is producing messages to one broker,
> and reading them from the other. I'd attach examples, but I understand
> attachments get stripped on this list :( I do have a patch of WIP if
> anyone's interested, that I can mail directly. Note: its heavy with
> trace statements so definitively WIP.
> 4) updating the code to propagate unbind() operations
> 5) adding support for reOrigion when things get a bit flappy. (flappy is
> a tech term :) )
>
> I've had good success so far, and have successfully completed 1), 2) &
> 3) in that messages are now being federated across instances of header
> exchanges in multiple brokers.
>
> But I need to implement the unbind and reOrigion cases for completeness
> before I can submit a proper patch. both of these cases require a lookup
> of an existing binding and updating the details of their associated
> fedBinding instances. In the headers exchange, unlike the
> Direct/Topic/Fanout exchanges, there needs to be a 1:1 affinity between
> a binding and a fedBinding. These other exchanges use a simple match
> based on either a match pattern or a routingKey.
>
> I've had some discussion with some folks on this already off list, but I
> wanted to get some feedback from a wider audience, as I've only been in
> this code for for a short time. So here goes:
>
> The current headers Exchange uses a CopyOnWriteArray (def in
> cpp/src/qpid/sys) There are several examples of using this class in the
> exchange context; the DirectExchange. However, my case is slightly
> different as I believe I need to be able to do a lookup for an existing
> binding using more that just a simple bindingKey.
> The HeadersExchange (like the XML exchange), for anyone who's not been
> near that code recently, uses matching during a route() call based on
> the args/headers rather than a simple key or match pattern string.
>
> so: In the HeadersExachnge
> 1) during the bind() call, a bindingKey is passed in, is this key of any
> value at all? i.e. is there always a 1:1 affinity between a bindingKey
> and a successful binding. Can I use this key for a successful retrieval
> of the associated binding. looking at the impl of route() this doesn't
> appear to be the case; its not even used in that method.
> 2) I've not been able to find any examples of a usage of a CopyOnWrite
> array where a lookup is done and an element is then updated. That is,
> all the lookup's appear to be const; meaning I can't update them.
>
> For the federated unbind case I need to be able to lookup up an element
> of a CopyOnWriteArray, and update the contents of its fedBinding. So
> having access to a const element isn't of much use. here's my BoundKey
> and Bindings defns et. al.:
>
> struct BoundKey
> {
> Binding::shared_ptr binding;
> FedBinding fedBinding;
> BoundKey(Binding::shared_ptr binding_) : binding(binding_) {}
> };
>
> typedef qpid::sys::CopyOnWriteArray<BoundKey> Bindings;
> Bindings bindings;
>
>
> So, has anyone used a CopyOnWriteArray to retrieve an element and then
> update it? All the uses I've been able to find so far are read only, so
> a const retval is ok in those cases.
> I need to be able to do a lookup and get a non-const reference that I
> can update.
>
> I tried the following, but keep getting a bunch of const related
> compiler errors:
>
> HeadersExchange::BoundKey*
> HeadersExchange::findIfExists(Queue::shared_ptr queue, const
> std::string& key, const FieldTable* args)
> {
> BoundKey * bk = 0;
>
> Bindings::ConstPtr p = bindings.snapshot();
> if (p.get()){
> for (std::vector<BoundKey>::iterator i = p->begin(); i != p->end(); ++i) {
> if (((*i).binding->queue == queue) && ((*i).binding->key == key) &&
> equal((*i).binding->args, *args)) {
> bk = &(*i);
> break;
> }
> }
> }
> return bk;
> }
>
>
> I also thought about having a fn() that return a bool and takes an inout
> param of type BoundKey&. Tried that and still got const compiler issues.
>
> If anyone can give me a hand, I'd really appreciate it!
>
The goal of CopyOnWriteArray is to provide low-locking thread safety. The idea
is that if you want to modify the array you actually create a new copy and
modify that, leaving the old copy in place for any threads that are still using
it. So you definitely don't want to get a non-const reference to an existing
element and modify it in place - those compile errors are protcting you :)
As it stands you could do
if (array.remove_if(...)) array.add(...)
If there's a performance issue with doing it in two steps like that you'd need
to add a new function to copy and mutate the array, I'd have to think a bit as
to the best signature for such a thing.
Cheers,
Alan.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: C++ usage of CopyOnWriteArray : background; adding federation
for the HeadersExchange
Posted by Gordon Sim <gs...@redhat.com>.
Just in case this part of the question is still unanswered:
On 12/22/2009 07:59 PM, Sam Joyce wrote:
> 1) during the bind() call, a bindingKey is passed in, is this key of any
> value at all? i.e. is there always a 1:1 affinity between a bindingKey
> and a successful binding. Can I use this key for a successful retrieval
> of the associated binding. looking at the impl of route() this doesn't
> appear to be the case; its not even used in that method.
The binding key is used on unbind to identify the correct binding to
remove. As you point out it is not used (by the headers exchange) when
routing messages.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org