laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.22k stars 10.9k forks source link

Reopening: Queue priorities & block_for shouldn't be used together #51612

Closed tomastan closed 3 months ago

tomastan commented 3 months ago

Laravel Version

All versions since 5.6 up to 9.19 and 11.9 at the moment

PHP Version

8.x

Queue driver

Redis

Description

This is a reopening of long standing issue. Original issue: #30728 was closed due to lack of confirmation if the issue exists in currently supported versions. I've tested actual ones, and the issue is still there up to the current 11.9.x.

For the convenience, copying essential parts of the original issue below:

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...

MohammadStemless commented 3 months ago

Does the same apply when using Horizon, since the sleep property is going to be at the supervisor level?

tomastan commented 3 months ago

@MohammadStemless Horizon is not used. The problem is in a stock Laravel from the repo and reproducible without supervisor.

What do you mean by sleep property? If you talk about --sleep CLI option, it makes no difference:

# config/queue.php:71
'block_for'   => 1, // Default is null
$ php artisan queue:work --queue=high,default --sleep=0|grep DONE
  2024-06-01 12:26:22 App\Jobs\TestJob ....................... 6.83ms DONE
  2024-06-01 12:26:23 App\Jobs\TestJob ....................... 1.39ms DONE
  2024-06-01 12:26:24 App\Jobs\TestJob ....................... 1.25ms DONE
  2024-06-01 12:26:26 App\Jobs\TestJob ....................... 1.27ms DONE

1 second lag per job per queue.

$ php artisan queue:work --queue=default --sleep=0|grep DONE
  2024-06-01 12:31:10 App\Jobs\TestJob ....................... 5.65ms DONE
  2024-06-01 12:31:10 App\Jobs\TestJob ....................... 1.12ms DONE
  2024-06-01 12:31:10 App\Jobs\TestJob ....................... 1.25ms DONE
  2024-06-01 12:31:10 App\Jobs\TestJob ....................... 1.15ms DONE

No lag.

themsaid commented 3 months ago

“shouldn’t be used together” doesn’t sound right at all. It’s completely fine to use block_for and dequeue from multiple queues.

Closing this since it's not a bug.

tomastan commented 3 months ago

@themsaid i'm not sure is my explanation and original author's is not enough, but please let me try explain the issue again.

In your own video how "block_for" works https://x.com/themsaid/status/1232896655982284801 you say that: "When you open a connection with Redis, and if you couldn't find any jobs, wait for 5 seconds, before sleeping and starting (process again)".

In the case:

Expected result:

100 jobs completed in 1 sec. Since there are jobs in the queue and worker can find jobs in Redis, "block_for" should not trigger.

Actual result

100 jobs completed in 500 secs. "block_for" triggers even there are jobs to do.

When it does not happen

If a single queue is used there is no such problem and "block_for" works as expected.

Workaround

What original author says, and what we had to do as well is resetting block_for to null to make jobs work at a designed performance, with a fallback and higher CPU usage.

ipa1981 commented 3 months ago

We have dropped "block_for" setting from the project when ended up with 500.000 jobs in several queues. It does not work as it should. And I see a bizzare practice closing unresolved issues by Laravel team.

mohsenfeyzzadeh commented 2 months ago

+1