Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default Zend_Translate cache config wipes Config manifest cache #2706

Closed
nyeholt opened this issue Dec 6, 2013 · 5 comments
Closed

Default Zend_Translate cache config wipes Config manifest cache #2706

nyeholt opened this issue Dec 6, 2013 · 5 comments

Comments

@nyeholt
Copy link
Contributor

nyeholt commented Dec 6, 2013

i18n::include_by_locale() calls $cache->clean(Zend_Cache::CLEANING_MODE_ALL); if "flush" is set in URL params. The default configuration of this cache is

Zend_Translate::setCache(
                SS_Cache::factory('i18n', 'Output', array('lifetime' => null, 'automatic_serialization' => true))
            );

which ends up pointing to the same silverstripe-cache/user/cache directory that the config manifest uses. The net effect is that the execution of dev/build?flush=1 will first clear out this folder and rebuild the config manifest as expected in ConfigManifest::regenerate(), but later when calling Folder::populateDefaults() the _t() function is triggered, which then triggers i18n::include_by_locale() which deletes those just built files

Typically, a subsequent request will hit code that checks the following in ConfigManifest

        if (false === $this->variantKeySpec) {
            $this->regenerate($includeTests);
        }

which rebuilds everything again, but in some cases this doesn't execute or the timing of requests is out (haven't nailed down the exact reason yet) and things don't regenerate, subsequently causing a failure by

PHP Warning:  Invalid argument supplied for foreach() in framework/core/manifest/ConfigManifest.php on line 140

A workaround is to manually configure a cache for Zend_Translate that stores data in a separate location.

@nyeholt
Copy link
Contributor Author

nyeholt commented Dec 6, 2013

Another workaround is changing

        if (false === $this->variantKeySpec) {
            $this->regenerate($includeTests);
        }

to

        if (false === $this->variantKeySpec || false === $this->phpConfigSources) {
            $this->regenerate($includeTests);
        }

@chillu
Copy link
Member

chillu commented Dec 6, 2013

Damn Zend_Cache::CLEANING_MODE_ALL... But there's not much we can do since pretty much no available caching backend supports tags. And changing the cache location isn't going to cut it either, since you can override the File backend with e.g. APC which doesn't respect a cache_dir or file_name_prefix option.

Also, since Zend_Cache::factory() returns you a new instance each time is called, it can lead to side effects when you instanciate caches before running through the config layer (where cache backends might be configured differently). So ConfigManifest retrieves the cache instance with a File backend early in bootstrap, but if it would re-request it later it might have an APC backend, and won't find any of the existing cached items. That's not a problem at the moment because the ConfigManifest cache is only read once, but it can still lead to nasty slowdowns if you're not aware of this limitation.

I see two options:

  • Change ConfigManifest to use ManifestCache instead of SS_Cache. It can't be configured to anything other than filesystem backend anyway (ManifestCache_APC exists, but you can't apply it). So behind the scenes not much will change.
  • Change the way we configure SS_Cache to be included separately from _config.php and the Config API, early in bootstrap. Declare all available caches there (rather than relying on execution path and chance), and clear all of them when "flush" is set. We could have a <module>/bootstrap.php file name convention, but at that point in Core.php we don't even know which modules are available yet. So a tricky option overall.

It has always bugged me that you can't actually clear all caches through "flush", and rely on both execution path and internals like the fact that Zend_Cache happens to clear all caches when called on a single cache (unless using tags). So if you change all caches apart from "aggregate" to APC, there's no way to clear the "aggregate" cache any more.

BTW, the "other workaround" from your last message is to prevent the PHP warning, correct? AFAICT the deletion of just-built caches still happens with that fix.

@nyeholt
Copy link
Contributor Author

nyeholt commented Dec 7, 2013

Yep - the first workaround solves the problem (because the translate cache is now separate), so it might be worth putting that into the i18n class when it actually creates the cache object. The second workaround simply prevents the PHP warning by triggering the rebuilding immediately.

@kinglozzer
Copy link
Member

Partially fixed (for backends that support tags) by #3558?

@chillu
Copy link
Member

chillu commented Feb 16, 2017

This uses Symfony\Component\Config\ResourceCheckerConfigCache now in 4.x, which has resolved this issue implicitly - see #6221 for context

@chillu chillu closed this as completed Feb 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants