From d9385014fe383d5e2382b3def4479cf583336ae0 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Tue, 15 Jul 2014 19:07:58 -0700
Subject: [PATCH] Avoid breaking DB transactions in SqlBagOStuff

* Callers still sees some decent time ordering since writes block
  until committed. This will not blindly flush transactions though.
* Basically reverts 59e8032

Change-Id: Ib9a91ac023f8e3fafb0bf0eef9dca6e31b867be9
---
 includes/objectcache/SqlBagOStuff.php | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index 8b0b738cd7..0c91dab43b 100644
--- a/includes/objectcache/SqlBagOStuff.php
+++ b/includes/objectcache/SqlBagOStuff.php
@@ -238,7 +238,12 @@ class SqlBagOStuff extends BagOStuff {
 					$res = $db->select( $tableName,
 						array( 'keyname', 'value', 'exptime' ),
 						array( 'keyname' => $tableKeys ),
-						__METHOD__ );
+						__METHOD__,
+						// Approximate write-on-the-fly BagOStuff API via blocking.
+						// This approximation fails if a ROLLBACK happens (which is rare).
+						// We do not want to flush the TRX as that can break callers.
+						$db->trxLevel() ? array( 'LOCK IN SHARE MODE' ) : array()
+					);
 					foreach ( $res as $row ) {
 						$row->serverIndex = $serverIndex;
 						$row->tableName = $tableName;
@@ -258,13 +263,11 @@ class SqlBagOStuff extends BagOStuff {
 					$db = $this->getDB( $row->serverIndex );
 					if ( $this->isExpired( $db, $row->exptime ) ) { // MISS
 						$this->debug( "get: key has expired, deleting" );
-						$db->commit( __METHOD__, 'flush' );
 						# Put the expiry time in the WHERE condition to avoid deleting a
 						# newly-inserted value
 						$db->delete( $row->tableName,
 							array( 'keyname' => $key, 'exptime' => $row->exptime ),
 							__METHOD__ );
-						$db->commit( __METHOD__, 'flush' );
 					} else { // HIT
 						$values[$key] = $this->unserialize( $db->decodeBlob( $row->value ) );
 					}
@@ -327,14 +330,12 @@ class SqlBagOStuff extends BagOStuff {
 				}
 
 				try {
-					$db->commit( __METHOD__, 'flush' );
 					$db->replace(
 						$tableName,
 						array( 'keyname' ),
 						$rows,
 						__METHOD__
 					);
-					$db->commit( __METHOD__, 'flush' );
 				} catch ( DBError $e ) {
 					$this->handleWriteError( $e, $serverIndex );
 					$result = false;
@@ -374,7 +375,6 @@ class SqlBagOStuff extends BagOStuff {
 
 				$encExpiry = $db->timestamp( $exptime );
 			}
-			$db->commit( __METHOD__, 'flush' );
 			// (bug 24425) use a replace if the db supports it instead of
 			// delete/insert to avoid clashes with conflicting keynames
 			$db->replace(
@@ -385,7 +385,6 @@ class SqlBagOStuff extends BagOStuff {
 					'value' => $db->encodeBlob( $this->serialize( $value ) ),
 					'exptime' => $encExpiry
 				), __METHOD__ );
-			$db->commit( __METHOD__, 'flush' );
 		} catch ( DBError $e ) {
 			$this->handleWriteError( $e, $serverIndex );
 			return false;
@@ -419,7 +418,6 @@ class SqlBagOStuff extends BagOStuff {
 				}
 				$encExpiry = $db->timestamp( $exptime );
 			}
-			$db->commit( __METHOD__, 'flush' );
 			// (bug 24425) use a replace if the db supports it instead of
 			// delete/insert to avoid clashes with conflicting keynames
 			$db->update(
@@ -435,7 +433,6 @@ class SqlBagOStuff extends BagOStuff {
 				),
 				__METHOD__
 			);
-			$db->commit( __METHOD__, 'flush' );
 		} catch ( DBQueryError $e ) {
 			$this->handleWriteError( $e, $serverIndex );
 
@@ -454,12 +451,10 @@ class SqlBagOStuff extends BagOStuff {
 		list( $serverIndex, $tableName ) = $this->getTableByKey( $key );
 		try {
 			$db = $this->getDB( $serverIndex );
-			$db->commit( __METHOD__, 'flush' );
 			$db->delete(
 				$tableName,
 				array( 'keyname' => $key ),
 				__METHOD__ );
-			$db->commit( __METHOD__, 'flush' );
 		} catch ( DBError $e ) {
 			$this->handleWriteError( $e, $serverIndex );
 			return false;
@@ -478,7 +473,6 @@ class SqlBagOStuff extends BagOStuff {
 		try {
 			$db = $this->getDB( $serverIndex );
 			$step = intval( $step );
-			$db->commit( __METHOD__, 'flush' );
 			$row = $db->selectRow(
 				$tableName,
 				array( 'value', 'exptime' ),
@@ -487,14 +481,12 @@ class SqlBagOStuff extends BagOStuff {
 				array( 'FOR UPDATE' ) );
 			if ( $row === false ) {
 				// Missing
-				$db->commit( __METHOD__, 'flush' );
 
 				return null;
 			}
 			$db->delete( $tableName, array( 'keyname' => $key ), __METHOD__ );
 			if ( $this->isExpired( $db, $row->exptime ) ) {
 				// Expired, do not reinsert
-				$db->commit( __METHOD__, 'flush' );
 
 				return null;
 			}
@@ -512,7 +504,6 @@ class SqlBagOStuff extends BagOStuff {
 				// Race condition. See bug 28611
 				$newValue = null;
 			}
-			$db->commit( __METHOD__, 'flush' );
 		} catch ( DBError $e ) {
 			$this->handleWriteError( $e, $serverIndex );
 			return null;
@@ -603,7 +594,6 @@ class SqlBagOStuff extends BagOStuff {
 							$maxExpTime = $row->exptime;
 						}
 
-						$db->commit( __METHOD__, 'flush' );
 						$db->delete(
 							$this->getTableNameByShard( $i ),
 							array(
@@ -612,7 +602,6 @@ class SqlBagOStuff extends BagOStuff {
 								'keyname' => $keys
 							),
 							__METHOD__ );
-						$db->commit( __METHOD__, 'flush' );
 
 						if ( $progressCallback ) {
 							if ( intval( $totalSeconds ) === 0 ) {
@@ -645,9 +634,7 @@ class SqlBagOStuff extends BagOStuff {
 			try {
 				$db = $this->getDB( $serverIndex );
 				for ( $i = 0; $i < $this->shards; $i++ ) {
-					$db->commit( __METHOD__, 'flush' );
 					$db->delete( $this->getTableNameByShard( $i ), '*', __METHOD__ );
-					$db->commit( __METHOD__, 'flush' );
 				}
 			} catch ( DBError $e ) {
 				$this->handleWriteError( $e, $serverIndex );
@@ -775,12 +762,10 @@ class SqlBagOStuff extends BagOStuff {
 			}
 
 			for ( $i = 0; $i < $this->shards; $i++ ) {
-				$db->commit( __METHOD__, 'flush' );
 				$db->query(
 					'CREATE TABLE ' . $db->tableName( $this->getTableNameByShard( $i ) ) .
 					' LIKE ' . $db->tableName( 'objectcache' ),
 					__METHOD__ );
-				$db->commit( __METHOD__, 'flush' );
 			}
 		}
 	}
-- 
2.20.1