Added slave/master fallback logic in Revision

* This is a more specific form of the logic removed in 3c2bc32ae1.
  It does not suffer the problem of causing constant master DB
  queries due to a bad template reference or such.
* It will use the master if writes from the current thread
  are pending or were recently committed. This deals with the
  common problem of code that needs to read things it just wrote,
  such as diffs on rollback or edit hooks.
* This commit reverts 8624e261f by making the hack obsolete.

Bug: T93866
Bug: T94407
Change-Id: Ib9ecb75e1236e767bdc86d124d5e22a03ae0fb5f

diff --git a/includes/Revision.php b/includes/Revision.php
index 356cd32..a13499a 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -297,7 +297,10 @@ class Revision implements IDBAccessObject {
        }
 
        /**
-        * Given a set of conditions, fetch a revision.
+        * Given a set of conditions, fetch a revision
+        *
+        * This method is used then a revision ID is qualified and
+        * will incorporate some basic slave/master fallback logic
         *
         * @param array $conditions
         * @param int $flags (optional)
@@ -305,10 +308,24 @@ class Revision implements IDBAccessObject {
         */
        private static function newFromConds( $conditions, $flags = 0 ) {
                $db = wfGetDB( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_SLAVE );
+
                $rev = self::loadFromConds( $db, $conditions, $flags );
+               // Make sure new pending/committed revision are visibile later on
+               // within web requests to certain avoid bugs like T93866 and T94407.
+               if ( !$rev
+                       && !( $flags & self::READ_LATEST )
+                       && wfGetLB()->getServerCount() > 1
+                       && wfGetLB()->hasOrMadeRecentMasterChanges()
+               ) {
+                       $flags = self::READ_LATEST;
+                       $db = wfGetDB( DB_MASTER );
+                       $rev = self::loadFromConds( $db, $conditions, $flags );
+               }
+
                if ( $rev ) {
                        $rev->mQueryFlags = $flags;
                }
+
                return $rev;
        }
 
@@ -1481,9 +1498,9 @@ class Revision implements IDBAccessObject {
         * @return string|bool The revision's text, or false on failure
         */
        protected function loadText() {
-
                // Caching may be beneficial for massive use of external storage
                global $wgRevisionCacheExpiry, $wgMemc;
+
                $textId = $this->getTextId();
                $key = wfMemcKey( 'revisiontext', 'textid', $textId );
                if ( $wgRevisionCacheExpiry ) {

diff --git a/includes/db/Database.php b/includes/db/Database.php
index b3c81f9..cbd8be5 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3611,6 +3611,7 @@ abstract class DatabaseBase implements IDatabase {
                        $this->runOnTransactionPreCommitCallbacks();
                        $this->doCommit( $fname );
                        if ( $this->mTrxDoneWrites ) {
+                               $this->mDoneWrites = microtime( true );
                                $this->getTransactionProfiler()->transactionWritingOut(
                                        $this->mServer, $this->mDBname, $this->mTrxShortId );
                        }
@@ -3691,6 +3692,7 @@ abstract class DatabaseBase implements IDatabase {
                $this->runOnTransactionPreCommitCallbacks();
                $this->doCommit( $fname );
                if ( $this->mTrxDoneWrites ) {
+                       $this->mDoneWrites = microtime( true );
                        $this->getTransactionProfiler()->transactionWritingOut(
                                $this->mServer, $this->mDBname, $this->mTrxShortId );
                }

diff --git a/includes/db/LoadBalancer.php b/includes/db/LoadBalancer.php
index 4e0a5b9..48c636e 100644 (file)
--- a/includes/db/LoadBalancer.php
+++ b/includes/db/LoadBalancer.php
@@ -1022,8 +1022,7 @@ class LoadBalancer {
        }
 
        /**
-        * Determine if there are any pending changes that need to be rolled back
-        * or committed.
+        * Determine if there are pending changes in a transaction by this thread
         * @since 1.23
         * @return bool
         */
@@ -1044,6 +1043,42 @@ class LoadBalancer {
                return false;
        }
 
+       /**
+        * Get the timestamp of the latest write query done by this thread
+        * @since 1.25
+        * @return float|bool UNIX timestamp or false
+        */
+       public function lastMasterChangeTimestamp() {
+               $lastTime = false;
+               // Always 0, but who knows.. :)
+               $masterIndex = $this->getWriterIndex();
+               foreach ( $this->mConns as $conns2 ) {
+                       if ( empty( $conns2[$masterIndex] ) ) {
+                               continue;
+                       }
+                       /** @var DatabaseBase $conn */
+                       foreach ( $conns2[$masterIndex] as $conn ) {
+                               $lastTime = max( $lastTime, $conn->lastDoneWrites() );
+                       }
+               }
+               return $lastTime;
+       }
+
+       /**
+        * Check if this load balancer object had any recent or still
+        * pending writes issued against it by this PHP thread
+        *
+        * @param float $age How many seconds ago is "recent" [defaults to mWaitTimeout]
+        * @return bool
+        * @since 1.25
+        */
+       public function hasOrMadeRecentMasterChanges( $age = null ) {
+               $age = ( $age === null ) ? $this->mWaitTimeout : $age;
+
+               return ( $this->hasMasterChanges()
+                       || $this->lastMasterChangeTimestamp() > microtime( true ) - $age );
+       }
+
        /**
         * @param mixed $value
         * @return mixed

diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php
index de8dd43..77bbd36 100644 (file)
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -1233,12 +1233,6 @@ class DifferenceEngine extends ContextSource {
                // Load the new revision object
                if ( $this->mNewid ) {
                        $this->mNewRev = Revision::newFromId( $this->mNewid );
-
-                       if ( !$this->mNewRev && wfGetLB()->getServerCount() > 1 ) {
-                               // Try harder… This is being hit after a rollback where we show the
-                               // diff immediately after the edit happened. T93866
-                               $this->mNewRev = Revision::newFromId( $this->mNewid, Revision::READ_LATEST );
-                       }
                } else {
                        $this->mNewRev = Revision::newFromTitle(
                                $this->getTitle(),