diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ec14cd164a..ceb2221168 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -79,6 +79,7 @@ phutil_register_library_map(array( 'AphrontQueryConnectionException' => 'storage/exception/connection', 'AphrontQueryConnectionLostException' => 'storage/exception/connectionlost', 'AphrontQueryCountException' => 'storage/exception/count', + 'AphrontQueryDeadlockException' => 'storage/exception/deadlock', 'AphrontQueryDuplicateKeyException' => 'storage/exception/duplicatekey', 'AphrontQueryException' => 'storage/exception/base', 'AphrontQueryObjectMissingException' => 'storage/exception/objectmissing', @@ -182,9 +183,9 @@ phutil_register_library_map(array( 'ConduitAPI_slowvote_info_Method' => 'applications/conduit/method/slowvote/info', 'ConduitAPI_user_Method' => 'applications/conduit/method/user/base', 'ConduitAPI_user_addstatus_Method' => 'applications/conduit/method/user/addstatus', - 'ConduitAPI_user_removestatus_Method' => 'applications/conduit/method/user/removestatus', 'ConduitAPI_user_find_Method' => 'applications/conduit/method/user/find', 'ConduitAPI_user_info_Method' => 'applications/conduit/method/user/info', + 'ConduitAPI_user_removestatus_Method' => 'applications/conduit/method/user/removestatus', 'ConduitAPI_user_whoami_Method' => 'applications/conduit/method/user/whoami', 'ConduitException' => 'applications/conduit/protocol/exception', 'DarkConsoleConfigPlugin' => 'aphront/console/plugin/config', @@ -1118,6 +1119,7 @@ phutil_register_library_map(array( 'AphrontQueryConnectionException' => 'AphrontQueryException', 'AphrontQueryConnectionLostException' => 'AphrontQueryRecoverableException', 'AphrontQueryCountException' => 'AphrontQueryException', + 'AphrontQueryDeadlockException' => 'AphrontQueryRecoverableException', 'AphrontQueryDuplicateKeyException' => 'AphrontQueryException', 'AphrontQueryObjectMissingException' => 'AphrontQueryException', 'AphrontQueryParameterException' => 'AphrontQueryException', @@ -1207,9 +1209,9 @@ phutil_register_library_map(array( 'ConduitAPI_slowvote_info_Method' => 'ConduitAPIMethod', 'ConduitAPI_user_Method' => 'ConduitAPIMethod', 'ConduitAPI_user_addstatus_Method' => 'ConduitAPI_user_Method', - 'ConduitAPI_user_removestatus_Method' => 'ConduitAPI_user_Method', 'ConduitAPI_user_find_Method' => 'ConduitAPI_user_Method', 'ConduitAPI_user_info_Method' => 'ConduitAPI_user_Method', + 'ConduitAPI_user_removestatus_Method' => 'ConduitAPI_user_Method', 'ConduitAPI_user_whoami_Method' => 'ConduitAPI_user_Method', 'DarkConsoleConfigPlugin' => 'DarkConsolePlugin', 'DarkConsoleController' => 'PhabricatorController', diff --git a/src/storage/connection/base/AphrontDatabaseConnection.php b/src/storage/connection/base/AphrontDatabaseConnection.php index 62e185db13..2f57cd417d 100644 --- a/src/storage/connection/base/AphrontDatabaseConnection.php +++ b/src/storage/connection/base/AphrontDatabaseConnection.php @@ -22,13 +22,12 @@ */ abstract class AphrontDatabaseConnection { - private static $transactionStates = array(); + private $transactionState; abstract public function getInsertID(); abstract public function getAffectedRows(); abstract public function selectAllResults(); abstract public function executeRawQuery($raw_query); - abstract protected function getTransactionKey(); abstract public function escapeString($string); abstract public function escapeColumnName($string); @@ -133,11 +132,62 @@ abstract class AphrontDatabaseConnection { * @task xaction */ protected function getTransactionState() { - $key = $this->getTransactionKey(); - if (empty(self::$transactionStates[$key])) { - self::$transactionStates[$key] = new AphrontDatabaseTransactionState(); + if (!$this->transactionState) { + $this->transactionState = new AphrontDatabaseTransactionState(); } - return self::$transactionStates[$key]; + return $this->transactionState; + } + + + /** + * @task xaction + */ + public function beginReadLocking() { + $this->getTransactionState()->beginReadLocking(); + return $this; + } + + + /** + * @task xaction + */ + public function endReadLocking() { + $this->getTransactionState()->endReadLocking(); + return $this; + } + + + /** + * @task xaction + */ + public function isReadLocking() { + return $this->getTransactionState()->isReadLocking(); + } + + + /** + * @task xaction + */ + public function beginWriteLocking() { + $this->getTransactionState()->beginWriteLocking(); + return $this; + } + + + /** + * @task xaction + */ + public function endWriteLocking() { + $this->getTransactionState()->endWriteLocking(); + return $this; + } + + + /** + * @task xaction + */ + public function isWriteLocking() { + return $this->getTransactionState()->isWriteLocking(); } } diff --git a/src/storage/connection/isolated/AphrontIsolatedDatabaseConnection.php b/src/storage/connection/isolated/AphrontIsolatedDatabaseConnection.php index 25f57e4cc9..e4a1eda11a 100644 --- a/src/storage/connection/isolated/AphrontIsolatedDatabaseConnection.php +++ b/src/storage/connection/isolated/AphrontIsolatedDatabaseConnection.php @@ -25,8 +25,6 @@ final class AphrontIsolatedDatabaseConnection private $configuration; private static $nextInsertID; private $insertID; - private static $nextTransactionKey = 1; - private $transactionKey; private $transcript = array(); @@ -38,8 +36,6 @@ final class AphrontIsolatedDatabaseConnection // collisions and make them distinctive. self::$nextInsertID = 55555000000 + mt_rand(0, 1000); } - - $this->transactionKey = 'iso-xaction-'.(self::$nextTransactionKey++); } public function escapeString($string) { @@ -70,10 +66,6 @@ final class AphrontIsolatedDatabaseConnection return $this->affectedRows; } - protected function getTransactionKey() { - return $this->transactionKey; - } - public function selectAllResults() { return $this->allResults; } diff --git a/src/storage/connection/mysql/base/AphrontMySQLDatabaseConnectionBase.php b/src/storage/connection/mysql/base/AphrontMySQLDatabaseConnectionBase.php index 7e7fda155e..8b0c8d0ce1 100644 --- a/src/storage/connection/mysql/base/AphrontMySQLDatabaseConnectionBase.php +++ b/src/storage/connection/mysql/base/AphrontMySQLDatabaseConnectionBase.php @@ -225,7 +225,7 @@ abstract class AphrontMySQLDatabaseConnectionBase throw new AphrontQueryConnectionLostException($exmsg); case 1213: // Deadlock case 1205: // Lock wait timeout exceeded - throw new AphrontQueryRecoverableException($exmsg); + throw new AphrontQueryDeadlockException($exmsg); case 1062: // Duplicate Key // NOTE: In some versions of MySQL we get a key name back here, but // older versions just give us a key index ("key 2") so it's not diff --git a/src/storage/connection/mysql/base/__init__.php b/src/storage/connection/mysql/base/__init__.php index 61fd3b1ca3..1607ede52d 100644 --- a/src/storage/connection/mysql/base/__init__.php +++ b/src/storage/connection/mysql/base/__init__.php @@ -12,8 +12,8 @@ phutil_require_module('phabricator', 'storage/connection/base'); phutil_require_module('phabricator', 'storage/exception/accessdenied'); phutil_require_module('phabricator', 'storage/exception/base'); phutil_require_module('phabricator', 'storage/exception/connectionlost'); +phutil_require_module('phabricator', 'storage/exception/deadlock'); phutil_require_module('phabricator', 'storage/exception/duplicatekey'); -phutil_require_module('phabricator', 'storage/exception/recoverable'); phutil_require_module('phabricator', 'storage/exception/schema'); phutil_require_module('phutil', 'error'); diff --git a/src/storage/connection/mysql/mysql/AphrontMySQLDatabaseConnection.php b/src/storage/connection/mysql/mysql/AphrontMySQLDatabaseConnection.php index 17c0e32680..9dfe01f9f6 100644 --- a/src/storage/connection/mysql/mysql/AphrontMySQLDatabaseConnection.php +++ b/src/storage/connection/mysql/mysql/AphrontMySQLDatabaseConnection.php @@ -34,10 +34,6 @@ final class AphrontMySQLDatabaseConnection return mysql_affected_rows($this->requireConnection()); } - protected function getTransactionKey() { - return (int)$this->requireConnection(); - } - protected function connect() { if (!function_exists('mysql_connect')) { // We have to '@' the actual call since it can spew all sorts of silly diff --git a/src/storage/connection/mysql/mysqli/AphrontMySQLiDatabaseConnection.php b/src/storage/connection/mysql/mysqli/AphrontMySQLiDatabaseConnection.php index cf0910d637..d7f75ff83b 100644 --- a/src/storage/connection/mysql/mysqli/AphrontMySQLiDatabaseConnection.php +++ b/src/storage/connection/mysql/mysqli/AphrontMySQLiDatabaseConnection.php @@ -36,10 +36,6 @@ final class AphrontMySQLiDatabaseConnection return $this->requireConnection()->affected_rows; } - protected function getTransactionKey() { - return spl_object_hash($this->requireConnection()); - } - protected function connect() { if (!class_exists('mysqli', false)) { throw new Exception( diff --git a/src/storage/exception/deadlock/AphrontQueryDeadlockException.php b/src/storage/exception/deadlock/AphrontQueryDeadlockException.php new file mode 100644 index 0000000000..6f0951956f --- /dev/null +++ b/src/storage/exception/deadlock/AphrontQueryDeadlockException.php @@ -0,0 +1,23 @@ +establishConnection('r'); $lock_clause = ''; -/* - - TODO: Restore this? - if ($connection->isReadLocking()) { $lock_clause = 'FOR UPDATE'; } else if ($connection->isWriteLocking()) { $lock_clause = 'LOCK IN SHARE MODE'; } -*/ $args = func_get_args(); $args = array_slice($args, 2); @@ -1342,8 +1337,74 @@ abstract class LiskDAO { } + /** + * Begins read-locking selected rows with SELECT ... FOR UPDATE, so that + * other connections can not read them (this is an enormous oversimplification + * of FOR UPDATE semantics; consult the MySQL documentation for details). To + * end read locking, call @{method:endReadLocking}. For example: + * + * $beach->openTransaction(); + * $beach->beginReadLocking(); + * + * $beach->reload(); + * $beach->setGrainsOfSand($beach->getGrainsOfSand() + 1); + * $beach->save(); + * + * $beach->endReadLocking(); + * $beach->saveTransaction(); + * + * @return this + * @task xaction + */ + public function beginReadLocking() { + $this->establishConnection('w')->beginReadLocking(); + return $this; + } + + + /** + * Ends read-locking that began at an earlier @{method:beginReadLocking} call. + * + * @return this + * @task xaction + */ + public function endReadLocking() { + $this->establishConnection('w')->endReadLocking(); + return $this; + } + + /** + * Begins write-locking selected rows with SELECT ... LOCK IN SHARE MODE, so + * that other connections can not update or delete them (this is an + * oversimplification of LOCK IN SHARE MODE semantics; consult the + * MySQL documentation for details). To end write locking, call + * @{method:endWriteLocking}. + * + * @return this + * @task xaction + */ + public function beginWriteLocking() { + $this->establishConnection('w')->beginWriteLocking(); + return $this; + } + + + /** + * Ends write-locking that began at an earlier @{method:beginWriteLocking} + * call. + * + * @return this + * @task xaction + */ + public function endWriteLocking() { + $this->establishConnection('w')->endWriteLocking(); + return $this; + } + + /* -( Isolation )---------------------------------------------------------- */ + /** * @task isolate */ diff --git a/src/storage/transaction/AphrontDatabaseTransactionState.php b/src/storage/transaction/AphrontDatabaseTransactionState.php index aaa49f7001..13f1a8a4ac 100644 --- a/src/storage/transaction/AphrontDatabaseTransactionState.php +++ b/src/storage/transaction/AphrontDatabaseTransactionState.php @@ -23,7 +23,9 @@ */ final class AphrontDatabaseTransactionState { - private $depth; + private $depth = 0; + private $readLockLevel = 0; + private $writeLockLevel = 0; public function getDepth() { return $this->depth; @@ -46,6 +48,40 @@ final class AphrontDatabaseTransactionState { return 'Aphront_Savepoint_'.$this->depth; } + public function beginReadLocking() { + $this->readLockLevel++; + return $this; + } + + public function endReadLocking() { + if ($this->readLockLevel == 0) { + throw new Exception("Too many calls to endReadLocking()!"); + } + $this->readLockLevel--; + return $this; + } + + public function isReadLocking() { + return ($this->readLockLevel > 0); + } + + public function beginWriteLocking() { + $this->writeLockLevel++; + return $this; + } + + public function endWriteLocking() { + if ($this->writeLockLevel == 0) { + throw new Exception("Too many calls to endWriteLocking()!"); + } + $this->writeLockLevel--; + return $this; + } + + public function isWriteLocking() { + return ($this->writeLockLevel > 0); + } + public function __destruct() { if ($this->depth) { throw new Exception( @@ -53,6 +89,16 @@ final class AphrontDatabaseTransactionState { 'implicitly rolled back. Calls to openTransaction() must always be '. 'paired with a call to saveTransaction() or killTransaction().'); } + if ($this->readLockLevel) { + throw new Exception( + 'Process exited with an open read lock! Call to beginReadLocking() '. + 'must always be paired with a call to endReadLocking().'); + } + if ($this->writeLockLevel) { + throw new Exception( + 'Process exited with an open write lock! Call to beginWriteLocking() '. + 'must always be paired with a call to endWriteLocking().'); + } } }