* Improved handling of corrupt values in cache for MemcLockManager.
Also improved the use of Status warnings a bit.
* Removed broken special-case handling for SH->EX lock escalation.
Updated MysqLockManager to compensate.
* Made FSLockManager only use one handle per file, which is more
efficient and avoids errors when escalating locks (SH->EX).
* Made lock unit tests have more useful output on failure.
Change-Id: Ib304712fa2b6b3fd02bfc1b08b6f238c771960c2
$status = Status::newGood();
$db = $this->getConnection( $lockSrv ); // checked in isServerUp()
- $keys = array_unique( array_map( array( $this, 'sha1Base36Absolute' ), $paths ) );
+
+ $keys = array(); // list of hash keys for the paths
+ $data = array(); // list of rows to insert
+ $checkEXKeys = array(); // list of hash keys that this has no EX lock on
# Build up values for INSERT clause
- $data = array();
- foreach ( $keys as $key ) {
+ foreach ( $paths as $path ) {
+ $key = $this->sha1Base36Absolute( $path );
+ $keys[] = $key;
$data[] = array( 'fls_key' => $key, 'fls_session' => $this->session );
+ if ( !isset( $this->locksHeld[$path][self::LOCK_EX] ) ) {
+ $checkEXKeys[] = $key;
+ }
}
- # Block new writers...
+
+ # Block new writers (both EX and SH locks leave entries here)...
$db->insert( 'filelocks_shared', $data, __METHOD__, array( 'IGNORE' ) );
# Actually do the locking queries...
if ( $type == self::LOCK_SH ) { // reader locks
+ $blocked = false;
# Bail if there are any existing writers...
- $blocked = $db->selectField( 'filelocks_exclusive', '1',
- array( 'fle_key' => $keys ),
- __METHOD__
- );
- # Prospective writers that haven't yet updated filelocks_exclusive
- # will recheck filelocks_shared after doing so and bail due to our entry.
+ if ( count( $checkEXKeys ) ) {
+ $blocked = $db->selectField( 'filelocks_exclusive', '1',
+ array( 'fle_key' => $checkEXKeys ),
+ __METHOD__
+ );
+ }
+ # Other prospective writers that haven't yet updated filelocks_exclusive
+ # will recheck filelocks_shared after doing so and bail due to this entry.
} else { // writer locks
$encSession = $db->addQuotes( $this->session );
# Bail if there are any existing writers...
- # The may detect readers, but the safe check for them is below.
+ # This may detect readers, but the safe check for them is below.
# Note: if two writers come at the same time, both bail :)
$blocked = $db->selectField( 'filelocks_shared', '1',
array( 'fls_key' => $keys, "fls_session != $encSession" ),
protected $lockDir; // global dir for all servers
- /** @var Array Map of (locked key => lock type => lock file handle) */
+ /** @var Array Map of (locked key => lock file handle) */
protected $handles = array();
/**
} elseif ( isset( $this->locksHeld[$path][self::LOCK_EX] ) ) {
$this->locksHeld[$path][$type] = 1;
} else {
- wfSuppressWarnings();
- $handle = fopen( $this->getLockPath( $path ), 'a+' );
- wfRestoreWarnings();
- if ( !$handle ) { // lock dir missing?
- wfMkdirParents( $this->lockDir );
- $handle = fopen( $this->getLockPath( $path ), 'a+' ); // try again
+ if ( isset( $this->handles[$path] ) ) {
+ $handle = $this->handles[$path];
+ } else {
+ wfSuppressWarnings();
+ $handle = fopen( $this->getLockPath( $path ), 'a+' );
+ wfRestoreWarnings();
+ if ( !$handle ) { // lock dir missing?
+ wfMkdirParents( $this->lockDir );
+ $handle = fopen( $this->getLockPath( $path ), 'a+' ); // try again
+ }
}
if ( $handle ) {
// Either a shared or exclusive lock
if ( flock( $handle, $lock | LOCK_NB ) ) {
// Record this lock as active
$this->locksHeld[$path][$type] = 1;
- $this->handles[$path][$type] = $handle;
+ $this->handles[$path] = $handle;
} else {
fclose( $handle );
$status->fatal( 'lockmanager-fail-acquirelock', $path );
--$this->locksHeld[$path][$type];
if ( $this->locksHeld[$path][$type] <= 0 ) {
unset( $this->locksHeld[$path][$type] );
- // If a LOCK_SH comes in while we have a LOCK_EX, we don't
- // actually add a handler, so check for handler existence.
- if ( isset( $this->handles[$path][$type] ) ) {
- if ( $type === self::LOCK_EX
- && isset( $this->locksHeld[$path][self::LOCK_SH] )
- && !isset( $this->handles[$path][self::LOCK_SH] ) )
- {
- // EX lock came first: move this handle to the SH one
- $this->handles[$path][self::LOCK_SH] = $this->handles[$path][$type];
- } else {
- // Mark this handle to be unlocked and closed
- $handlesToClose[] = $this->handles[$path][$type];
- }
- unset( $this->handles[$path][$type] );
- }
}
if ( !count( $this->locksHeld[$path] ) ) {
unset( $this->locksHeld[$path] ); // no locks on this path
+ if ( isset( $this->handles[$path] ) ) {
+ $handlesToClose[] = $this->handles[$path];
+ unset( $this->handles[$path] );
+ }
}
// Unlock handles to release locks and delete
// any lock files that end up with no locks on them...
* This is meant for multi-wiki systems that may share files.
* All locks are non-blocking, which avoids deadlocks.
*
- * All lock requests for a resource, identified by a hash string, will map
- * to one bucket. Each bucket maps to one or several peer servers, each running memcached.
+ * All lock requests for a resource, identified by a hash string, will map to one
+ * bucket. Each bucket maps to one or several peer servers, each running memcached.
* A majority of peers must agree for a lock to be acquired.
*
* @ingroup LockManager
protected $serversUp = array(); // (server name => bool)
protected $lockExpiry; // integer; maximum time locks can be held
- protected $session = ''; // string; random SHA-1 UUID
+ protected $session = ''; // string; random UUID
/**
* Construct a new instance from configuration.
foreach ( $paths as $path ) {
$locksKey = $this->recordKeyForPath( $path );
$locksHeld = isset( $lockRecords[$locksKey] )
- ? $lockRecords[$locksKey]
- : array( self::LOCK_SH => array(), self::LOCK_EX => array() ); // init
+ ? self::sanitizeLockArray( $lockRecords[$locksKey] )
+ : self::newLockArray(); // init
foreach ( $locksHeld[self::LOCK_EX] as $session => $expiry ) {
if ( $expiry < $now ) { // stale?
unset( $locksHeld[self::LOCK_EX][$session] );
// If there were no lock conflicts, update all the lock records...
if ( $status->isOK() ) {
- foreach ( $lockRecords as $locksKey => $locksHeld ) {
- $memc->set( $locksKey, $locksHeld );
- wfDebug( __METHOD__ . ": acquired lock on key $locksKey.\n" );
+ foreach ( $paths as $path ) {
+ $locksKey = $this->recordKeyForPath( $path );
+ $locksHeld = $lockRecords[$locksKey];
+ $ok = $memc->set( $locksKey, $locksHeld, 7*86400 );
+ if ( !$ok ) {
+ $status->fatal( 'lockmanager-fail-acquirelock', $path );
+ } else {
+ wfDebug( __METHOD__ . ": acquired lock on key $locksKey.\n" );
+ }
}
}
foreach ( $paths as $path ) {
$locksKey = $this->recordKeyForPath( $path ); // lock record
if ( !isset( $lockRecords[$locksKey] ) ) {
+ $status->warning( 'lockmanager-fail-releaselock', $path );
continue; // nothing to do
}
- $locksHeld = $lockRecords[$locksKey];
- if ( is_array( $locksHeld ) && isset( $locksHeld[$type] ) ) {
- unset( $locksHeld[$type][$this->session] );
- $ok = $memc->set( $locksKey, $locksHeld );
+ $locksHeld = self::sanitizeLockArray( $lockRecords[$locksKey] );
+ if ( isset( $locksHeld[$type][$this->session] ) ) {
+ unset( $locksHeld[$type][$this->session] ); // unregister this session
+ if ( $locksHeld === self::newLockArray() ) {
+ $ok = $memc->delete( $locksKey );
+ } else {
+ $ok = $memc->set( $locksKey, $locksHeld );
+ }
+ if ( !$ok ) {
+ $status->fatal( 'lockmanager-fail-releaselock', $path );
+ }
} else {
- $ok = true;
- }
- if ( !$ok ) {
- $status->fatal( 'lockmanager-fail-releaselock', $path );
+ $status->warning( 'lockmanager-fail-releaselock', $path );
}
wfDebug( __METHOD__ . ": released lock on key $locksKey.\n" );
}
return implode( ':', array( __CLASS__, 'locks', $this->sha1Base36Absolute( $path ) ) );
}
+ /**
+ * @return Array An empty lock structure for a key
+ */
+ protected static function newLockArray() {
+ return array( self::LOCK_SH => array(), self::LOCK_EX => array() );
+ }
+
+ /**
+ * @param $a array
+ * @return Array An empty lock structure for a key
+ */
+ protected static function sanitizeLockArray( $a ) {
+ if ( is_array( $a ) && isset( $a[self::LOCK_EX] ) && isset( $a[self::LOCK_SH] ) ) {
+ return $a;
+ } else {
+ trigger_error( __METHOD__ . ": reset invalid lock array.", E_USER_WARNING );
+ return self::newLockArray();
+ }
+ }
+
/**
* @param $memc MemcachedBagOStuff
* @param array $keys List of keys to acquire
continue; // acquire in order
}
}
- } while ( count( $lockedKeys ) < count( $keys ) && ( microtime( true ) - $start ) <= 6 );
+ } while ( count( $lockedKeys ) < count( $keys ) && ( microtime( true ) - $start ) <= 3 );
if ( count( $lockedKeys ) != count( $keys ) ) {
- $this->releaseMutexes( $lockedKeys ); // failed; release what was locked
+ $this->releaseMutexes( $memc, $lockedKeys ); // failed; release what was locked
return false;
}
foreach ( $paths as $path ) {
if ( isset( $this->locksHeld[$path][$type] ) ) {
++$this->locksHeld[$path][$type];
- } elseif ( isset( $this->locksHeld[$path][self::LOCK_EX] ) ) {
- $this->locksHeld[$path][$type] = 1;
} else {
$bucket = $this->getBucketFromPath( $path );
$pathsToLock[$bucket][] = $path;
private function doTestLockCalls() {
$backendName = $this->backendClass();
- for ( $i=0; $i<50; $i++ ) {
- $paths = array(
- "test1.txt",
- "test2.txt",
- "test3.txt",
- "subdir1",
- "subdir1", // duplicate
- "subdir1/test1.txt",
- "subdir1/test2.txt",
- "subdir2",
- "subdir2", // duplicate
- "subdir2/test3.txt",
- "subdir2/test4.txt",
- "subdir2/subdir",
- "subdir2/subdir/test1.txt",
- "subdir2/subdir/test2.txt",
- "subdir2/subdir/test3.txt",
- "subdir2/subdir/test4.txt",
- "subdir2/subdir/test5.txt",
- "subdir2/subdir/sub",
- "subdir2/subdir/sub/test0.txt",
- "subdir2/subdir/sub/120-px-file.txt",
- );
+ $paths = array(
+ "test1.txt",
+ "test2.txt",
+ "test3.txt",
+ "subdir1",
+ "subdir1", // duplicate
+ "subdir1/test1.txt",
+ "subdir1/test2.txt",
+ "subdir2",
+ "subdir2", // duplicate
+ "subdir2/test3.txt",
+ "subdir2/test4.txt",
+ "subdir2/subdir",
+ "subdir2/subdir/test1.txt",
+ "subdir2/subdir/test2.txt",
+ "subdir2/subdir/test3.txt",
+ "subdir2/subdir/test4.txt",
+ "subdir2/subdir/test5.txt",
+ "subdir2/subdir/sub",
+ "subdir2/subdir/sub/test0.txt",
+ "subdir2/subdir/sub/120-px-file.txt",
+ );
+ for ( $i=0; $i<25; $i++ ) {
$status = $this->backend->lockFiles( $paths, LockManager::LOCK_EX );
- $this->assertEquals( array(), $status->errors,
- "Locking of files succeeded ($backendName)." );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName) ($i)." );
$this->assertEquals( true, $status->isOK(),
- "Locking of files succeeded with OK status ($backendName)." );
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
$status = $this->backend->lockFiles( $paths, LockManager::LOCK_SH );
- $this->assertEquals( array(), $status->errors,
- "Locking of files succeeded ($backendName)." );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName) ($i)." );
$this->assertEquals( true, $status->isOK(),
- "Locking of files succeeded with OK status ($backendName)." );
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
$status = $this->backend->unlockFiles( $paths, LockManager::LOCK_SH );
- $this->assertEquals( array(), $status->errors,
- "Locking of files succeeded ($backendName)." );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName) ($i)." );
+ $this->assertEquals( true, $status->isOK(),
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
+
+ $status = $this->backend->unlockFiles( $paths, LockManager::LOCK_EX );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName). ($i)" );
$this->assertEquals( true, $status->isOK(),
- "Locking of files succeeded with OK status ($backendName)." );
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
+
+ ## Flip the acquire/release ordering around ##
+
+ $status = $this->backend->lockFiles( $paths, LockManager::LOCK_SH );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName) ($i)." );
+ $this->assertEquals( true, $status->isOK(),
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
+
+ $status = $this->backend->lockFiles( $paths, LockManager::LOCK_EX );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName) ($i)." );
+ $this->assertEquals( true, $status->isOK(),
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
$status = $this->backend->unlockFiles( $paths, LockManager::LOCK_EX );
- $this->assertEquals( array(), $status->errors,
- "Locking of files succeeded ($backendName)." );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName). ($i)" );
+ $this->assertEquals( true, $status->isOK(),
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
+
+ $status = $this->backend->unlockFiles( $paths, LockManager::LOCK_SH );
+ $this->assertEquals( print_r( array(), true ), print_r( $status->errors, true ),
+ "Locking of files succeeded ($backendName) ($i)." );
$this->assertEquals( true, $status->isOK(),
- "Locking of files succeeded with OK status ($backendName)." );
+ "Locking of files succeeded with OK status ($backendName) ($i)." );
}
$status = Status::newGood();
dépôts / lhc / web / wiklou.git / commitdiff