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