jenkins-bot has submitted this change and it was merged.
Change subject: Avoid "Unable to set value to APCBagOStuff" exceptions
......................................................................
Avoid "Unable to set value to APCBagOStuff" exceptions
* This can happen due to incr/add races. Use incrWithInit()
instead to handle such cases.
* Also made BagOStuff:incrWithInit() return the new value like incr().
Change-Id: I0e3b02a4cff7c20544a9db2eaabd3f61e5a470b1
---
M includes/libs/objectcache/BagOStuff.php
M includes/utils/UIDGenerator.php
M tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
3 files changed, 27 insertions(+), 9 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/libs/objectcache/BagOStuff.php
b/includes/libs/objectcache/BagOStuff.php
index 703c195..b9be43d 100644
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -507,18 +507,27 @@
/**
* Increase stored value of $key by $value while preserving its TTL
*
- * This will create the key with value $init and TTL $ttl if not present
+ * This will create the key with value $init and TTL $ttl instead if
not present
*
* @param string $key
* @param int $ttl
* @param int $value
* @param int $init
- * @return bool
+ * @return int|bool New value or false on failure
* @since 1.24
*/
public function incrWithInit( $key, $ttl, $value = 1, $init = 1 ) {
- return $this->incr( $key, $value ) ||
- $this->add( $key, (int)$init, $ttl ) || $this->incr(
$key, $value );
+ $newValue = $this->incr( $key, $value );
+ if ( $newValue === false ) {
+ // No key set; initialize
+ $newValue = $this->add( $key, (int)$init, $ttl ) ?
$init : false;
+ }
+ if ( $newValue === false ) {
+ // Raced out initializing; increment
+ $newValue = $this->incr( $key, $value );
+ }
+
+ return $newValue;
}
/**
diff --git a/includes/utils/UIDGenerator.php b/includes/utils/UIDGenerator.php
index e2de900..ed7ddb8 100644
--- a/includes/utils/UIDGenerator.php
+++ b/includes/utils/UIDGenerator.php
@@ -373,12 +373,9 @@
$cache = ObjectCache::getLocalServerInstance();
}
if ( $cache ) {
- $counter = $cache->incr( $bucket, $count );
+ $counter = $cache->incrWithInit( $bucket,
$cache::TTL_INDEFINITE, $count, $count );
if ( $counter === false ) {
- if ( !$cache->add( $bucket, (int)$count ) ) {
- throw new RuntimeException( 'Unable to
set value to ' . get_class( $cache ) );
- }
- $counter = $count;
+ throw new RuntimeException( 'Unable to set
value to ' . get_class( $cache ) );
}
}
diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
index 94b74cb..b9fd6ab 100644
--- a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
+++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
@@ -184,6 +184,18 @@
}
/**
+ * @covers BagOStuff::incrWithInit
+ */
+ public function testIncrWithInit() {
+ $key = wfMemcKey( 'test' );
+ $val = $this->cache->incrWithInit( $key, 0, 1, 3 );
+ $this->assertEquals( 3, $val, "Correct init value" );
+
+ $val = $this->cache->incrWithInit( $key, 0, 1, 3 );
+ $this->assertEquals( 4, $val, "Correct init value" );
+ }
+
+ /**
* @covers BagOStuff::getMulti
*/
public function testGetMulti() {
--
To view, visit https://gerrit.wikimedia.org/r/260435
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0e3b02a4cff7c20544a9db2eaabd3f61e5a470b1
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
