From: Aaron Schulz Date: Thu, 30 Mar 2017 21:56:22 +0000 (-0700) Subject: Database: clean up lockTables() and add postgres support X-Git-Tag: 1.31.0-rc.0~3563^2~1 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=1a1c43bdff483a2c74b5fa98d6e21910a3d17a72 Database: clean up lockTables() and add postgres support A new method is now available to check whether session scope locks are supported, which callers typically want when using lock(). Its usage can avoid deadlock prone and expensive row-level locks for some maintenance tasks. For Postgres, table locks are tied to the transaction. Trigger startAtomic() in lockTables() and endAtomic() in unlockTables() to assure that a transaction is present. Also remove LOW_PRIORITY feature, which is ignored by mysql. Change-Id: I499061bcc2763afb1ff4a43319064eed4ba3a8fe --- diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 49ddd14ee8..40bcc1b02c 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -3295,26 +3295,37 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware return false; } - /** - * Lock specific tables - * - * @param array $read Array of tables to lock for read access - * @param array $write Array of tables to lock for write access - * @param string $method Name of caller - * @param bool $lowPriority Whether to indicate writes to be LOW PRIORITY - * @return bool - */ - public function lockTables( $read, $write, $method, $lowPriority = true ) { + public function tableLocksHaveTransactionScope() { return true; } - /** - * Unlock specific tables - * - * @param string $method The caller - * @return bool - */ - public function unlockTables( $method ) { + final public function lockTables( array $read, array $write, $method ) { + if ( $this->writesOrCallbacksPending() ) { + throw new DBUnexpectedError( $this, "Transaction writes or callbacks still pending." ); + } + + if ( $this->tableLocksHaveTransactionScope() ) { + $this->startAtomic( $method ); + } + + return $this->doLockTables( $read, $write, $method ); + } + + protected function doLockTables( array $read, array $write, $method ) { + return true; + } + + final public function unlockTables( $method ) { + if ( $this->tableLocksHaveTransactionScope() ) { + $this->endAtomic( $method ); + + return true; // locks released on COMMIT/ROLLBACK + } + + return $this->doUnlockTables( $method ); + } + + protected function doUnlockTables( $method ) { return true; } diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php index 672509002d..e2b522610c 100644 --- a/includes/libs/rdbms/database/DatabaseMysqlBase.php +++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php @@ -1054,36 +1054,26 @@ abstract class DatabaseMysqlBase extends Database { return true; } - /** - * @param array $read - * @param array $write - * @param string $method - * @param bool $lowPriority - * @return bool - */ - public function lockTables( $read, $write, $method, $lowPriority = true ) { - $items = []; + public function tableLocksHaveTransactionScope() { + return false; // tied to TCP connection + } + protected function doLockTables( array $read, array $write, $method ) { + $items = []; foreach ( $write as $table ) { - $tbl = $this->tableName( $table ) . - ( $lowPriority ? ' LOW_PRIORITY' : '' ) . - ' WRITE'; - $items[] = $tbl; + $items[] = $this->tableName( $table ) . ' WRITE'; } foreach ( $read as $table ) { $items[] = $this->tableName( $table ) . ' READ'; } + $sql = "LOCK TABLES " . implode( ',', $items ); $this->query( $sql, $method ); return true; } - /** - * @param string $method - * @return bool - */ - public function unlockTables( $method ) { + protected function doUnlockTables( $method ) { $this->query( "UNLOCK TABLES", $method ); return true; diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php index af9716d451..5bcd4a8e56 100644 --- a/includes/libs/rdbms/database/DatabasePostgres.php +++ b/includes/libs/rdbms/database/DatabasePostgres.php @@ -1305,6 +1305,33 @@ SQL; return parent::streamStatementEnd( $sql, $newLine ); } + public function doLockTables( array $read, array $write, $method ) { + $tablesWrite = []; + foreach ( $write as $table ) { + $tablesWrite[] = $this->tableName( $table ); + } + $tablesRead = []; + foreach ( $read as $table ) { + $tablesRead[] = $this->tableName( $table ); + } + + // Acquire locks for the duration of the current transaction... + if ( $tablesWrite ) { + $this->query( + 'LOCK TABLE ONLY ' . implode( ',', $tablesWrite ) . ' IN EXCLUSIVE MODE', + $method + ); + } + if ( $tablesRead ) { + $this->query( + 'LOCK TABLE ONLY ' . implode( ',', $tablesRead ) . ' IN SHARE MODE', + $method + ); + } + + return true; + } + public function lockIsFree( $lockName, $method ) { // http://www.postgresql.org/docs/8.2/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS $key = $this->addQuotes( $this->bigintFromLockName( $lockName ) ); diff --git a/includes/libs/rdbms/database/IMaintainableDatabase.php b/includes/libs/rdbms/database/IMaintainableDatabase.php index 138cf2de9e..b984c42544 100644 --- a/includes/libs/rdbms/database/IMaintainableDatabase.php +++ b/includes/libs/rdbms/database/IMaintainableDatabase.php @@ -210,6 +210,73 @@ interface IMaintainableDatabase extends IDatabase { public function duplicateTableStructure( $oldName, $newName, $temporary = false, $fname = __METHOD__ ); + + /** + * Checks if table locks acquired by lockTables() are transaction-bound in their scope + * + * Transaction-bound table locks will be released when the current transaction terminates. + * Table locks that are not bound to a transaction are not effected by BEGIN/COMMIT/ROLLBACK + * and will last until either lockTables()/unlockTables() is called or the TCP connection to + * the database is closed. + * + * @return bool + * @since 1.29 + */ + public function tableLocksHaveTransactionScope(); + + /** + * Lock specific tables + * + * Any pending transaction should be resolved before calling this method, since: + * a) Doing so resets any REPEATABLE-READ snapshot of the data to a fresh one. + * b) Previous row and table locks from the transaction or session may be released + * by LOCK TABLES, which may be unsafe for the changes in such a transaction. + * c) The main use case of lockTables() is to avoid deadlocks and timeouts by locking + * entire tables in order to do long-running, batched, and lag-aware, updates. Batching + * and replication lag checks do not work when all the updates happen in a transaction. + * + * Always get all relevant table locks up-front in one call, since LOCK TABLES might release + * any prior table locks on some RDBMes (e.g MySQL). + * + * For compatibility, callers should check tableLocksHaveTransactionScope() before using + * this method. If locks are scoped specifically to transactions then caller must either: + * - a) Start a new transaction and acquire table locks for the scope of that transaction, + * doing all row updates within that transaction. It will not be possible to update + * rows in batches; this might result in high replication lag. + * - b) Forgo table locks entirely and avoid calling this method. Careful use of hints like + * LOCK IN SHARE MODE and FOR UPDATE and the use of query batching may be preferrable + * to using table locks with a potentially large transaction. Use of MySQL and Postges + * style REPEATABLE-READ (Snapshot Isolation with or without First-Committer-Rule) can + * also be considered for certain tasks that require a consistent view of entire tables. + * + * If session scoped locks are not supported, then calling lockTables() will trigger + * startAtomic(), with unlockTables() triggering endAtomic(). This will automatically + * start a transaction if one is not already present and cause the locks to be released + * when the transaction finishes (normally during the unlockTables() call). + * + * In any case, avoid using begin()/commit() in code that runs while such table locks are + * acquired, as that breaks in case when a transaction is needed. The startAtomic() and + * endAtomic() methods are safe, however, since they will join any existing transaction. + * + * @param array $read Array of tables to lock for read access + * @param array $write Array of tables to lock for write access + * @param string $method Name of caller + * @return bool + * @since 1.29 + */ + public function lockTables( array $read, array $write, $method ); + + /** + * Unlock all tables locked via lockTables() + * + * If table locks are scoped to transactions, then locks might not be released until the + * transaction ends, which could happen after this method is called. + * + * @param string $method The caller + * @return bool + * @since 1.29 + */ + public function unlockTables( $method ); } class_alias( 'Wikimedia\Rdbms\IMaintainableDatabase', 'IMaintainableDatabase' ); diff --git a/includes/libs/rdbms/database/MaintainableDBConnRef.php b/includes/libs/rdbms/database/MaintainableDBConnRef.php index 30c62deb1f..8238f3edd3 100644 --- a/includes/libs/rdbms/database/MaintainableDBConnRef.php +++ b/includes/libs/rdbms/database/MaintainableDBConnRef.php @@ -68,6 +68,18 @@ class MaintainableDBConnRef extends DBConnRef implements IMaintainableDatabase { ) { return $this->__call( __FUNCTION__, func_get_args() ); } + + public function tableLocksHaveTransactionScope() { + return $this->__call( __FUNCTION__, func_get_args() ); + } + + public function lockTables( array $read, array $write, $method ) { + return $this->__call( __FUNCTION__, func_get_args() ); + } + + public function unlockTables( $method ) { + return $this->__call( __FUNCTION__, func_get_args() ); + } } class_alias( 'Wikimedia\Rdbms\MaintainableDBConnRef', 'MaintainableDBConnRef' );