rdbms: make Database::insertSelect() stricter about replication safety
[lhc/web/wiklou.git] / includes / libs / rdbms / database / DatabaseMysqlBase.php
index 6a545ce..1e845e8 100644 (file)
@@ -24,7 +24,7 @@ namespace Wikimedia\Rdbms;
 
 use DateTime;
 use DateTimeZone;
-use MediaWiki;
+use Wikimedia;
 use InvalidArgumentException;
 use Exception;
 use stdClass;
@@ -60,11 +60,15 @@ abstract class DatabaseMysqlBase extends Database {
        protected $sqlMode;
        /** @var bool Use experimental UTF-8 transmission encoding */
        protected $utf8Mode;
+       /** @var bool|null */
+       protected $defaultBigSelects = null;
 
        /** @var string|null */
        private $serverVersion = null;
        /** @var bool|null */
        private $insertSelectIsSafe = null;
+       /** @var stdClass|null */
+       private $replicationInfoRow = null;
 
        /**
         * Additional $params include:
@@ -126,14 +130,14 @@ abstract class DatabaseMysqlBase extends Database {
                # Close/unset connection handle
                $this->close();
 
-               $this->mServer = $server;
-               $this->mUser = $user;
-               $this->mPassword = $password;
-               $this->mDBname = $dbName;
+               $this->server = $server;
+               $this->user = $user;
+               $this->password = $password;
+               $this->dbName = $dbName;
 
                $this->installErrorHandler();
                try {
-                       $this->mConn = $this->mysqlConnect( $this->mServer );
+                       $this->conn = $this->mysqlConnect( $this->server );
                } catch ( Exception $ex ) {
                        $this->restoreErrorHandler();
                        throw $ex;
@@ -141,7 +145,7 @@ abstract class DatabaseMysqlBase extends Database {
                $error = $this->restoreErrorHandler();
 
                # Always log connection errors
-               if ( !$this->mConn ) {
+               if ( !$this->conn ) {
                        if ( !$error ) {
                                $error = $this->lastError();
                        }
@@ -159,10 +163,10 @@ abstract class DatabaseMysqlBase extends Database {
                        $this->reportConnectionError( $error );
                }
 
-               if ( $dbName != '' ) {
-                       MediaWiki\suppressWarnings();
+               if ( strlen( $dbName ) ) {
+                       Wikimedia\suppressWarnings();
                        $success = $this->selectDB( $dbName );
-                       MediaWiki\restoreWarnings();
+                       Wikimedia\restoreWarnings();
                        if ( !$success ) {
                                $this->queryLogger->error(
                                        "Error selecting database {db_name} on server {db_server}",
@@ -171,7 +175,7 @@ abstract class DatabaseMysqlBase extends Database {
                                        ] )
                                );
                                $this->queryLogger->debug(
-                                       "Error selecting database $dbName on server {$this->mServer}" );
+                                       "Error selecting database $dbName on server {$this->server}" );
 
                                $this->reportConnectionError( "Error selecting database $dbName" );
                        }
@@ -190,7 +194,7 @@ abstract class DatabaseMysqlBase extends Database {
                }
                // Set any custom settings defined by site config
                // (e.g. https://dev.mysql.com/doc/refman/4.1/en/innodb-parameters.html)
-               foreach ( $this->mSessionVars as $var => $val ) {
+               foreach ( $this->sessionVars as $var => $val ) {
                        // Escape strings but not numbers to avoid MySQL complaining
                        if ( !is_int( $val ) && !is_float( $val ) ) {
                                $val = $this->addQuotes( $val );
@@ -213,7 +217,7 @@ abstract class DatabaseMysqlBase extends Database {
                        }
                }
 
-               $this->mOpened = true;
+               $this->opened = true;
 
                return true;
        }
@@ -257,9 +261,9 @@ abstract class DatabaseMysqlBase extends Database {
                if ( $res instanceof ResultWrapper ) {
                        $res = $res->result;
                }
-               MediaWiki\suppressWarnings();
+               Wikimedia\suppressWarnings();
                $ok = $this->mysqlFreeResult( $res );
-               MediaWiki\restoreWarnings();
+               Wikimedia\restoreWarnings();
                if ( !$ok ) {
                        throw new DBUnexpectedError( $this, "Unable to free MySQL result" );
                }
@@ -282,9 +286,9 @@ abstract class DatabaseMysqlBase extends Database {
                if ( $res instanceof ResultWrapper ) {
                        $res = $res->result;
                }
-               MediaWiki\suppressWarnings();
+               Wikimedia\suppressWarnings();
                $row = $this->mysqlFetchObject( $res );
-               MediaWiki\restoreWarnings();
+               Wikimedia\restoreWarnings();
 
                $errno = $this->lastErrno();
                // Unfortunately, mysql_fetch_object does not reset the last errno.
@@ -318,9 +322,9 @@ abstract class DatabaseMysqlBase extends Database {
                if ( $res instanceof ResultWrapper ) {
                        $res = $res->result;
                }
-               MediaWiki\suppressWarnings();
+               Wikimedia\suppressWarnings();
                $row = $this->mysqlFetchArray( $res );
-               MediaWiki\restoreWarnings();
+               Wikimedia\restoreWarnings();
 
                $errno = $this->lastErrno();
                // Unfortunately, mysql_fetch_array does not reset the last errno.
@@ -354,9 +358,9 @@ abstract class DatabaseMysqlBase extends Database {
                if ( $res instanceof ResultWrapper ) {
                        $res = $res->result;
                }
-               MediaWiki\suppressWarnings();
+               Wikimedia\suppressWarnings();
                $n = $this->mysqlNumRows( $res );
-               MediaWiki\restoreWarnings();
+               Wikimedia\restoreWarnings();
 
                // Unfortunately, mysql_num_rows does not reset the last errno.
                // We are not checking for any errors here, since
@@ -465,19 +469,19 @@ abstract class DatabaseMysqlBase extends Database {
         * @return string
         */
        public function lastError() {
-               if ( $this->mConn ) {
+               if ( $this->conn ) {
                        # Even if it's non-zero, it can still be invalid
-                       MediaWiki\suppressWarnings();
-                       $error = $this->mysqlError( $this->mConn );
+                       Wikimedia\suppressWarnings();
+                       $error = $this->mysqlError( $this->conn );
                        if ( !$error ) {
                                $error = $this->mysqlError();
                        }
-                       MediaWiki\restoreWarnings();
+                       Wikimedia\restoreWarnings();
                } else {
                        $error = $this->mysqlError();
                }
                if ( $error ) {
-                       $error .= ' (' . $this->mServer . ')';
+                       $error .= ' (' . $this->server . ')';
                }
 
                return $error;
@@ -506,19 +510,32 @@ abstract class DatabaseMysqlBase extends Database {
                return $this->nativeReplace( $table, $rows, $fname );
        }
 
-       protected function nativeInsertSelect(
-               $destTable, $srcTable, $varMap, $conds,
-               $fname = __METHOD__, $insertOptions = [], $selectOptions = [], $selectJoinConds = []
-       ) {
-               if ( $this->insertSelectIsSafe === null ) {
-                       // In MySQL, an INSERT SELECT is only replication safe with row-based
-                       // replication or if innodb_autoinc_lock_mode is 0. When those
-                       // conditions aren't met, use non-native mode.
-                       // While we could try to determine if the insert is safe anyway by
-                       // checking if the target table has an auto-increment column that
-                       // isn't set in $varMap, that seems unlikely to be worth the extra
-                       // complexity.
-                       $row = $this->selectRow(
+       protected function isInsertSelectSafe( array $insertOptions, array $selectOptions ) {
+               $row = $this->getReplicationSafetyInfo();
+               // For row-based-replication, the resulting changes will be relayed, not the query
+               if ( $row->binlog_format === 'ROW' ) {
+                       return true;
+               }
+               // LIMIT requires ORDER BY on a unique key or it is non-deterministic
+               if ( isset( $selectOptions['LIMIT'] ) ) {
+                       return false;
+               }
+               // In MySQL, an INSERT SELECT is only replication safe with row-based
+               // replication or if innodb_autoinc_lock_mode is 0. When those
+               // conditions aren't met, use non-native mode.
+               // While we could try to determine if the insert is safe anyway by
+               // checking if the target table has an auto-increment column that
+               // isn't set in $varMap, that seems unlikely to be worth the extra
+               // complexity.
+               return ( (int)$row->innodb_autoinc_lock_mode === 0 );
+       }
+
+       /**
+        * @return stdClass Process cached row
+        */
+       private function getReplicationSafetyInfo() {
+               if ( $this->replicationInfoRow === null ) {
+                       $this->replicationInfoRow = $this->selectRow(
                                false,
                                [
                                        'innodb_autoinc_lock_mode' => '@@innodb_autoinc_lock_mode',
@@ -527,33 +544,9 @@ abstract class DatabaseMysqlBase extends Database {
                                [],
                                __METHOD__
                        );
-                       $this->insertSelectIsSafe = $row->binlog_format === 'ROW' ||
-                               (int)$row->innodb_autoinc_lock_mode === 0;
-               }
-
-               if ( !$this->insertSelectIsSafe ) {
-                       return $this->nonNativeInsertSelect(
-                               $destTable,
-                               $srcTable,
-                               $varMap,
-                               $conds,
-                               $fname,
-                               $insertOptions,
-                               $selectOptions,
-                               $selectJoinConds
-                       );
-               } else {
-                       return parent::nativeInsertSelect(
-                               $destTable,
-                               $srcTable,
-                               $varMap,
-                               $conds,
-                               $fname,
-                               $insertOptions,
-                               $selectOptions,
-                               $selectJoinConds
-                       );
                }
+
+               return $this->replicationInfoRow;
        }
 
        /**
@@ -594,7 +587,7 @@ abstract class DatabaseMysqlBase extends Database {
                list( $database, , $prefix, $table ) = $this->qualifiedTableComponents( $table );
                $tableName = "{$prefix}{$table}";
 
-               if ( isset( $this->mSessionTempTables[$tableName] ) ) {
+               if ( isset( $this->sessionTempTables[$tableName] ) ) {
                        return true; // already known to exist and won't show in SHOW TABLES anyway
                }
 
@@ -889,17 +882,21 @@ abstract class DatabaseMysqlBase extends Database {
                        return 0; // already reached this point for sure
                }
 
-               $useGTID = ( $this->useGTIDs && $pos->gtids );
-
                // Call doQuery() directly, to avoid opening a transaction if DBO_TRX is set
-               if ( $useGTID ) {
+               if ( $pos->getGTIDs() ) {
+                       // Ignore GTIDs from domains exclusive to the master DB (presumably inactive)
+                       $rpos = $this->getReplicaPos();
+                       $gtidsWait = $rpos ? MySQLMasterPos::getCommonDomainGTIDs( $pos, $rpos ) : [];
+                       if ( !$gtidsWait ) {
+                               return -1; // $pos is from the wrong cluster?
+                       }
                        // Wait on the GTID set (MariaDB only)
-                       $gtidArg = $this->addQuotes( implode( ',', $pos->gtids ) );
+                       $gtidArg = $this->addQuotes( implode( ',', $gtidsWait ) );
                        $res = $this->doQuery( "SELECT MASTER_GTID_WAIT($gtidArg, $timeout)" );
                } else {
                        // Wait on the binlog coordinates
-                       $encFile = $this->addQuotes( $pos->file );
-                       $encPos = intval( $pos->pos );
+                       $encFile = $this->addQuotes( $pos->getLogFile() );
+                       $encPos = intval( $pos->pos[1] );
                        $res = $this->doQuery( "SELECT MASTER_POS_WAIT($encFile, $encPos, $timeout)" );
                }
 
@@ -912,7 +909,7 @@ abstract class DatabaseMysqlBase extends Database {
                // Result can be NULL (error), -1 (timeout), or 0+ per the MySQL manual
                $status = ( $row[0] !== null ) ? intval( $row[0] ) : null;
                if ( $status === null ) {
-                       if ( !$useGTID ) {
+                       if ( !$pos->getGTIDs() ) {
                                // T126436: jobs programmed to wait on master positions might be referencing
                                // binlogs with an old master hostname; this makes MASTER_POS_WAIT() return null.
                                // Try to detect this case and treat the replica DB as having reached the given
@@ -938,26 +935,26 @@ abstract class DatabaseMysqlBase extends Database {
         * @return MySQLMasterPos|bool
         */
        public function getReplicaPos() {
-               $res = $this->query( 'SHOW SLAVE STATUS', __METHOD__ );
-               $row = $this->fetchObject( $res );
+               $now = microtime( true );
 
-               if ( $row ) {
-                       $pos = isset( $row->Exec_master_log_pos )
-                               ? $row->Exec_master_log_pos
-                               : $row->Exec_Master_Log_Pos;
-                       // Also fetch the last-applied GTID set (MariaDB)
-                       if ( $this->useGTIDs ) {
-                               $res = $this->query( "SHOW GLOBAL VARIABLES LIKE 'gtid_slave_pos'", __METHOD__ );
-                               $gtidRow = $this->fetchObject( $res );
-                               $gtidSet = $gtidRow ? $gtidRow->Value : '';
-                       } else {
-                               $gtidSet = '';
+               if ( $this->useGTIDs ) {
+                       $res = $this->query( "SELECT @@global.gtid_slave_pos AS Value", __METHOD__ );
+                       $gtidRow = $this->fetchObject( $res );
+                       if ( $gtidRow && strlen( $gtidRow->Value ) ) {
+                               return new MySQLMasterPos( $gtidRow->Value, $now );
                        }
+               }
 
-                       return new MySQLMasterPos( $row->Relay_Master_Log_File, $pos, $gtidSet );
-               } else {
-                       return false;
+               $res = $this->query( 'SHOW SLAVE STATUS', __METHOD__ );
+               $row = $this->fetchObject( $res );
+               if ( $row && strlen( $row->Relay_Master_Log_File ) ) {
+                       return new MySQLMasterPos(
+                               "{$row->Relay_Master_Log_File}/{$row->Exec_Master_Log_Pos}",
+                               $now
+                       );
                }
+
+               return false;
        }
 
        /**
@@ -966,23 +963,23 @@ abstract class DatabaseMysqlBase extends Database {
         * @return MySQLMasterPos|bool
         */
        public function getMasterPos() {
-               $res = $this->query( 'SHOW MASTER STATUS', __METHOD__ );
-               $row = $this->fetchObject( $res );
+               $now = microtime( true );
 
-               if ( $row ) {
-                       // Also fetch the last-written GTID set (MariaDB)
-                       if ( $this->useGTIDs ) {
-                               $res = $this->query( "SHOW GLOBAL VARIABLES LIKE 'gtid_binlog_pos'", __METHOD__ );
-                               $gtidRow = $this->fetchObject( $res );
-                               $gtidSet = $gtidRow ? $gtidRow->Value : '';
-                       } else {
-                               $gtidSet = '';
+               if ( $this->useGTIDs ) {
+                       $res = $this->query( "SELECT @@global.gtid_binlog_pos AS Value", __METHOD__ );
+                       $gtidRow = $this->fetchObject( $res );
+                       if ( $gtidRow && strlen( $gtidRow->Value ) ) {
+                               return new MySQLMasterPos( $gtidRow->Value, $now );
                        }
+               }
 
-                       return new MySQLMasterPos( $row->File, $row->Position, $gtidSet );
-               } else {
-                       return false;
+               $res = $this->query( 'SHOW MASTER STATUS', __METHOD__ );
+               $row = $this->fetchObject( $res );
+               if ( $row && strlen( $row->File ) ) {
+                       return new MySQLMasterPos( "{$row->File}/{$row->Position}", $now );
                }
+
+               return false;
        }
 
        public function serverIsReadOnly() {
@@ -1081,6 +1078,10 @@ abstract class DatabaseMysqlBase extends Database {
         * @since 1.20
         */
        public function lockIsFree( $lockName, $method ) {
+               if ( !parent::lockIsFree( $lockName, $method ) ) {
+                       return false; // already held
+               }
+
                $encName = $this->addQuotes( $this->makeLockName( $lockName ) );
                $result = $this->query( "SELECT IS_FREE_LOCK($encName) AS lockstatus", $method );
                $row = $this->fetchObject( $result );
@@ -1172,14 +1173,14 @@ abstract class DatabaseMysqlBase extends Database {
         */
        public function setBigSelects( $value = true ) {
                if ( $value === 'default' ) {
-                       if ( $this->mDefaultBigSelects === null ) {
+                       if ( $this->defaultBigSelects === null ) {
                                # Function hasn't been called before so it must already be set to the default
                                return;
                        } else {
-                               $value = $this->mDefaultBigSelects;
+                               $value = $this->defaultBigSelects;
                        }
-               } elseif ( $this->mDefaultBigSelects === null ) {
-                       $this->mDefaultBigSelects =
+               } elseif ( $this->defaultBigSelects === null ) {
+                       $this->defaultBigSelects =
                                (bool)$this->selectField( false, '@@sql_big_selects', '', __METHOD__ );
                }
                $encValue = $value ? '1' : '0';
@@ -1378,7 +1379,7 @@ abstract class DatabaseMysqlBase extends Database {
         */
        public function listViews( $prefix = null, $fname = __METHOD__ ) {
                // The name of the column containing the name of the VIEW
-               $propertyName = 'Tables_in_' . $this->mDBname;
+               $propertyName = 'Tables_in_' . $this->dbName;
 
                // Query for the VIEWS
                $res = $this->query( 'SHOW FULL TABLES WHERE TABLE_TYPE = "VIEW"' );
@@ -1447,6 +1448,11 @@ abstract class DatabaseMysqlBase extends Database {
                        return $index;
                }
        }
+
+       protected function isTransactableQuery( $sql ) {
+               return parent::isTransactableQuery( $sql ) &&
+                       !preg_match( '/^SELECT\s+(GET|RELEASE|IS_FREE)_LOCK\(/', $sql );
+       }
 }
 
 class_alias( DatabaseMysqlBase::class, 'DatabaseMysqlBase' );