Skip to content

/ symfony

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

Jump to bottom

[Messenger] Remove indices in messenger table on MySQL to prevent deadlocks while removing messages when running multiple consumers #42345

Merged
merged 1 commit into from Aug 26, 2021
Merged

[Messenger] Remove indices in messenger table on MySQL to prevent deadlocks while removing messages when running multiple consumers #42345

merged 1 commit into from Aug 26, 2021
+60 −3

Conversation

@jeroennoten
Copy link
Contributor

@jeroennoten jeroennoten commented Aug 2, 2021 •
edited

SELECT ... FOR UPDATE locks rows but also relevant indices. Since locking rows and indices is not one atomic operation,
this might cause deadlocks when running multiple workers. Removing indices on queue_name and available_at
resolves this problem.

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #41541 #39041
License MIT

Using Doctrine transport with multiple consumers occasionally results in MySQL deadlocks while removing a message from the messages database table.

This can be reproduced consistently by setting up a default async queue with the Doctrine transport and creating an empty TestMessage and TestMessageHandler. Create a command that dispatches 10000 of these messages in a for loop en start 4 message consumers. After a while, several consumers report a deadlock:

In Connection.php line 227:
                                                                                                                   
  [Symfony\Component\Messenger\Exception\TransportException]                                                       
  An exception occurred while executing 'DELETE FROM messenger_messages WHERE id = ?' with params ["32903"]:       
                                                                                                                   
  SQLSTATE[40001]: Serialization failure: 1213 Deadlock found when trying to get lock; try restarting transaction  
                                                                                                                   

Exception trace:
  at /var/www/vendor/symfony/messenger/Transport/Doctrine/Connection.php:227
 Symfony\Component\Messenger\Transport\Doctrine\Connection->ack() at /var/www/vendor/symfony/messenger/Transport/Doctrine/DoctrineReceiver.php:79
 Symfony\Component\Messenger\Transport\Doctrine\DoctrineReceiver->ack() at /var/www/vendor/symfony/messenger/Transport/Doctrine/DoctrineTransport.php:50
 Symfony\Component\Messenger\Transport\Doctrine\DoctrineTransport->ack() at /var/www/vendor/symfony/messenger/Worker.php:150
 Symfony\Component\Messenger\Worker->handleMessage() at /var/www/vendor/symfony/messenger/Worker.php:81
 Symfony\Component\Messenger\Worker->run() at /var/www/vendor/symfony/messenger/Command/ConsumeMessagesCommand.php:202
 Symfony\Component\Messenger\Command\ConsumeMessagesCommand->execute() at /var/www/vendor/symfony/console/Command/Command.php:255
 Symfony\Component\Console\Command\Command->run() at /var/www/vendor/symfony/console/Application.php:1027
 Symfony\Component\Console\Application->doRunCommand() at /var/www/vendor/symfony/framework-bundle/Console/Application.php:97
 Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() at /var/www/vendor/symfony/console/Application.php:273
 Symfony\Component\Console\Application->doRun() at /var/www/vendor/symfony/framework-bundle/Console/Application.php:83
 Symfony\Bundle\FrameworkBundle\Console\Application->doRun() at /var/www/vendor/symfony/console/Application.php:149
 Symfony\Component\Console\Application->run() at /var/www/bin/console:42

A similar problem with Laravel's queue worker (and a solution) is reported here: laravel/framework#31660

The solution is to remove indices on the queue_name and available_at columns. After removing these indices, I could not reproduce the issue anymore. Also, I did not notice any performance degradations.
@jeroennoten jeroennoten requested a review from sroze as a code owner Aug 2, 2021
@carsonbot carsonbot added this to the 4.4 milestone Aug 2, 2021
@OskarStark OskarStark requested a review from lyrixx Aug 2, 2021
@fabpot
Copy link
Member

@fabpot fabpot commented Aug 26, 2021

IIUC, this issue only exists on MySQL. It this is the case, the change should only be done for MySQL and not for all database engines.

@jeroennoten jeroennoten force-pushed the jeroennoten:feature/remove-index-from-messenger-database-table branch 3 times, most recently from 790721a to ee0c97b Aug 26, 2021
@jeroennoten
Copy link
Contributor Author

@jeroennoten jeroennoten commented Aug 26, 2021

@fabpot, you are right. To be sure, I tested with PostgreSQL and I could not reproduce the issues there.

So I changed this to only apply to MySQL. I also added a unit test for these changes.

@jeroennoten jeroennoten changed the title [Messenger] Remove indices in messenger table to prevent deadlocks while removing messages when running multiple consumers [Messenger] Remove indices in messenger table on MySQL to prevent deadlocks while removing messages when running multiple consumers Aug 26, 2021
@fabpot
fabpot requested changes Aug 26, 2021
View changes
src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php Show resolved Hide resolved
@jeroennoten jeroennoten force-pushed the jeroennoten:feature/remove-index-from-messenger-database-table branch 2 times, most recently from 364079e to f7cc0b1 Aug 26, 2021
@jeroennoten
Remove indices in messenger table on MySQL to prevent deadlocks while…
8c3c0a3 
… removing messages when running multiple consumers

SELECT ... FOR UPDATE row locks also locks indices. Since locking rows and indices is not one atomic operation,
this might cause deadlocks when running multiple workers. Removing indices on queue_name and available_at
resolves this problem.
@jeroennoten jeroennoten force-pushed the jeroennoten:feature/remove-index-from-messenger-database-table branch from f7cc0b1 to 8c3c0a3 Aug 26, 2021
@jeroennoten jeroennoten requested a review from fabpot Aug 26, 2021
@fabpot
fabpot approved these changes Aug 26, 2021
View changes
@fabpot
Copy link
Member

@fabpot fabpot commented Aug 26, 2021

Thank you @jeroennoten.

@fabpot fabpot merged commit 7080940 into symfony:4.4 Aug 26, 2021
10 checks passed
10 checks passed
@github-actions
Tests (7.1)
Details
@github-actions
Tests (7.2)
Details
@github-actions
Psalm
Details
@github-actions
Tests (8.0)
Details
@github-actions
Tests (8.0)
Details
@github-actions
Tests (7.4, high-deps)
Details
@github-actions
Tests (8.0, low-deps)
Details
@github-actions
Tests (8.1, experimental) Tests (8.1, experimental)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
fabbot.io Your code looks good.
Details
This was referenced Aug 30, 2021
Merged
Merged
@bobvandevijver
Copy link
Contributor

@bobvandevijver bobvandevijver commented Aug 30, 2021

Thank you @jeroennoten, I just saw this exact error in production for the very first time this weekend. Very nice to see it has already been resolved!

@jeroennoten
Copy link
Contributor Author

@jeroennoten jeroennoten commented Aug 30, 2021

@bobvandevijver, you're welcome!

Don't forget to run bin/console messenger:setup-transports after updating Symfony to the latest version to apply these changes to the database. The command will update the existing table or create a new one if it does not exist.

@jeroennoten jeroennoten deleted the jeroennoten:feature/remove-index-from-messenger-database-table branch Aug 30, 2021
@bobvandevijver
Copy link
Contributor

@bobvandevijver bobvandevijver commented Sep 1, 2021

We're using migrations to define the table, but thanks for the tip!

@torbenbr torbenbr mentioned this pull request Sep 3, 2021
Open
@PalTamasWBA
Copy link

@PalTamasWBA PalTamasWBA commented Sep 3, 2021 •
edited

@jeroennoten removing the indexes fixes the lock issue but caused us significant performance issues. I was searching the issues and it seems to me that multiple consumers are not working very well for others either, be it redis or doctrine transport, no matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot

@sroze sroze

@lyrixx lyrixx

Assignees
No one assigned
Labels
Bug Messenger Status: Reviewed
Projects
None yet
Milestone
4.4
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@jeroennoten @fabpot @bobvandevijver @PalTamasWBA @carsonbot