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

MySQL + deadlock leads to SAVEPOINT DOCTRINE_x does not exist #6651

Open
nikophil opened this issue Dec 18, 2024 · 15 comments
Open

MySQL + deadlock leads to SAVEPOINT DOCTRINE_x does not exist #6651

nikophil opened this issue Dec 18, 2024 · 15 comments

Comments

@nikophil
Copy link
Contributor

nikophil commented Dec 18, 2024

Bug Report

Q A
Version any

Summary

Hello,

this issue is a follow up of doctrine/orm#11230: in MySQL, when a deadlock occurs because of concurrency, the system tries to rollback, and then an error "SAVEPOINT DOCTRINE_x does not exist" occurs.

At first, the savepoint error did hide the original deadlock error. This has been fixed thanks to doctrine/orm#11646 - and this will soon be fixed as well in symfony/messenger.

Nevertheless, doctrine/orm#11230 has been closed, but the original problem is not fixed, and the rollback still fails, which results in closing the EM, and breaking the worker.

Since the dbal is the only one aware of savepoints, I think this should be fixed here.

Current behavior

When a deadlock occur, within a transaction with savepoints in MySQL, the rollback fails with SAVEPOINT DOCTRINE_x does not exist

Expected behavior

No error on roll back 😅

From MySQL doc:

When deadlock detection is enabled (the default) and a deadlock does occur, InnoDB detects the condition and rolls back one of the transactions (the victim).

So maybe when a deadlock occurs, DBAL should be resilient when the rollback fails? Or it should check the existence of the given savepoint/transaction before rolling back?

@petermanders89
Copy link

I am not sure how related this issue is; but I experience the following. I get the same error related to the doctrine:migrations:migrate command.

I am using symfony with doctrine. I have a symfony command that prepares the application. My installation command is pretty simple. I run: doctrine:migrations:migrate --no-interaction and in the same command I load an entity; persist and flush. This gives me the similair error: 1305 SAVEPOINT DOCTRINE_2 does not exist. I use Doctrine\ORM\EntityManagerInterface with dependency injection as variable entityManager. The initial database is empty.

$this->getApplication()->setAutoExit(false);
$this->getApplication()->run(new StringInput('doctrine:migrations:migrate --no-interaction'), $output);

$post = new Post();
$post->setTitle('this is a title');

$this->entityManager->persist($policy);
$this->entityManager->flush();

If I put $this->entityManager->getConnection()->isTransactionActive() after the migrations command, this returns true.

If I run the command twice, there is no error due to the fact that the migrations are already executed and up to date. Even if I add multiple new entities, persist and flush them there are no issues if I run the migrations first.

This issue only occurs after I updated from doctrine/dbal: 3.* to doctrine/dbal: 4.*.

I have create an repo to reproduce this issue: https://github.com/petermanders89/dbal-issue. This is repo created with symfony new my_project_directory --webapp. I modified it slightly.

When running the install command; in this case with symfony console app:install it'll throw this same error.

@nikophil
Copy link
Contributor Author

hey @petermanders89

if using MySQL, you should try to declare your migrations as "non transactional". In MySQL, "ALTER TABLE" statements, and all other statements which modify the db structure implicitly closes the transaction. Connection::isTransactionActive() does not check the DB to see if a transaction is actually active.

At the end, when doctrine commits the changes, it tries to commit a non-existent transaction, and your migration fails 💥
I think you're facing this problem with migration to dbal 4, because dbal 4 forces usage of savepoints, but I think it's actually not the same problem than the one I'm talking about.

@petermanders89
Copy link

Hey @nikophil . Thank you! That indeed solves all my issues related to this. Sorry to mess up your issue.

@morozov
Copy link
Member

morozov commented Dec 22, 2024

@nikophil please provide a sample script or other instructions that will reproduce your issue. Ideally, it should use only DBAL as a dependency.

@nikophil
Copy link
Contributor Author

nikophil commented Dec 23, 2024

Hi @morozov

here it is: https://github.com/nikophil/dbal-repro

you can run it by:

  • set the DSN of a MySQL connection in src/create-connection.php
  • run composer install
  • run php ./src/create-deadlock.php

I've somehow reproduced a concurrency problem by running twice src/update-line.php at the same time, using symfony/process

The concurrency creates a deadlock. On my machine, I have a deadlock, and an error on rollback every time I'm running the script.

The script outputs the error, basically:

PHP Fatal error:  Uncaught PDOException: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE_2 does not exist in /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Driver/PDO/Connection.php:27
Stack trace:
#0 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Driver/PDO/Connection.php(27): PDO->exec()
#1 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(878): Doctrine\DBAL\Driver\PDO\Connection->exec()
#2 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1140): Doctrine\DBAL\Connection->executeStatement()
#3 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1080): Doctrine\DBAL\Connection->rollbackSavepoint()
#4 /home/nicolas/works/github.com/nikophil/dbal-repro/src/update-line.php(32): Doctrine\DBAL\Connection->rollBack()
#5 {main}

Next Doctrine\DBAL\Driver\PDO\Exception: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE_2 does not exist in /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Driver/PDO/Exception.php:28
Stack trace:
#0 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Driver/PDO/Connection.php(33): Doctrine\DBAL\Driver\PDO\Exception::new()
#1 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(878): Doctrine\DBAL\Driver\PDO\Connection->exec()
#2 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1140): Doctrine\DBAL\Connection->executeStatement()
#3 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1080): Doctrine\DBAL\Connection->rollbackSavepoint()
#4 /home/nicolas/works/github.com/nikophil/dbal-repro/src/update-line.php(32): Doctrine\DBAL\Connection->rollBack()
#5 {main}

Next Doctrine\DBAL\Exception\DriverException: An exception occurred while executing a query: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE_2 does not exist in /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Driver/API/MySQL/ExceptionConverter.php:91
Stack trace:
#0 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1373): Doctrine\DBAL\Driver\API\MySQL\ExceptionConverter->convert()
#1 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1315): Doctrine\DBAL\Connection->handleDriverException()
#2 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(880): Doctrine\DBAL\Connection->convertExceptionDuringQuery()
#3 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1140): Doctrine\DBAL\Connection->executeStatement()
#4 /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Connection.php(1080): Doctrine\DBAL\Connection->rollbackSavepoint()
#5 /home/nicolas/works/github.com/nikophil/dbal-repro/src/update-line.php(32): Doctrine\DBAL\Connection->rollBack()
#6 {main}
thrown in /home/nicolas/works/github.com/nikophil/dbal-repro/vendor/doctrine/dbal/src/Driver/API/MySQL/ExceptionConverter.php on line 91

as you can see, the rollback throws an error, and the problem occurs for both dbal 3 et 4

@nikophil
Copy link
Contributor Author

hey @morozov

by any chance, did you check the reproducer I gave?

Do you think we can fix this? I'd be willing to provide a PR, but I'd prefer that we agree on a solution first.

thanks 🙏

@morozov
Copy link
Member

morozov commented Jan 21, 2025

@nikophil thanks for the reproducer. I was able to run it and observe the same behavior as described in the issue.

The question is, why do you believe this is a bug? In my understanding, a nested transaction attempts to roll back to a savepoint. This savepoint no longer exists because the whole transaction in the session where it was previously created was rolled back as a victim of a deadlock. So the attempt fails, which is reported via an exception. This looks correct.

If it doesn't matter to your business logic whether the transaction was rolled back to the savepoint or fully rolled back, why don't you catch this exception and do nothing?

@nikophil
Copy link
Contributor Author

In my understanding, the underlying error (the deadlock) is hidden by the savepoint error, so it's quite hard to reason about the problem.

@morozov
Copy link
Member

morozov commented Jan 21, 2025

So, if you're saying that "if a deadlock occurs in a nested transaction, a DeadlockException should be thrown to the user-land", then I think I agree.

@nikophil
Copy link
Contributor Author

Yeah that's were I'd like to go :)

But what should we do if a save point error occurs?
My opinion would be to totally ignore the savepoint error. The whole transaction is closed anyway 🤷‍♂️

But as far as I know this is only a mysql problem 🤔

@morozov
Copy link
Member

morozov commented Jan 21, 2025

If a savepoint error occurs due to a deadlock, it should throw a deadlock exception. From the API consumer's standpoint, there are no savepoints, there are nested transactions, and we need to tell all consumers that started a transaction at any level that their transaction was rolled back due to a deadlock.

@nikophil
Copy link
Contributor Author

If a savepoint error occurs due to a deadlock

I'm wondering how this can be achieved, there is no way to know what was the reason why rollback() was called. And I'm pretty sure no parameter can be added to this method 😅

And always muting the error from rollbackSavepoint() does not seem the way to go neither...

any hints about this?

@morozov
Copy link
Member

morozov commented Jan 21, 2025

And always muting the error from rollbackSavepoint() does not seem the way to go neither...

That's for sure.

any hints about this?

I'm not really familiar with transactions in DBAL, so I don't have any immediate hints, and there isn't necessary an easy solution to this. If I were to implement this, I'd start with a space-time diagram (like this) and document the current behavior. The actors there would be:

  1. The component that starts the top-level transaction in the victim thread.
  2. The component that starts the nested transaction in the victim thread.
  3. The component that starts a transaction in the winning thread.

I'd documented the interaction between them in the form of method calls, returns and/or exceptions. Then it may become clear how the communication needs to change.

@nikophil
Copy link
Contributor Author

well, it seems that it can't be fixed directly in the DBAL without a BC break 🤔 but maybe this can be mitigated in the callers.

In my case, the component which starts the transactions is symfony/messenger (through DoctrineTransactionMiddleware)

I think this could also be done in the ORM: here and here.

Those are the same places where doctrine/orm#11230 and symfony/symfony#59103 fixed the previous error where a rollback error did hide the original error.

any thoughts @greg0ire?

@greg0ire
Copy link
Member

@nikophil I'm not sure I understand the issue. Are you saying:

  1. there is a deadlock
  2. code inside the finally throws another exception about a savepoint that does not exist
  3. despite what I recently did, the deadlock exception is not visible as a "previous exception" of the exception mentioned in step 2. ?

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

No branches or pull requests

4 participants