transactional() doesn't correctly manage nested transactions with savepoints upon MySQL deadlock

aboks commented 3 years ago

Bug Report

Q	A
BC Break	no
Version	2.10.2

Summary

Using DBAL with MySQL, if a deadlock occurs inside a nested transactional() (with savepoints to emulate nested transactions), the DBAL connection tries to rollback the savepoint. However, MySQL automatically rolls back the transaction (and thus releases all savepoints) upon a deadlock. Therefore the attempt to rollback the savepoint fails with SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE2_SAVEPOINT_2 does not exist.

Current behaviour

The DBAL connection does not correctly take into account that the deadlock rolled back the transaction and all savepoints, and thus tries to rollback to the nonexisting savepoint. This fails with

Fatal error: Uncaught PDOException: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE2_SAVEPOINT_2 does not exist in /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:41
Stack trace:
#0 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php(41): PDO->exec()
#1 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php(1439): Doctrine\DBAL\Driver\PDOConnection->exec()
#2 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php(1371): Doctrine\DBAL\Connection->rollbackSavepoint()
#3 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php(1181): Doctrine\DBAL\Connection->rollBack()
#4 /var/www/html/deadlock.php(43): Doctrine\DBAL\Connection->transactional()
#5 {main}

Next Doctrine\DBAL\Driver\PDOException: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE2_SAVEPOINT_2 does not exist in /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php on line 43

How to reproduce

Have a running MySQL server with a database in it. Create a script like below and fill in the DSN of the database:

<?php
// deadlock.php
use Doctrine\DBAL\DriverManager;

require_once(__DIR__ . "/vendor/autoload.php");

$thread = intval($argv[1]);

$conn = DriverManager::getConnection([
    "url" => "(DSN to MySQL database)",
]);
$conn->setNestTransactionsWithSavepoints(true);

if ($thread === 1) {
    $conn->exec("CREATE TABLE IF NOT EXISTS `test1` (
      `id` int(11) NOT NULL AUTO_INCREMENT,
      PRIMARY KEY (`id`)
    ) ENGINE=InnoDB DEFAULT CHARSET=latin1;");
    $conn->exec("INSERT INTO test1 VALUES()");

    $conn->exec("CREATE TABLE IF NOT EXISTS `test2` (
      `id` int(11) NOT NULL AUTO_INCREMENT,
      PRIMARY KEY (`id`)
    ) ENGINE=InnoDB DEFAULT CHARSET=latin1;");
    $conn->query("INSERT INTO test2 VALUES()");
    print "Setup done\n";
}

$conn->transactional(function () use ($conn, $thread) {
    $conn->transactional(function () use ($conn, $thread) {
        print "Running thread $thread\n";
        if ($thread === 1) {
            $conn->query("DELETE FROM test1");
            sleep(10);
            $conn->query("DELETE FROM test2");
        } else {
            $conn->query("DELETE FROM test2");
            sleep(10);
            $conn->query("DELETE FROM test1");
        }
    });
});

print "DONE!\n";

Now open two separate shells. From the one, run php deadlock.php 1 and (within 10 seconds) run from the other shell php deadlock.php 2 The script in the second shell will now fail with the SAVEPOINT DOCTRINE2_SAVEPOINT_2 does not exist message above.

Expected behaviour

The expected behaviour is that the DBAL connection knows that a deadlock has already rolled back the entire transaction. It therefore does not attempt to rollback the savepoints or the transaction anymore. The script in the second shell will then fail with the original deadlock exception (SQLSTATE[40001]: Serialization failure: 1213 Deadlock found when trying to get lock; try restarting transaction).

davidrans commented 3 years ago

FYI, this may be related: https://github.com/php/php-src/commit/7b9519a792a04d6943ff7082ff343a96ec00157f

hvt commented 3 years ago

Think this might be related to the fact there is no exception conversion being done inside Doctrine\DBAL\Driver\PDO::commit().

Is there a particular reason why that is not being done there?

Let me add to that, that in my case when issuing the commit, a deadlock is detected. In vanilla MySQL (or MariaDB), in my knowledge this never happens. However, when using Galera in a master-master configuration, deadlock detection is done async with other masters after the query has been answered. If in the end that query would end up in a deadlock, this is only propagated back to the client when the client issues a following query. Which in our case is the commit. This concept is described here.

So in our situation, exception conversion would also be preferred around the commit statement. So that we can nicely catch a DeadlockException instead of manually going through the native PDOException.

doctrine / dbal