Queue priorities & `block_for` shouldn't be used together

trevorgehman commented 4 years ago

Laravel Version: 5.6.x & above
Queue Driver: Redis

Description:

If you use block_for and also utilize queue priorities, the worker will block on each queue sequentially. This could result in much longer queue processing times.

// config/queue.php

'redis' => [
   'driver'      => 'redis',
   'connection'  => 'queue',
   'queue'       => 'default',
   'retry_after' => 90,
   'block_for'   => 5,
],

Steps To Reproduce:

For example, suppose you set block_for=5 and spawn a worker like this:

php artisan queue:work --queue=high,medium,low,default --sleep=0

The worker will block on the high queue for 5 seconds, then the medium queue, then the low queue, and finally the default queue. This means that it could take up to 15 seconds before a job in the default queue is picked up, even if it is the only job in any of the queues.

More seriously, if you set block_for=0 the worker will indefinitely block on the high queue, and the other queues will never be reached.

Solution:

I'm not sure if this is a bug, but I initially assumed that the worker would block on all the queues set for the worker and immediately pick up a new job in any of them.

Perhaps all that's needed is an addition to the documentation recommending that the block_for setting only be used when workers listen to a single queue...

driesvints commented 4 years ago

Laravel Version: 5.6.x & above

Did you test this on Laravel 6?

trevorgehman commented 4 years ago

I've actually only just noticed it in 5.8.35. I imagine this works the same in 6.x since this code is the same:

The worker calls getNextJob for each queue in order:

https://github.com/laravel/framework/blob/dfb4c1c3111d63383ed1f7ae67c79c1f83c8ec49/src/Illuminate/Queue/Worker.php#L260-L267

Which calls pop and retrieveNextJob on the Redis queue:

https://github.com/laravel/framework/blob/dfb4c1c3111d63383ed1f7ae67c79c1f83c8ec49/src/Illuminate/Queue/RedisQueue.php#L219-L238

You can see that BLPOP is called for each queue (line 233):

https://github.com/laravel/framework/blob/dfb4c1c3111d63383ed1f7ae67c79c1f83c8ec49/src/Illuminate/Queue/RedisQueue.php#L232-L235

So it's going to block for (in this case) 5 seconds before checking the next queue in the priority list.

driesvints commented 4 years ago

@trevorgehman feel free to open up a new issue once you were able to confirm this on 6.x

z5864703 commented 4 years ago

@trevorgehman feel free to open up a new issue once you were able to confirm this on 6.x

It can be confirmed that this problem exists in 6.x and 7.x

tomastan commented 5 months ago

The same is in 9.x, 10.x, and 11.x as well. Just spent a day on the jobs performance bottlenecks and reverse engineering both empty laravel and our own project to finally find this issue out. ;)

laravel / framework