> On 2012-03-20 15:20:42, Alan Conway wrote: > > branches/asyncstore/cpp/src/qpid/broker/AsyncStore.h, line 83 > > <https://reviews.apache.org/r/4243/diff/4/?file=90821#file90821line83> > > > > 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.
Yes, that is the idea here (see the comment at the forward declaration). > On 2012-03-20 15:20:42, Alan Conway wrote: > > branches/asyncstore/cpp/src/qpid/broker/AsyncStore.h, line 63 > > <https://reviews.apache.org/r/4243/diff/4/?file=90821#file90821line63> > > > > 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. > > > > I had thought that the instance of BrokerContext would supply the necessary context, containing whatever is needed for the broker to appropriately handle the callback. This object is opaque to the store, and is simply passed back to the broker in the callback. > On 2012-03-20 15:20:42, Alan Conway wrote: > > branches/asyncstore/cpp/src/qpid/broker/AsyncStore.h, line 77 > > <https://reviews.apache.org/r/4243/diff/4/?file=90821#file90821line77> > > > > 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. The latter of these - it supplies a callback for another worker thread to asynchronously make a callback on the write() method. In the case of the store, it would call write(addr) with addr pointing to the correct location in the page buffers, and the source (the message object in the case of an enqueue op) would perform the write. This does presuppose that the broker (or whomever supplies the write() callback) would keep the data to be written stable and available (effectively locked?) until the appropriate completion has been received from the store. The completion could then delete the DataSource object, although this does not preclude using a RefCounted solution. - Kim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4243/#review6119 ----------------------------------------------------------- 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 > >
