Skip to content

[dbal] requeue leads to duplicate entries #814

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

Closed
adrianrudnik opened this issue Apr 1, 2019 · 4 comments
Closed

[dbal] requeue leads to duplicate entries #814

adrianrudnik opened this issue Apr 1, 2019 · 4 comments
Assignees
Labels

Comments

@adrianrudnik
Copy link

adrianrudnik commented Apr 1, 2019

I Just tried to implement a simple JSON submit queue where the endpoint can either respond HTTP status code 200 or fail. For that fail case I wanted to use the requeue option.

Building a very basic approach I always end up with endless new entries in the enqueue table, every retry leading to a new entry. Maybe I'm totally reading the docs wrong:

Interop\Queue\Processor::REQUEUE defines itself as The original message is removed from the queue but a copy is published to the queue again.. Looking at the code this is exact scenario is not reached, as the function returns prior removing the old message.

$this->deleteMessage($message->getDeliveryId());

In addition to this the Enqueue\Dbal\DbalProducer::send() never looks for duplicates and updates them, instead always creating a new entry. This also leads to an infinite loop that fills the database with new entries which are directly processed in the same run.

Is this intended behaviour?

@Steveb-p
Copy link
Contributor

Steveb-p commented Apr 1, 2019

It does seem that early exit a line before should probably be removed.

public function reject(Message $message, bool $requeue = false): void
{
InvalidMessageException::assertMessageInstanceOf($message, DbalMessage::class);
if ($requeue) {
$message = clone $message;
$message->setRedelivered(false);
$this->getContext()->createProducer()->send($this->queue, $message);
return;
}
$this->deleteMessage($message->getDeliveryId());
}

It was superfluous at the moment it was introduced a year ago (

/**
* {@inheritdoc}
*
* @param DbalMessage $message
*/
public function reject(PsrMessage $message, $requeue = false)
{
InvalidMessageException::assertMessageInstanceOf($message, DbalMessage::class);
if ($requeue) {
$this->context->createProducer()->send($this->queue, $message);
return;
}
}
) and was probably left over without much thought.

Clearly, it is an error in the library. I'll provide a PR with functional test covering it.

@Steveb-p
Copy link
Contributor

Steveb-p commented Apr 1, 2019

@adrianrudnik could you check in your local version if you swap the DbalConsumer code with the one in the PR that it behaves how you'd expect?

@Steveb-p Steveb-p self-assigned this Apr 1, 2019
@Steveb-p Steveb-p added the bug label Apr 1, 2019
@adrianrudnik
Copy link
Author

@Steveb-p just checked, yes it does, thanks!

@Steveb-p
Copy link
Contributor

Steveb-p commented Apr 1, 2019

Tests without the change in code:

PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

...............................................................  63 / 144 ( 43%)
.........F..................................................... 126 / 144 ( 87%)
...............F..                                              144 / 144 (100%)

Time: 33.65 seconds, Memory: 12.00MB

There were 2 failures:

1) Enqueue\Dbal\Tests\Functional\DbalConsumerTest::testShouldRemoveOriginalMessageThatHaveBeenRejectedWithRequeue
Failed asserting that 2 is identical to 1.

...

Tests with changes removing the return.

PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

...............................................................  63 / 144 ( 43%)
............................................................... 126 / 144 ( 87%)
..................                                              144 / 144 (100%)

Time: 33.41 seconds, Memory: 12.00MB

OK (144 tests, 261 assertions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants