Sending 0 in heartbeat negotiation doesn't disable heartbeats

[anurag90x]

Based on pika/pika@3027890 and

Heartbeat Timeout Value
The heartbeat timeout value defines after what period of time the peer TCP connection should be considered unreachable (down) by RabbitMQ and client libraries. This value is negotiated between the client and RabbitMQ server at the time of connection. The client must be configured to request heartbeats.

The broker and client will attempt to negotiate heartbeats by default. When both values are non-0, the lower of the requested values will be used. If one side uses a zero value (attempts to disable heartbeats) but the other does not, the non-zero value will be used.

The timeout is in seconds, and default value is 60. Older releases used 580 seconds by default.

it looks like - https://github.com/celery/py-amqp/blob/master/amqp/connection.py#L423 should be removed, since sending 0 does nothing. Does that seem reasonable?

[anurag90x]

Right, but self.heartbeat is being set to 0 if self.client_heartbeat == 0. So, the client will not send heartbeats but from that pika commit and the docs it looks like the server will still send heartbeats even if the client wants to disable heartbeats (by sending 0 in the tuneOk method).

[anurag90x]

Ah I'm probably not explaining it properly. Essentially, there's this line -

https://github.com/celery/py-amqp/blob/master/amqp/connection.py#L421

which is saying if the client_heartbeat is set to 0 (the default, which also goes into the tuneOk method), the lib turns off heartbeats. However, according to pika/pika@3027890 and the rabbitmq documentation, even if the client sends 0 as the value in the tuneOk call, the server will still send heartbeats.

My question is, if that's the case should that line be removed i.e do not set self.heartbeat to 0 if client_heartbeat is 0. That should make it, similar to pika's implementation which is in the commit I linked.

[anurag90x]

👍 I checked the spec, so just to clarify the fact that the client is ignoring the heartbeat won't cause the connection to be terminated by the broker, right?

If that is not the case (termination of the connection) I can see the justification on a client by client basis but I think it can lead to some confusion between different clients especially since pika and the java client are both using the other logic.