Move MySQL-specific fakeMaster/fakeSlave stuff to DatabaseMysqlBase
authorBrad Jorsch <bjorsch@wikimedia.org>
Mon, 13 Jan 2014 19:48:36 +0000 (14:48 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Mon, 13 Jan 2014 20:00:27 +0000 (15:00 -0500)
Probably the fakeMaster/fakeSlave stuff was originally intended to be
generic, but the existing code in Database.php is making some very
MySQL-specific assumptions. So let's evict it from Database.php (except
for the minimal support functions) and put it in DatabaseMysqlBase where
it makes more sense.

This also takes care of fixing the breakage described in I5d2a1696 by
reverting Id6268193 and the problematic bit of I364e192e (again).

Change-Id: I3dc6ca216bf2c2f07d3090e86f2539292e1fa86b

includes/db/Database.php
includes/db/DatabaseMysqlBase.php
includes/db/DatabaseOracle.php
includes/db/DatabasePostgres.php
includes/db/DatabaseUtility.php

index 6e87c40..90e658f 100644 (file)
@@ -249,7 +249,6 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
        protected $mForeign;
        protected $mErrorCount = 0;
        protected $mLBInfo = array();
-       protected $mFakeSlaveLag = null, $mFakeMaster = false;
        protected $mDefaultBigSelects = null;
        protected $mSchemaVars = false;
 
@@ -478,10 +477,11 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
        /**
         * Set lag time in seconds for a fake slave
         *
-        * @param int $lag
+        * @param mixed $lag Valid values for this parameter are determined by the
+        *   subclass, but should be a PHP scalar or array that would be sensible
+        *   as part of $wgLBFactoryConf.
         */
        public function setFakeSlaveLag( $lag ) {
-               $this->mFakeSlaveLag = $lag;
        }
 
        /**
@@ -490,7 +490,6 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
         * @param bool $enabled
         */
        public function setFakeMaster( $enabled = true ) {
-               $this->mFakeMaster = $enabled;
        }
 
        /**
@@ -3117,32 +3116,6 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
         *   zero if we timed out.
         */
        public function masterPosWait( DBMasterPos $pos, $timeout ) {
-               wfProfileIn( __METHOD__ );
-
-               if ( !is_null( $this->mFakeSlaveLag ) ) {
-                       $wait = intval( ( $pos->pos - microtime( true ) + $this->mFakeSlaveLag ) * 1e6 );
-
-                       if ( $wait > $timeout * 1e6 ) {
-                               wfDebug( "Fake slave timed out waiting for $pos ($wait us)\n" );
-                               wfProfileOut( __METHOD__ );
-
-                               return -1;
-                       } elseif ( $wait > 0 ) {
-                               wfDebug( "Fake slave waiting $wait us\n" );
-                               usleep( $wait );
-                               wfProfileOut( __METHOD__ );
-
-                               return 1;
-                       } else {
-                               wfDebug( "Fake slave up to date ($wait us)\n" );
-                               wfProfileOut( __METHOD__ );
-
-                               return 0;
-                       }
-               }
-
-               wfProfileOut( __METHOD__ );
-
                # Real waits are implemented in the subclass.
                return 0;
        }
@@ -3153,15 +3126,8 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
         * @return DBMasterPos|bool False if this is not a slave.
         */
        public function getSlavePos() {
-               if ( !is_null( $this->mFakeSlaveLag ) ) {
-                       $pos = new MySQLMasterPos( 'fake', microtime( true ) - $this->mFakeSlaveLag );
-                       wfDebug( __METHOD__ . ": fake slave pos = $pos\n" );
-
-                       return $pos;
-               } else {
-                       # Stub
-                       return false;
-               }
+               # Stub
+               return false;
        }
 
        /**
@@ -3170,11 +3136,8 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
         * @return DBMasterPos|bool False if this is not a master
         */
        public function getMasterPos() {
-               if ( $this->mFakeMaster ) {
-                       return new MySQLMasterPos( 'fake', microtime( true ) );
-               } else {
-                       return false;
-               }
+               # Stub
+               return false;
        }
 
        /**
@@ -3642,7 +3605,7 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
         * @return int Database replication lag in seconds
         */
        public function getLag() {
-               return intval( $this->mFakeSlaveLag );
+               return 0;
        }
 
        /**
index 2c448ed..647a42a 100644 (file)
@@ -33,6 +33,11 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
        /** @var MysqlMasterPos */
        protected $lastKnownSlavePos;
 
+       /** @var null|int */
+       protected $mFakeSlaveLag = null;
+
+       protected $mFakeMaster = false;
+
        /**
         * @return string
         */
@@ -578,6 +583,24 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
         */
        abstract protected function mysqlPing();
 
+       /**
+        * Set lag time in seconds for a fake slave
+        *
+        * @param int $lag
+        */
+       public function setFakeSlaveLag( $lag ) {
+               $this->mFakeSlaveLag = $lag;
+       }
+
+       /**
+        * Make this connection a fake master
+        *
+        * @param bool $enabled
+        */
+       public function setFakeMaster( $enabled = true ) {
+               $this->mFakeMaster = $enabled;
+       }
+
        /**
         * Returns slave lag.
         *
@@ -661,7 +684,9 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
         *
         * @param DBMasterPos|MySQLMasterPos $pos
         * @param int $timeout The maximum number of seconds to wait for synchronisation
-        * @return bool|string
+        * @return int Zero if the slave was past that position already,
+        *   greater than zero if we waited for some period of time, less than
+        *   zero if we timed out.
         */
        function masterPosWait( DBMasterPos $pos, $timeout ) {
                if ( $this->lastKnownSlavePos && $this->lastKnownSlavePos->hasReached( $pos ) ) {
@@ -673,15 +698,30 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                $this->commit( __METHOD__, 'flush' );
 
                if ( !is_null( $this->mFakeSlaveLag ) ) {
-                       $status = parent::masterPosWait( $pos, $timeout );
-                       wfProfileOut( __METHOD__ );
+                       $wait = intval( ( $pos->pos - microtime( true ) + $this->mFakeSlaveLag ) * 1e6 );
 
-                       return $status;
+                       if ( $wait > $timeout * 1e6 ) {
+                               wfDebug( "Fake slave timed out waiting for $pos ($wait us)\n" );
+                               wfProfileOut( __METHOD__ );
+
+                               return -1;
+                       } elseif ( $wait > 0 ) {
+                               wfDebug( "Fake slave waiting $wait us\n" );
+                               usleep( $wait );
+                               wfProfileOut( __METHOD__ );
+
+                               return 1;
+                       } else {
+                               wfDebug( "Fake slave up to date ($wait us)\n" );
+                               wfProfileOut( __METHOD__ );
+
+                               return 0;
+                       }
                }
 
                # Call doQuery() directly, to avoid opening a transaction if DBO_TRX is set
                $encFile = $this->addQuotes( $pos->file );
-               $encPos = intval( $pos->getMasterPos() );
+               $encPos = intval( $pos->pos );
                $sql = "SELECT MASTER_POS_WAIT($encFile, $encPos, $timeout)";
                $res = $this->doQuery( $sql );
 
@@ -705,7 +745,10 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
         */
        function getSlavePos() {
                if ( !is_null( $this->mFakeSlaveLag ) ) {
-                       return parent::getSlavePos();
+                       $pos = new MySQLMasterPos( 'fake', microtime( true ) - $this->mFakeSlaveLag );
+                       wfDebug( __METHOD__ . ": fake slave pos = $pos\n" );
+
+                       return $pos;
                }
 
                $res = $this->query( 'SHOW SLAVE STATUS', 'DatabaseBase::getSlavePos' );
@@ -729,7 +772,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
         */
        function getMasterPos() {
                if ( $this->mFakeMaster ) {
-                       return parent::getMasterPos();
+                       return new MySQLMasterPos( 'fake', microtime( true ) );
                }
 
                $res = $this->query( 'SHOW MASTER STATUS', 'DatabaseBase::getMasterPos' );
@@ -1232,8 +1275,8 @@ class MySQLMasterPos implements DBMasterPos {
        /** @var string */
        public $file;
 
-       /** @var int */
-       private $pos;
+       /** @var int timestamp */
+       public $pos;
 
        function __construct( $file, $pos ) {
                $this->file = $file;
@@ -1263,11 +1306,4 @@ class MySQLMasterPos implements DBMasterPos {
 
                return ( $thisPos && $thatPos && $thisPos >= $thatPos );
        }
-
-       /**
-        * @return int
-        */
-       public function getMasterPos() {
-               return $this->pos;
-       }
 }
index d90b3e1..d304444 100644 (file)
@@ -1513,9 +1513,6 @@ class DatabaseOracle extends DatabaseBase {
                return 'BITOR(' . $fieldLeft . ', ' . $fieldRight . ')';
        }
 
-       function setFakeMaster( $enabled = true ) {
-       }
-
        function getDBname() {
                return $this->mDBname;
        }
index cb671d8..83d8cae 100644 (file)
@@ -1573,9 +1573,6 @@ SQL;
                return array( $startOpts, $useIndex, $preLimitTail, $postLimitTail );
        }
 
-       function setFakeMaster( $enabled = true ) {
-       }
-
        function getDBname() {
                return $this->mDBname;
        }
index 2b4b634..87687db 100644 (file)
@@ -326,12 +326,8 @@ class LikeMatch {
 
 /**
  * An object representing a master or slave position in a replicated setup.
+ *
+ * The implementation details of this opaque type are up to the database subclass.
  */
 interface DBMasterPos {
-       /**
-        * Return the master position.
-        *
-        * @return mixed Master position
-        */
-       public function getMasterPos();
 }