jenkins-bot has submitted this change and it was merged.
Change subject: Added reentrant lock support to BagOStuff
......................................................................
Added reentrant lock support to BagOStuff
* Also fixed HashBagOStuff::delete() return value for non-keys
Change-Id: I9a977750c6fc6b8406bc1c9366a6b1b34bf48b6b
---
M includes/libs/objectcache/BagOStuff.php
M includes/libs/objectcache/HashBagOStuff.php
M includes/libs/objectcache/ReplicatedBagOStuff.php
M includes/objectcache/MultiWriteBagOStuff.php
M tests/phpunit/includes/objectcache/BagOStuffTest.php
5 files changed, 71 insertions(+), 36 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/libs/objectcache/BagOStuff.php
b/includes/libs/objectcache/BagOStuff.php
index 5004a8a..ddbe8ea 100644
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -43,15 +43,16 @@
* @ingroup Cache
*/
abstract class BagOStuff implements LoggerAwareInterface {
- private $debugMode = false;
-
+ /** @var array[] Lock tracking */
+ protected $locks = array();
/** @var integer */
protected $lastError = self::ERR_NONE;
- /**
- * @var LoggerInterface
- */
+ /** @var LoggerInterface */
protected $logger;
+
+ /** @var bool */
+ private $debugMode = false;
/** Possible values for getLastError() */
const ERR_NONE = 0; // no error
@@ -221,49 +222,77 @@
}
/**
+ * Acquire an advisory lock on a key string
+ *
+ * Note that if reentry is enabled, duplicate calls ignore $expiry
+ *
* @param string $key
* @param int $timeout Lock wait timeout; 0 for non-blocking [optional]
* @param int $expiry Lock expiry [optional]; 1 day maximum
+ * @param string $rclass Allow reentry if set and the current lock used
this value
* @return bool Success
*/
- public function lock( $key, $timeout = 6, $expiry = 6 ) {
+ public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) {
+ // Avoid deadlocks and allow lock reentry if specified
+ if ( isset( $this->locks[$key] ) ) {
+ if ( $rclass != '' && $this->locks[$key]['class'] ===
$rclass ) {
+ ++$this->locks[$key]['depth'];
+ return true;
+ } else {
+ return false;
+ }
+ }
+
$expiry = min( $expiry ?: INF, 86400 );
$this->clearLastError();
$timestamp = microtime( true ); // starting UNIX timestamp
if ( $this->add( "{$key}:lock", 1, $expiry ) ) {
- return true;
+ $locked = true;
} elseif ( $this->getLastError() || $timeout <= 0 ) {
- return false; // network partition or non-blocking
+ $locked = false; // network partition or non-blocking
+ } else {
+ $uRTT = ceil( 1e6 * ( microtime( true ) - $timestamp )
); // estimate RTT (us)
+ $sleep = 2 * $uRTT; // rough time to do get()+set()
+
+ $attempts = 0; // failed attempts
+ do {
+ if ( ++$attempts >= 3 && $sleep <= 5e5 ) {
+ // Exponentially back off after failed
attempts to avoid network spam.
+ // About 2*$uRTT*(2^n-1) us of "sleep"
happen for the next n attempts.
+ $sleep *= 2;
+ }
+ usleep( $sleep ); // back off
+ $this->clearLastError();
+ $locked = $this->add( "{$key}:lock", 1, $expiry
);
+ if ( $this->getLastError() ) {
+ $locked = false; // network partition
+ break;
+ }
+ } while ( !$locked && ( microtime( true ) - $timestamp
) < $timeout );
}
- $uRTT = ceil( 1e6 * ( microtime( true ) - $timestamp ) ); //
estimate RTT (us)
- $sleep = 2 * $uRTT; // rough time to do get()+set()
-
- $attempts = 0; // failed attempts
- do {
- if ( ++$attempts >= 3 && $sleep <= 5e5 ) {
- // Exponentially back off after failed attempts
to avoid network spam.
- // About 2*$uRTT*(2^n-1) us of "sleep" happen
for the next n attempts.
- $sleep *= 2;
- }
- usleep( $sleep ); // back off
- $this->clearLastError();
- $locked = $this->add( "{$key}:lock", 1, $expiry );
- if ( $this->getLastError() ) {
- return false; // network partition
- }
- } while ( !$locked && ( microtime( true ) - $timestamp ) <
$timeout );
+ if ( $locked ) {
+ $this->locks[$key] = array( 'class' => $rclass, 'depth'
=> 1 );
+ }
return $locked;
}
/**
+ * Release an advisory lock on a key string
+ *
* @param string $key
* @return bool Success
*/
public function unlock( $key ) {
- return $this->delete( "{$key}:lock" );
+ if ( isset( $this->locks[$key] ) &&
--$this->locks[$key]['depth'] <= 0 ) {
+ unset( $this->locks[$key] );
+
+ return $this->delete( "{$key}:lock" );
+ }
+
+ return true;
}
/**
@@ -278,13 +307,14 @@
* @param string $key
* @param int $timeout Lock wait timeout; 0 for non-blocking [optional]
* @param int $expiry Lock expiry [optional]; 1 day maximum
+ * @param string $rclass Allow reentry if set and the current lock used
this value
* @return ScopedCallback|null Returns null on failure
* @since 1.26
*/
- final public function getScopedLock( $key, $timeout = 6, $expiry = 30 )
{
+ final public function getScopedLock( $key, $timeout = 6, $expiry = 30,
$rclass = '' ) {
$expiry = min( $expiry ?: INF, 86400 );
- if ( !$this->lock( $key, $timeout, $expiry ) ) {
+ if ( !$this->lock( $key, $timeout, $expiry, $rclass ) ) {
return null;
}
diff --git a/includes/libs/objectcache/HashBagOStuff.php
b/includes/libs/objectcache/HashBagOStuff.php
index e03c83f..b685e41 100644
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -68,10 +68,6 @@
}
function delete( $key ) {
- if ( !isset( $this->bag[$key] ) ) {
- return false;
- }
-
unset( $this->bag[$key] );
return true;
diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php
b/includes/libs/objectcache/ReplicatedBagOStuff.php
index 20e146d..9e80e9f 100644
--- a/includes/libs/objectcache/ReplicatedBagOStuff.php
+++ b/includes/libs/objectcache/ReplicatedBagOStuff.php
@@ -104,8 +104,8 @@
return $this->writeStore->decr( $key, $value );
}
- public function lock( $key, $timeout = 6, $expiry = 6 ) {
- return $this->writeStore->lock( $key, $timeout, $expiry );
+ public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) {
+ return $this->writeStore->lock( $key, $timeout, $expiry,
$rclass );
}
public function unlock( $key ) {
diff --git a/includes/objectcache/MultiWriteBagOStuff.php
b/includes/objectcache/MultiWriteBagOStuff.php
index b5f3bd9..bbfaa5e 100644
--- a/includes/objectcache/MultiWriteBagOStuff.php
+++ b/includes/objectcache/MultiWriteBagOStuff.php
@@ -121,12 +121,13 @@
* @param string $key
* @param int $timeout
* @param int $expiry
+ * @param string $rclass
* @return bool
*/
- public function lock( $key, $timeout = 6, $expiry = 6 ) {
+ public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) {
// Lock only the first cache, to avoid deadlocks
if ( isset( $this->caches[0] ) ) {
- return $this->caches[0]->lock( $key, $timeout, $expiry
);
+ return $this->caches[0]->lock( $key, $timeout, $expiry,
$rclass );
} else {
return true;
}
diff --git a/tests/phpunit/includes/objectcache/BagOStuffTest.php
b/tests/phpunit/includes/objectcache/BagOStuffTest.php
index fcc15d3..f5814e4 100644
--- a/tests/phpunit/includes/objectcache/BagOStuffTest.php
+++ b/tests/phpunit/includes/objectcache/BagOStuffTest.php
@@ -1,6 +1,7 @@
<?php
/**
* @author Matthias Mullie <[email protected]>
+ * @group BagOStuff
*/
class BagOStuffTest extends MediaWikiTestCase {
/** @var BagOStuff */
@@ -169,5 +170,12 @@
$value3 = $this->cache->getScopedLock( $key, 0 );
$this->assertType( 'ScopedCallback', $value3, 'Lock returned
callback after release' );
+ unset( $value3 );
+
+ $value1 = $this->cache->getScopedLock( $key, 0, 5, 'reentry' );
+ $value2 = $this->cache->getScopedLock( $key, 0, 5, 'reentry' );
+
+ $this->assertType( 'ScopedCallback', $value1, 'First reentrant
call returned lock' );
+ $this->assertType( 'ScopedCallback', $value1, 'Second reentrant
call returned lock' );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/233062
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9a977750c6fc6b8406bc1c9366a6b1b34bf48b6b
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
