From: daniel <daniel.kinzler@wikimedia.de>
Date: Tue, 10 Jul 2018 18:54:33 +0000 (+0200)
Subject: In RevisionStore, use DB_MASTER when READ_LATEST is set.
X-Git-Tag: 1.34.0-rc.0~4815^2
X-Git-Url: https://git.heureux-cyclage.org/?a=commitdiff_plain;h=b4ffe54ca18ba9152dbaeb0507e4450180c3284c;p=lhc%2Fweb%2Fwiklou.git

In RevisionStore, use DB_MASTER when READ_LATEST is set.

This fixes a few instances where a replica connection was used to read
auxilliary data even if the READ_LATEST flag was passed.

Bug: T198869
Change-Id: Ie3b5f7921f939e6bd5216130dc4adcf242e5c624
---

diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php
index 390cc19d22..7a5c1f4e01 100644
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -255,6 +255,16 @@ class RevisionStore
 		return $lb->getConnection( $mode, [], $this->wikiId );
 	}
 
+	/**
+	 * @param int $queryFlags a bit field composed of READ_XXX flags
+	 *
+	 * @return DBConnRef
+	 */
+	private function getDBConnectionRefForQueryFlags( $queryFlags ) {
+		list( $mode, ) = DBAccessObjectUtils::getDBOptions( $queryFlags );
+		return $this->getDBConnectionRef( $mode );
+	}
+
 	/**
 	 * @param IDatabase $connection
 	 */
@@ -1000,9 +1010,8 @@ class RevisionStore
 	 * @return null|RecentChange
 	 */
 	public function getRecentChange( RevisionRecord $rev, $flags = 0 ) {
-		$dbr = $this->getDBConnection( DB_REPLICA );
-
 		list( $dbType, ) = DBAccessObjectUtils::getDBOptions( $flags );
+		$db = $this->getDBConnection( $dbType );
 
 		$userIdentity = $rev->getUser( RevisionRecord::RAW );
 
@@ -1013,18 +1022,18 @@ class RevisionStore
 		}
 
 		// TODO: Select by rc_this_oldid alone - but as of Nov 2017, there is no index on that!
-		$actorWhere = $this->actorMigration->getWhere( $dbr, 'rc_user', $rev->getUser(), false );
+		$actorWhere = $this->actorMigration->getWhere( $db, 'rc_user', $rev->getUser(), false );
 		$rc = RecentChange::newFromConds(
 			[
 				$actorWhere['conds'],
-				'rc_timestamp' => $dbr->timestamp( $rev->getTimestamp() ),
+				'rc_timestamp' => $db->timestamp( $rev->getTimestamp() ),
 				'rc_this_oldid' => $rev->getId()
 			],
 			__METHOD__,
 			$dbType
 		);
 
-		$this->releaseDBConnection( $dbr );
+		$this->releaseDBConnection( $db );
 
 		// XXX: cache this locally? Glue it to the RevisionRecord?
 		return $rc;
@@ -1221,8 +1230,7 @@ class RevisionStore
 		// $slot will be the inherited slot, while $mainSlotRow still refers to the original slot.
 		$mainSlotRow->slot_content_id =
 			function ( SlotRecord $slot ) use ( $queryFlags, $mainSlotRow ) {
-				list( $dbMode, ) = DBAccessObjectUtils::getDBOptions( $queryFlags );
-				$db = $this->getDBConnectionRef( $dbMode );
+				$db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
 				return $this->findSlotContentId( $db, $mainSlotRow->slot_revision_id, 'main' );
 			};
 
@@ -1344,12 +1352,11 @@ class RevisionStore
 			// and fall back to master. The assumption is that we only want to force the fallback
 			// if we are quite sure the revision exists because the caller supplied a revision ID.
 			// If the page isn't found at all on a replica, it probably simply does not exist.
-			$db = $this->getDBConnection( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_REPLICA );
+			$db = $this->getDBConnectionRefForQueryFlags( $flags );
 
 			$conds[] = 'rev_id=page_latest';
 			$rev = $this->loadRevisionFromConds( $db, $conds, $flags );
 
-			$this->releaseDBConnection( $db );
 			return $rev;
 		}
 	}
@@ -1386,12 +1393,11 @@ class RevisionStore
 			// and fall back to master. The assumption is that we only want to force the fallback
 			// if we are quite sure the revision exists because the caller supplied a revision ID.
 			// If the page isn't found at all on a replica, it probably simply does not exist.
-			$db = $this->getDBConnection( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_REPLICA );
+			$db = $this->getDBConnectionRefForQueryFlags( $flags );
 
 			$conds[] = 'rev_id=page_latest';
 			$rev = $this->loadRevisionFromConds( $db, $conds, $flags );
 
-			$this->releaseDBConnection( $db );
 			return $rev;
 		}
 	}
@@ -1559,9 +1565,9 @@ class RevisionStore
 			$user = new UserIdentityValue( 0, '', 0 );
 		}
 
-		$comment = $this->commentStore
-			// Legacy because $row may have come from self::selectFields()
-			->getCommentLegacy( $this->getDBConnection( DB_REPLICA ), 'ar_comment', $row, true );
+		$db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
+		// Legacy because $row may have come from self::selectFields()
+		$comment = $this->commentStore->getCommentLegacy( $db, 'ar_comment', $row, true );
 
 		$slots = $this->newRevisionSlots( $row->ar_rev_id, $row, $queryFlags, $title );
 
@@ -1607,9 +1613,9 @@ class RevisionStore
 			$user = new UserIdentityValue( 0, '', 0 );
 		}
 
-		$comment = $this->commentStore
-			// Legacy because $row may have come from self::selectFields()
-			->getCommentLegacy( $this->getDBConnection( DB_REPLICA ), 'rev_comment', $row, true );
+		$db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
+		// Legacy because $row may have come from self::selectFields()
+		$comment = $this->commentStore->getCommentLegacy( $db, 'rev_comment', $row, true );
 
 		$slots = $this->newRevisionSlots( $row->rev_id, $row, $queryFlags, $title );
 
@@ -1902,9 +1908,8 @@ class RevisionStore
 	 * @return RevisionRecord|null
 	 */
 	private function newRevisionFromConds( $conditions, $flags = 0, Title $title = null ) {
-		$db = $this->getDBConnection( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_REPLICA );
+		$db = $this->getDBConnectionRefForQueryFlags( $flags );
 		$rev = $this->loadRevisionFromConds( $db, $conditions, $flags, $title );
-		$this->releaseDBConnection( $db );
 
 		$lb = $this->getDBLoadBalancer();
 
@@ -1916,9 +1921,9 @@ class RevisionStore
 			&& $lb->hasOrMadeRecentMasterChanges()
 		) {
 			$flags = self::READ_LATEST;
-			$db = $this->getDBConnection( DB_MASTER );
-			$rev = $this->loadRevisionFromConds( $db, $conditions, $flags, $title );
-			$this->releaseDBConnection( $db );
+			$dbw = $this->getDBConnection( DB_MASTER );
+			$rev = $this->loadRevisionFromConds( $dbw, $conditions, $flags, $title );
+			$this->releaseDBConnection( $dbw );
 		}
 
 		return $rev;
@@ -2409,15 +2414,12 @@ class RevisionStore
 	 * @return string|bool False if not found
 	 */
 	public function getTimestampFromId( $title, $id, $flags = 0 ) {
-		$db = $this->getDBConnection(
-			( $flags & IDBAccessObject::READ_LATEST ) ? DB_MASTER : DB_REPLICA
-		);
+		$db = $this->getDBConnectionRefForQueryFlags( $flags );
 
 		$conds = [ 'rev_id' => $id ];
 		$conds['rev_page'] = $title->getArticleID();
 		$timestamp = $db->selectField( 'revision', 'rev_timestamp', $conds, __METHOD__ );
 
-		$this->releaseDBConnection( $db );
 		return ( $timestamp !== false ) ? wfTimestamp( TS_MW, $timestamp ) : false;
 	}