----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1317 -----------------------------------------------------------
/branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp <https://reviews.apache.org/r/1312/#comment2990> Not too keen on this lookup. Can it be avoided? E.g. can we modify the Queue::acquire() to simply take the consumer name as the second parameter? (That is for the present at least all that is required) Alternatively the DeliveryRecord could be modified to hold a pointer to the Consumer rather than simply the tag. /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp <https://reviews.apache.org/r/1312/#comment2991> The purpose of the check is to ensure that an acquire attempt for a message that has since been replaced, does not acquire the message that has replaced it instead. I believe it is still necessary, though I concede the form is ugly and unintuitive. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h <https://reviews.apache.org/r/1312/#comment2992> I'm not keen on the terminology here. For me selector implies something subtly different from the role this object is serving (at least from the role I *think* it is serving). I'd prefer something like 'allocator'. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h <https://reviews.apache.org/r/1312/#comment2993> Again, I'm not too keen on the term 'consumed' in this context. Though I can see how it is justified, it is potentially confusing in my view (could imply the actual dequeue of a message). I'd prefer 'acquired', 'allocated' or even just 'locked' as they are all less ambiguous on the state in question. - Gordon On 2011-08-05 20:46:15, Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1312/ > ----------------------------------------------------------- > > (Updated 2011-08-05 20:46:15) > > > Review request for qpid. > > > Summary > ------- > > Some initial refactoring of Queue/Consumer interface to allow for message > grouping support. Very preliminary. > > > This addresses bug qpid-3346. > https://issues.apache.org/jira/browse/qpid-3346 > > > Diffs > ----- > > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144324 > > Diff: https://reviews.apache.org/r/1312/diff > > > Testing > ------- > > minimal - passes unit tests on linux. > > > Thanks, > > Kenneth > >
