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

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

Open
oprudkyi opened this issue Feb 24, 2018 · 2 comments

Comments

@oprudkyi
Copy link

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 added a commit to martianoff/laravel-amqp that referenced this issue Mar 29, 2018
increment required php version to real value
martianoff added a commit to martianoff/laravel-amqp that referenced this issue Mar 29, 2018
increment required php version to real value
@martianoff
Copy link
Contributor

martianoff commented Mar 30, 2018

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

martianoff added a commit to martianoff/laravel-amqp that referenced this issue Mar 30, 2018
increment required php version to real value
@FractalizeR
Copy link
Collaborator

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

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

3 participants