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

Incorrect handling of Heartbeats #190

Open
mrtus opened this issue Nov 12, 2024 · 0 comments
Open

Incorrect handling of Heartbeats #190

mrtus opened this issue Nov 12, 2024 · 0 comments

Comments

@mrtus
Copy link

mrtus commented Nov 12, 2024

👋 Hello Team

Since we bumped to v2.0.3 (upgraded from v0.5.11) we are noticing some heartbeat issues with broken connections as a result.
Even when the heartbeat is configured to 60s, our connections seem to fail after a while.
This results in AMQProxy warnings such as 2024-11-12T13:31:44.102913Z WARN amq_proxy.client[remote_address: "127.0.0.1:54210"] No heartbeat response in 899s (max 61s), closing connection and application side this results in a "broken connection" as a result

It seems like there is a deviation in heartbeat behaviour between the expectations of AMQP & the proxy.

The proxy client defines "last heartbeat" as the time when a "heartbeat" frame actually occurred https://github.com/cloudamqp/amqproxy/blob/main/src/amqproxy/client.cr#L65.

While the official AMQP PHP implementation seems to only send heartbeats when the last activity (defined as last write or read time) is longer in the past than the heartbeat / 2 as described here https://github.com/php-amqplib/php-amqplib/blob/master/PhpAmqpLib/Connection/Heartbeat/AbstractSignalHeartbeatSender.php#L90.

So based on that behaviour I assume they consider ?every? network frame as a "heartbeat" event and not only heartbeat frames.
Which would make sense since a heartbeat is mainly to ensure the connection stays alive when idle, and not to create extra frames on an active reading/writing connection.

I have not verified this behaviour with the server implementation though.

Eager to hear your thoughts on this.

Thank you!

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

1 participant