Skip to content

Commit

Permalink
feature #53892 [Messenger][AMQP] Automatically reconnect on connectio…
Browse files Browse the repository at this point in the history
…n loss (ostrolucky)

This PR was merged into the 7.1 branch.

Discussion
----------

[Messenger][AMQP] Automatically reconnect on connection loss

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      |no
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

When using Rabbitmq in cluster, there is a common need of having to upgrade the nodes, while keeping the existing connections. The way this is normally done is by putting nodes in cluster to `maintenance mode`, ensuring cluster is healthy at all times. However, symfony/messenger nor php-amqp handle this use case at the moment. What happens instead is that exception when getting the message is thrown, worker crashes, error is logged and process manager has to respin it. This all happens without having a way in user space to handle this case better. Messenger's retry mechanism does not work here, because that one kicks in only when exception is thrown in handlers. Concrete exception is following:

>  [AMQPConnectionException (320)]
>  Server connection error: 320, message: CONNECTION_FORCED - Node was put into maintenance mode

What I'm proposing in this PR is that if connection error is detected _try to reconnect once_ before throwing exception. That should handle the outlined case. This goes line in line with recommendation from AWS's support we got:
> kindly ensure that the client connecting to the broker attempts a retry in case the above error message is observed during a maintenance window. In RabbitMQ cluster deployments, the nodes are restarted one-by-one, meaning at least two nodes will be up and running at all times. Even if a connection is severed, a connection retry will result in the other nodes accepting the connection, and the clients can keep using the broker.

I've also reported issue at php-amqplib/php-amqplib#1161 with hope that this could be fixed at some point upstream, but I don't give it a big chance.

Commits
-------

056b4a5 [Messenger] AMQP:Automatically reconnect on connection loss
  • Loading branch information
fabpot committed Feb 23, 2024
2 parents 17c5508 + 056b4a5 commit f78f932
Showing 1 changed file with 10 additions and 0 deletions.
Expand Up @@ -52,6 +52,16 @@ private function getEnvelope(string $queueName): iterable
{
try {
$amqpEnvelope = $this->connection->get($queueName);
} catch (\AMQPConnectionException) {
// Try to reconnect once to accommodate need for one of the nodes in cluster needing to stop serving the
// traffic. This may happen for example when one of the nodes in cluster is going into maintenance node.
// see https://github.com/php-amqplib/php-amqplib/issues/1161
try {
$this->connection->queue($queueName)->getConnection()->reconnect();
$amqpEnvelope = $this->connection->get($queueName);
} catch (\AMQPException $exception) {
throw new TransportException($exception->getMessage(), 0, $exception);
}
} catch (\AMQPException $exception) {
throw new TransportException($exception->getMessage(), 0, $exception);
}
Expand Down

0 comments on commit f78f932

Please sign in to comment.