search

vyuldashev / laravel-queue-rabbitmq

RabbitMQ driver for Laravel Queue. Supports Laravel Horizon.
MIT License
1.93k stars 382 forks source link

Create delayed queues as quorum when it's enabled #564

Closed TomKeur closed 1 year ago

TomKeur commented 1 year ago

Fixes #563

khepin commented 1 year ago

I've enabled the test runs. Can you take a look at the failures there.

TomKeur commented 1 year ago

Thanks for the quick reply! I've fixed the styling issue, somehow the last version of Laravel Pint did not come up this error, but I've fixed it by using the same composer command used in the GitHub Action workflow :).

khepin commented 1 year ago

@adm-bome any thoughts? If not I'll merge tomorrow.

TomKeur commented 1 year ago

For your info: today I've upgraded to RabbitMQ 3.12.2, now I'm able to set the Default Queue type per vhost, when it's set to quorum, every queue generated will be a quorum queue.

But I guess it's still a nice addition for people working with an older version of RabbitMQ.

adm-bome commented 1 year ago

@khepin I will look in to it. (Will comment in 2 à 3 days)

TomKeur commented 1 year ago

Any news? I was thinking, maybe we should make it a own configuration parameter, since this could be a "breaking" change, because it changes the default behavior.

adm-bome commented 1 year ago

My concern.

When enabling delay queues, an inline option could be set to also make delay queues quorum. When you want to and quorum was set for the processing queue.

Also testing for this config and the functional part would be nice addition.

TomKeur commented 1 year ago

After doing some research, it seems like for the default we do not want to use Quorum queues for Delay queues, see:

https://www.rabbitmq.com/quorum-queues.html#use-cases

"When Not to Use Quorum Queues",

Temporary nature of queues: transient or exclusive queues, high queue churn (declaration and deletion rates)

But since RabbitMQ 3.12 you can set the default on vhost level in the GUI, when creating a new vhost. So maybe it should be an option, to set the "Default" for delay queues, for example

'x-queue-type' => 'classic',

adm-bome commented 1 year ago

So, it makes no sense and will not add any performance to the processing of messages.