lockmanager: QuorumLockManager subclasses can get EX/SH locks at once
authorAaron Schulz <aschulz@wikimedia.org>
Sun, 16 Jun 2013 20:48:17 +0000 (13:48 -0700)
committerBryanDavis <bdavis@wikimedia.org>
Tue, 22 Oct 2013 17:18:07 +0000 (17:18 +0000)
* Also reduced rount trips in doUnlockingRequestBucket().
* Also removed some redundant doc comments.

Change-Id: I81878e92332509bd7fda9ddeef950b774f5b015d

includes/filebackend/lockmanager/DBLockManager.php
includes/filebackend/lockmanager/LockManager.php
includes/filebackend/lockmanager/MemcLockManager.php
includes/filebackend/lockmanager/QuorumLockManager.php
includes/filebackend/lockmanager/RedisLockManager.php

index e081987..3e934ba 100644 (file)
@@ -110,6 +110,19 @@ abstract class DBLockManager extends QuorumLockManager {
                $this->session = wfRandomString( 31 );
        }
 
+       // @TODO: change this code to work in one batch
+       protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
+               $status = Status::newGood();
+               foreach ( $pathsByType as $type => $paths ) {
+                       $status->merge( $this->doGetLocksOnServer( $lockSrv, $paths, $type ) );
+               }
+               return $status;
+       }
+
+       protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
+               return Status::newGood();
+       }
+
        /**
         * @see QuorumLockManager::isServerUp()
         * @return bool
@@ -252,7 +265,7 @@ class MySqlLockManager extends DBLockManager {
         * @see DBLockManager::getLocksOnServer()
         * @return Status
         */
-       protected function getLocksOnServer( $lockSrv, array $paths, $type ) {
+       protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) {
                $status = Status::newGood();
 
                $db = $this->getConnection( $lockSrv ); // checked in isServerUp()
@@ -318,14 +331,6 @@ class MySqlLockManager extends DBLockManager {
                return $status;
        }
 
-       /**
-        * @see QuorumLockManager::freeLocksOnServer()
-        * @return Status
-        */
-       protected function freeLocksOnServer( $lockSrv, array $paths, $type ) {
-               return Status::newGood(); // not supported
-       }
-
        /**
         * @see QuorumLockManager::releaseAllLocks()
         * @return Status
@@ -361,7 +366,7 @@ class PostgreSqlLockManager extends DBLockManager {
                self::LOCK_EX => self::LOCK_EX
        );
 
-       protected function getLocksOnServer( $lockSrv, array $paths, $type ) {
+       protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) {
                $status = Status::newGood();
                if ( !count( $paths ) ) {
                        return $status; // nothing to lock
@@ -407,14 +412,6 @@ class PostgreSqlLockManager extends DBLockManager {
                return $status;
        }
 
-       /**
-        * @see QuorumLockManager::freeLocksOnServer()
-        * @return Status
-        */
-       protected function freeLocksOnServer( $lockSrv, array $paths, $type ) {
-               return Status::newGood(); // not supported
-       }
-
        /**
         * @see QuorumLockManager::releaseAllLocks()
         * @return Status
index f0e54ec..dad8a62 100644 (file)
@@ -98,7 +98,7 @@ abstract class LockManager {
        /**
         * Lock the resources at the given abstract paths
         *
-        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
         * @param integer $timeout Timeout in seconds (0 means non-blocking) (since 1.21)
         * @return Status
         * @since 1.22
@@ -125,7 +125,7 @@ abstract class LockManager {
        /**
         * Unlock the resources at the given abstract paths
         *
-        * @param array $paths List of storage paths
+        * @param array $paths List of paths
         * @param $type integer LockManager::LOCK_* constant
         * @return Status
         */
@@ -136,7 +136,7 @@ abstract class LockManager {
        /**
         * Unlock the resources at the given abstract paths
         *
-        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
         * @return Status
         * @since 1.22
         */
@@ -176,7 +176,7 @@ abstract class LockManager {
         * Normalize the $paths array by converting LOCK_UW locks into the
         * appropriate type and removing any duplicated paths for each lock type.
         *
-        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @param array $paths Map of LockManager::LOCK_* constants to lists of paths
         * @return Array
         * @since 1.22
         */
@@ -190,7 +190,7 @@ abstract class LockManager {
 
        /**
         * @see LockManager::lockByType()
-        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @param array $paths Map of LockManager::LOCK_* constants to lists of paths
         * @return Status
         * @since 1.22
         */
@@ -215,7 +215,7 @@ abstract class LockManager {
        /**
         * Lock resources with the given keys and lock type
         *
-        * @param array $paths List of storage paths
+        * @param array $paths List of paths
         * @param $type integer LockManager::LOCK_* constant
         * @return Status
         */
@@ -223,7 +223,7 @@ abstract class LockManager {
 
        /**
         * @see LockManager::unlockByType()
-        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @param array $paths Map of LockManager::LOCK_* constants to lists of paths
         * @return Status
         * @since 1.22
         */
@@ -238,7 +238,7 @@ abstract class LockManager {
        /**
         * Unlock resources with the given keys and lock type
         *
-        * @param array $paths List of storage paths
+        * @param array $paths List of paths
         * @param $type integer LockManager::LOCK_* constant
         * @return Status
         */
@@ -250,22 +250,10 @@ abstract class LockManager {
  * @since 1.19
  */
 class NullLockManager extends LockManager {
-       /**
-        * @see LockManager::doLock()
-        * @param $paths array
-        * @param $type int
-        * @return Status
-        */
        protected function doLock( array $paths, $type ) {
                return Status::newGood();
        }
 
-       /**
-        * @see LockManager::doUnlock()
-        * @param $paths array
-        * @param $type int
-        * @return Status
-        */
        protected function doUnlock( array $paths, $type ) {
                return Status::newGood();
        }
index 757aeee..5eab03e 100644 (file)
@@ -88,11 +88,44 @@ class MemcLockManager extends QuorumLockManager {
                $this->session = wfRandomString( 32 );
        }
 
+       // @TODO: change this code to work in one batch
+       protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
+               $status = Status::newGood();
+
+               $lockedPaths = array();
+               foreach ( $pathsByType as $type => $paths ) {
+                       $status->merge( $this->doGetLocksOnServer( $lockSrv, $paths, $type ) );
+                       if ( $status->isOK() ) {
+                               $lockedPaths[$type] = isset( $lockedPaths[$type] )
+                                       ? array_merge( $lockedPaths[$type], $paths )
+                                       : $paths;
+                       } else {
+                               foreach ( $lockedPaths as $type => $paths ) {
+                                       $status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) );
+                               }
+                               break;
+                       }
+               }
+
+               return $status;
+       }
+
+       // @TODO: change this code to work in one batch
+       protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
+               $status = Status::newGood();
+
+               foreach ( $pathsByType as $type => $paths ) {
+                       $status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) );
+               }
+
+               return $status;
+       }
+
        /**
         * @see QuorumLockManager::getLocksOnServer()
         * @return Status
         */
-       protected function getLocksOnServer( $lockSrv, array $paths, $type ) {
+       protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) {
                $status = Status::newGood();
 
                $memc = $this->getCache( $lockSrv );
@@ -164,7 +197,7 @@ class MemcLockManager extends QuorumLockManager {
         * @see QuorumLockManager::freeLocksOnServer()
         * @return Status
         */
-       protected function freeLocksOnServer( $lockSrv, array $paths, $type ) {
+       protected function doFreeLocksOnServer( $lockSrv, array $paths, $type ) {
                $status = Status::newGood();
 
                $memc = $this->getCache( $lockSrv );
index 3b96ad6..8356d32 100644 (file)
 abstract class QuorumLockManager extends LockManager {
        /** @var Array Map of bucket indexes to peer server lists */
        protected $srvsByBucket = array(); // (bucket index => (lsrv1, lsrv2, ...))
+       /** @var Array Map of degraded buckets */
+       protected $degradedBuckets = array(); // (buckey index => UNIX timestamp)
 
-       /**
-        * @see LockManager::doLock()
-        * @param $paths array
-        * @param $type int
-        * @return Status
-        */
        final protected function doLock( array $paths, $type ) {
+               return $this->doLockByType( array( $type => $paths ) );
+       }
+
+       final protected function doUnlock( array $paths, $type ) {
+               return $this->doUnlockByType( array( $type => $paths ) );
+       }
+
+       protected function doLockByType( array $pathsByType ) {
                $status = Status::newGood();
 
-               $pathsToLock = array(); // (bucket => paths)
+               $pathsToLock = array(); // (bucket => type => paths)
                // Get locks that need to be acquired (buckets => locks)...
-               foreach ( $paths as $path ) {
-                       if ( isset( $this->locksHeld[$path][$type] ) ) {
-                               ++$this->locksHeld[$path][$type];
-                       } else {
-                               $bucket = $this->getBucketFromPath( $path );
-                               $pathsToLock[$bucket][] = $path;
+               foreach ( $pathsByType as $type => $paths ) {
+                       foreach ( $paths as $path ) {
+                               if ( isset( $this->locksHeld[$path][$type] ) ) {
+                                       ++$this->locksHeld[$path][$type];
+                               } else {
+                                       $bucket = $this->getBucketFromPath( $path );
+                                       $pathsToLock[$bucket][$type][] = $path;
+                               }
                        }
                }
 
-               $lockedPaths = array(); // files locked in this attempt
+               $lockedPaths = array(); // files locked in this attempt (type => paths)
                // Attempt to acquire these locks...
-               foreach ( $pathsToLock as $bucket => $paths ) {
+               foreach ( $pathsToLock as $bucket => $pathsToLockByType ) {
                        // Try to acquire the locks for this bucket
-                       $status->merge( $this->doLockingRequestBucket( $bucket, $paths, $type ) );
+                       $status->merge( $this->doLockingRequestBucket( $bucket, $pathsToLockByType ) );
                        if ( !$status->isOK() ) {
-                               $status->merge( $this->doUnlock( $lockedPaths, $type ) );
+                               $status->merge( $this->doUnlockByType( $lockedPaths ) );
                                return $status;
                        }
                        // Record these locks as active
-                       foreach ( $paths as $path ) {
-                               $this->locksHeld[$path][$type] = 1; // locked
+                       foreach ( $pathsToLockByType as $type => $paths ) {
+                               foreach ( $paths as $path ) {
+                                       $this->locksHeld[$path][$type] = 1; // locked
+                                       // Keep track of what locks were made in this attempt
+                                       $lockedPaths[$type][] = $path;
+                               }
                        }
-                       // Keep track of what locks were made in this attempt
-                       $lockedPaths = array_merge( $lockedPaths, $paths );
                }
 
                return $status;
        }
 
-       /**
-        * @see LockManager::doUnlock()
-        * @param $paths array
-        * @param $type int
-        * @return Status
-        */
-       final protected function doUnlock( array $paths, $type ) {
+       protected function doUnlockByType( array $pathsByType ) {
                $status = Status::newGood();
 
-               $pathsToUnlock = array();
-               foreach ( $paths as $path ) {
-                       if ( !isset( $this->locksHeld[$path][$type] ) ) {
-                               $status->warning( 'lockmanager-notlocked', $path );
-                       } else {
-                               --$this->locksHeld[$path][$type];
-                               // Reference count the locks held and release locks when zero
-                               if ( $this->locksHeld[$path][$type] <= 0 ) {
-                                       unset( $this->locksHeld[$path][$type] );
-                                       $bucket = $this->getBucketFromPath( $path );
-                                       $pathsToUnlock[$bucket][] = $path;
-                               }
-                               if ( !count( $this->locksHeld[$path] ) ) {
-                                       unset( $this->locksHeld[$path] ); // no SH or EX locks left for key
+               $pathsToUnlock = array(); // (bucket => type => paths)
+               foreach ( $pathsByType as $type => $paths ) {
+                       foreach ( $paths as $path ) {
+                               if ( !isset( $this->locksHeld[$path][$type] ) ) {
+                                       $status->warning( 'lockmanager-notlocked', $path );
+                               } else {
+                                       --$this->locksHeld[$path][$type];
+                                       // Reference count the locks held and release locks when zero
+                                       if ( $this->locksHeld[$path][$type] <= 0 ) {
+                                               unset( $this->locksHeld[$path][$type] );
+                                               $bucket = $this->getBucketFromPath( $path );
+                                               $pathsToUnlock[$bucket][$type][] = $path;
+                                       }
+                                       if ( !count( $this->locksHeld[$path] ) ) {
+                                               unset( $this->locksHeld[$path] ); // no SH or EX locks left for key
+                                       }
                                }
                        }
                }
 
                // Remove these specific locks if possible, or at least release
                // all locks once this process is currently not holding any locks.
-               foreach ( $pathsToUnlock as $bucket => $paths ) {
-                       $status->merge( $this->doUnlockingRequestBucket( $bucket, $paths, $type ) );
+               foreach ( $pathsToUnlock as $bucket => $pathsToUnlockByType ) {
+                       $status->merge( $this->doUnlockingRequestBucket( $bucket, $pathsToUnlockByType ) );
                }
                if ( !count( $this->locksHeld ) ) {
                        $status->merge( $this->releaseAllLocks() );
+                       $this->degradedBuckets = array(); // safe to retry the normal quorum
                }
 
                return $status;
@@ -116,11 +121,10 @@ abstract class QuorumLockManager extends LockManager {
         * This is all or nothing; if any key is locked then this totally fails.
         *
         * @param $bucket integer
-        * @param array $paths List of resource keys to lock
-        * @param $type integer LockManager::LOCK_EX or LockManager::LOCK_SH
+        * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
         * @return Status
         */
-       final protected function doLockingRequestBucket( $bucket, array $paths, $type ) {
+       final protected function doLockingRequestBucket( $bucket, array $pathsByType ) {
                $status = Status::newGood();
 
                $yesVotes = 0; // locks made on trustable servers
@@ -131,10 +135,11 @@ abstract class QuorumLockManager extends LockManager {
                        if ( !$this->isServerUp( $lockSrv ) ) {
                                --$votesLeft;
                                $status->warning( 'lockmanager-fail-svr-acquire', $lockSrv );
+                               $this->degradedBuckets[$bucket] = time();
                                continue; // server down?
                        }
                        // Attempt to acquire the lock on this peer
-                       $status->merge( $this->getLocksOnServer( $lockSrv, $paths, $type ) );
+                       $status->merge( $this->getLocksOnServer( $lockSrv, $pathsByType ) );
                        if ( !$status->isOK() ) {
                                return $status; // vetoed; resource locked
                        }
@@ -158,21 +163,33 @@ abstract class QuorumLockManager extends LockManager {
         * Attempt to release locks with the peers for a bucket
         *
         * @param $bucket integer
-        * @param array $paths List of resource keys to lock
-        * @param $type integer LockManager::LOCK_EX or LockManager::LOCK_SH
+        * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
         * @return Status
         */
-       final protected function doUnlockingRequestBucket( $bucket, array $paths, $type ) {
+       final protected function doUnlockingRequestBucket( $bucket, array $pathsByType ) {
                $status = Status::newGood();
 
+               $yesVotes = 0; // locks freed on trustable servers
+               $votesLeft = count( $this->srvsByBucket[$bucket] ); // remaining peers
+               $quorum = floor( $votesLeft / 2 + 1 ); // simple majority
+               $isDegraded = isset( $this->degradedBuckets[$bucket] ); // not the normal quorum?
                foreach ( $this->srvsByBucket[$bucket] as $lockSrv ) {
                        if ( !$this->isServerUp( $lockSrv ) ) {
-                               $status->fatal( 'lockmanager-fail-svr-release', $lockSrv );
+                               $status->warning( 'lockmanager-fail-svr-release', $lockSrv );
                        // Attempt to release the lock on this peer
                        } else {
-                               $status->merge( $this->freeLocksOnServer( $lockSrv, $paths, $type ) );
+                               $status->merge( $this->freeLocksOnServer( $lockSrv, $pathsByType ) );
+                               ++$yesVotes; // success for this peer
+                               // Normally the first peers form the quorum, and the others are ignored.
+                               // Ignore them in this case, but not when an alternative quorum was used.
+                               if ( $yesVotes >= $quorum && !$isDegraded ) {
+                                       break; // lock released
+                               }
                        }
                }
+               // Set a bad status if the quorum was not met.
+               // Assumes the same "up" servers as during the acquire step.
+               $status->setResult( $yesVotes >= $quorum );
 
                return $status;
        }
@@ -190,7 +207,8 @@ abstract class QuorumLockManager extends LockManager {
        }
 
        /**
-        * Check if a lock server is up
+        * Check if a lock server is up.
+        * This should process cache results to reduce RTT.
         *
         * @param $lockSrv string
         * @return bool
@@ -198,14 +216,13 @@ abstract class QuorumLockManager extends LockManager {
        abstract protected function isServerUp( $lockSrv );
 
        /**
-        * Get a connection to a lock server and acquire locks on $paths
+        * Get a connection to a lock server and acquire locks
         *
         * @param $lockSrv string
-        * @param $paths array
-        * @param $type integer
+        * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
         * @return Status
         */
-       abstract protected function getLocksOnServer( $lockSrv, array $paths, $type );
+       abstract protected function getLocksOnServer( $lockSrv, array $pathsByType );
 
        /**
         * Get a connection to a lock server and release locks on $paths.
@@ -213,11 +230,10 @@ abstract class QuorumLockManager extends LockManager {
         * Subclasses must effectively implement this or releaseAllLocks().
         *
         * @param $lockSrv string
-        * @param $paths array
-        * @param $type integer
+        * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
         * @return Status
         */
-       abstract protected function freeLocksOnServer( $lockSrv, array $paths, $type );
+       abstract protected function freeLocksOnServer( $lockSrv, array $pathsByType );
 
        /**
         * Release all locks that this session is holding.
index e8e5996..43b0198 100644 (file)
@@ -78,7 +78,40 @@ class RedisLockManager extends QuorumLockManager {
                $this->session = wfRandomString( 32 );
        }
 
-       protected function getLocksOnServer( $lockSrv, array $paths, $type ) {
+       // @TODO: change this code to work in one batch
+       protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
+               $status = Status::newGood();
+
+               $lockedPaths = array();
+               foreach ( $pathsByType as $type => $paths ) {
+                       $status->merge( $this->doGetLocksOnServer( $lockSrv, $paths, $type ) );
+                       if ( $status->isOK() ) {
+                               $lockedPaths[$type] = isset( $lockedPaths[$type] )
+                                       ? array_merge( $lockedPaths[$type], $paths )
+                                       : $paths;
+                       } else {
+                               foreach ( $lockedPaths as $type => $paths ) {
+                                       $status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) );
+                               }
+                               break;
+                       }
+               }
+
+               return $status;
+       }
+
+       // @TODO: change this code to work in one batch
+       protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
+               $status = Status::newGood();
+
+               foreach ( $pathsByType as $type => $paths ) {
+                       $status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) );
+               }
+
+               return $status;
+       }
+
+       protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) {
                $status = Status::newGood();
 
                $server = $this->lockServers[$lockSrv];
@@ -162,7 +195,7 @@ LUA;
                return $status;
        }
 
-       protected function freeLocksOnServer( $lockSrv, array $paths, $type ) {
+       protected function doFreeLocksOnServer( $lockSrv, array $paths, $type ) {
                $status = Status::newGood();
 
                $server = $this->lockServers[$lockSrv];