Reset nested transaction level on deadlock exceptions #6103
Conversation
The test validates broken behaviour with nested transactions.
| $this->connection->beginTransaction(); | ||
| $this->connection->executeStatement('DELETE FROM test1'); | ||
|
|
||
| $pid = pcntl_fork(); |
There was a problem hiding this comment.
Do we really need to fork the process? Wouldn't opening a second connection be enough to provoke a deadlock?
There was a problem hiding this comment.
Unfortunately, we can't as PHP is single threaded and if we attempt to simply work with two connections it just hangs till a timeout is reached. Already tried it.
Another possible solution to skip the output buffer of phpunit is to simply die in the child process, but for me it seems a bit "hacky", if that is the thing that bothers you.
| $this->connection->commit(); | ||
| $this->connection->commit(); | ||
| } catch (DeadlockException $ex) { | ||
| self::assertFalse($this->connection->isTransactionActive()); |
There was a problem hiding this comment.
This assertion does not tell me that the connection's transaction status is in sync with the driver.
| { | ||
| parent::setUp(); | ||
|
|
||
| $supportedDrivers = ['pdo_mysql', 'mysqli', 'pdo_pgsql']; |
There was a problem hiding this comment.
You said that this is an issue of MySQL drivers only, yet you're including PDO_pgsql as well. Does it make sense to exclude any drivers here? Can we come up with a driver-agnostic functional test that makes sure we can start a new transaction aftera deadlock?
There was a problem hiding this comment.
Only mysql/postgres of the supported drivers have deadlock exceptions. Other databases do not have such exceptions.
But, it seems that after attempting what you have suggested, to open a new transaction, this fix isn't enough and the pdo driver library must first be patched, as it stays in "active transaction state".
See the link here:
https://github.com/php/php-src/blob/b132b7ab7efb40628dedf2eae1cf7d6949684bcb/ext/pdo/pdo_dbh.c#L605
For now will postpone that pull request and willl attempt to fix firstly the php pdo driver and then will modify that PR.
Sorry for bothering you at this time.
|
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. |
|
This pull request was closed due to inactivity. |
Summary
All mysql flavors on deadlock exception rollback all transactions, thus making the internal counter in DBAL invalid.
This pull request fixes that behaviour by reseting the dbal nested transactions counter on such exceptions.