https://www.mediawiki.org/wiki/Special:Code/MediaWiki/108302
Revision: 108302
Author: raindrift
Date: 2012-01-07 01:53:27 +0000 (Sat, 07 Jan 2012)
Log Message:
-----------
Adding concurrency checking backend class
Note: This is not done! That's why it's in a branch!
Modified Paths:
--------------
branches/concurrency/includes/AutoLoader.php
branches/concurrency/includes/installer/MysqlUpdater.php
branches/concurrency/maintenance/tables.sql
Added Paths:
-----------
branches/concurrency/includes/ConcurrencyCheck.php
branches/concurrency/maintenance/archives/patch-concurrencycheck.sql
branches/concurrency/tests/phpunit/includes/ConcurrencyCheckTest.php
Modified: branches/concurrency/includes/AutoLoader.php
===================================================================
--- branches/concurrency/includes/AutoLoader.php 2012-01-07 01:45:06 UTC
(rev 108301)
+++ branches/concurrency/includes/AutoLoader.php 2012-01-07 01:53:27 UTC
(rev 108302)
@@ -43,6 +43,7 @@
'ChannelFeed' => 'includes/Feed.php',
'Collation' => 'includes/Collation.php',
'ConcatenatedGzipHistoryBlob' => 'includes/HistoryBlob.php',
+ 'ConcurrencyCheck' => 'includes/ConcurrencyCheck.php',
'ConfEditor' => 'includes/ConfEditor.php',
'ConfEditorParseError' => 'includes/ConfEditor.php',
'ConfEditorToken' => 'includes/ConfEditor.php',
@@ -52,7 +53,6 @@
'DeferredUpdates' => 'includes/DeferredUpdates.php',
'DerivativeRequest' => 'includes/WebRequest.php',
'DiffHistoryBlob' => 'includes/HistoryBlob.php',
-
'DoubleReplacer' => 'includes/StringUtils.php',
'DummyLinker' => 'includes/Linker.php',
'Dump7ZipOutput' => 'includes/Export.php',
Added: branches/concurrency/includes/ConcurrencyCheck.php
===================================================================
--- branches/concurrency/includes/ConcurrencyCheck.php
(rev 0)
+++ branches/concurrency/includes/ConcurrencyCheck.php 2012-01-07 01:53:27 UTC
(rev 108302)
@@ -0,0 +1,304 @@
+<?php
+
+/**
+ * Class for cooperative locking of web resources
+ *
+ * Each resource is identified by a combination of the "resource type" (the
application, the type
+ * of content, etc), and the resource's primary key or some other unique
numeric ID.
+ *
+ * Currently, a resource can only be checked out by a single user. Other
attempts to check it out result
+ * in the checkout failing. In the future, an option for multiple
simulataneous checkouts could be added
+ * without much trouble.
+ *
+ * This could be done with named locks, except then it would be impossible to
build a list of all the
+ * resources currently checked out for a given application. There's no good
way to construct a query
+ * that answers the question, "What locks do you have starting with [foo]"
This could be done really well
+ * with a concurrent, reliable, distributed key/value store, but we don't have
one of those right now.
+ *
+ * @author Ian Baker <[email protected]>
+ */
+class ConcurrencyCheck {
+
+
+ // TODO: docblock
+ public function __construct( $resourceType, $user, $expirationTime =
null ) {
+
+ // All database calls are to the master, since the whole point
of this class is maintaining
+ // concurrency. Most reads should come from cache anyway.
+ $this->dbw = wfGetDb( DB_MASTER );
+
+ $this->user = $user;
+ $this->resourceType = $resourceType;
+ $this->setExpirationTime( $expirationTime );
+ }
+
+ /**
+ * Check out a resource. This establishes an atomically generated,
cooperative lock
+ * on a key. The lock is tied to the current user.
+ *
+ * @var $record Integer containing the record id to check out
+ * @var $override Boolean (optional) describing whether to override an
existing checkout
+ * @return boolean
+ */
+ public function checkout( $record, $override = null ) {
+ $this->validateId( $record );
+ $dbw = $this->dbw;
+ $userId = $this->user->getId();
+
+ // attempt an insert, check success (this is atomic)
+ $insertError = null;
+ $res = $dbw->insert(
+ 'concurrencycheck',
+ array(
+ 'cc_resource_type' => $this->resourceType,
+ 'cc_record' => $record,
+ 'cc_user' => $userId,
+ 'cc_expiration' => time() +
$this->expirationTime,
+ ),
+ __METHOD__,
+ array('IGNORE')
+ );
+
+ // if the insert succeeded, checkout is done.
+ if( $dbw->affectedRows() === 1 ) {
+ //TODO: delete cache key
+ return true;
+ }
+
+ $dbw->begin();
+ $row = $dbw->selectRow(
+ 'concurrencycheck',
+ array( 'cc_user', 'cc_expiration' ),
+ array(
+ 'cc_resource_type' => $this->resourceType,
+ 'cc_record' => $record,
+ ),
+ __METHOD__,
+ array()
+ );
+
+ // not checked out by current user, checkout is unexpired,
override is unset
+ if( ! ( $override || $row->cc_user == $userId ||
$row->cc_expiration <= time() ) ) {
+ $dbw->rollback();
+ return false;
+ }
+
+ // execute a replace
+ $res = $dbw->replace(
+ 'concurrencycheck',
+ array( array('cc_resource_type', 'cc_record') ),
+ array(
+ 'cc_resource_type' => $this->resourceType,
+ 'cc_record' => $record,
+ 'cc_user' => $userId,
+ 'cc_expiration' => time() +
$this->expirationTime,
+ ),
+ __METHOD__
+ );
+
+ // TODO cache the result.
+
+ $dbw->commit();
+ return true;
+
+ // if insert succeeds, delete the cache key. don't make a new
one since they have to be created atomically.
+ //
+ // if insert fails:
+ // begin transaction
+ // select where key=key and expiration > now()
+ // if row is missing or user matches:
+ // execute a replace()
+ // overwrite the cache key (might as well, since this is
inside a transaction)
+ // commit
+ // if select returned an unexpired row owned by someone else,
return failure.
+
+ // optional: check to see if the current user already has the
resource checked out, and if so,
+ // return that checkout information instead. (does anyone want
that?)
+ }
+
+ /**
+ * Check in a resource. Only works if the resource is checked out by
the current user.
+ *
+ * @var $record Integer containing the record id to checkin
+ * @return Boolean
+ */
+ public function checkin( $record ) {
+ $this->validateId( $record );
+ $dbw = $this->dbw;
+ $userId = $this->user->getId();
+
+ $dbw->delete(
+ 'concurrencycheck',
+ array(
+ 'cc_resource_type' => $this->resourceType,
+ 'cc_record' => $record,
+ 'cc_user' => $userId, // only the owner can
perform a checkin
+ ),
+ __METHOD__,
+ array()
+ );
+
+ // check row count (this is atomic, select would not be)
+ if( $dbw->affectedRows() === 1 ) {
+ // TODO: remove record from cache
+ return true;
+ }
+
+ return false;
+
+ // delete the row, specifying the username in the where clause
(keeps users from checking in stuff that's not theirs).
+ // if a row was deleted:
+ // remove the record from memcache. (removing cache key
doesn't require atomicity)
+ // return true
+ // else
+ // return false
+
+ }
+
+ /**
+ * Remove all expired checkouts.
+ *
+ * @return Integer describing the number of records expired.
+ */
+ public function expire() {
+ $dbw = $this->dbw;
+
+ $now = time();
+
+ // get the rows to remove from cache.
+ $res = $dbw->select(
+ 'concurrencycheck',
+ array( '*' ),
+ array(
+ 'cc_expiration <= ' . $now,
+ ),
+ __METHOD__,
+ array()
+ );
+
+ // remove the rows from the db
+ $dbw->delete(
+ 'concurrencycheck',
+ array(
+ 'cc_expiration <= ' . $now,
+ ),
+ __METHOD__,
+ array()
+ );
+
+ // TODO: fetch the rows here, remove them from cache.
+
+ // return the number of rows removed.
+ return $dbw->affectedRows();
+
+ // grab a unixtime.
+ // select all rows where expiration < time
+ // delete all rows where expiration < time
+ // remove selected rows from memcache
+ //
+ // previous idea, probably wrong:
+ // select all expired rows.
+ // foreach( expired )
+ // delete row where id=id and expiration < now() (accounts
for updates)
+ // if delete succeeded, remove cache key (txn not required,
since removing cache key doesn't require atomicity)
+ // (sadly, this must be many deletes to coordinate removal
from memcache)
+ // (is it necessary to remove expired cache entries?)
+ }
+
+ public function status( $keys ) {
+ $dbw = $this->dbw;
+
+ // validate keys.
+ foreach( $keys as $key ) {
+ $this->validateId( $key );
+ }
+
+ $checkouts = array();
+
+ // TODO: check cache before selecting
+
+ // check for each of $keys in cache (also check expiration)
+ // build a list of the missing ones.
+ // run the below select with that list.
+ // when finished, re-add any missing keys with the 'invalid'
status.
+
+ $this->expire();
+
+ $dbw->begin();
+ $res = $dbw->select(
+ 'concurrencycheck',
+ array( '*' ),
+ array(
+ 'cc_resource_type' => $this->resourceType,
+ 'cc_record IN (' . implode( ',', $keys ) . ')',
+ 'cc_expiration > unix_timestamp(now())'
+ ),
+ __METHOD__,
+ array()
+ );
+
+ while( $res && $record = $res->fetchRow() ) {
+ $record['status'] = 'valid';
+ # cache the row.
+ $checkouts[ $record['cc_record'] ] = $record;
+ }
+
+ // if a key was passed in but has no (unexpired) checkout,
include it in the
+ // result set to make things easier and more consistent on the
client-side.
+ foreach( $keys as $key ) {
+ if( ! array_key_exists( $key, $checkouts ) ) {
+ $checkouts[$key]['status'] = 'invalid';
+ }
+ }
+
+ return $checkouts;
+
+ // fetch keys from cache or db (keys are an array)
+ //
+ // for all unexpired keys present in cache, store cached return
value for returning later.
+ //
+ // if some keys remain (missing from cache or expired):
+ // execute expire() to make sure db records are cleared
+ // for all remaining keys:
+ // begin transaction
+ // select rows where key in (keys) and expiration > now()
+ // overwrite any memcache entry
+ // commit
+ // return values that were added to cache, plus values pulled
from cache
+ }
+
+ public function list() {
+
+ }
+
+ public function setUser ( $user ) {
+ $this->user = $user;
+ }
+
+ public function setExpirationTime ( $expirationTime = null ) {
+ // check to make sure the time is digits only, so it can be
used in queries
+ // negative number are allowed, though mostly only used for
testing
+ // TODO: define maximum and minimum times in configuration, to
prevent DoS
+ if( $expirationTime && preg_match('/^[\d-]+$/',
$expirationTime) ) {
+ $this->expirationTime = $expirationTime; // the amount
of time before a checkout expires.
+ } else {
+ $this->expirationTime = 60 * 15; // 15 mins. TODO: make
a configurable default for this.
+ }
+ }
+
+ /**
+ * Check to make sure a record ID is numeric, throw an exception if not.
+ *
+ * @var $record Integer
+ * @throws ConcurrencyCheckBadRecordIdException
+ * @return boolean
+ */
+ private static function validateId ( $record ) {
+ if( ! preg_match('/^\d+$/', $record) ) {
+ throw new ConcurrencyCheckBadRecordIdException( 'Record
ID ' . $record . ' must be a positive integer' );
+ }
+ return true;
+ }
+}
+
+class ConcurrencyCheckBadRecordIdException extends MWException {};
Property changes on: branches/concurrency/includes/ConcurrencyCheck.php
___________________________________________________________________
Added: svn:eol-style
+ native
Modified: branches/concurrency/includes/installer/MysqlUpdater.php
===================================================================
--- branches/concurrency/includes/installer/MysqlUpdater.php 2012-01-07
01:45:06 UTC (rev 108301)
+++ branches/concurrency/includes/installer/MysqlUpdater.php 2012-01-07
01:53:27 UTC (rev 108302)
@@ -192,6 +192,7 @@
array( 'modifyField', 'user', 'ug_group',
'patch-ug_group-length-increase.sql' ),
array( 'addField', 'uploadstash', 'us_chunk_inx',
'patch-uploadstash_chunk.sql' ),
array( 'addfield', 'job', 'job_timestamp',
'patch-jobs-add-timestamp.sql' ),
+ array( 'addTable', 'concurrencycheck',
'patch-concurrencycheck.sql'),
);
}
Added: branches/concurrency/maintenance/archives/patch-concurrencycheck.sql
===================================================================
--- branches/concurrency/maintenance/archives/patch-concurrencycheck.sql
(rev 0)
+++ branches/concurrency/maintenance/archives/patch-concurrencycheck.sql
2012-01-07 01:53:27 UTC (rev 108302)
@@ -0,0 +1,25 @@
+--
+-- Store atomic locking information for web resources, to permit
+-- UI that warns users when concurrently editing things that aren't
+-- concurrently editable.
+--
+CREATE TABLE /*_*/concurrencycheck (
+ -- a string describing the resource or application being checked out.
+ cc_resource_type varchar(255) NOT NULL,
+
+ -- the (numeric) ID of the record that's being checked out.
+ cc_record int unsigned NOT NULL,
+
+ -- the user who has control of the resource
+ cc_user int unsigned NOT NULL,
+
+ -- the date/time on which this record expires
+ cc_expiration varbinary(14) not null
+
+) /*$wgDBTableOptions*/;
+-- composite pk.
+CREATE UNIQUE INDEX /*i*/cc_resource_record ON /*_*/concurrencycheck
(cc_resource_type, cc_record);
+-- sometimes there's a delete based on userid.
+CREATE INDEX /*i*/cc_user ON /*_*/concurrencycheck (cc_user);
+-- sometimes there's a delete based on expiration
+CREATE INDEX /*i*/cc_expiration ON /*_*/concurrencycheck (cc_expiration);
Property changes on:
branches/concurrency/maintenance/archives/patch-concurrencycheck.sql
___________________________________________________________________
Added: svn:eol-style
+ native
Modified: branches/concurrency/maintenance/tables.sql
===================================================================
--- branches/concurrency/maintenance/tables.sql 2012-01-07 01:45:06 UTC (rev
108301)
+++ branches/concurrency/maintenance/tables.sql 2012-01-07 01:53:27 UTC (rev
108302)
@@ -1483,4 +1483,31 @@
-- Should cover *most* configuration - strings, ints, bools, etc.
CREATE INDEX /*i*/cf_name_value ON /*_*/config (cf_name,cf_value(255));
+--
+-- Store atomic locking information for web resources, to permit
+-- UI that warns users when concurrently editing things that aren't
+-- concurrently editable.
+--
+CREATE TABLE /*_*/concurrencycheck (
+ -- a string describing the resource or application being checked out.
+ cc_resource_type varchar(255) NOT NULL,
+
+ -- the (numeric) ID of the record that's being checked out.
+ cc_record int unsigned NOT NULL,
+
+ -- the user who has control of the resource
+ cc_user int unsigned NOT NULL,
+
+ -- the date/time on which this record expires
+ cc_expiration varbinary(14) not null
+
+) /*$wgDBTableOptions*/;
+-- composite pk.
+CREATE UNIQUE INDEX /*i*/cc_resource_record ON /*_*/concurrencycheck
(cc_resource_type, cc_record);
+-- sometimes there's a delete based on userid.
+CREATE INDEX /*i*/cc_user ON /*_*/concurrencycheck (cc_user);
+-- sometimes there's a delete based on expiration
+CREATE INDEX /*i*/cc_expiration ON /*_*/concurrencycheck (cc_expiration);
+
+
-- vim: sw=2 sts=2 et
Added: branches/concurrency/tests/phpunit/includes/ConcurrencyCheckTest.php
===================================================================
--- branches/concurrency/tests/phpunit/includes/ConcurrencyCheckTest.php
(rev 0)
+++ branches/concurrency/tests/phpunit/includes/ConcurrencyCheckTest.php
2012-01-07 01:53:27 UTC (rev 108302)
@@ -0,0 +1,93 @@
+<?php
+
+class ConcurrencyCheckTest extends MediaWikiTestCase {
+ /**
+ * @var Array of test users
+ */
+ public static $users;
+
+ // Prepare test environment
+
+ public function setUp() {
+ parent::setUp();
+
+ self::$users = array(
+ 'user1' => new ApiTestUser(
+ 'Concurrencychecktestuser1',
+ 'ConcurrencyCheck Test User 1',
+ '[email protected]',
+ array()
+ ),
+ 'user2' => new ApiTestUser(
+ 'Concurrencychecktestuser2',
+ 'ConcurrencyCheck Test User 2',
+ '[email protected]',
+ array()
+ ),
+ );
+ }
+
+ public function tearDown() {
+ parent::tearDown();
+
+ // perhaps clean up all the cruft left in the db
+ }
+
+ // Actual tests from here down
+
+ public function testCheckoutCheckin() {
+ $first = new ConcurrencyCheck( 'CCUnitTest',
self::$users['user1']->user );
+ $second = new ConcurrencyCheck( 'CCUnitTest',
self::$users['user2']->user );
+ $testKey = 1337;
+
+ // clean up after any previously failed tests
+ $first->checkin($testKey);
+ $second->checkin($testKey);
+
+ // tests
+ $this->assertTrue( $first->checkout($testKey), "Initial
checkout" );
+ $this->assertFalse( $second->checkout($testKey), "Checkout of
locked resource fails as different user" );
+ $this->assertTrue( $first->checkout($testKey), "Checkout of
locked resource succeeds as original user" );
+ $this->assertFalse( $second->checkin($testKey), "Checkin of
locked resource fails as different user" );
+ $this->assertTrue( $first->checkin($testKey), "Checkin of
locked resource succeeds as original user" );
+ $second->setExpirationTime(-5);
+ $this->assertTrue( $second->checkout($testKey), "Checked-in
resource is now available to second user" );
+ $second->setExpirationTime();
+ $this->assertTrue( $first->checkout($testKey), "Checkout of
expired resource succeeds as first user");
+ $this->assertTrue( $second->checkout($testKey, true), "Checkout
override" );
+ $this->assertFalse( $first->checkout($testKey), "Checkout of
overriden resource fails as different user" );
+
+ // cleanup
+ $this->assertTrue( $second->checkin($testKey), "Checkin of
record with changed ownership" );
+ }
+
+ public function testExpire() {
+ $cc = new ConcurrencyCheck( 'CCUnitTest',
self::$users['user1']->user );
+ $cc->setExpirationTime(-1);
+ $cc->checkout( 1338 ); // these numbers are test record ids.
+ $cc->checkout( 1339 );
+ $cc->setExpirationTime();
+ $cc->checkout( 13310 );
+
+ // tests
+ $this->assertEquals( 2, $cc->expire(), "Resource expiration" );
+ $this->assertTrue( $cc->checkin( 13310 ), "Checkin succeeds
after expiration" );
+ }
+
+ public function testStatus() {
+ $cc = new ConcurrencyCheck( 'CCUnitTest',
self::$users['user1']->user );
+ $cc->checkout( 1337 );
+ $cc->checkout( 1338 );
+ $cc->setExpirationTime(-5);
+ $cc->checkout( 1339 );
+ $cc->setExpirationTime();
+
+ // tests
+ $output = $cc->status( array( 1337, 1338, 1339, 13310 ) );
+ $this->assertEquals( true, is_array( $output ), "Status returns
values" );
+ $this->assertEquals( 4, count( $output ), "Output has the
correct number of records" );
+ $this->assertEquals( 'valid', $output[1337]['status'], "Current
checkouts are listed as valid");
+ $this->assertEquals( 'invalid', $output[1339]['status'],
"Expired checkouts are invalid");
+ $this->assertEquals( 'invalid', $output[13310]['status'],
"Missing checkouts are invalid");
+ }
+}
\ No newline at end of file
Property changes on:
branches/concurrency/tests/phpunit/includes/ConcurrencyCheckTest.php
___________________________________________________________________
Added: svn:eol-style
+ native
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs
