fhteam / laravel-amqp

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

argument "x-expires" to the deyed queue added #4

Closed abachmann closed 9 years ago

abachmann commented 9 years ago

I need to send messages with a high range of different delays. In order to reduce the number of queues I need to make use of the argument "x-expires".

A detailed description can be found here: https://www.rabbitmq.com/ttl.html#queue-ttl

Creating a queue is not time intensive and an additional delay of one second is sufficient. In my opinion it's not necessary to make this argument configurable.

matyunin commented 9 years ago

@abachmann, thanks for your pr!

I think we need to move the x-expires to the function arguments, or pass it as an array of additional arguments.

I.e. like this:

public function declareDelayedQueue($destinationQueueName, $delay, array $additionalArguments = [])
{
    $destinationQueueName = $this->getQueueName($destinationQueueName);
    $deferredQueueName = $destinationQueueName . '_deferred_' . $delay;

    $arguments = [
        'x-dead-letter-exchange' => ['S', ''],
        'x-dead-letter-routing-key' => ['S', $destinationQueueName],
        'x-message-ttl' => ['I', $delay * 1000],
        'x-expires' => ['I', ($delay + 1) * 1000],
    ];

    if (!empty($additionalArguments)) {
        $arguments = array_merge($arguments, $additionalArguments);
    }

    $flags = array_replace([
        'queue' => '',
        'passive' => false,
        'durable' => false,
        'exclusive' => false,
        'auto_delete' => true,
        'nowait' => false,
        'arguments' => null,
        'ticket' => null,
    ], $this->queueFlags, [
        'queue' => $deferredQueueName,
        'durable' => true,
        'arguments' => $arguments,
    ]);

    call_user_func_array([$this->channel, 'queue_declare'], $flags);

    return $deferredQueueName;
}

This way can help us not only specify any additional arguments but also redeclare predefined array values:

$amqpQueue->declareDelayedQueue('queue:name', 1, [
    'x-dead-letter-exchange' => ['S', 'deadXchg'],
    'x-expires' => ['I', 4000],
]);

@FractalizeR, what can you say?

FractalizeR commented 9 years ago

This solution will make automatic queue destruction unavoidable which is not always desired.

I'd propose to just change array_replace call inside declareDelayedQueue to array_replace_recursive. That would allow you to pass whatever queue additional settings using $queueFlags parameter to the constructor. You will be able to make it like this then:

$queueFlags = [
    'arguments' => [
        'x-expires' => ['I', 5000],
    ],

This way you will have complete control over the situation. And this allows to support whatever additional flags you want without changing declareDelayedQueue signature.

What do you, guys, think?

matyunin commented 9 years ago

@FractalizeR, :+1:

FractalizeR commented 9 years ago

@abachmann ? ;)

abachmann commented 9 years ago

@FractalizeR I like your approach and it's the cleanest one!

In my case the problem is that it's still not possible to define a value for x-expires that's relative to the passed $delay in declareDelayedQueue. Therfore I'd prefer ['I', ($delay + 1) * 1000] as a default value for x-expires.

Is it ok for both of you?

FractalizeR commented 9 years ago

I think I've found a good temporary solution. The problem is that the current architecture doesn't provide flexible work with multiple queues and multiple delayed queues. This is something to think about in 2.0.

Please have a look at 411932a93ba830bb44d6fc5396c5746b2ec699b3. I've provided some way to enable dynamic queue configuration via use of closures in configuration array. If that suites you well, you can test it with your case. If all goes well, we will tag this.

abachmann commented 9 years ago

@FractalizeR Great!