Only apply DB_MASTER fallback in Revision::fetchText() if READ_LATEST

Add support to DBAccessObjectUtils for fallback logic to make
this simple for other callers too.

Change-Id: I58ab7bd7d31a481f9dc9a773392ea90feb1ebeac

diff --git a/includes/Revision.php b/includes/Revision.php
index ef0f03d..0f8d0bd 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -1608,25 +1608,38 @@ class Revision implements IDBAccessObject {
                        $row = null;
                }
 
+               // Callers doing updates will pass in READ_LATEST as usual. Since the text/blob tables
+               // do not normally get rows changed around, set READ_LATEST_IMMUTABLE in those cases.
+               $flags = $this->mQueryFlags;
+               $flags |= DBAccessObjectUtils::hasFlags( $flags, self::READ_LATEST )
+                       ? self::READ_LATEST_IMMUTABLE
+                       : 0;
+
+               list( $index, $options, $fallbackIndex, $fallbackOptions ) =
+                       DBAccessObjectUtils::getDBOptions( $flags );
+
                if ( !$row ) {
                        // Text data is immutable; check replica DBs first.
-                       $dbr = wfGetDB( DB_REPLICA );
-                       $row = $dbr->selectRow( 'text',
+                       $row = wfGetDB( $index )->selectRow(
+                               'text',
                                [ 'old_text', 'old_flags' ],
                                [ 'old_id' => $textId ],
-                               __METHOD__ );
+                               __METHOD__,
+                               $options
+                       );
                }
 
-               // Fallback to the master in case of replica DB lag. Also use FOR UPDATE if it was
-               // used to fetch this revision to avoid missing the row due to REPEATABLE-READ.
-               $forUpdate = ( $this->mQueryFlags & self::READ_LOCKING == self::READ_LOCKING );
-               if ( !$row && ( $forUpdate || wfGetLB()->getServerCount() > 1 ) ) {
-                       $dbw = wfGetDB( DB_MASTER );
-                       $row = $dbw->selectRow( 'text',
+               // Fallback to DB_MASTER in some cases if the row was not found
+               if ( !$row && $fallbackIndex !== null ) {
+                       // Use FOR UPDATE if it was used to fetch this revision. This avoids missing the row
+                       // due to REPEATABLE-READ. Also fallback to the master if READ_LATEST is provided.
+                       $row = wfGetDB( $fallbackIndex )->selectRow(
+                               'text',
                                [ 'old_text', 'old_flags' ],
                                [ 'old_id' => $textId ],
                                __METHOD__,
-                               $forUpdate ? [ 'FOR UPDATE' ] : [] );
+                               $fallbackOptions
+                       );
                }
 
                if ( !$row ) {

diff --git a/includes/dao/DBAccessObjectUtils.php b/includes/dao/DBAccessObjectUtils.php
index 1fc9ae7..cc63446 100644 (file)
--- a/includes/dao/DBAccessObjectUtils.php
+++ b/includes/dao/DBAccessObjectUtils.php
@@ -26,7 +26,7 @@
  *
  * @since 1.26
  */
-class DBAccessObjectUtils {
+class DBAccessObjectUtils implements IDBAccessObject {
        /**
         * @param integer $bitfield
         * @param integer $flags IDBAccessObject::READ_* constant
@@ -37,23 +37,45 @@ class DBAccessObjectUtils {
        }
 
        /**
-        * Get an appropriate DB index and options for a query
+        * Get an appropriate DB index, options, and fallback DB index for a query
         *
-        * @param integer $bitfield
-        * @return array (DB_MASTER/DB_REPLICA, SELECT options array)
+        * The fallback DB index and options are to be used if the entity is not found
+        * with the initial DB index, typically querying the master DB to avoid lag
+        *
+        * @param integer $bitfield Bitfield of IDBAccessObject::READ_* constants
+        * @return array List of DB indexes and options in this order:
+        *   - DB_MASTER or DB_REPLICA constant for the initial query
+        *   - SELECT options array for the initial query
+        *   - DB_MASTER constant for the fallback query; null if no fallback should happen
+        *   - SELECT options array for the fallback query; empty if no fallback should happen
         */
        public static function getDBOptions( $bitfield ) {
-               $index = self::hasFlags( $bitfield, IDBAccessObject::READ_LATEST )
-                       ? DB_MASTER
-                       : DB_REPLICA;
+               if ( self::hasFlags( $bitfield, self::READ_LATEST_IMMUTABLE ) ) {
+                       $index = DB_REPLICA; // override READ_LATEST if set
+                       $fallbackIndex = DB_MASTER;
+               } elseif ( self::hasFlags( $bitfield, self::READ_LATEST ) ) {
+                       $index = DB_MASTER;
+                       $fallbackIndex = null;
+               } else {
+                       $index = DB_REPLICA;
+                       $fallbackIndex = null;
+               }
+
+               $lockingOptions = [];
+               if ( self::hasFlags( $bitfield, self::READ_EXCLUSIVE ) ) {
+                       $lockingOptions[] = 'FOR UPDATE';
+               } elseif ( self::hasFlags( $bitfield, self::READ_LOCKING ) ) {
+                       $lockingOptions[] = 'LOCK IN SHARE MODE';
+               }
 
-               $options = [];
-               if ( self::hasFlags( $bitfield, IDBAccessObject::READ_EXCLUSIVE ) ) {
-                       $options[] = 'FOR UPDATE';
-               } elseif ( self::hasFlags( $bitfield, IDBAccessObject::READ_LOCKING ) ) {
-                       $options[] = 'LOCK IN SHARE MODE';
+               if ( $fallbackIndex !== null ) {
+                       $options = []; // locks on DB_REPLICA make no sense
+                       $fallbackOptions = $lockingOptions;
+               } else {
+                       $options = $lockingOptions;
+                       $fallbackOptions = []; // no fallback
                }
 
-               return [ $index, $options ];
+               return [ $index, $options, $fallbackIndex, $fallbackOptions ];
        }
 }