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