Skip to content

[Messenger] [amqp-messenger] When heartbeat expired, disconnect to force a reconnect #47574

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

Closed

Conversation

surfingmig
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
License MIT

Network can fail in many ways, sometimes pretty subtle.

To avoid error messages "Library error: a socket error occurred", we detect when heartbeat as expired, and disconnect the chanmel connection, and force a reconnect.

Here a explaination to justify the condition :
https://www.rabbitmq.com/heartbeats.html#:~:text=Heartbeat%20frames%20are%20sent%20about,TCP%20connection%20will%20be%20closed.

@ostrolucky
Copy link
Contributor

I'm also interested in this. Would be nice to have a solution for auto reconnect, eg. when OPs are restarting rabbitmq nodes.

@@ -510,6 +515,11 @@ static function (): bool {
}
);
}

$this->lastActivityTime = time();
} elseif (!empty($this->connectionOptions['heartbeat']) && time() > $this->lastActivityTime + ($this->connectionOptions['heartbeat'] * 2)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} elseif (0 < ($this->connectionOptions['heartbeat'] ?? 0) && time() > $this->lastActivityTime + 2 * $this->connectionOptions['heartbeat']) {

@@ -102,6 +102,9 @@ class Connection
*/
private $amqpDelayExchange;

/** @var int */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our CS is to put these on many lines IIRC

GurvanVgx added a commit to GurvanVgx/symfony that referenced this pull request Oct 11, 2022
I'm taking the work done from someone else and and adding the remarks mentionned because there is no activity on this PR

PR: symfony#47574
@nicolas-grekas
Copy link
Member

Continued in #47831, thanks for submitting.

fabpot added a commit that referenced this pull request Oct 15, 2022
This PR was merged into the 5.4 branch.

Discussion
----------

[Messenger] Fix amqp socket lost

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #42825
| License       | MIT
| Doc PR        | N/A

I'm taking the work done from someone else and and adding the remarks mentionned because there is no activity on this PR

PR: #47574

Network can fail in many ways, sometimes pretty subtle.

To avoid error messages "Library error: a socket error occurred", we detect when heartbeat as expired, and disconnect the chanmel connection, and force a reconnect.

Here a explanation to justify the condition :
https://www.rabbitmq.com/heartbeats.html#:~:text=Heartbeat%20frames%20are%20sent%20about,TCP%20connection%20will%20be%20closed.

Commits
-------

bdeef74 [Messenger] Fix amqp socket lost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants