Skip Menu |

This queue is for tickets about the AnyEvent-RabbitMQ CPAN distribution.

Report information
The Basics
Id: 79511
Status: resolved
Priority: 0/
Queue: AnyEvent-RabbitMQ

People
Owner: Nobody in particular
Requestors: chip [...] pobox.com
Cc:
AdminCc:

Bug Information
Severity: Critical
Broken in: 1.08
Fixed in: (no value)



Subject: Apparent memory leak in _return_cbs
I find entries added to _return_cbs but never removed from them.
On Sat Sep 08 00:36:26 2012, CHIPS wrote: Show quoted text
> I find entries added to _return_cbs but never removed from them.
Yep, if you use an infinite set of routing keys whilst publishing messages, you'll use infinite ram for callbacks. Unfortunately the server doesn't ack messages as ok when they're published, but can reject messages (if the immediate or mandatory flags are set), ergo there being no hook to cleanup the callback. I've adjusted things to not create the callback (and thus not leak) if neither of the flags in question are set. Commit: 0d79a27 What are your feelings on how to fix this properly? The only solution I can think of is to set a timer to remove the callback some pre-defined time after the message has been flushed to the server, but this feels icky..
Subject: Re: [rt.cpan.org #79511] Apparent memory leak in _return_cbs
Date: Sat, 08 Sep 2012 14:22:51 -0700
To: bug-AnyEvent-RabbitMQ [...] rt.cpan.org
From: Chip Salzenberg <chip [...] pobox.com>
On 9/8/2012 1:02 PM, Tomas Doran via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=79511 > > > On Sat Sep 08 00:36:26 2012, CHIPS wrote:
>> I find entries added to _return_cbs but never removed from them.
> Yep, if you use an infinite set of routing keys whilst publishing > messages, you'll use infinite ram for callbacks. > > Unfortunately the server doesn't ack messages as ok when they're > published, but can reject messages (if the immediate or mandatory flags > are set), ergo there being no hook to cleanup the callback.
I have some ideas I'm going to code. For example, I think basic.return should be handled as a callback to the application at the Connection or Channel level, since return actually doesn't identify the message specifically. Let the user decide how to handle it. So you don't need to keep a list at all. I'm also putting in support for ack mode, though, which will maintain a list, but which won't leak when I do it.
On Sat Sep 08 17:22:58 2012, CHIPS wrote: Show quoted text
> I have some ideas I'm going to code. For example, I think basic.return > should be handled as a callback to the application at the Connection or > Channel level, since return actually doesn't identify the message > specifically. Let the user decide how to handle it. So you don't need > to keep a list at all. > > I'm also putting in support for ack mode, though, which will maintain a > list, but which won't leak when I do it.
+1, I agree that handling it per message doesn't make sense as it's per channel. And awesome, I will await patches. Thanks in advance!
Subject: Re: [rt.cpan.org #79511] Apparent memory leak in _return_cbs
Date: Sat, 08 Sep 2012 18:02:22 -0700
To: bug-AnyEvent-RabbitMQ [...] rt.cpan.org
From: Chip Salzenberg <chip [...] pobox.com>
Good news: the basic.return actually comes back with full headers and body, as well as the return frame itself. So if you make sure that all outgoing messages are uniquely identifiable -- adding a field to the user-defined headers can do this, as for example my "sender" and "line" entries -- we should be able to do this right. I want to integrate it with support for basic.ack. Seems like it could be as simple as extending the possible parameters of on_failure callback. For example, here's a quote from a dumped return message, printed with ->verbose. Unfortunately if you try to send to a nonexistent exchange Rabbit just closes your connection and makes you start over. Impolite. But if the exchange exists, you get the below. Also unfortunately, it seems that the Return frame does NOT include the message_id you used when sending. Which is silly. EV: error in callback (ignoring): { body => bless( { channel => 1, payload => "hello", type_id => 3 }, 'Net::AMQP::Frame::Body' ), header => bless( { content_type => "application/octet-stream", delivery_mode => 2, headers => { line => 1, sender => "feed_xxx_3393" }, priority => 1, timestamp => "1347152061", user_id => "guest" }, 'Net::AMQP::Protocol::Basic::ContentHeader' ), return => bless( { channel => 1, method_frame => bless( { exchange => "nowhere", reply_code => 312, reply_text => "NO_ROUTE", routing_key => "mand1" }, 'Net::AMQP::Protocol::Basic::Return' ), payload => "", type_id => 1 }, 'Net::AMQP::Frame::Method' ) } at lib/AnyEvent/RabbitMQ/Channel.pm line 665.
From: chip [...] pobox.com
Actually it *does* include the message_id you put in the Header object, if any. Thus: EV: error in callback (ignoring): { body => bless( { channel => 1, payload => "hello", type_id => 3 }, 'Net::AMQP::Frame::Body' ), header => bless( { content_type => "application/octet-stream", delivery_mode => 2, headers => { line => 1, sender => "feed_xxx_18315" }, message_id => 3, ... where "3" is a number I made up. Presto, reliable callbacks with message_id. Now let's see how the basic.ack system works with this.
From: chip [...] pobox.com
This ticket is obsoleted by my amqp 0.9 patches