Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification on amqp_maybe_release_buffers usage #211

Closed
pinepain opened this issue Sep 8, 2014 · 5 comments
Closed

Clarification on amqp_maybe_release_buffers usage #211

pinepain opened this issue Sep 8, 2014 · 5 comments

Comments

@pinepain
Copy link

pinepain commented Sep 8, 2014

Hi Alan,

could you provide some details on amqp_maybe_release_buffers (as well as amqp_maybe_release_buffers_on_channel) usage?

Does library utilize buffers in continuous amqp_simple_rpc calls one by one or is it necessary to call amqp_maybe_release_buffers after each amqp_simple_rpc?

Am I right that it is good idea to call amqp_maybe_release_buffers only after dealing with potentially large read operations, say after basic.get-ok, basic.return, basic.deliver processing?

Maybe I'm missing something else, so if you will have time - highlight the intended usage of this function.

Thanks!

P.S.: I saw this mailing list discussion http://lists.rabbitmq.com/pipermail/rabbitmq-discuss/2013-June/027986.html but it is a bit old, so please give a note about whether all whitened there still valid or invalid.

@alanxz
Copy link
Owner

alanxz commented Sep 8, 2014

could you provide some details on amqp_maybe_release_buffers (as well as amqp_maybe_release_buffers_on_channel) usage?

The recommendation for using these functions is to call one of after you're done using an amqp_frame_t, an amqp_method_t or returned to you by rabbitmq-c. e.g., amqp_declare_queue() returns an amqp_declare_queue_ok_t*, after you're done using that, you should call amqp_maybe_release_buffers(). If you use amqp_simple_wait_frame() the same applies to the amqp_frame_t structs it populates.

amqp_maybe_release_buffers_on_channel() was added for the use case in which you're using more than one channel and need to recycle memory on each channel independently.

Does library utilize buffers in continuous amqp_simple_rpc calls one by one or is it necessary to call amqp_maybe_release_buffers after each amqp_simple_rpc?

Internally the library uses a memory pool for each channel. It allocates from this pool, expanding it as necessary to fulfill memory needs. The the memory is re-used after calling amqp_maybe_release_buffers(). As an implementation detail, if there is an especially large allocation this maybe returned to the OS after calling amqp_maybe_release_buffers().

To be more direct: you don't have to call amqp_maybe_release_buffers after each amqp_simple_rpc, however not calling it as close as you can to when you're done using the memory, will result in the memory pool growing.

Am I right that it is good idea to call amqp_maybe_release_buffers only after dealing with potentially large read operations, say after basic.get-ok, basic.return, basic.deliver processing?

No, I would include other RPC operations as well (e.g., queue.declare, exchange.bind).

The message you're referencing on the email list is still largely applicable. The only thing that has changed since that was written is internally memory is now pooled on a per-channel basis. Calling amqp_maybe_release_buffers_on_channel will recycle memory associated with the specified channel only, amqp_maybe_release_buffers will recycle memory associated with all channels.

@alanxz alanxz closed this as completed Sep 8, 2014
@pinepain
Copy link
Author

pinepain commented Sep 9, 2014

Thanks! Is it possible to destroy (not release) buffer for specific channel rather than all destroying it for whole connections? I mean situation where we close specific channel and do not intended it to use later, but there are pre-allocated buffer which still takes memory. Can you also consider how safe is to do such things while there are some chance that client and server channels list may be out of sync which channels are opened?

@alanxz
Copy link
Owner

alanxz commented Sep 9, 2014

rabbitmq-c does not provide a public API to free a memory pool for a channel (or even connection-wide).

It not all that dangerous to free a memory pool for a channel that is potentially still open - memory pools are created on demand. The only noticeable effect would be performance, and even that wouldn't be huge on most systems with a reasonable malloc implementation.

I would need to think carefully about what a reasonable API for this would be. Having both an amqp_maybe_release_buffers() and something like amqp_maybe_destroy_pools() is confusing and I'm fairly certain would be used incorrectly (the rabbitmq-c API isn't great, I'm trying hard not to make it worse). My gut reaction here to to improve the way amqp_maybe_release_buffers_on_channel() works so that it destroys the memory pool when it detects the channel has been closed.

@petersilva
Copy link

When I have amqp_maybe_release_buffers, I get message header corruption at some random point.
If I remove it, I get thousands of messages processed with no errors. I'm guessing that it is using more memory? Do I need to use amqp_maybe_release_buffers, or is it OK to leave it out?

@petersilva
Copy link

I found my bobo... wasn't checking presence of AMQP_BASIC_HEADERS_FLAG... now I am, and it's fine...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants