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

Redelivering #393

Open
aivchen opened this issue Sep 21, 2020 · 4 comments
Open

Redelivering #393

aivchen opened this issue Sep 21, 2020 · 4 comments
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug

Comments

@aivchen
Copy link
Contributor

aivchen commented Sep 21, 2020

What steps will reproduce the problem?

  1. Add some tasks to the queue.
  2. Run yii queue/listen and interrupt it by Ctrl+C.

What's expected?

The failed task (since RabbitMQ 2.7.0 ) must go to the head of the queue to keep the order of the tasks (https://www.rabbitmq.com/semantics.html#ordering).

What do you get instead?

The failed task goes to the back of the queue.

Additional info

Q A
Yii version 2.0.37
PHP version 7.2
Operating system Linux Alpine
RabbitMQ version 3.8.8
RabbitMQ Driver amqp_interop

Command:

<?php
namespace app\commands;


use app\jobs\bc\TestJob;
use Yii;
use yii\console\Controller;

class TestController extends Controller
{
    public function actionIndex(): void
    {
        for ($i = 0; $i < 5; $i++) {
            \Yii::$app->queue->push(Yii::createObject([
                'class' => TestJob::class,
                'id' => $i,
            ]));
        }
    }
}

Job:

<?php


namespace app\jobs\bc;


use yii\queue\JobInterface;

class TestJob implements JobInterface
{
    public $id;

    public function execute($queue)
    {
        print "$this->id\n";

        sleep(3);
    }
}

Output:

bash-5.0# php yii test
bash-5.0# php yii queue/listen
0
1
^C
bash-5.0# php yii queue/listen
2
3
4
1

@samdark
Copy link
Member

samdark commented Sep 21, 2020

That was made intentionally not to halt the rest of the queue while re-trying a single task. That's desired behavior for the case with more than a single worker (or another case when order isn't important). I guess it can be made configurable...

@aivchen
Copy link
Contributor Author

aivchen commented Sep 22, 2020

That was made intentionally not to halt the rest of the queue while re-trying a single task.

But in my example TestJob doesn't implement RetryableJobInterface. Why the library sends the task to the back of the queue?

@samdark
Copy link
Member

samdark commented Sep 22, 2020

That's a good question. Since were talking about desired behavior of the job to be sent to the head of queue I've thought we're talking about retries. If there are no retries, why do you expect the job to re-enter queue at all?

@aivchen
Copy link
Contributor Author

aivchen commented Sep 22, 2020

Because of this (https://www.rabbitmq.com/semantics.html#ordering):

Messages can be returned to the queue using AMQP methods that feature a requeue parameter (basic.recover, basic.reject and basic.nack), or due to a channel closing while holding unacknowledged messages.

As I said I pressed Ctrl+c to stop the queue worker (no exception were thrown). In this case, as I understand, the message must be returned to the head of the queue because of the closed channel and unacknowledged message.

Also, I've done some debug. Indeed, the message returns to the head of the queue, but the library on the next launch marks the message as acknowledged and redelivers to the back of the queue.
It happens here: https://github.com/yiisoft/yii2-queue/blob/master/src/drivers/amqp_interop/Queue.php#L265

But if the job throws an exception the library works as expected.

For example:

class TestJob implements JobInterface
{
    public $id;

    public function execute($queue)
    {
        print "$this->id\n";

        if ($this->id === 3) {
            throw new \RuntimeException('Some error');
        }

        sleep(3);
    }
}

In this case, the task won't be redelivered to the queue as I assume the library marks the message as acknowledged.

@samdark samdark added status:ready for adoption Feel free to implement this issue. type:bug Bug and removed status:under discussion labels Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug
Projects
None yet
Development

No branches or pull requests

2 participants