From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 12 Jun 2013 19:31:15 +0000 (-0700)
Subject: filebackend: try to combine SH and EX lock acquisition
X-Git-Tag: 1.31.0-rc.0~18468
X-Git-Url: https://git.heureux-cyclage.org/?a=commitdiff_plain;h=ef2f2610abcdad351177746e77e71bdf6aab14d6;p=lhc%2Fweb%2Fwiklou.git

filebackend: try to combine SH and EX lock acquisition

* getPathsToLockForOpsInternal() now returns an array in the format LockManager can use
* Also fixed getScopedLocksForOps() for multiwrite backend (it should accept proxy backend paths)
* Updated getScopedFileLocks() docs

Change-Id: Id0dc50c159c5fbc8fca10a9e46c5be23afcb3e9a
---

diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php
index bdeb578eff..f586578b2a 100644
--- a/includes/filebackend/FileBackend.php
+++ b/includes/filebackend/FileBackend.php
@@ -1183,8 +1183,10 @@ abstract class FileBackend {
 	 * Once the return value goes out scope, the locks will be released and
 	 * the status updated. Unlock fatals will not change the status "OK" value.
 	 *
-	 * @param array $paths Storage paths
-	 * @param integer $type LockManager::LOCK_* constant
+	 * @see ScopedLock::factory()
+	 *
+	 * @param array $paths List of storage paths or map of lock types to path lists
+	 * @param integer|string $type LockManager::LOCK_* constant or "mixed"
 	 * @param Status $status Status to update on lock/unlock
 	 * @return ScopedLock|null Returns null on failure
 	 */
diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php
index 7d354870d7..97584a7169 100644
--- a/includes/filebackend/FileBackendMultiWrite.php
+++ b/includes/filebackend/FileBackendMultiWrite.php
@@ -141,17 +141,10 @@ class FileBackendMultiWrite extends FileBackend {
 
 		$mbe = $this->backends[$this->masterIndex]; // convenience
 
-		// Get the paths to lock from the master backend
-		$realOps = $this->substOpBatchPaths( $ops, $mbe );
-		$paths = $mbe->getPathsToLockForOpsInternal( $mbe->getOperationsInternal( $realOps ) );
-		// Get the paths under the proxy backend's name
-		$paths['sh'] = $this->unsubstPaths( $paths['sh'] );
-		$paths['ex'] = $this->unsubstPaths( $paths['ex'] );
 		// Try to lock those files for the scope of this function...
 		if ( empty( $opts['nonLocking'] ) ) {
 			// Try to lock those files for the scope of this function...
-			$scopeLockS = $this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status );
-			$scopeLockE = $this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status );
+			$scopeLock = $this->getScopedLocksForOps( $ops, $status );
 			if ( !$status->isOK() ) {
 				return $status; // abort
 			}
@@ -178,6 +171,7 @@ class FileBackendMultiWrite extends FileBackend {
 			}
 		}
 		// Actually attempt the operation batch on the master backend...
+		$realOps = $this->substOpBatchPaths( $ops, $mbe );
 		$masterStatus = $mbe->doOperations( $realOps, $opts );
 		$status->merge( $masterStatus );
 		// Propagate the operations to the clone backends if there were no unexpected errors
@@ -624,15 +618,16 @@ class FileBackendMultiWrite extends FileBackend {
 	}
 
 	public function getScopedLocksForOps( array $ops, Status $status ) {
-		$fileOps = $this->backends[$this->masterIndex]->getOperationsInternal( $ops );
+		$realOps = $this->substOpBatchPaths( $ops, $this->backends[$this->masterIndex] );
+		$fileOps = $this->backends[$this->masterIndex]->getOperationsInternal( $realOps );
 		// Get the paths to lock from the master backend
 		$paths = $this->backends[$this->masterIndex]->getPathsToLockForOpsInternal( $fileOps );
 		// Get the paths under the proxy backend's name
-		$paths['sh'] = $this->unsubstPaths( $paths['sh'] );
-		$paths['ex'] = $this->unsubstPaths( $paths['ex'] );
-		return array(
-			$this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status ),
-			$this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status )
+		$pbPaths = array(
+			LockManager::LOCK_UW => $this->unsubstPaths( $paths[LockManager::LOCK_UW] ),
+			LockManager::LOCK_EX => $this->unsubstPaths( $paths[LockManager::LOCK_EX] )
 		);
+		// Actually acquire the locks
+		return array( $this->getScopedFileLocks( $pbPaths, 'mixed', $status ) );
 	}
 }
diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php
index 8ff383bd21..0921e99f38 100644
--- a/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@ -953,12 +953,13 @@ abstract class FileBackendStore extends FileBackend {
 
 	/**
 	 * Get a list of storage paths to lock for a list of operations
-	 * Returns an array with 'sh' (shared) and 'ex' (exclusive) keys,
-	 * each corresponding to a list of storage paths to be locked.
-	 * All returned paths are normalized.
+	 * Returns an array with LockManager::LOCK_UW (shared locks) and
+	 * LockManager::LOCK_EX (exclusive locks) keys, each corresponding
+	 * to a list of storage paths to be locked. All returned paths are
+	 * normalized.
 	 *
 	 * @param array $performOps List of FileOp objects
-	 * @return Array ('sh' => list of paths, 'ex' => list of paths)
+	 * @return Array (LockManager::LOCK_UW => path list, LockManager::LOCK_EX => path list)
 	 */
 	final public function getPathsToLockForOpsInternal( array $performOps ) {
 		// Build up a list of files to lock...
@@ -972,15 +973,15 @@ abstract class FileBackendStore extends FileBackend {
 		// Get a shared lock on the parent directory of each path changed
 		$paths['sh'] = array_merge( $paths['sh'], array_map( 'dirname', $paths['ex'] ) );
 
-		return $paths;
+		return array(
+			LockManager::LOCK_UW => $paths['sh'],
+			LockManager::LOCK_EX => $paths['ex']
+		);
 	}
 
 	public function getScopedLocksForOps( array $ops, Status $status ) {
 		$paths = $this->getPathsToLockForOpsInternal( $this->getOperationsInternal( $ops ) );
-		return array(
-			$this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status ),
-			$this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status )
-		);
+		return array( $this->getScopedFileLocks( $paths, 'mixed', $status ) );
 	}
 
 	final protected function doOperationsInternal( array $ops, array $opts ) {
@@ -998,8 +999,7 @@ abstract class FileBackendStore extends FileBackend {
 			// Build up a list of files to lock...
 			$paths = $this->getPathsToLockForOpsInternal( $performOps );
 			// Try to lock those files for the scope of this function...
-			$scopeLockS = $this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status );
-			$scopeLockE = $this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status );
+			$scopeLock = $this->getScopedFileLocks( $paths, 'mixed', $status );
 			if ( !$status->isOK() ) {
 				return $status; // abort
 			}