search

vyuldashev / laravel-queue-rabbitmq

RabbitMQ driver for Laravel Queue. Supports Laravel Horizon.
MIT License
1.91k stars 377 forks source link

Queue deamon continue after a RabbitMQ connection lost #286

Closed Broutard closed 4 years ago

Broutard commented 4 years ago

Describe the bug After a Lost connection or a Connection reset by peer the job is re-run and the queue deamon continue with a lost rabbitmq connection.

Steps To Reproduce Run a long job (e.g. with a sleep(20)) and restart rabbitmq (or stop networking...) during the job.

Expected behavior The queue deamon must stop.

Additional context In RabbitMQJob::fire() method, when a causedByLostConnection($exception) occurs, the job is retry after 2sec. Caused by this update : https://github.com/vyuldashev/laravel-queue-rabbitmq/commit/edd9cc43082d2846fe517db30aeb29a03b04b04e It is a causedByDeadlock() before this update.

I think, $this->causedByLostConnection($exception) should be deleted here.

image

Broutard commented 4 years ago

If I remove the $this->causedByLostConnection($exception) from RabbitMQJob::fire(), it failed with a Connection reset by peer and Laravel worker stop the deamon as expected.

Broutard commented 4 years ago

If I set sleep_on_error=false in the queue config, the worker stops but, my job is processed 3 times (2 times when the RabbitMQ connection is lost... and a 3rd when the worker is restarted).

vyuldashev commented 4 years ago

This issue will be fixed in v10

vyuldashev commented 4 years ago

Actually, it is already fixed in v10 but it has not been released yet.

vyuldashev commented 4 years ago

v10 is released. You can try it now.

judgej commented 3 years ago

Are we sure this is fixed in in v10? I'm using V10 and finding the same issue. The problem seems to be that the Laravel Worker method (in a trait) the determines whether the connetion has been lost, looks for key phrases in the exception message that is thrown. Those messages are: (a) geared towards database connections being dropped (seems like a Laravel bug to me); and (b) do not include any of the phrases that the underlying AMQP library spits out (with the ex exception of the "reset by peer" as stated above).

Since the AMQP package throws some specific conenctiob lost exceptions, it should be possible to look for those exceptions instead of passing it back to Laravel core to make a best-guess at the exception cause.

I'm using Lumen 6.3.3 so there may be some differences in the way the connection lost detection works. (Looks much the same to as Laravel 8 though https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/DetectsLostConnections.php#L16)