Add heartbeat option defaults & README documentation.

[ssafejava]

heartbeat is a very important setting that shouldn't be ignored. Without it, clients that die will hold onto unacked messages indefinitely, until the connection is manually reaped.

This PR properly sets the heartbeat option to the RabbitMQ default of 580 and documents the option in the README.

[jgato]

Please, could you give us a little more details on that? If a client is subscribed in a queue and this die without a clean close, the server is not aware and continue sending messages, isn't it? With the heartbeat the server is aware and thus, it will not send more messages?

[ssafejava]

Yes exactly that - if the client dies, or the connection between the client & server is interrupted, the server will not detect this the previous default of heartbeat: 0 and unacked messages will stay unacked for eternity.

When a heartbeat is set, the server will send messages to the client & expect a response every heartbeat seconds, the RabbitMQ default being 580 seconds. In that case, if the connection were interrupted, the old connection would be reaped at some time within the next ~10min and any unacked messages held by that connection will be released back into the ready queue.

What really necessitates this patch is that even a small connection interruption can have disastrous results on the queue. Connected clients will reconnect (thus creating a new connection) but the old connections will live on indefinitely, holding their unacked messages in purgatory along with them.

[jgato]

For sure, we need this patch. Many thanks!!

On Wed, Jan 15, 2014 at 9:19 AM, ssafejava notifications@github.com wrote:

Yes exactly that - if the client dies, or the connection between the client & server is interrupted, the server will not detect this the previous default of heartbeat: 0 and unacked messages will stay unacked for eternity.

When a heartbeat is set, the server will send messages to the client & expect a response every heartbeat seconds, the RabbitMQ default being 580 seconds. In that case, if the connection were interrupted, the old connection would be reaped at some time within the next ~10min.

What really necessitates this patch is that even a small connection interruption can have disastrous results on the queue. Connected clients will reconnect (thus creating a new connection) but the old connections will live on indefinitely, holding their unacked messages in purgatory along with them.

— Reply to this email directly or view it on GitHubhttps://github.com/postwait/node-amqp/pull/287#issuecomment-32342160 .

[ssafejava]

FYI right now you can fix this behavior (without this patch) by simply adding heartbeat: 580 (or whichever value you're comfortable with) to your connection options. This patch simply sets a non-zero default and documentation.

[postwait]

There shouldn't be a change in default behavior. I've adopted and adapted your documentation.

[ssafejava]

I strongly believe that the previous default was a bug; L452 of connection.js should have read this.options.heartbeat || 580, as the rabbitMQ default is 580 and it needs to be explicitly set to something else in order to disable it. I think this is very confusing to users that expect the defaults to be respected, especially since the example option hashes do not include a heartbeat attribute.

Please consider changing this in the next major release, and in the meantime documenting it as heartbeat: 0 // by default, heartbeats are disabled, see below in the example connection hashes to explain what is happening. Heartbeats are a fundamental feature of rabbitMQ and it is nonsensical to have them disabled by default - and we are going to leave it that way, it needs to be extremely explicit or we risk a great deal of hard-to-find bugs.