fhteam / laravel-amqp

AMQP driver for Laravel queue
GNU General Public License v2.0
28 stars 12 forks source link

driver doesn't work correctly when there are failing jobs (as result of timeout or php memory limits) #21

Open oprudkyi opened 6 years ago

oprudkyi commented 6 years ago

Hi,

expected behavior - jobs that fail (by 'memory exhausted' - i.e. memory_limit or timeout) after count of tries marked as failed and go to failed_jobs table

current behavior: - jobs that fail are kept in the top of queue and are blocking any other jobs entirely (in case of single worker)

some investigations

with amqp there are different approach, pop() just receives message from queue https://github.com/fhteam/laravel-amqp/blob/master/src/Queue/AMQPQueue.php#L350 without acknowledging it (i.e. without calling $this->channel->basic_ack ) so even if worker/job failed as result of fatal error (memory exhausted) or kill (timeout) is still atop of queue without chances to be removed (this is by design of amqp/rabbitmq)

simple fix can be designed as calling basic_ack in function pop() and in case of explicit exception just save it back to queue - such approach will lost any jobs with fatal errors, though queue by self won't be blocked more complex approach will involve additional queues just to keep track of running jobs with fatal errors

martianoff commented 6 years ago

@oprudkyi try my commit with "retry_after" option it will restart failed jobs with timeout, so queue won't be locked

FractalizeR commented 6 years ago

I think we fixed this with our recent PR in master I've just merged. Could you please test?