You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@qpid.apache.org by Chug Rolke <cr...@redhat.com> on 2013/03/19 22:02:50 UTC

Review Request: C++ Broker 'rebind' to steer messages from one queue to others

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

Review request for qpid, Gordon Sim and Ted Ross.


Summary (updated)
-----------------

C++ Broker 'rebind' to steer messages from one queue to others


Description (updated)
-------

To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
A third option would be (3) to freeze each individual binding while it is moved. 

Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.

The change proposed here is a prototype that implements lightweight strategy #3.

This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
* Queue settings and status.
* Management method to trigger the rebind.
* Exchange methods to effect the rebind for each exchange type.
* Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
* Some test suite code to trigger the rebind method and its error paths.
* A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.

Before this feature could transition to 'Ship It' it still needs:
* An ACL property to control specification of rebind queues.
* A handler for queue deletion while the queue is part of a rebind set.
* Code to restore a queue from rebind state back to normal.
* Proof that traffic can be properly recovered through a rebind


This addresses bug QPID-4650.
    https://issues.apache.org/jira/browse/QPID-4650


Diffs (updated)
-----

  trunk/qpid/cpp/src/qpid/broker/Broker.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/Exchange.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/Link.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/Queue.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/QueueBindings.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/QueueRebindHelpers.h PRE-CREATION 
  trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1458431 
  trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1458431 
  trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1458431 
  trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1458431 
  trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1458431 
  trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1458431 
  trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1458431 
  trunk/qpid/cpp/src/qpid/sys/CopyOnWriteArray.h 1458431 
  trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1458431 
  trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1458431 
  trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION 
  trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION 
  trunk/qpid/specs/management-schema.xml 1458431 
  trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1458431 

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


Testing (updated)
-------

Several tests to exercise rebind code paths.


Thanks,

Chug Rolke

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Chug Rolke <cr...@redhat.com>.


> On March 28, 2013, 7:50 p.m., Alan Conway wrote:
> > I'm not completely following this. It looks like declaring an overflow-partnership sends messages to queue X to queue Y and vice versa? Why would you want to redirect in both directions? Also I'm a bit puzzled by the name overflow-partner. What's overflowing?

There was a PDF attached to the Jira. The essence is that the broker is providing a real-time delivery service and a consumer goes slow. Rather than having this consumer queue go too big or drop messages a 'backup agent' steps in and starts receiving all the consumer's messages on a different queue. The backup agent is big and fast and buffers the consumer data until some time at which the consumer queue 'has more room'. Then the backup agent starts feeding messages into the original queue.


> On March 28, 2013, 7:50 p.m., Alan Conway wrote:
> > trunk/qpid/cpp/src/qpid/broker/Broker.cpp, line 1104
> > <https://reviews.apache.org/r/10020/diff/3/?file=274539#file274539line1104>
> >
> >     I can't find anywhere that the value of isOverflowSource is used. Why is it needed?

This is anticipating a command that takes the queue pair out of the swapped state. The command must be directed at the same queue that received the original swap command.


> On March 28, 2013, 7:50 p.m., Alan Conway wrote:
> > trunk/qpid/cpp/src/qpid/broker/Broker.cpp, line 1118
> > <https://reviews.apache.org/r/10020/diff/3/?file=274539#file274539line1118>
> >
> >     suggest mention dstQueue in the log message

will do


- Chug


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


On March 26, 2013, 1:54 a.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated March 26, 2013, 1:54 a.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1460944 
>   trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1460944 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1460944 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Alan Conway <ac...@redhat.com>.


> On March 28, 2013, 7:50 p.m., Alan Conway wrote:
> > I'm not completely following this. It looks like declaring an overflow-partnership sends messages to queue X to queue Y and vice versa? Why would you want to redirect in both directions? Also I'm a bit puzzled by the name overflow-partner. What's overflowing?
> 
> Chug Rolke wrote:
>     There was a PDF attached to the Jira. The essence is that the broker is providing a real-time delivery service and a consumer goes slow. Rather than having this consumer queue go too big or drop messages a 'backup agent' steps in and starts receiving all the consumer's messages on a different queue. The backup agent is big and fast and buffers the consumer data until some time at which the consumer queue 'has more room'. Then the backup agent starts feeding messages into the original queue.

Thanks, that explains it.


- Alan


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


On March 26, 2013, 1:54 a.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated March 26, 2013, 1:54 a.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1460944 
>   trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1460944 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1460944 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Alan Conway <ac...@redhat.com>.

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


I'm not completely following this. It looks like declaring an overflow-partnership sends messages to queue X to queue Y and vice versa? Why would you want to redirect in both directions? Also I'm a bit puzzled by the name overflow-partner. What's overflowing? 


trunk/qpid/cpp/src/qpid/broker/Broker.cpp
<https://reviews.apache.org/r/10020/#comment38724>

    I can't find anywhere that the value of isOverflowSource is used. Why is it needed?



trunk/qpid/cpp/src/qpid/broker/Broker.cpp
<https://reviews.apache.org/r/10020/#comment38723>

    suggest mention dstQueue in the log message


- Alan Conway


On March 26, 2013, 1:54 a.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated March 26, 2013, 1:54 a.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1460944 
>   trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1460944 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1460944 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Alan Conway <ac...@redhat.com>.

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


This looks much nicer! Only minor comments.


trunk/qpid/cpp/src/qpid/broker/Broker.cpp
<https://reviews.apache.org/r/10020/#comment40731>

    I would make this info rather than notice



trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/10020/#comment40732>

    Can this be private or protected?
    Don't like the name but can't think of a better.



trunk/qpid/cpp/src/tests/queue_redirect.py
<https://reviews.apache.org/r/10020/#comment40733>

    Mixing function name conventions: python should be lowercase_with_underscore.



trunk/qpid/cpp/src/tests/queue_redirect.py
<https://reviews.apache.org/r/10020/#comment40734>

    Terminology: redirect rather than rebind.


- Alan Conway


On April 3, 2013, 10:08 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated April 3, 2013, 10:08 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1464210 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1464210 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1464210 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1464210 
>   trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1464210 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1464210 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Gordon Sim <gs...@redhat.com>.


> On April 30, 2013, 12:13 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Queue.h, line 172
> > <https://reviews.apache.org/r/10020/diff/5/?file=285775#file285775line172>
> >
> >     What happens if someone deletes the queues without explicitly cancelling the redirect? There is a circular reference that needs to be broken for the underlying objects to be deleted.
> 
> Chug Rolke wrote:
>     If either queue is deleted then the references are broken and there is no attempt to move messages from the backup queue to the target queue. Code to do that is in Broker::deleteQueue() at line 1312.

Sorry, my mistake! I missed that.


> On April 30, 2013, 12:13 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 240
> > <https://reviews.apache.org/r/10020/diff/5/?file=285776#file285776line240>
> >
> >     This means anything sent to the target will be redirected to the source, right? Is there a reason for that? Seems a little unintuitive to me, at least without further explanation.
> 
> Chug Rolke wrote:
>     In previous renditions of this feature I had proposed a special binding that does not get redirected. This becomes tricky to code and to use as an end user. The redirect is basically a swap which has some of the attributes of a code structure swap since no new resources are allocated nor can they fail to be allocated. There is further explanation in a doc connected to the jira https://issues.apache.org/jira/secure/attachment/12576800/SlowConsumerRedirect_v1.0.pdf
>

I get why messages delivered to source are redirected to target. I'm still a little unclear on why messages sent to the target are redirected to source. What is the use case for the reverse direction? Maybe it would be clearer to use terminology other than source and target (e.g. original and shadow or copy)?


> On April 30, 2013, 12:13 p.m., Gordon Sim wrote:
> > trunk/qpid/specs/management-schema.xml, line 579
> > <https://reviews.apache.org/r/10020/diff/5/?file=285780#file285780line579>
> >
> >     Not crazy about the naming. I think 'cancelled' might be a little clearer than 'destroy'.
> 
> Chug Rolke wrote:
>     OK.

That is a fairly minor quibble. Not a blocker. It just seems clearer what has happened when you say 'queue redirect cancelled' as compared to 'queue redirect destroyed' which to me at least makes me wonder if the redirect queue was deleted also.


- Gordon


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


On April 29, 2013, 6:12 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated April 29, 2013, 6:12 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1477103 
>   trunk/qpid/cpp/src/tests/CMakeLists.txt 1477103 
>   trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1477103 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1477103 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Chug Rolke <cr...@redhat.com>.


> On April 30, 2013, 12:13 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Queue.h, line 172
> > <https://reviews.apache.org/r/10020/diff/5/?file=285775#file285775line172>
> >
> >     What happens if someone deletes the queues without explicitly cancelling the redirect? There is a circular reference that needs to be broken for the underlying objects to be deleted.

If either queue is deleted then the references are broken and there is no attempt to move messages from the backup queue to the target queue. Code to do that is in Broker::deleteQueue() at line 1312.


> On April 30, 2013, 12:13 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 240
> > <https://reviews.apache.org/r/10020/diff/5/?file=285776#file285776line240>
> >
> >     This means anything sent to the target will be redirected to the source, right? Is there a reason for that? Seems a little unintuitive to me, at least without further explanation.

In previous renditions of this feature I had proposed a special binding that does not get redirected. This becomes tricky to code and to use as an end user. The redirect is basically a swap which has some of the attributes of a code structure swap since no new resources are allocated nor can they fail to be allocated. There is further explanation in a doc connected to the jira https://issues.apache.org/jira/secure/attachment/12576800/SlowConsumerRedirect_v1.0.pdf


> On April 30, 2013, 12:13 p.m., Gordon Sim wrote:
> > trunk/qpid/specs/management-schema.xml, line 287
> > <https://reviews.apache.org/r/10020/diff/5/?file=285780#file285780line287>
> >
> >     Do we need three separate variables here? Why not simply 'redirected', which has the name of the target queue if redirection is active and is blank otherwise?

It could be trimmed down to two variables: the name of the peer and a bool to identify which is the source and which is the target. The bool determines to which queue the accumulated messages are moved when the partnerwhip is broken.


> On April 30, 2013, 12:13 p.m., Gordon Sim wrote:
> > trunk/qpid/specs/management-schema.xml, line 579
> > <https://reviews.apache.org/r/10020/diff/5/?file=285780#file285780line579>
> >
> >     Not crazy about the naming. I think 'cancelled' might be a little clearer than 'destroy'.

OK.


- Chug


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


On April 29, 2013, 6:12 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated April 29, 2013, 6:12 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1477103 
>   trunk/qpid/cpp/src/tests/CMakeLists.txt 1477103 
>   trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1477103 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1477103 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Gordon Sim <gs...@redhat.com>.

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



trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/10020/#comment41160>

    What happens if someone deletes the queues without explicitly cancelling the redirect? There is a circular reference that needs to be broken for the underlying objects to be deleted.



trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/10020/#comment41161>

    This means anything sent to the target will be redirected to the source, right? Is there a reason for that? Seems a little unintuitive to me, at least without further explanation.



trunk/qpid/specs/management-schema.xml
<https://reviews.apache.org/r/10020/#comment41158>

    Do we need three separate variables here? Why not simply 'redirected', which has the name of the target queue if redirection is active and is blank otherwise?



trunk/qpid/specs/management-schema.xml
<https://reviews.apache.org/r/10020/#comment41159>

    Not crazy about the naming. I think 'cancelled' might be a little clearer than 'destroy'.


- Gordon Sim


On April 29, 2013, 6:12 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated April 29, 2013, 6:12 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1477103 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1477103 
>   trunk/qpid/cpp/src/tests/CMakeLists.txt 1477103 
>   trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1477103 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1477103 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Alan Conway <ac...@redhat.com>.

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

Ship it!


Ship It!

- Alan Conway


On May 3, 2013, 7:15 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated May 3, 2013, 7:15 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1478892 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1478892 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1478892 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1478892 
>   trunk/qpid/cpp/src/tests/CMakeLists.txt 1478892 
>   trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1478892 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1478892 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Gordon Sim <gs...@redhat.com>.

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

Ship it!


I can live with this. Thanks for your patience Chuck!

- Gordon Sim


On May 3, 2013, 7:15 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated May 3, 2013, 7:15 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1478892 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1478892 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1478892 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1478892 
>   trunk/qpid/cpp/src/tests/CMakeLists.txt 1478892 
>   trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1478892 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1478892 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Chug Rolke <cr...@redhat.com>.

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

(Updated May 3, 2013, 7:15 p.m.)

Review request for qpid, Gordon Sim and Ted Ross.

Changes
-------

Rename redirect stopped event to QueueRedirectCancelled.
Reduced mgmt status indicators to one string and one bool.

Description
-------

To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed.
Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed.
A third option would be (3) to freeze each individual binding while it is moved.

Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.

The change proposed here is a prototype that implements lightweight strategy #3.

This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
* Queue settings and status.
* Management method to trigger the rebind.
* Exchange methods to effect the rebind for each exchange type.
* Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
* Some test suite code to trigger the rebind method and its error paths.
* A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.

Before this feature could transition to 'Ship It' it still needs:
* An ACL property to control specification of rebind queues.
* A handler for queue deletion while the queue is part of a rebind set.
* Code to restore a queue from rebind state back to normal.
* Proof that traffic can be properly recovered through a rebind

This addresses bug QPID-4650.
https://issues.apache.org/jira/browse/QPID-4650

Diffs (updated)
-----

trunk/qpid/cpp/src/qpid/broker/Broker.h 1478892
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1478892
trunk/qpid/cpp/src/qpid/broker/Queue.h 1478892
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1478892
trunk/qpid/cpp/src/tests/CMakeLists.txt 1478892
trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION
trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION
trunk/qpid/specs/management-schema.xml 1478892
trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1478892

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

Testing
-------

Several tests to exercise rebind code paths.

Thanks,

Chug Rolke

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Chug Rolke <cr...@redhat.com>.

(Updated April 29, 2013, 6:12 p.m.)

Review request for qpid, Gordon Sim and Ted Ross.

Changes
-------

Another review answering all of Alan's review comments of April 25.

Description
-------

The change proposed here is a prototype that implements lightweight strategy #3.

This addresses bug QPID-4650.
https://issues.apache.org/jira/browse/QPID-4650

Diffs (updated)
-----

trunk/qpid/cpp/src/qpid/broker/Broker.h 1477103
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1477103
trunk/qpid/cpp/src/qpid/broker/Queue.h 1477103
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1477103
trunk/qpid/cpp/src/tests/CMakeLists.txt 1477103
trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION
trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION
trunk/qpid/specs/management-schema.xml 1477103
trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1477103

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

Testing
-------

Several tests to exercise rebind code paths.

Thanks,

Chug Rolke

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Chug Rolke <cr...@redhat.com>.

(Updated April 3, 2013, 10:08 p.m.)

Review request for qpid, Gordon Sim and Ted Ross.

Changes
-------

This version incorporates review comments and is a more polished product.
A pdf file describes in pictures what this patch does https://issues.apache.org/jira/secure/attachment/12576800/SlowConsumerRedirect_v1.0.pdf

This patch destroys a redirect pairing when either of the queues is deleted. In that case there is no attempt at save messages in the backup queue or put them back into the source queue. The management method has a nominal message mover to put messages left on the target queue back into the source queue when the redirect is destroyed. This, however, is not a finished feature.

Description
-------

The change proposed here is a prototype that implements lightweight strategy #3.

This addresses bug QPID-4650.
https://issues.apache.org/jira/browse/QPID-4650

Diffs (updated)
-----

trunk/qpid/cpp/src/qpid/broker/Broker.h 1464210
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1464210
trunk/qpid/cpp/src/qpid/broker/Queue.h 1464210
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1464210
trunk/qpid/cpp/src/tests/queue_redirect.py PRE-CREATION
trunk/qpid/cpp/src/tests/run_queue_redirect PRE-CREATION
trunk/qpid/specs/management-schema.xml 1464210
trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1464210

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

Testing
-------

Several tests to exercise rebind code paths.

Thanks,

Chug Rolke

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Chug Rolke <cr...@redhat.com>.


> On March 27, 2013, 2:56 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Queue.h, line 171
> > <https://reviews.apache.org/r/10020/diff/3/?file=274540#file274540line171>
> >
> >     Is there a reason to use a raw pointer rather than a shared pointer? (Not that I see any insurmountable issue either way)

Shared pointer - done.


> On March 27, 2013, 2:56 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Broker.h, line 167
> > <https://reviews.apache.org/r/10020/diff/3/?file=274538#file274538line167>
> >
> >     I wonder if we could come up with a nicer name...

How does 'redirect' sound?


- Chug


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


On March 26, 2013, 1:54 a.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated March 26, 2013, 1:54 a.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1460944 
>   trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1460944 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1460944 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Gordon Sim <gs...@redhat.com>.


> On March 27, 2013, 2:56 p.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Broker.h, line 167
> > <https://reviews.apache.org/r/10020/diff/3/?file=274538#file274538line167>
> >
> >     I wonder if we could come up with a nicer name...
> 
> Chug Rolke wrote:
>     How does 'redirect' sound?

much better!


- Gordon


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


On March 26, 2013, 1:54 a.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated March 26, 2013, 1:54 a.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1460944 
>   trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1460944 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1460944 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Gordon Sim <gs...@redhat.com>.

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



trunk/qpid/cpp/src/qpid/broker/Broker.h
<https://reviews.apache.org/r/10020/#comment38627>

    I wonder if we could come up with a nicer name... 



trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/10020/#comment38629>

    Is there a reason to use a raw pointer rather than a shared pointer? (Not that I see any insurmountable issue either way)



trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/10020/#comment38628>

    Does deliverTo() need to be public?


- Gordon Sim


On March 26, 2013, 1:54 a.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10020/
> -----------------------------------------------------------
> 
> (Updated March 26, 2013, 1:54 a.m.)
> 
> 
> Review request for qpid, Gordon Sim and Ted Ross.
> 
> 
> Description
> -------
> 
> To get a true 'atomic rebind' one should (1) freeze all traffic going through all exchanges that have bindings to be changed. 
> Failing that, one could (2) freeze all traffic going through each exchange while that exchange's bindings are changed. 
> A third option would be (3) to freeze each individual binding while it is moved. 
> 
> Options (1) and (2) require per-message locking at the exchange level; these locks do not exist today and adding them would undoubtedly introduce performance degredation. For discussion please see https://issues.apache.org/jira/browse/QPID-4616 and review comments at https://reviews.apache.org/r/9698/
> Option (3) requires no new locking and could leverage the locking methods that the exchanges already use.
> 
> The change proposed here is a prototype that implements lightweight strategy #3.
> 
> This review exposes what the feature is trying to accomplish and the basic framework is complete. It has:
> * Queue settings and status.
> * Management method to trigger the rebind.
> * Exchange methods to effect the rebind for each exchange type.
> * Broker changes to handle queues in the 'rebound' state where bind/unbind operations on them actually go to other queues.
> * Some test suite code to trigger the rebind method and its error paths.
> * A qpid.rebind exchange for backup agents to use to refill queues that are in rebind state and not accessable through normal bindings.
> 
> Before this feature could transition to 'Ship It' it still needs:
> * An ACL property to control specification of rebind queues.
> * A handler for queue deletion while the queue is part of a rebind set.
> * Code to restore a queue from rebind state back to normal.
> * Proof that traffic can be properly recovered through a rebind
> 
> 
> This addresses bug QPID-4650.
>     https://issues.apache.org/jira/browse/QPID-4650
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/Broker.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.h 1460944 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1460944 
>   trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION 
>   trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION 
>   trunk/qpid/specs/management-schema.xml 1460944 
>   trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1460944 
> 
> Diff: https://reviews.apache.org/r/10020/diff/
> 
> 
> Testing
> -------
> 
> Several tests to exercise rebind code paths.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Re: Review Request: C++ Broker 'rebind' to steer messages from one queue to others

Posted by Chug Rolke <cr...@redhat.com>.

(Updated March 26, 2013, 1:54 a.m.)

Review request for qpid, Gordon Sim and Ted Ross.

Changes
-------

This version does two things that greatly simplify the operation:

1. Only two queues are involved: the source queue and its backup queue. There is no list of backup target queues.
2. Messages for the queues are essentially swapped at the Queue::deliver level.

There are no queue attributes to describe a backup queue. Queues are all normal. Exchanges and bindings are unaware of any changes.

To effect a backup operation, a broker method that names the two queues is invoked.
If the queues pass a series of checks then each queue gets a pointer to its partner queue.
In function Queue::deliver the pointer is checked. If null then the message goes to the current queue else the message goes to the backup queue.

In order for the backup engine to put messages back on to the original queue there is no need for a backup exchange or more bindings. The backup engine simply puts the the messages into the backup queue. By virture of the swap the messages then go back into the original queue.

The patch presented here is still missing protections around queue deletions. It also needs a way to get out of backup state.

Description
-------

The change proposed here is a prototype that implements lightweight strategy #3.

This addresses bug QPID-4650.
https://issues.apache.org/jira/browse/QPID-4650

Diffs (updated)
-----

trunk/qpid/cpp/src/qpid/broker/Broker.h 1460944
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1460944
trunk/qpid/cpp/src/qpid/broker/Queue.h 1460944
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1460944
trunk/qpid/cpp/src/tests/queue_rebind.py PRE-CREATION
trunk/qpid/cpp/src/tests/run_queue_rebind PRE-CREATION
trunk/qpid/specs/management-schema.xml 1460944
trunk/qpid/tools/src/py/qpidtoollibs/broker.py 1460944

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

Testing
-------

Several tests to exercise rebind code paths.

Thanks,

Chug Rolke