qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Stitcher <astitc...@redhat.com>
Subject Re: Review Request: Develop Asynchronous Store Interface for Qpid (v.2)
Date Tue, 28 Feb 2012 21:47:51 GMT
On Tue, 2012-02-28 at 21:16 +0000, Gordon Sim wrote:
> On 02/28/2012 08:45 PM, Andrew Stitcher wrote:
> > On Tue, 2012-02-28 at 20:24 +0000, Gordon Sim wrote:
> >> What I do think is important is that the broker can easily get specific
> >> context passed back in some way. I don't see how your design would allow
> >> this but as you note below that may simply be because I don't understand
> >> how you intend the ResultFactory and BrokerContext to be used.
> >
> > The intent is that BrokerContext is a base class which encapsulates
> > whatever context the broker wantsd to associate with a handle so that it
> > can simply use that BrokerContext to figure out exactly what the result
> > relates to.
> >
> > The point of the ResultFactory is simply that the broker side must have
> > control of creating and destroying the Results since they end up on the
> > broker side.
> Why do we need AsyncResult objects as distinct from a callback context 
> and the status of the operation?

That goes with using a single callback for the result - there needs to
be a single base class passed back. If you used 2 callbacks then the
AsyncResult is no longer necessary.

> [...]
> >> I don't think it is noticeably shorter. Whether it is simpler is I think
> >> again a matter of personal preference.
> >
> > Well, it is noticeably simpler than either of Kim's proposals because
> > I've dropped all the class definitions that aren't necessary for the
> > specific interface being defined - that certainly makes it easier for me
> > to read - it doesn't make the actual usage very much different I agree.
> Ignoring the AsyncStorePlus which is a strictly optional additonal 
> utility, I think the first of Kim's proposals is as simple as this (in 
> fact around callbacks I think it is simpler, clearer and no less safe).

Well only if you ignore the complexity of the Ops structure which is not
really fully outlined in v1. I'm assuming here that the intent would be
to use a (pseudo) discriminated union rather than the structure actually
shown there.

> >> What would createDataHandle() actually do in general? What about
> >> createFlushHandle()?
> >
> > The most important point of the Handle factories is to associate the
> > BrokerContext with a Handle so that the broker knows what the handle
> > refers to.
> How do I get the BrokerContext back from a handle?

A simple accessor would do it - remember the base class is not shown in
the interface because it's not needed there, but I think it could be
something like:

class AsyncHandle {
  ~AsyncHandle() = 0;

  const BrokerContext* getBrokerContext() const = 0;

>  Should it be passed 
> to the createXxxResult() methods on Result Factory rather than the 
> AsyncHandle (which is really only interesting to the store)?

You pass back the AsyncHandle because the broker might want to do
something further with it. If you only pass back the BrokerContext then
you'd somehow need to create a circular link between them.

> I prefer having the callback context simply passed in with the 
> operation. That allows you for example to use a different context for 
> two operations that share a handle (e.g. create/destroy or enqueue/dequeue).

That is definitely a good option. The (perhaps small) benefit of this
wrapped scheme is that it passes fewer things back to the result, given
that you need to have the store contexts passed back as well for the
reasons above.

> >> (I prefer createMessageHandle() to createEnqueueHandle() - seems more
> >> consistent with the others and more intuitive).
> >
> > Actually I simply replaced "next" in the v2 proposal with "create" - I
> > meant nothing semantic here.
> >
> > Although I'd note that Messages and Enqueues are in fact separate
> > things, especially if we want to avoid copying the same message to
> > multiple on disk queues some day.
> >
> > In the context of this v2 proposal I think that the DataHandle actually
> > is closer to the idea of Message.
> You don't dequeue an enqueue, you dequeue a message that was previously 
> enqueued. In the same way you have a QueueHandle that you use to 
> associate the destroy with what was previously created, I think it is 
> more consistent to use a MessageHandle to associate the dequeue with the 
> previous enqueue.

Well if we want to be picky then they should be EnqueuedMessageHandle
and MessageHandle (or something). I don't we should get too distracted
by this level of bike-shed namingness though!


> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project:      http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org

Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org

View raw message