ctidigital / configuration-cache-lock Goto Github PK

Hey I saw this mentioned at Mage Titans and thought I'd give it a look.

I can see that while you've put locking around the Mage_Core_Model_App::_initModules there's still a chance for unlocked config cache regenration.

There is a chance that any request that retrieves configuration using Mage_Core_Model_Config:getNode() (or setNode, or init or a number of the functions in this class) could trigger an unlocked regeneration by doing the following steps:

1. Mage_Core_Model_Config:getNode()
2. Mage_Core_Model_Config:getSectionNode($path);
3. Mage_Core_Model_Config:_getSectionConfig($path);
4. Mage_Core_Model_Config:_loadSectionCache($path);

_loadSectionCache() is the killer here. Look at how it's defined:

protected function _loadSectionCache($sectionName)
{
    $cacheId = $this->getCacheId() . '_' . $sectionName;
    $xmlString = $this->_loadCache($cacheId);
    /**
     * If we can't load section cache (problems with cache storage)
     */
    if (!$xmlString) {
        $this->_useCache = false;
        $this->reinit($this->_options);
        return false;
    } else {
        $xml = simplexml_load_string($xmlString, $this->_elementClass);
        return $xml;
    }
}

If we fail to load this single node from cache for any reason, we invalidate the entire config cache and reinitialise it. This reinit functionally is fully internal to Mage_Core_Model_Cache and will not be subject to your cache locking mechanism. reinit will eventually call init which should look scarily familiar to you as something which needs proper locking applied to it.

public function init($options=array())
{
    $this->setCacheChecksum(null);
    $this->_cacheLoadedSections = array();
    $this->setOptions($options);
    $this->loadBase();
    $cacheLoad = $this->loadModulesCache();
    if ($cacheLoad) {
        return $this;
    }
    $this->loadModules();
    $this->loadDb();
    $this->saveCache();
    return $this;
}

Magento had actually made an attempt at cache locking but it's obviously not working very well. It is around the saveCache but perhaps it also needs a little extra locking in the init() function. I wonder if it would be possible to amalgamate the two rather than having two - potentially competing - cache locks.

Also just a little thing you might find interesting, during the talk you mentioned this cache locking had a little bonus feature as only the first request would trigger any available setup scripts. With a little config you can completley disable the automatic application of setup scripts which puts the control in your hands, allowing you to trigger setup scripts as a final part of your deployment using magerun.

All in all, neat stuff. Thanks.