Database: clean up lockTables() and add postgres support
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 30 Mar 2017 21:56:22 +0000 (14:56 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 7 Apr 2017 23:28:36 +0000 (16:28 -0700)
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

includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabaseMysqlBase.php
includes/libs/rdbms/database/DatabasePostgres.php
includes/libs/rdbms/database/IMaintainableDatabase.php
includes/libs/rdbms/database/MaintainableDBConnRef.php

index 49ddd14..40bcc1b 100644 (file)
@@ -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;
        }
 
index 6725090..e2b5226 100644 (file)
@@ -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;
index af9716d..5bcd4a8 100644 (file)
@@ -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 ) );
index 138cf2d..b984c42 100644 (file)
@@ -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' );
index 30c62de..8238f3e 100644 (file)
@@ -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' );