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

[Alexis Richardson]

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
>> >
>
>