On 09/10/2009 08:14 AM, Gordon Sim wrote:
On 09/09/2009 10:21 PM, Alan Conway wrote:
* ClientImportExport.h
I viewed these as related to libraries, not namespaces, but I agree the
dependency is not ideal. Are you suggesting that there should be a
separate library for the new API or just that there should be a separate
header file defining those macros for use in headers in the messaging
namespace?
They really are about libraries so I guess its OK. I was mostly bothered by the
#include "qpid/client/...", I'd like to see a clean break with the old API. It's
not a critical point.
* Address class
The reply-to address for a message must be encoded into the AMQP 0-10
format for transmission and this requires interpreting what the address
refers to (e.g. a 'topic', a 'queue' or some other 0-10 specific
'address'). It is obviously desirable to avoid doing unnecessary work
for every message sent with a reply-to defined, and the Address class
was introduced as the type of the ReplyTo header on a message in order
to hold details of a 'pre-interpreted' address (which may have required
parsing or even lookup on the broker) that can be directly encoded more
quickly.
It may be that the parsing does not add significant overhead and we
could simply remove the Address struct and rely on a simple string. That
would certainly be preferable for simplicity.
I'd be inclined make Address a PIMPL so we can do whatever we want with regard
to pre-parsing etc. We can use the flyweight pattern to avoid allocations and
make them light weight if we need to.
* MessageContent & Codec
I agree that the encode/decode methods should not be on the message and
will remove them.
The concept at present is that a message content can be one of two
'structured types' (list or map), or just a sequence of bytes. The
structured types represent the 'logical' structure of the content and
are not intended to imply any particular encoding.
I'm still not sure whether the structured content should be a part of
the message or whether that aspect should be handled separately and am
not totally comfortable with the current form. The reason for having it
'built in' to the message is convenience for the user; they can simply
pass a message whose content is a map of values to be sent and can
receive a message already decoded into a map for processing. Of course
for transmission over the network even the structured types will need to
be encoded into a sequence of bytes and encoding and decoding will
happen 'under the covers' by choosing a particular encoding (ideally in
some configurable manner).
The alternative would be to have message simply represent the bytes to
be sent or received and have the encode/decode and structural
representations kept distinct. That would I think certainly result in a
simpler API. The question is whether there is value in doing this
automatically, and if so whether there is a better way of doing so.
I'll think about this some more and will very much welcome any further
comments or suggestions you may have.
Planning for non-contiguous sequences of bytes is also a good idea, even
if the implementation that exploits it follows later on. Keen to see
your thoughts on that.
How about this: a Message is a container for data which internally might be
non-contiguous etc. But Message itself provides no access to the data.
A set of *View classes provide "views" of the message data.
So you'd have StringView to view the message as a contiguous string, MapView to
parse the message as a map etc.
Message m;
std::string x = StringView(m);
MapView mm(m); // throw if cant be parsed as map.
cout << mm["foo"]
If/when we support multi-buffer messages we can add a BuffersView which allows
the user to view the underlying buffers directly with no copying.
Views are read-only and may or may not copy data from the message so modifying
the message may invalidate some types of view (like modifying a std::collection
may invalidate an iterator)
When we want to create or modify a message we use corresponding Content classes.
Views are read-only, Contents are read write. You can use Contents in two ways
1. pass to message constructor or assign
Message m(StringContent("foo"));
m = StringContent("bar")
2. modify existing message
Message m;
MapContent mm(m);
mm["foo"] = ...;
The Message API will have constructor/assign etc. for the Content base class,
we'll figure out the type under the covers. That means we can expand the range
of content types without any change to the Message API.
If/when we support multi-buffer messages a BuffersContent will allow the user to
provide pointers to their own buffers, extend a message by adding buffers etc.
We may provide some options around avoiding copies, the default should be that
we copy though as there are lifcycle issues to work out if we don't copy.
We could also provide view/content that implement std::streambuf for efficient
integration with iostreams.
I'm in two minds as to whether View and Content are separate classes or should
be combined into just one read-write Content class. The motivation for
separation is const correctness - FooView(const Message&) but
FooContent(Message&). Probably in most cases the Content would inherit from the
View for the read operations.
* Receiver: capacity (& start/stop), session-level fetch, message listeners
The capacity of a listener is indeed a flow control window. If the
capacity is 0 then messages are not prefetched and are only transferred
from the server in response to an explicit fetch() call.
I think it may be better to remove the start/stop as they aren't really
needed. However at present while the capacity defines the credit window,
the start and stop control whether it is in effect or not (and have no
effect when capacity = 0).
What does "in effect" mean? How does stopped the stopped state differ
capacity==0?
The session level fetch allows you to get messages from any of the
subscriptions created for the session, in order, as they arrive. I think
this is very useful.
Agreed. What are the rules if a message arrives when there are any/all of
receiver::fetch, session::fetch and receiver listener active?
In addition to the straightforward pull style of session level fetch,
there is the ability dispatch the next available message to the listener
defined for the subscription it relates to (if a listener was defined).
(See e.g. the topic_listener for an example, though not necessarily the
most illuminating in its current form).
I haven't addressed some of your earlier points on threading but I
haven't forgotten about them either. I hope to respond on that point in
more detail before too long.
I would be inclined to say something like this: once a listener is provided to a
receiver, its received() may be called at any time in an unspecified thread
until the receiver is destroyed or the listener is un-associated from it (do we
even allow that?) Calls to received() are guaranteed to be sequential but
they're not guaranteed to be all in the same thread.
That gives us a workable API behind which we can implement a sensible
client-side thread pool initially and add more user-controlled threading options
going forward. I would be inclined to avoid any other threading knobs and dials
(e.g. start() stop() run() type stuff) as that wires assumptions about threading
into the API and restricts what we can do.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
