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

Reply via email to