> On 2011-06-10 15:28:20, Kenneth Giusti wrote: > > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp, line 1229 > > <https://reviews.apache.org/r/860/diff/1/?file=20822#file20822line1229> > > > > "All you need to know is when they all complete" - not true: we need > > to know when _each_ dequeue completes. This is not like the enqueue case - > > we need to known when each dequeue is complete as the Message.Accept may be > > only interested in that one particular dequeue event. > > > > Which is what QPID-3079 is specifically trying to solve.
Right but you can still count rather than holding a set/list. If the message.accept is interested in N completions it can just count down to 0 then complete. I'm wary of adding new dynamic data structures on the critical path. - Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/860/#review801 ----------------------------------------------------------- On 2011-06-08 20:31:05, Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/860/ > ----------------------------------------------------------- > > (Updated 2011-06-08 20:31:05) > > > Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet. > > > Summary > ------- > > Modifies the broker's handling of Message.Accept to hold off the completion > of the command until all messages related to the accept have completed > dequeue. This particularly applies to persistent messages, as the > store::dequeue() operation must complete before the message is considered > fully dequeued. > > Note this bugfix requires some changes to the broker's store module > interface: previously, the store only identified the message when a dequeue > was completed. This is not enough information - the queue from which is was > removed must also be identified (the message may be in the process of being > dequeued on several queues at once). > > > This addresses bug qpid-3079. > https://issues.apache.org/jira/browse/qpid-3079 > > > Diffs > ----- > > /branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp > 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 > /branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 > > Diff: https://reviews.apache.org/r/860/diff > > > Testing > ------- > > broker unit tests, store unit tests (modified jboss store). Still needs to > be vetted on non-linux, and have latest trunk merged in. > > > Thanks, > > Kenneth > >
