Avoid stack overflow in LoadBalancer with CACHE_DB WAN/server cache
authorAaron Schulz <aschulz@wikimedia.org>
Tue, 2 Aug 2016 10:57:52 +0000 (03:57 -0700)
committerAddshore <addshorewiki@gmail.com>
Tue, 2 Aug 2016 13:06:49 +0000 (13:06 +0000)
* Add the ability to expose key BagOStuff attributes like whether the
  emulation SQL cache is being used. Several callers use hacks to detect
  this and can be updated later.
* Fallback to a safe empty WAN cache in the CACHE_DB case. This fixes
  a regression from f4bf52e8438.
* Also add this protection to the server cache, which could break
  similarly before too.
* Also fix CACHE_ANYTHING by avoid the recursion risk from
  fc1d4d796024 by just checking the disabled service map.

Bug: T123829
Bug: T141804
Change-Id: I17ee26138f69e01ec1aaddb55ab27caa4d542193

includes/Services/ServiceContainer.php
includes/db/loadbalancer/LoadBalancer.php
includes/libs/objectcache/BagOStuff.php
includes/libs/objectcache/CachedBagOStuff.php
includes/libs/objectcache/IExpiringStore.php
includes/libs/objectcache/MultiWriteBagOStuff.php
includes/libs/objectcache/ReplicatedBagOStuff.php
includes/libs/objectcache/WANObjectCache.php
includes/objectcache/ObjectCache.php
includes/objectcache/SqlBagOStuff.php

index b336795..bad0ef9 100644 (file)
@@ -367,4 +367,12 @@ class ServiceContainer implements DestructibleService {
                return $service;
        }
 
+       /**
+        * @param string $name
+        * @return bool Whether the service is disabled
+        * @since 1.28
+        */
+       public function isServiceDisabled( $name ) {
+               return isset( $this->disabled[$name] );
+       }
 }
index dbf031e..b44b559 100644 (file)
@@ -138,8 +138,20 @@ class LoadBalancer {
                        }
                }
 
-               $this->srvCache = ObjectCache::getLocalServerInstance();
-               $this->wanCache = ObjectCache::getMainWANInstance();
+               // Use APC/memcached style caching, but avoids loops with CACHE_DB (T141804)
+               // @TODO: inject these in via LBFactory at some point
+               $cache = ObjectCache::getLocalServerInstance();
+               if ( $cache->getQoS( $cache::ATTR_EMULATION ) > $cache::QOS_EMULATION_SQL ) {
+                       $this->srvCache = $cache;
+               } else {
+                       $this->srvCache = new EmptyBagOStuff();
+               }
+               $wCache = ObjectCache::getMainWANInstance();
+               if ( $wCache->getQoS( $wCache::ATTR_EMULATION ) > $wCache::QOS_EMULATION_SQL ) {
+                       $this->wanCache = $wCache;
+               } else {
+                       $this->wanCache = WANObjectCache::newEmpty();
+               }
 
                if ( isset( $params['trxProfiler'] ) ) {
                        $this->trxProfiler = $params['trxProfiler'];
index 1a2711a..25a5a26 100644 (file)
@@ -46,7 +46,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
        /** @var array[] Lock tracking */
        protected $locks = [];
 
-       /** @var integer */
+       /** @var integer ERR_* class constant */
        protected $lastError = self::ERR_NONE;
 
        /** @var string */
@@ -70,6 +70,9 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
        /** @var bool */
        private $dupeTrackScheduled = false;
 
+       /** @var integer[] Map of (ATTR_* class constant => QOS_* class constant) */
+       protected $attrMap = [];
+
        /** Possible values for getLastError() */
        const ERR_NONE = 0; // no error
        const ERR_NO_RESPONSE = 1; // no response
@@ -734,4 +737,34 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
        public function makeKey() {
                return $this->makeKeyInternal( $this->keyspace, func_get_args() );
        }
+
+       /**
+        * @param integer $flag ATTR_* class constant
+        * @return integer QOS_* class constant
+        * @since 1.28
+        */
+       public function getQoS( $flag ) {
+               return isset( $this->attrMap[$flag] ) ? $this->attrMap[$flag] : self::QOS_UNKNOWN;
+       }
+
+       /**
+        * Merge the flag maps of one or more BagOStuff objects into a "lowest common denominator" map
+        *
+        * @param BagOStuff[] $bags
+        * @return integer[] Resulting flag map (class ATTR_* constant => class QOS_* constant)
+        */
+       protected function mergeFlagMaps( array $bags ) {
+               $map = [];
+               foreach ( $bags as $bag ) {
+                       foreach ( $bag->attrMap as $attr => $rank ) {
+                               if ( isset( $map[$attr] ) ) {
+                                       $map[$attr] = min( $map[$attr], $rank );
+                               } else {
+                                       $map[$attr] = $rank;
+                               }
+                       }
+               }
+
+               return $map;
+       }
 }
index 60ec922..e70a51f 100644 (file)
@@ -42,8 +42,10 @@ class CachedBagOStuff extends HashBagOStuff {
         * @param array $params Parameters for HashBagOStuff
         */
        function __construct( BagOStuff $backend, $params = [] ) {
-               $this->backend = $backend;
                parent::__construct( $params );
+
+               $this->backend = $backend;
+               $this->attrMap = $backend->attrMap;
        }
 
        protected function doGet( $key, $flags = 0 ) {
index 91e7934..62c4fa5 100644 (file)
@@ -42,4 +42,11 @@ interface IExpiringStore {
        const TTL_PROC_LONG = 30; // loose cache time that can survive slow web requests
 
        const TTL_INDEFINITE = 0;
+
+       // Attribute and QoS constants; higher QOS values with the same prefix rank higher...
+       // Medium attributes constants related to emulation or media type
+       const ATTR_EMULATION = 1;
+       const QOS_EMULATION_SQL = 1;
+       // Generic "unknown" value that is useful for comparisons (e.g. always good enough)
+       const QOS_UNKNOWN = INF;
 }
index fe61470..9dcfa7c 100644 (file)
@@ -83,6 +83,7 @@ class MultiWriteBagOStuff extends BagOStuff {
                                $this->caches[] = ObjectFactory::getObjectFromSpec( $cacheInfo );
                        }
                }
+               $this->mergeFlagMaps( $this->caches );
 
                $this->asyncWrites = (
                        isset( $params['replication'] ) &&
index 5f2c509..f2ba9de 100644 (file)
@@ -65,6 +65,7 @@ class ReplicatedBagOStuff extends BagOStuff {
                $this->readStore = ( $params['readFactory'] instanceof BagOStuff )
                        ? $params['readFactory']
                        : ObjectFactory::getObjectFromSpec( $params['readFactory'] );
+               $this->attrMap = $this->mergeFlagMaps( [ $this->readStore, $this->writeStore ] );
        }
 
        public function setDebug( $debug ) {
index b5d014f..4fd40e2 100644 (file)
@@ -977,7 +977,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
        /**
         * Get the "last error" registered; clearLastError() should be called manually
-        * @return int ERR_* constant for the "last error" registry
+        * @return int ERR_* class constant for the "last error" registry
         */
        final public function getLastError() {
                if ( $this->lastRelayError ) {
@@ -1019,6 +1019,15 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                $this->procCache->clear();
        }
 
+       /**
+        * @param integer $flag ATTR_* class constant
+        * @return integer QOS_* class constant
+        * @since 1.28
+        */
+       public function getQoS( $flag ) {
+               return $this->cache->getQoS( $flag );
+       }
+
        /**
         * Do the actual async bus purge of a key
         *
index e1bb2db..26f3356 100644 (file)
@@ -228,14 +228,12 @@ class ObjectCache {
                        }
                }
 
-               try {
-                       // Make sure we actually have a DB backend before falling back to CACHE_DB
-                       MediaWikiServices::getInstance()->getDBLoadBalancer();
-                       $candidate = CACHE_DB;
-               } catch ( ServiceDisabledException $e ) {
+               if ( MediaWikiServices::getInstance()->isServiceDisabled( 'DBLoadBalancer' ) ) {
                        // The LoadBalancer is disabled, probably because
                        // MediaWikiServices::disableStorageBackend was called.
                        $candidate = CACHE_NONE;
+               } else {
+                       $candidate = CACHE_DB;
                }
 
                return self::getInstance( $candidate );
index 98b6eb9..c48880f 100644 (file)
@@ -91,6 +91,9 @@ class SqlBagOStuff extends BagOStuff {
         */
        public function __construct( $params ) {
                parent::__construct( $params );
+
+               $this->attrMap[self::ATTR_EMULATION] = self::QOS_EMULATION_SQL;
+
                if ( isset( $params['servers'] ) ) {
                        $this->serverInfos = [];
                        $this->serverTags = [];