From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 22 Aug 2016 02:14:54 +0000 (-0700)
Subject: Optimize ChronologyProtector writes for multi-DC case
X-Git-Tag: 1.31.0-rc.0~5899^2
X-Git-Url: https://git.cyclocoop.org/%242?a=commitdiff_plain;h=6675d682197ea3b8ac5f3bcea3b89d9ada416bed;p=lhc%2Fweb%2Fwiklou.git

Optimize ChronologyProtector writes for multi-DC case

Change-Id: Iecd218043814ac0963c67b16d043e697a6933741
---

diff --git a/includes/db/ChronologyProtector.php b/includes/db/ChronologyProtector.php
index cc35999612..2539b87f72 100644
--- a/includes/db/ChronologyProtector.php
+++ b/includes/db/ChronologyProtector.php
@@ -147,31 +147,20 @@ class ChronologyProtector {
 			implode( ', ', array_keys( $this->shutdownPositions ) ) . "\n"
 		);
 
-		$shutdownPositions = $this->shutdownPositions;
-		$ok = $this->store->merge(
-			$this->key,
-			function ( $store, $key, $curValue ) use ( $shutdownPositions ) {
-				/** @var $curPositions DBMasterPos[] */
-				if ( $curValue === false ) {
-					$curPositions = $shutdownPositions;
-				} else {
-					$curPositions = $curValue['positions'];
-					// Use the newest positions for each DB master
-					foreach ( $shutdownPositions as $db => $pos ) {
-						if ( !isset( $curPositions[$db] )
-							|| $pos->asOfTime() > $curPositions[$db]->asOfTime()
-						) {
-							$curPositions[$db] = $pos;
-						}
-					}
-				}
-
-				return [ 'positions' => $curPositions ];
-			},
-			BagOStuff::TTL_MINUTE,
-			10,
-			BagOStuff::WRITE_SYNC // visible in all datacenters
-		);
+		// CP-protected writes should overwhemingly go to the master datacenter, so get DC-local
+		// lock to merge the values. Use a DC-local get() and a synchronous all-DC set(). This
+		// makes it possible for the BagOStuff class to write in parallel to all DCs with one RTT.
+		if ( $this->store->lock( $this->key, 3 ) ) {
+			$ok = $this->store->set(
+				$this->key,
+				self::mergePositions( $this->store->get( $this->key ), $this->shutdownPositions ),
+				BagOStuff::TTL_MINUTE,
+				BagOStuff::WRITE_SYNC
+			);
+			$this->store->unlock( $this->key );
+		} else {
+			$ok = false;
+		}
 
 		if ( !$ok ) {
 			// Raced out too many times or stash is down
@@ -206,4 +195,28 @@ class ChronologyProtector {
 			wfDebugLog( 'replication', __METHOD__ . ": key is {$this->key} (unread)\n" );
 		}
 	}
+
+	/**
+	 * @param array|bool $curValue
+	 * @param DBMasterPos[] $shutdownPositions
+	 * @return array
+	 */
+	private static function mergePositions( $curValue, array $shutdownPositions ) {
+		/** @var $curPositions DBMasterPos[] */
+		if ( $curValue === false ) {
+			$curPositions = $shutdownPositions;
+		} else {
+			$curPositions = $curValue['positions'];
+			// Use the newest positions for each DB master
+			foreach ( $shutdownPositions as $db => $pos ) {
+				if ( !isset( $curPositions[$db] )
+					|| $pos->asOfTime() > $curPositions[$db]->asOfTime()
+				) {
+					$curPositions[$db] = $pos;
+				}
+			}
+		}
+
+		return [ 'positions' => $curPositions ];
+	}
 }