From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 8 Sep 2016 20:07:15 +0000 (-0700)
Subject: Add LBFactory::getChronologyProtectorTouched() method
X-Git-Tag: 1.31.0-rc.0~5661^2~1
X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin/?a=commitdiff_plain;h=6b8effebb32af5685528c8fd2ebaadac8880a0c1;p=lhc%2Fweb%2Fwiklou.git

Add LBFactory::getChronologyProtectorTouched() method

This lets callers check whether a user recently change a DB and
possibly try harder to reflect certain changes or refresh caches.

Also simplified some logging code a bit.

Change-Id: Ia1168cf0d46cfdee046838ce4c5a6294e4d81760
---

diff --git a/includes/db/ChronologyProtector.php b/includes/db/ChronologyProtector.php
index b4619f3004..39c995780f 100644
--- a/includes/db/ChronologyProtector.php
+++ b/includes/db/ChronologyProtector.php
@@ -31,8 +31,8 @@ class ChronologyProtector {
 
 	/** @var string Storage key name */
 	protected $key;
-	/** @var array Map of (ip: <IP>, agent: <user-agent>) */
-	protected $client;
+	/** @var string Hash of client parameters */
+	protected $clientId;
 	/** @var bool Whether to no-op all method calls */
 	protected $enabled = true;
 	/** @var bool Whether to check and wait on positions */
@@ -44,6 +44,8 @@ class ChronologyProtector {
 	protected $startupPositions = [];
 	/** @var DBMasterPos[] Map of (DB master name => position) */
 	protected $shutdownPositions = [];
+	/** @var float[] Map of (DB master name => 1) */
+	protected $shutdownTouchDBs = [];
 
 	/**
 	 * @param BagOStuff $store
@@ -52,11 +54,8 @@ class ChronologyProtector {
 	 */
 	public function __construct( BagOStuff $store, array $client ) {
 		$this->store = $store;
-		$this->client = $client;
-		$this->key = $store->makeGlobalKey(
-			'ChronologyProtector',
-			md5( $client['ip'] . "\n" . $client['agent'] )
-		);
+		$this->clientId = md5( $client['ip'] . "\n" . $client['agent'] );
+		$this->key = $store->makeGlobalKey( __CLASS__, $this->clientId );
 	}
 
 	/**
@@ -95,10 +94,8 @@ class ChronologyProtector {
 
 		$masterName = $lb->getServerName( $lb->getWriterIndex() );
 		if ( !empty( $this->startupPositions[$masterName] ) ) {
-			$info = $lb->parentInfo();
 			$pos = $this->startupPositions[$masterName];
-			wfDebugLog( 'replication', __METHOD__ .
-				": LB '" . $info['id'] . "' waiting for master pos $pos\n" );
+			wfDebugLog( 'replication', __METHOD__ . ": LB for '$masterName' set to pos $pos\n" );
 			$lb->waitFor( $pos );
 		}
 	}
@@ -111,35 +108,47 @@ class ChronologyProtector {
 	 * @return void
 	 */
 	public function shutdownLB( LoadBalancer $lb ) {
-		if ( !$this->enabled || $lb->getServerCount() <= 1 ) {
-			return; // non-replicated setup or disabled
+		if ( !$this->enabled ) {
+			return; // not enabled
+		} elseif ( !$lb->hasOrMadeRecentMasterChanges( INF ) ) {
+			// Only save the position if writes have been done on the connection
+			return;
 		}
 
-		$info = $lb->parentInfo();
 		$masterName = $lb->getServerName( $lb->getWriterIndex() );
-
-		// Only save the position if writes have been done on the connection
-		$db = $lb->getAnyOpenConnection( $lb->getWriterIndex() );
-		if ( !$db || !$db->doneWrites() ) {
-			wfDebugLog( 'replication', __METHOD__ . ": LB {$info['id']}, no writes done\n" );
-
-			return; // nothing to do
+		if ( $lb->getServerCount() > 1 ) {
+			$pos = $lb->getMasterPos();
+			wfDebugLog( 'replication', __METHOD__ . ": LB for '$masterName' has pos $pos\n" );
+			$this->shutdownPositions[$masterName] = $pos;
+		} else {
+			wfDebugLog( 'replication', __METHOD__ . ": DB '$masterName' touched\n" );
 		}
-
-		$pos = $db->getMasterPos();
-		wfDebugLog( 'replication', __METHOD__ . ": LB {$info['id']} has master pos $pos\n" );
-		$this->shutdownPositions[$masterName] = $pos;
+		$this->shutdownTouchDBs[$masterName] = 1;
 	}
 
 	/**
 	 * Notify the ChronologyProtector that the LBFactory is done calling shutdownLB() for now.
 	 * May commit chronology data to persistent storage.
 	 *
-	 * @return array Empty on success; returns the (db name => position) map on failure
+	 * @return DBMasterPos[] Empty on success; returns the (db name => position) map on failure
 	 */
 	public function shutdown() {
-		if ( !$this->enabled || !count( $this->shutdownPositions ) ) {
-			return true; // nothing to save
+		if ( !$this->enabled ) {
+			return [];
+		}
+
+		// Some callers might want to know if a user recently touched a DB.
+		// These writes do not need to block on all datacenters receiving them.
+		foreach ( $this->shutdownTouchDBs as $dbName => $unused ) {
+			$this->store->set(
+				$this->getTouchedKey( $this->store, $dbName ),
+				microtime( true ),
+				BagOStuff::TTL_DAY
+			);
+		}
+
+		if ( !count( $this->shutdownPositions ) ) {
+			return []; // nothing to save
 		}
 
 		wfDebugLog( 'replication',
@@ -175,6 +184,24 @@ class ChronologyProtector {
 		return [];
 	}
 
+	/**
+	 * @param string $dbName DB master name (e.g. "db1052")
+	 * @return float|bool UNIX timestamp when client last touched the DB; false if not on record
+	 * @since 1.28
+	 */
+	public function getTouched( $dbName ) {
+		return $this->store->get( $this->getTouchedKey( $this->store, $dbName ) );
+	}
+
+	/**
+	 * @param BagOStuff $store
+	 * @param string $dbName
+	 * @return string
+	 */
+	private function getTouchedKey( BagOStuff $store, $dbName ) {
+		return $store->makeGlobalKey( __CLASS__, 'mtime', $this->clientId, $dbName );
+	}
+
 	/**
 	 * Load in previous master positions for the client
 	 */
@@ -187,11 +214,9 @@ class ChronologyProtector {
 		if ( $this->wait ) {
 			$data = $this->store->get( $this->key );
 			$this->startupPositions = $data ? $data['positions'] : [];
-
 			wfDebugLog( 'replication', __METHOD__ . ": key is {$this->key} (read)\n" );
 		} else {
 			$this->startupPositions = [];
-
 			wfDebugLog( 'replication', __METHOD__ . ": key is {$this->key} (unread)\n" );
 		}
 	}
diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php
index 62a5286566..cd8dff32a1 100644
--- a/includes/db/loadbalancer/LBFactory.php
+++ b/includes/db/loadbalancer/LBFactory.php
@@ -555,6 +555,15 @@ abstract class LBFactory implements DestructibleService {
 		}
 	}
 
+	/**
+	 * @param string $dbName DB master name (e.g. "db1052")
+	 * @return float|bool UNIX timestamp when client last touched the DB or false if not recent
+	 * @since 1.28
+	 */
+	public function getChronologyProtectorTouched( $dbName ) {
+		return $this->chronProt->getTouched( $dbName );
+	}
+
 	/**
 	 * Disable the ChronologyProtector for all load balancers
 	 *
diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php
index 24c5d9296d..bf78d13f26 100644
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -155,11 +155,14 @@ class LBFactoryTest extends MediaWikiTestCase {
 		// (a) First HTTP request
 		$mPos = new MySQLMasterPos( 'db1034-bin.000976', '843431247' );
 
+		$now = microtime( true );
 		$mockDB = $this->getMockBuilder( 'DatabaseMysql' )
 			->disableOriginalConstructor()
 			->getMock();
 		$mockDB->expects( $this->any() )
-			->method( 'doneWrites' )->will( $this->returnValue( true ) );
+			->method( 'writesOrCallbacksPending' )->will( $this->returnValue( true ) );
+		$mockDB->expects( $this->any() )
+			->method( 'lastDoneWrites' )->will( $this->returnValue( $now ) );
 		$mockDB->expects( $this->any() )
 			->method( 'getMasterPos' )->will( $this->returnValue( $mPos ) );
 
@@ -174,6 +177,18 @@ class LBFactoryTest extends MediaWikiTestCase {
 			->method( 'parentInfo' )->will( $this->returnValue( [ 'id' => "main-DEFAULT" ] ) );
 		$lb->expects( $this->any() )
 			->method( 'getAnyOpenConnection' )->will( $this->returnValue( $mockDB ) );
+		$lb->expects( $this->any() )
+			->method( 'hasOrMadeRecentMasterChanges' )->will( $this->returnCallback(
+				function () use ( $mockDB ) {
+					$p = 0;
+					$p |= call_user_func( [ $mockDB, 'writesOrCallbacksPending' ] );
+					$p |= call_user_func( [ $mockDB, 'lastDoneWrites' ] );
+
+					return (bool)$p;
+				}
+			) );
+		$lb->expects( $this->any() )
+			->method( 'getMasterPos' )->will( $this->returnValue( $mPos ) );
 
 		$bag = new HashBagOStuff();
 		$cp = new ChronologyProtector(
@@ -184,7 +199,8 @@ class LBFactoryTest extends MediaWikiTestCase {
 			]
 		);
 
-		$mockDB->expects( $this->exactly( 2 ) )->method( 'doneWrites' );
+		$mockDB->expects( $this->exactly( 2 ) )->method( 'writesOrCallbacksPending' );
+		$mockDB->expects( $this->exactly( 2 ) )->method( 'lastDoneWrites' );
 
 		// Nothing to wait for
 		$cp->initLB( $lb );