From 7cc14e02d6045f8a7faeb1209fa390200fd156cb Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 7 Sep 2016 12:42:03 -0700
Subject: [PATCH] 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
---
 includes/Revision.php                | 33 +++++++++++++------
 includes/dao/DBAccessObjectUtils.php | 48 ++++++++++++++++++++--------
 2 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/includes/Revision.php b/includes/Revision.php
index ef0f03df8f..0f8d0bd1d2 100644
--- 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 1fc9ae784e..cc63446cb3 100644
--- 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 ];
 	}
 }
-- 
2.20.1