-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Reset nested transaction level on deadlock exceptions #6103
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
<?php | ||
|
||
namespace Doctrine\DBAL\Tests\Functional\Driver\PDO\MySQL; | ||
|
||
use Doctrine\DBAL\Exception\DeadlockException; | ||
use Doctrine\DBAL\Schema\Table; | ||
use Doctrine\DBAL\Tests\FunctionalTestCase; | ||
use Doctrine\DBAL\Tests\TestUtil; | ||
|
||
use function implode; | ||
use function pcntl_fork; | ||
use function sleep; | ||
use function sprintf; | ||
|
||
/** @require extension pcntl */ | ||
class DeadlockTest extends FunctionalTestCase | ||
{ | ||
protected function setUp(): void | ||
{ | ||
parent::setUp(); | ||
|
||
$supportedDrivers = ['pdo_mysql', 'mysqli', 'pdo_pgsql']; | ||
if (! TestUtil::isDriverOneOf(...$supportedDrivers)) { | ||
self::markTestSkipped(sprintf('This supports one of %s drivers', implode(', ', $supportedDrivers))); | ||
} | ||
|
||
$table = new Table('test1'); | ||
$table->addColumn('id', 'integer'); | ||
$this->dropAndCreateTable($table); | ||
|
||
$table = new Table('test2'); | ||
$table->addColumn('id', 'integer'); | ||
$this->dropAndCreateTable($table); | ||
|
||
$this->connection->executeStatement('INSERT INTO test1 VALUES(1)'); | ||
$this->connection->executeStatement('INSERT INTO test2 VALUES(1)'); | ||
} | ||
|
||
public function testNestedTransactionsDeadlockExceptionHandling(): void | ||
{ | ||
$this->connection->setNestTransactionsWithSavepoints(true); | ||
|
||
$this->connection->beginTransaction(); | ||
$this->connection->beginTransaction(); | ||
$this->connection->executeStatement('DELETE FROM test1'); | ||
|
||
$pid = pcntl_fork(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
if ($pid) { | ||
$connection = TestUtil::getConnection(); | ||
$connection->beginTransaction(); | ||
$connection->executeStatement('DELETE FROM test2'); | ||
$connection->executeStatement('DELETE FROM test1'); | ||
$connection->commit(); | ||
$connection->close(); | ||
|
||
$this->markTestSkipped('Gracefully skip child process in deadlock test'); | ||
} | ||
|
||
try { | ||
$this->waitForTableLock(); | ||
$this->connection->executeStatement('DELETE FROM test2'); | ||
$this->connection->commit(); | ||
$this->connection->commit(); | ||
} catch (DeadlockException $ex) { | ||
self::assertFalse($this->connection->isTransactionActive()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion does not tell me that the connection's transaction status is in sync with the driver. |
||
|
||
return; | ||
} | ||
|
||
$this->fail('Expected deadlock exception did not happen.'); | ||
} | ||
|
||
private function waitForTableLock(): void | ||
{ | ||
sleep(5); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.