> On 2011-08-16 13:56:46, Kenneth Giusti wrote: > > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h, line 419 > > <https://reviews.apache.org/r/1312/diff/2/?file=32699#file32699line419> > > > > Agreed - I don't like adding "type" info here, as it would probably be > > abused. You are correct - I wanted to validate the config, and added this > > to simplify the code. > > > > I'll back it out and specifically check the settings map for > > incompatible options.
Perhaps what we want here is a QueueConfiguration class that is an internal only, strongly typed, explicitly named set of configuration options. We would create this from the arguments specified in one place (within this class itself), and then all other decisions based on configuration could use the simpler, more explicit interface. What do you think? > On 2011-08-16 13:56:46, Kenneth Giusti wrote: > > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp, line 660 > > <https://reviews.apache.org/r/1312/diff/2/?file=32700#file32700line660> > > > > Originally tried that, but the eventual call to acquire() would fail as > > the message was already removed from the queue (we need to call > > acquire/dequeue to maintain correct state). > > > > I can use removeIf, but I'll have to provide a variant of acquire() > > that assumes the message has already been removed from the queue. > > > > Ah, yes I see what you mean... I was confusing Messages::remove() and Queue::acquire() there for a minute... That said however, is much of Queue::acquire() not actually irrelevant here (i.e. in purge/move/reroute) anyway? We don't actually want to check whether we can acquire the message. All we need to do (other than remove it from messages) is notify the observers. Or am I missing something? - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1473 ----------------------------------------------------------- On 2011-08-16 00:35:20, Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1312/ > ----------------------------------------------------------- > > (Updated 2011-08-16 00:35:20) > > > 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/Broker.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144319 > /branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144319 > /branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144319 > /branches/qpid-3346/qpid/specs/management-schema.xml 1144324 > /branches/qpid-3346/qpid/tests/src/py/qpid_tests/broker_0_10/management.py > 1144324 > > Diff: https://reviews.apache.org/r/1312/diff > > > Testing > ------- > > minimal - passes unit tests on linux. > > > Thanks, > > Kenneth > >
