On 09/10/2009 05:29 PM, Alan Conway wrote:
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.
Its easy enough to add in a new header with effectively the same macros.
Alternatively I guess we could move ClientImportExport.h up a level
alongside CommonImportExport.h.
* 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.
Ok, I'll see what I can do there.
* 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"] = ...;
In this second example, at what point would the map be encoded?
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.
I like the idea here! I'm not sure I fully see how to implement it yet
but will mull it over a bit and perhaps come back with some questions.
* 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?
In effect means 'actually granted'. I.e. I can set a receiver to have a
window of 50 messages. Only when I start() the receiver is that credit
granted. If I stop() the receiver then the broker is asked to reset
granted credit to 0. (I can then call start() again to get messages
flowing again).
When the capacity is 0 start and stop don't have any effect.
Like I say, I think they may well not be needed and could be removed.
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?
If you call Receiver::fetch() concurrently from more than one thread
then each thread will get some set of the available messages (no strict
round robin is supported). The situation is pretty much identical if you
have Session::fetch() called concurrently as well. All these calls are
serialised at the session level.
Listeners are currently only relevant if you call Session::dispatch().
This will in effect fetch the next available message for any receiver
that has a listener set and then pass that message to the listener. If
called concurrently it would behave similarly to Session::fetch().
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.
Agreed and it is these methods I've tried to avoid. The current
stop()/start() calls don't start or stop threads in anyway; in fact the
new API implementation does not start any threads at all (it does
however use the qpid::client::Connection class which for now at least
starts its own IO thread).
(I wasn't mad on the names at first as I felt it implied something like
a thread being started. If I get rid of them that will end the confusion!).
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
