You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Gordon Sim <gs...@redhat.com> on 2013/07/03 17:22:31 UTC
Review Request 12249: add support for 1.0 lifetime policy to queues
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/
-----------------------------------------------------------
Review request for qpid, Alan Conway and Kenneth Giusti.
Bugs: QPID-4976
https://issues.apache.org/jira/browse/QPID-4976
Repository: qpid
Description
-------
This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
Diffs
-----
/trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
/trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
/trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
/trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
/trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
Diff: https://reviews.apache.org/r/12249/diff/
Testing
-------
Thanks,
Gordon Sim
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Gordon Sim <gs...@redhat.com>.
> On July 5, 2013, 3:15 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.h, line 342
> > <https://reviews.apache.org/r/12249/diff/1-2/?file=317951#file317951line342>
> >
> > this should read "indicate _the_ queue is being used in some other context _than_ by a subscriber", right?
Oops, yes, I'll fix.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22783
-----------------------------------------------------------
On July 4, 2013, 7:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 4, 2013, 7:39 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1499373
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1499373
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22783
-----------------------------------------------------------
Ship it!
I like the renaming - the intent is much clearer.
/trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/12249/#comment46486>
this should read "indicate _the_ queue is being used in some other context _than_ by a subscriber", right?
- Kenneth Giusti
On July 4, 2013, 7:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 4, 2013, 7:39 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1499373
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1499373
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Gordon Sim <gs...@redhat.com>.
> On July 5, 2013, 2:20 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.h, line 126
> > <https://reviews.apache.org/r/12249/diff/1-2/?file=317951#file317951line126>
> >
> > Not sure what "use case" means. Is the controller a subscriber?
It could be. Right now in practical terms it is either a sending or receiving link over 1.0 and is only relevant for policy options that are currently only exposed for queues created in response to an attach with the dynamic flag set on the terminus. However in answering the question I realise I haven't set it for sending links, so will fix that.
I'll see if I can come up with wording for this comment that is a clearer also.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22781
-----------------------------------------------------------
On July 4, 2013, 7:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 4, 2013, 7:39 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1499373
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1499373
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22781
-----------------------------------------------------------
Ship it!
/trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/12249/#comment46484>
Not sure what "use case" means. Is the controller a subscriber?
- Alan Conway
On July 4, 2013, 7:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 4, 2013, 7:39 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1499373
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1499373
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1499373
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1499373
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/
-----------------------------------------------------------
(Updated July 4, 2013, 7:39 p.m.)
Review request for qpid, Alan Conway and Kenneth Giusti.
Changes
-------
Updated following reviews by Alan and Ken (many thanks to both of you!).
Bugs: QPID-4976
https://issues.apache.org/jira/browse/QPID-4976
Repository: qpid
Description
-------
This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
Diffs (updated)
-----
/trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1499373
/trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/Queue.h 1499373
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1499373
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1499373
/trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1499373
/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1499373
/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1499373
/trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1499373
/trunk/qpid/cpp/src/tests/QueueTest.cpp 1499373
Diff: https://reviews.apache.org/r/12249/diff/
Testing
-------
Thanks,
Gordon Sim
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Gordon Sim <gs...@redhat.com>.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 848
> > <https://reviews.apache.org/r/12249/diff/1/?file=317952#file317952line848>
> >
> > Lock order, this is called with messageLock held. Need a note somewhere to explicitly state the lock order rules for Queue.
I removed the ownership lock entirely, as it was only used for the owner field and even there not consistently. Now just uses the same lock as the other users fields.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp, line 267
> > <https://reviews.apache.org/r/12249/diff/1/?file=317960#file317960line267>
> >
> > I don't understand the usePrimary/trackPrimary code. I don't see how it relates to HA primary status.
>
> Gordon Sim wrote:
> It has nothing to do with HA. I'll come up with a different name. It indicates whether the link that caused it to be created is still using it.
Changed avoid this confusion and hopefully signal purpose more clearly.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22765
-----------------------------------------------------------
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Alan Conway <ac...@redhat.com>.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 495
> > <https://reviews.apache.org/r/12249/diff/1/?file=317952#file317952line495>
> >
> > Naming is not clear, inUse looks like a question and release is very general. Perhaps addUser() removeUser() ?
>
> Gordon Sim wrote:
> How about markInUse()/releaseFromUse()? I felt addUser()/removeUser() might sound like permissioning.
That works.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 452
> > <https://reviews.apache.org/r/12249/diff/1/?file=317955#file317955line452>
> >
> > How is this handled now? Need to auto-del queues with only replicating subscriptions.
>
> Gordon Sim wrote:
> I didn't change that; it was not run for uncounted consumers previously so I left it as is. (On backups, a queue is prevented from being autodeleted by being marked in use until deleted on the primary).
Gotcha
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp, line 124
> > <https://reviews.apache.org/r/12249/diff/1/?file=317966#file317966line124>
> >
> > Can we move the isAutoDelete() check into inUse()?
>
> Gordon Sim wrote:
> I'm not sure what you mean. They seem to me to track two different things?
Never mind, brain hiccup on my part.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22765
-----------------------------------------------------------
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Gordon Sim <gs...@redhat.com>.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/amqp/descriptors.h, line 93
> > <https://reviews.apache.org/r/12249/diff/1/?file=317948#file317948line93>
> >
> > Why apache.org? I thought these were AMQP policies.
Quite right, that's a typo, I'll fix it.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 495
> > <https://reviews.apache.org/r/12249/diff/1/?file=317952#file317952line495>
> >
> > Naming is not clear, inUse looks like a question and release is very general. Perhaps addUser() removeUser() ?
How about markInUse()/releaseFromUse()? I felt addUser()/removeUser() might sound like permissioning.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 835
> > <https://reviews.apache.org/r/12249/diff/1/?file=317952#file317952line835>
> >
> > Can we convert the policy into an integer code during setup and do a switch here rather than if/elif? This is on the critical path.
Yes, thats a good idea, I'll change that. I don't think it is on the critical path though as it should only be called (a) when a consumer cancels or (b) when the queue becomes empty (and there is a check on empty before calling this).
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 452
> > <https://reviews.apache.org/r/12249/diff/1/?file=317955#file317955line452>
> >
> > How is this handled now? Need to auto-del queues with only replicating subscriptions.
I didn't change that; it was not run for uncounted consumers previously so I left it as is. (On backups, a queue is prevented from being autodeleted by being marked in use until deleted on the primary).
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp, line 267
> > <https://reviews.apache.org/r/12249/diff/1/?file=317960#file317960line267>
> >
> > I don't understand the usePrimary/trackPrimary code. I don't see how it relates to HA primary status.
It has nothing to do with HA. I'll come up with a different name. It indicates whether the link that caused it to be created is still using it.
> On July 4, 2013, 2:43 p.m., Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp, line 124
> > <https://reviews.apache.org/r/12249/diff/1/?file=317966#file317966line124>
> >
> > Can we move the isAutoDelete() check into inUse()?
I'm not sure what you mean. They seem to me to track two different things?
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22765
-----------------------------------------------------------
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22765
-----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/amqp/descriptors.h
<https://reviews.apache.org/r/12249/#comment46456>
Why apache.org? I thought these were AMQP policies.
/trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/12249/#comment46460>
Needs a comment describing the purpose of the class.
/trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/12249/#comment46461>
Needs a brief comment describing the purpose of the class.
/trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/12249/#comment46458>
This and isEmpty should take a ScopedLock& argument, they are called with the lock held.
/trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/12249/#comment46459>
Remove dead code.
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/12249/#comment46462>
Naming is not clear, inUse looks like a question and release is very general. Perhaps addUser() removeUser() ?
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/12249/#comment46463>
Can we convert the policy into an integer code during setup and do a switch here rather than if/elif? This is on the critical path.
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/12249/#comment46464>
Lock order, this is called with messageLock held. Need a note somewhere to explicitly state the lock order rules for Queue.
/trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp
<https://reviews.apache.org/r/12249/#comment46466>
How is this handled now? Need to auto-del queues with only replicating subscriptions.
/trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp
<https://reviews.apache.org/r/12249/#comment46469>
I don't understand the usePrimary/trackPrimary code. I don't see how it relates to HA primary status.
/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp
<https://reviews.apache.org/r/12249/#comment46467>
Can we move the isAutoDelete() check into inUse()?
- Alan Conway
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Gordon Sim <gs...@redhat.com>.
> On July 3, 2013, 7:21 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/tests/QueueTest.cpp, line 431
> > <https://reviews.apache.org/r/12249/diff/1/?file=317968#file317968line431>
> >
> > Ah, interesting... should the test have called queue->consume() on c3 originally?
>
> Gordon Sim wrote:
> Well, its not strictly required, but if consume() is not called then cancel() certainly shouldn't be either.
>
> Kenneth Giusti wrote:
> This was originally my oversight - I'd recommend adding the missing consume() to be more consistent with the broker's treatment of clients [actually, both tests in that file seem to have the same issue!]
I'll do that
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22731
-----------------------------------------------------------
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Kenneth Giusti <kg...@apache.org>.
> On July 3, 2013, 7:21 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp, line 270
> > <https://reviews.apache.org/r/12249/diff/1/?file=317954#file317954line270>
> >
> > No DELETE_ON_CLOSE check?
> >
>
> Gordon Sim wrote:
> I didn't think that made as much sense for 0-10 or for 'shared' queues more generally (which would be where the verify method was called). In 1.0 it means delete the queue when the link that caused it to be created is closed. I suppose I could interpret that to mean the session that created it in 0-10. That would be like autodelete+exclusive, but without locking other users out entirely as exclusive does...
Ah, I understand. I thought it may have been an oversight, but this make sense. I'd mark the issue closed, but I can't seem to do it :(
> On July 3, 2013, 7:21 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/tests/QueueTest.cpp, line 431
> > <https://reviews.apache.org/r/12249/diff/1/?file=317968#file317968line431>
> >
> > Ah, interesting... should the test have called queue->consume() on c3 originally?
>
> Gordon Sim wrote:
> Well, its not strictly required, but if consume() is not called then cancel() certainly shouldn't be either.
This was originally my oversight - I'd recommend adding the missing consume() to be more consistent with the broker's treatment of clients [actually, both tests in that file seem to have the same issue!]
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22731
-----------------------------------------------------------
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Gordon Sim <gs...@redhat.com>.
> On July 3, 2013, 7:21 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/tests/QueueTest.cpp, line 431
> > <https://reviews.apache.org/r/12249/diff/1/?file=317968#file317968line431>
> >
> > Ah, interesting... should the test have called queue->consume() on c3 originally?
Well, its not strictly required, but if consume() is not called then cancel() certainly shouldn't be either.
> On July 3, 2013, 7:21 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp, line 270
> > <https://reviews.apache.org/r/12249/diff/1/?file=317954#file317954line270>
> >
> > No DELETE_ON_CLOSE check?
> >
I didn't think that made as much sense for 0-10 or for 'shared' queues more generally (which would be where the verify method was called). In 1.0 it means delete the queue when the link that caused it to be created is closed. I suppose I could interpret that to mean the session that created it in 0-10. That would be like autodelete+exclusive, but without locking other users out entirely as exclusive does...
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22731
-----------------------------------------------------------
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 12249: add support for 1.0 lifetime policy to queues
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12249/#review22731
-----------------------------------------------------------
Ship it!
Looks good, aside from a couple of questions.
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp
<https://reviews.apache.org/r/12249/#comment46424>
No DELETE_ON_CLOSE check?
/trunk/qpid/cpp/src/tests/QueueTest.cpp
<https://reviews.apache.org/r/12249/#comment46425>
Ah, interesting... should the test have called queue->consume() on c3 originally?
- Kenneth Giusti
On July 3, 2013, 3:22 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12249/
> -----------------------------------------------------------
>
> (Updated July 3, 2013, 3:22 p.m.)
>
>
> Review request for qpid, Alan Conway and Kenneth Giusti.
>
>
> Bugs: QPID-4976
> https://issues.apache.org/jira/browse/QPID-4976
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This adds the ability to have queue autodeleted when no longer used, when empty or when both no longer used and empty as specified in the standard lifetime policies for AMQP 1.0. The ability to only delete if unused and empty is useful for 0-10 also so I've allowed these to be set there as well.
>
> The basic change is to have an optional policy specified that dictates what autodelete means. The default is to autodelete when not used. A queue is in use if it is being consumed or browsed, or if in 0-10 it has been declared as an exclusive queue and the declaring session is still active. Additionally over 1.0 the queue is in use if there is any sender attached to it. Autodeletion is now triggered automatically by the queue when some other operation moves it to a state eligible for deletion. To avoid this on a backup in ha, the queue replicator flags the replica as in use.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/LossyQueue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Lvq.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/DataReader.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/NodeProperties.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1497827
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1497827
> /trunk/qpid/cpp/src/qpid/messaging/amqp/AddressHelper.cpp 1497827
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1497827
>
> Diff: https://reviews.apache.org/r/12249/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>