https://www.mediawiki.org/wiki/Special:Code/MediaWiki/103790
Revision: 103790
Author: aaron
Date: 2011-11-20 22:03:58 +0000 (Sun, 20 Nov 2011)
Log Message:
-----------
* Cleanups to DBFileLockManager; improved $config layout
* Moved locking to doOperations() to reduce queries for DB-based locking
(better batching)
* Renamed $transaction to $fileOp in doOperations()
* Fixed bug in attempt() wrt setting failedAttempt field
* Added missing storagePathsToLock() to FileMoveOp
* Better use of Status::merge() function in various places
Modified Paths:
--------------
branches/FileBackend/phase3/includes/filerepo/ForeignDBViaLBRepo.php
branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php
branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
branches/FileBackend/phase3/includes/filerepo/backend/FileLockManager.php
Modified: branches/FileBackend/phase3/includes/filerepo/ForeignDBViaLBRepo.php
===================================================================
--- branches/FileBackend/phase3/includes/filerepo/ForeignDBViaLBRepo.php
2011-11-20 22:03:13 UTC (rev 103789)
+++ branches/FileBackend/phase3/includes/filerepo/ForeignDBViaLBRepo.php
2011-11-20 22:03:58 UTC (rev 103790)
@@ -30,6 +30,7 @@
function getSlaveDB() {
return wfGetDB( DB_SLAVE, array(), $this->wiki );
}
+
function hasSharedCache() {
return $this->hasSharedCache;
}
Modified: branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php
===================================================================
--- branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php
2011-11-20 22:03:13 UTC (rev 103789)
+++ branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php
2011-11-20 22:03:58 UTC (rev 103790)
@@ -258,7 +258,7 @@
// Get params for this operation
$params = $operation;
unset( $params['operation'] ); // don't need
this
- // Add operation on to the list of things to do
+ // Append the FileOp class
$performOps[] = new $class( $params );
} else {
throw new MWException( "Operation `$opName` is
not supported." );
@@ -270,31 +270,49 @@
final public function doOperations( array $ops ) {
$status = Status::newGood();
+
// Build up a list of FileOps...
$performOps = $this->getOperations( $ops );
+
+ // Build up a list of files to lock...
+ $filesToLock = array();
+ foreach ( $performOps as $index => $fileOp ) {
+ $filesToLock = array_merge( $filesToLock,
$fileOp->storagePathsToLock() );
+ }
+ $filesToLock = array_unique( $filesToLock ); // avoid warnings
+
+ // Try to lock those files...
+ $status->merge( $this->lockFiles( $filesToLock ) );
+ if ( !$status->isOK() ) {
+ return $status; // abort
+ }
+
// Attempt each operation; abort on failure...
- foreach ( $performOps as $index => $transaction ) {
- $subStatus = $transaction->attempt();
- $status->merge( $subStatus );
- if ( !$subStatus->isOK() ) { // operation failed?
+ foreach ( $performOps as $index => $fileOp ) {
+ $status->merge( $fileOp->attempt() );
+ if ( !$status->isOK() ) { // operation failed?
// Revert everything done so far and abort.
// Do this by reverting each previous operation
in reverse order.
$pos = $index - 1; // last one failed; no need
to revert()
while ( $pos >= 0 ) {
- $subStatus =
$performOps[$pos]->revert();
- $status->merge( $subStatus );
+ $status->merge(
$performOps[$pos]->revert() );
$pos--;
}
return $status;
}
}
+
// Finish each operation...
- foreach ( $performOps as $index => $transaction ) {
- $subStatus = $transaction->finish();
- $status->merge( $subStatus );
+ foreach ( $performOps as $index => $fileOp ) {
+ $status->merge( $fileOp->finish() );
}
- // Make sure status is OK, despite any finish() fatals
+
+ // Unlock all the files
+ $status->merge( $this->unlockFiles( $filesToLock ) );
+
+ // Make sure status is OK, despite any finish() or
unlockFiles() fatals
$status->setResult( true );
+
return $status;
}
@@ -354,10 +372,8 @@
throw new MWException( "Cannot attempt operation called
twice." );
}
$this->state = self::STATE_ATTEMPTED;
- $status = $this->setLocks();
- if ( $status->isOK() ) {
- $status = $this->doAttempt();
- } else {
+ $status = $this->doAttempt();
+ if ( !$status->isOK() ) {
$this->failedAttempt = true;
}
return $status;
@@ -373,12 +389,11 @@
throw new MWException( "Cannot rollback an unstarted or
finished operation." );
}
$this->state = self::STATE_DONE;
- if ( !$this->failedAttempt ) {
+ if ( $this->failedAttempt ) {
+ $status = Status::newGood(); // nothing to revert
+ } else {
$status = $this->doRevert();
- } else {
- $status = Status::newGood(); // nothing to revert
}
- $this->unsetLocks();
return $status;
}
@@ -392,39 +407,20 @@
throw new MWException( "Cannot cleanup an unstarted or
finished operation." );
}
$this->state = self::STATE_DONE;
- if ( !$this->failedAttempt ) {
+ if ( $this->failedAttempt ) {
+ $status = Status::newGood(); // nothing to revert
+ } else {
$status = $this->doFinish();
- } else {
- $status = Status::newGood(); // nothing to revert
}
- $this->unsetLocks();
return $status;
}
/**
- * Try to lock any files before changing them
- *
- * @return Status
- */
- private function setLocks() {
- return $this->backend->lockFiles( $this->storagePathsToLock() );
- }
-
- /**
- * Try to unlock any files that this locked
- *
- * @return Status
- */
- private function unsetLocks() {
- return $this->backend->unlockFiles( $this->storagePathsToLock()
);
- }
-
- /**
* Get a list of storage paths to lock for this operation
*
* @return Array
*/
- protected function storagePathsToLock() {
+ public function storagePathsToLock() {
return array();
}
@@ -632,6 +628,10 @@
}
return $status;
}
+
+ function storagePathsToLock() {
+ return array( $this->params['source'], $this->params['dest'] );
+ }
}
/**
Modified:
branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
===================================================================
---
branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
2011-11-20 22:03:13 UTC (rev 103789)
+++
branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
2011-11-20 22:03:58 UTC (rev 103790)
@@ -1,7 +1,5 @@
<?php
/**
- * Class for a "backend" consisting of a orioritized list of backend
- *
* @file
* @ingroup FileRepo
*/
@@ -41,35 +39,56 @@
final public function doOperations( array $ops ) {
$status = Status::newGood();
+
// Build up a list of FileOps. The list will have all the ops
// for one backend, then all the ops for the next, and so on.
+ // Also build up a list of files to lock...
$performOps = array();
- foreach ( $this->fileBackends as $backend ) {
+ $filesToLock = array();
+ foreach ( $this->fileBackends as $index => $backend ) {
$performOps = array_merge( $performOps,
$backend->getOperations( $ops ) );
+ // Set $filesToLock from the first backend so we don't
try to set all
+ // locks two or three times (depending on the number of
backends).
+ if ( $index == 0 ) {
+ foreach ( $performOps as $index => $fileOp ) {
+ $filesToLock = array_merge(
$filesToLock, $fileOp->storagePathsToLock() );
+ }
+ $filesToLock = array_unique( $filesToLock ); //
avoid warnings
+ }
}
+
+ // Try to lock those files...
+ $status->merge( $this->lockFiles( $filesToLock ) );
+ if ( !$status->isOK() ) {
+ return $status; // abort
+ }
+
// Attempt each operation; abort on failure...
- foreach ( $performOps as $index => $transaction ) {
- $subStatus = $transaction->attempt();
- $status->merge( $subStatus );
- if ( !$subStatus->isOK() ) { // operation failed?
+ foreach ( $performOps as $index => $fileOp ) {
+ $status->merge( $fileOp->attempt() );
+ if ( !$status->isOK() ) { // operation failed?
// Revert everything done so far and abort.
// Do this by reverting each previous operation
in reverse order.
$pos = $index - 1; // last one failed; no need
to revert()
while ( $pos >= 0 ) {
- $subStatus =
$performOps[$pos]->revert();
- $status->merge( $subStatus );
+ $status->merge(
$performOps[$pos]->revert() );
$pos--;
}
return $status;
}
}
+
// Finish each operation...
- foreach ( $performOps as $index => $transaction ) {
- $subStatus = $transaction->finish();
- $status->merge( $subStatus );
+ foreach ( $performOps as $index => $fileOp ) {
+ $status->merge( $fileOp->finish() );
}
- // Make sure status is OK, despite any finish() fatals
+
+ // Unlock all the files
+ $status->merge( $this->unlockFiles( $filesToLock ) );
+
+ // Make sure status is OK, despite any finish() or
unlockFiles() fatals
$status->setResult( true );
+
return $status;
}
@@ -145,13 +164,11 @@
function lockFiles( array $paths ) {
$status = Status::newGood();
foreach ( $this->backends as $index => $backend ) {
- $subStatus = $backend->lockFiles( $paths );
- $status->merge( $subStatus );
- if ( !$subStatus->isOk() ) {
+ $status->merge( $backend->lockFiles( $paths ) );
+ if ( !$status->isOk() ) {
// Lock failed: release the locks done so far
each backend
for ( $i=0; $i < $index; $i++ ) { // don't do
backend $index since it failed
- $subStatus = $backend->unlockFiles(
$paths );
- $status->merge( $subStatus );
+ $status->merge( $backend->unlockFiles(
$paths ) );
}
return $status;
}
@@ -162,8 +179,7 @@
function unlockFiles( array $paths ) {
$status = Status::newGood();
foreach ( $this->backends as $backend ) {
- $subStatus = $backend->unlockFile( $paths );
- $status->merge( $subStatus );
+ $status->merge( $backend->unlockFile( $paths ) );
}
return $status;
}
Modified:
branches/FileBackend/phase3/includes/filerepo/backend/FileLockManager.php
===================================================================
--- branches/FileBackend/phase3/includes/filerepo/backend/FileLockManager.php
2011-11-20 22:03:13 UTC (rev 103789)
+++ branches/FileBackend/phase3/includes/filerepo/backend/FileLockManager.php
2011-11-20 22:03:58 UTC (rev 103790)
@@ -97,8 +97,7 @@
$lockedKeys[] = $key;
} else {
// Abort and unlock everything
- $subStatus = $this->doUnlock( $lockedKeys );
- $status->merge( $subStatus );
+ $status->merge( $this->doUnlock( $lockedKeys )
);
return $status;
}
}
@@ -110,8 +109,7 @@
$status = Status::newGood();
foreach ( $keys as $key ) {
- $subStatus = $this->doSingleUnlock( $key );
- $status->merge( $subStatus );
+ $status->merge( $this->doSingleUnlock( $key ) );
}
return $status;
@@ -181,21 +179,19 @@
* One or several lock database servers are set up having a `file_locking`
* table with one field, fl_key, the PRIMARY KEY. The table engine should
* have row-level locking. For performance, deadlock detection should be
- * disabled and a low lock-wait timeout should be put in the server config.
+ * disabled and a low lock-wait timeout should be set via server config.
*
* All lock requests for an item (identified by an abstract key string) will
- * map to one bucket. Each bucket maps to a single server, and each server can
- * have several or no fallback servers. Fallback servers recieve the same lock
- * statements as the servers they are set as fallbacks for. This propagation is
- * only best-effort; lock requests will not be blocked just because a fallback
- * server cannot be contacted to recieve a copy of the lock request.
+ * map to one bucket. Each bucket maps to a single server, though each server
+ * can have several fallback servers.
+ *
+ * Fallback servers recieve the same lock statements as the servers they
standby for.
+ * This propagation is only best-effort; lock requests will not be blocked just
+ * because a fallback server cannot recieve a copy of the lock request.
*/
class DBFileLockManager extends FileLockManager {
/** @var Array Map of bucket indexes to server names */
- protected $serverMap = array(); // (index => server name)
- /** @var Array Map of servers to fallback server names */
- protected $serverFallbackMap = array(); // (server =>
(server1,server2,...))
-
+ protected $serverMap = array(); // (index => (server1,server2,...))
/** @var Array List of active lock key names */
protected $locksHeld = array(); // (key => 1)
/** $var Array Map Lock-active database connections (name => Database)
*/
@@ -203,54 +199,64 @@
/**
* Construct a new instance from configuration.
- * The 'serverMap' param of $config has is an array of consecutive
- * integer keys, starting from 0, with server name strings as values.
- * It should have no more than 16 items in the array.
- *
- * The `file_locking` table should have row-level locking (e.g. innoDB).
- *
- * @param array $config
+ * $config paramaters include:
+ * 'serverMap' : Array of no more than 16 consecutive integer keys,
+ * starting from 0, with a list of servers as values.
+ * The first server in each list is the main server and
+ * the others are fallback servers.
+ *
+ * @param Array $config
*/
function __construct( array $config ) {
- $this->serverMap = $config['serverMap'];
- $this->serverFallbackMap = $config['serverFallbackMap'];
+ $this->serverMap = (array)$config['serverMap'];
+ // Sanitize serverMap against bad config to prevent PHP errors
+ for ( $i=0; $i < count( $this->serverMap ); $i++ ) {
+ if (
+ !isset( $this->serverMap[$i] ) || // not
consecutive
+ !is_array( $this->serverMap[$i] ) || // bad type
+ !count( $this->serverMap[$i] ) // empty list
+ ) {
+ $this->serverMap[$i] = null; // see
getBucketFromKey()
+ wfWarn( "No key for bucket $i in serverMap or
server list is empty." );
+ }
+ }
}
function doLock( array $keys ) {
$status = Status::newGood();
$keysToLock = array();
- // Get locks that need to be acquired and which server they map
to...
+ // Get locks that need to be acquired (buckets => locks)...
foreach ( $keys as $key ) {
if ( isset( $this->locksHeld[$key] ) ) {
$status->warning( 'File already locked.' );
} else {
- $server = $this->getDBServerFromKey( $key );
- if ( $server === null ) { // config error?
- $status->fatal( "Lock server for $key
is not set." );
+ $bucket = $this->getBucketFromKey( $key );
+ if ( $bucket === null ) { // config error?
+ $status->fatal( "Lock servers for key
$key is not set." );
return $status;
}
- $keysToLock[$server][] = $key;
+ $keysToLock[$bucket][] = $key;
}
}
$lockedKeys = array(); // files locked in this attempt
// Attempt to acquire these locks...
- foreach ( $keysToLock as $server => $keys ) {
+ foreach ( $keysToLock as $bucket => $keys ) {
+ $server = $this->serverMap[$bucket][0]; // primary lock
server
+ $propagateToFallbacks = true; // give lock statements
to fallback servers
// Acquire the locks for this server. Three main cases
can happen:
// (a) Server is up; common case
// (b) Server is down but a fallback is up
// (c) Server is down and no fallbacks are up (or none
defined)
- $propagateToFallbacks = true;
try {
$this->lockingSelect( $server, $keys ); //
SELECT FOR UPDATE
} catch ( DBError $e ) {
// Can we manage to lock on any of the fallback
servers?
- if ( !$this->lockingSelectFallbacks( $server,
$keys ) ) {
- // Abort and unlock everything
+ if ( !$this->lockingSelectFallbacks( $bucket,
$keys ) ) {
+ // Abort and unlock everything we just
locked
$status->fatal( "Could not contact the
lock server." );
- $subStatus = $this->doUnlock(
$lockedKeys );
- $status->merge( $subStatus );
+ $status->merge( $this->doUnlock(
$lockedKeys ) );
return $status;
} else { // recovered using fallbacks
$propagateToFallbacks = false; // done
already
@@ -258,7 +264,7 @@
}
// Propagate any locks to the fallback servers (best
effort)
if ( $propagateToFallbacks ) {
- $this->lockingSelectFallbacks( $server, $keys );
+ $this->lockingSelectFallbacks( $bucket, $keys );
}
// Record locks as active
foreach ( $keys as $key ) {
@@ -290,7 +296,7 @@
}
/**
- * Get a database connection for $server and lock the rows for $keys
+ * Get a DB connection to a lock server and acquire locks on $keys.
*
* @param $server string
* @param $keys Array
@@ -311,24 +317,23 @@
}
/**
- * Propagate any locks to the fallback servers for $server.
+ * Propagate any locks to the fallback servers for a bucket.
* This should avoid throwing any exceptions.
*
- * @param $server string
+ * @param $bucket integer
* @param $keys Array
* @return bool Locks made on at least one fallback server
*/
- protected function lockingSelectFallbacks( $server, array $keys ) {
+ protected function lockingSelectFallbacks( $bucket, array $keys ) {
$locksMade = false;
- if ( isset( $this->serverFallbackMap[$server] ) ) {
- // Propagate the $server locks to each fallback for
$server...
- foreach ( $this->serverFallbackMap[$server] as
$fallbackServer ) {
- try {
- $this->doLockingSelect(
$fallbackServer, $keys ); // SELECT FOR UPDATE
- $locksMade = true;
- } catch ( DBError $e ) {
- // oh well; best effort
- }
+ $count = count( $this->serverMap[$bucket] );
+ for ( $i=1; $i < $count; $i++ ) { // start at 1 to only include
fallbacks
+ $server = $this->serverMap[$bucket][$i];
+ try {
+ $this->doLockingSelect( $server, $keys ); //
SELECT FOR UPDATE
+ $locksMade = true; // success for this fallback
+ } catch ( DBError $e ) {
+ // oh well; best effort
}
}
return $locksMade;
@@ -341,14 +346,14 @@
* @return void
*/
protected function commitLockTransactions() {
- try {
- foreach ( $this->activeConns as $server => $db ) {
+ foreach ( $this->activeConns as $server => $db ) {
+ try {
$db->commit(); // finish transaction
- unset( $this->activeConns[$server] );
+ } catch ( DBError $e ) {
+ // oh well
}
- } catch ( DBError $e ) {
- // oh well
}
+ $this->activeConns = array();
}
/**
@@ -356,29 +361,17 @@
* This should avoid throwing any exceptions.
*
* @param $key string
- * @return int
+ * @return integer
*/
protected function getBucketFromKey( $key ) {
$hash = str_pad( md5( $key ), 32, '0', STR_PAD_LEFT ); // 32
char hash
$prefix = substr( $hash, 0, 2 ); // first 2 hex chars (8 bits)
- return ( intval( base_convert( $prefix, 16, 10 ) ) % count(
$this->serverMap ) );
- }
-
- /**
- * Get the server name for lock key.
- * This should avoid throwing any exceptions.
- *
- * @param $key string
- * @return string|null
- */
- protected function getDBServerFromKey( $key ) {
- $bucket = $this->getBucketFromKey( $key );
- if ( isset( $this->serverMap[$bucket] ) ) {
- return $this->serverMap[$bucket];
- } else {
- wfWarn( "No key for bucket $bucket in serverMap." );
- return null; // disabled? bad config?
+ $bucket = intval( base_convert( $prefix, 16, 10 ) ) % count(
$this->serverMap );
+ // Sanity check that at least one server is handling this bucket
+ if ( !isset( $this->serverMap[$bucket] ) ) {
+ return null; // bad config?
}
+ return $bucket;
}
}
_______________________________________________
MediaWiki-CVS mailing list
MediaWiki-CVS@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs
