[rabbitmq-discuss] rabbitmq-c API improvements for multiple channels

[Alan Antonuk]

Ok, I've gone ahead and implemented what I think should be a channel-based
memory pooling, which you can find here:

https://github.com/alanxz/rabbitmq-c/tree/channel_owns_memory_pool

Its not quite API/ABI backwards compatible, the change that breaks things
being adding an extra field to the amqp_pool_t struct:
https://github.com/alanxz/rabbitmq-c/commit/2bc1dc9e22152506cf07f21d10e6a79e7ed2ccc3#diff-2

This could be fixed easy enough, the trade-off being some complexity
internally.

The change isn't a small patch, and should probably have a few more pairs
of eyes look at it, despite having a few unit tests written for
the machinery that moves the internal memory pools around.

For an explanation of what my implementation tries to do:
https://github.com/alanxz/rabbitmq-c/commit/2bc1dc9e22152506cf07f21d10e6a79e7ed2ccc3#L5R119

I have no idea how this will scale, especially to things like embedded
devices.  Hopefully I have not created a monster.

If the overarching idea looks sane to people, I will create a pull request
and we can see about merging this in.

-Alan


On Fri, Jan 13, 2012 at 5:15 PM, Alexis Richardson <alexis at rabbitmq.com>wrote:

> Alan,
>
> I'm afraid everyone has been distracted.
>
> Could anyone on the list opine?
>
> alexis
>
>
> On Fri, Jan 13, 2012 at 9:19 PM, Alan Antonuk <alan.antonuk at gmail.com>
> wrote:
> > I haven't really had a huge amount of time to work on it since I sent the
> > first message message, with the holidays and starting up a new project at
> > work.
> >
> > Also I was hoping to get some input from the rabbitmq guys so that any
> > changes I make would have a better chance of being accepted.
> >
> > Taking a quick look at what I wrote a month ago, today my plan would be
> to
> > implement having a amqp_pool_t associated with each amqp_channel_t and
> see
> > how far I could get with that.
> >
> > -Alan
> >
> > On Thu, Jan 12, 2012 at 11:39 AM, Alexis Richardson <alexis at rabbitmq.com
> >
> > wrote:
> >>
> >> Alan
> >>
> >> It appears that you stunned everyone into silence ;-)
> >>
> >> How are you getting on with this?
> >>
> >> alexis
> >>
> >>
> >> On Wed, Dec 14, 2011 at 8:15 PM, Alan Antonuk <alan.antonuk at gmail.com>
> >> wrote:
> >> > I've been working on a c++ wrapper around rabbitmq-c that presents a
> >> > "humane" API to the programmer heavily inspired by the Puka project.
> >> > (see: https://github.com/alanxz/SimpleAmqpClient).
> >> >
> >> > In developing this library I've run across several limitations of the
> >> > rabbitmq-c when working with multiple channels, the biggest issue
> being:
> >> >
> >> > - There is no way to wait for a list of methods on a channel.
> >> > There is amqp_simple_wait_method() - however this suffers from some
> >> > serious
> >> > drawbacks:
> >> > + you can only specify one method to listen for
> >> > + it calls abort() if a different method, or a method on a different
> >> > channel
> >> > is received
> >> > A use case for this might be: doing a basic.publish, and you want to
> >> > want to
> >> > wait for a basic.ack or a basic.return on a channel with confirms
> >> > enabled
> >> >
> >> > The way I got around this in SimpleAmqpClient was to only use
> >> > amqp_simple_wait_frame() and maintain queues of amqp_frame_t for each
> >> > channel that I have open.
> >> >
> >> > However, this comes with one serious drawback: memory management. Each
> >> > decoded frame is allocated in a connection-wide amqp_pool_t.  Because
> of
> >> > this - it is impossible to recycle the pool and release memory unless
> >> > you
> >> > have dealt with all of your pending amqp_frame_t's.  This becomes a
> >> > problem
> >> > in pathological cases where you have two consumers sending
> >> > simultaneously,
> >> > you can get in the situation that even though the client program
> >> > eventually
> >> > deals with every consumed message, memory never gets released, because
> >> > there
> >> > is always at least one frame queued up.
> >> >
> >> > The above demonstrates the second biggest issue with the rabbitmq-c
> API:
> >> > memory management when dealing with multiple channels.  There is no
> way
> >> > to
> >> > separate out memory allocation on, for example, a per-channel basis
> >> > (with
> >> > the library client keeping track of the memory pools used for
> example).
> >> >
> >> > Before I go on I'd like to mention one feature that I find useful with
> >> > the
> >> > current API: it is possible to use something like select() before
> >> > calling
> >> > amqp_simple_wait_frame() to setup a timeout while waiting for a
> >> > consumer,
> >> > which is useful when developing single-threaded RPC apps.
> >> >
> >> >
> >> > So now the interesting part: how could the API be improved?
> >> >
> >> > Some thoughts I've had dealing with the memory management:
> >> > 1. Create a amqp_simple_wait_frame() variant that as one of the
> >> > parameters
> >> > provides a callback to allocate memory, either something like a
> malloc()
> >> > call, which the client would then be responsible for freeing, or
> perhaps
> >> > something like get_amqp_pool(amqp_frame_t) which allows the client to
> >> > return
> >> > a memory pool which rabbitmq-c would use to allocate memory for that
> >> > frame.
> >> >  The amqp_frame_t would have to have some minimal amount of
> information
> >> > filled in - such as frame type and channel to be useful.
> >> >
> >> > 2. amqp_channel_t becomes a struct containing both the the channel_id
> >> > and a
> >> > amqp_pool_t.  The amqp_pool would be used by the library to allocate
> >> > frames
> >> > received on that channel.  The client would then be responsible for
> >> > calling
> >> > amqp_pool_recycle at an appropriate point.
> >> >
> >> > Some thoughts on improving the API to deal with multiple channels:
> >> > 1. Add the following to the API:
> >> > amqp_simple_wait_frame_on_channel - wait for a frame on a specified
> >> > channel
> >> > amqp_simple_wait_methods - wait for multiple methods on a specified
> >> > channel.
> >> > Don't abort() if the wrong method, or channel is received, instead
> queue
> >> > up
> >> > the frame as is done in amqp_simple_rpc
> >> > amqp_frames_enqueued_for_channel to add feature parity with
> >> > amqp_frames_enqueued
> >> > I started to code this up at one point but abandoned it as it didn't
> >> > support
> >> > the interesting property of being able to use select() to specify a
> >> > timeout
> >> > when calling amqp_simple_wait_frame_on_channel.  At least not without
> >> > adding
> >> > timeout to the rabbitmq-c api, which didn't fit with how the current
> >> > rabbitmq-c api. Here's the implementation I came up with before I
> >> > abandoned
> >> > it. I can't guarantee is completely free of
> >> > bugs. https://github.com/alanxz/rabbitmq-c/commits/simple_wait_frame/
> >> >
> >> > 2. Continue to duck the issue and allow clients to write their own
> code
> >> > to
> >> > deal with multiple channels (fixing the memory management issue using
> >> > one of
> >> > the above techniques)
> >> >
> >> > 3. Something I haven't thought of yet.
> >> >
> >> > So anyone have any thoughts on all of this?
> >> >
> >> > -Alan
> >> >
> >> > _______________________________________________
> >> > rabbitmq-discuss mailing list
> >> > rabbitmq-discuss at lists.rabbitmq.com
> >> > https://lists.rabbitmq.com/cgi-bin/mailman/listinfo/rabbitmq-discuss
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rabbitmq.com/pipermail/rabbitmq-discuss/attachments/20120123/62151982/attachment.htm>