Aaron Schulz has uploaded a new change for review. https://gerrit.wikimedia.org/r/260435
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 2 files changed, 15 insertions(+), 9 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/35/260435/1 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 ) ); } } -- To view, visit https://gerrit.wikimedia.org/r/260435 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0e3b02a4cff7c20544a9db2eaabd3f61e5a470b1 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits