----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4243/#review6119 -----------------------------------------------------------
branches/asyncstore/cpp/src/qpid/broker/AsyncStore.h <https://reviews.apache.org/r/4243/#comment13149> There's not enough context here for the callback to determine what request this result is for. Suggest making result callback a functor object and pass it by pointer. class ResultCallback { virtual void operator()(const AsyncResult&, BrokerContext*) = 0; } and pass it by pointer to the sumbit*() functions. That allows the user to create an individual result object for each submit call with whatever extra. The lifecycle is clear: the ResultCallback can be deleted in or after it's operator(). If a user doesn't need per-operation data they can just create a single callback instance and pass it to every submit. branches/asyncstore/cpp/src/qpid/broker/AsyncStore.h <https://reviews.apache.org/r/4243/#comment13146> What are the call semantics for DataSource? Is it called synchronously in createMessageHandle or called asynchronsouly in another thread? If the latter then DataStore needs to be ref-counted. It's worth a comment in the header file. Same thing applies to all the later instances of DataStore* parameters. branches/asyncstore/cpp/src/qpid/broker/AsyncStore.h <https://reviews.apache.org/r/4243/#comment13148> What's the scope of BrokerContext, and how do we ensure it is not deleted while still being used? It may need to be refcounted. - Alan On 2012-03-09 18:33:10, Kim van der Riet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4243/ > ----------------------------------------------------------- > > (Updated 2012-03-09 18:33:10) > > > Review request for qpid, Andrew Stitcher, Alan Conway, Gordon Sim, Ted Ross, > and Steve Huston. > > > Summary > ------- > > The async store interface is intended to replace the current syncronous store > interface in broker/MessageStore.h. It presents a bidirectional async > interface, allowing async store operations which contain optional callbacks > to be queued up for processing by the store, and for the results of these > operations (if the callback is present in the operation) to be queued up for > action by the broker itself. > > After a good deal of back-and-forth on the list, this is the latest iteration > on the async store interface. Thanks to Andrew and Gordon who have provided > much valuable feedback. > > If this interface is accepted, it will need to be implemented in two phases: > > 1. Broker wiring - the broker will need to be hooked into this interface > using async semantics. This could be a disrptive phase, as it would likely > break or disconnect the current sync interface. An async-sync adaptor would > also be developed to allow current syncronous stores to continue operating > under the new interface. > > 2. Async store development - the current async store is not currently part of > the Qpid codebase as it was developed under an incompatible license (LGPL2). > A new async store would be developed under Apache, with any bits moved from > the old code base dual-licenced to Apache. (This will require some legal > checkups.) > > Comments welcome. > > Coordinating JIRA: https://issues.apache.org/jira/browse/QPID-3858 > > > Diffs > ----- > > branches/asyncstore/cpp/src/CMakeLists.txt 1291264 > branches/asyncstore/cpp/src/Makefile.am 1291264 > branches/asyncstore/cpp/src/qpid/broker/AsyncStore.h PRE-CREATION > branches/asyncstore/cpp/src/qpid/broker/AsyncStore.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/4243/diff > > > Testing > ------- > > Compiles as written, but is not implemented. > > > Thanks, > > Kim > >
