From c6ec9309fa2ca83ee64842b4c6263d2810ffecab Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Wed, 8 May 2013 15:38:14 +1200 Subject: [PATCH 1/4] MDL-39526 cache: now re-parses definitions during upgrade. The solution to this issue was to make the disabled cache factory return a proper cache writer object when requested rather than a disabled cache object. This only changes things when CACHE_DISABLE_ALL has been defined. --- cache/disabledlib.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/cache/disabledlib.php b/cache/disabledlib.php index 18cc11eb07d..1e95bbc466c 100644 --- a/cache/disabledlib.php +++ b/cache/disabledlib.php @@ -256,10 +256,22 @@ class cache_factory_disabled extends cache_factory { * @return cache_config|cache_config_writer */ public function create_config_instance($writer = false) { + // We are always going to use the cache_config_disabled class for all regular request. + // However if the code has requested the writer then likely something is changing and + // we're going to need to interact with the config.php file. + // In this case we will still use the cache_config_writer. $class = 'cache_config_disabled'; + if ($writer) { + // If the writer was requested then something is changing. + $class = 'cache_config_writer'; + } if (!array_key_exists($class, $this->configs)) { self::set_state(self::STATE_INITIALISING); - $configuration = $class::create_default_configuration(); + if (!$writer) { + $configuration = $class::create_default_configuration(); + } else { + $configuration = false; + } $this->configs[$class] = new $class; $this->configs[$class]->load($configuration); } From b0dd08dd63fc04d36f69ac4782e6c4d4265b666b Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Wed, 8 May 2013 17:42:55 +1200 Subject: [PATCH 2/4] MDL-39430 cache: gets purged by force during upgrade and install --- admin/cli/upgrade.php | 6 ++++++ admin/index.php | 7 +++++++ cache/classes/config.php | 12 ++++++++++-- cache/classes/helper.php | 28 ++++++++++++++++++---------- cache/disabledlib.php | 10 +++++++--- cache/locallib.php | 5 +++-- lib/upgradelib.php | 10 +--------- 7 files changed, 52 insertions(+), 26 deletions(-) diff --git a/admin/cli/upgrade.php b/admin/cli/upgrade.php index a6b36ba7251..ca6ef6ae384 100644 --- a/admin/cli/upgrade.php +++ b/admin/cli/upgrade.php @@ -151,6 +151,12 @@ if ($interactive) { } if ($version > $CFG->version) { + // We purge all of MUC's caches here. + // Caches are disabled for upgrade by CACHE_DISABLE_ALL so we must set the first arg to true. + // This ensures a real config object is loaded and the stores will be purged. + // This is the only way we can purge custom caches such as memcache or APC. + // Note: all other calls to caches will still used the disabled API. + cache_helper::purge_all(true); upgrade_core($version, true); } set_config('release', $release); diff --git a/admin/index.php b/admin/index.php index 329c79b6463..c54e8388791 100644 --- a/admin/index.php +++ b/admin/index.php @@ -222,6 +222,13 @@ if ($cache) { } if ($version > $CFG->version) { // upgrade + // We purge all of MUC's caches here. + // Caches are disabled for upgrade by CACHE_DISABLE_ALL so we must set the first arg to true. + // This ensures a real config object is loaded and the stores will be purged. + // This is the only way we can purge custom caches such as memcache or APC. + // Note: all other calls to caches will still used the disabled API. + cache_helper::purge_all(true); + // We then purge the regular caches. purge_all_caches(); $PAGE->set_pagelayout('maintenance'); diff --git a/cache/classes/config.php b/cache/classes/config.php index 83b2b920b8e..322035fa093 100644 --- a/cache/classes/config.php +++ b/cache/classes/config.php @@ -396,10 +396,18 @@ class cache_config { * @param string $storename * @return array Associative array of definitions, id=>definition */ - public static function get_definitions_by_store($storename) { + public function get_definitions_by_store($storename) { $definitions = array(); - $config = cache_config::instance(); + // This function was accidentally made static at some stage in the past. + // It was converted to an instance method but to be backwards compatible + // we must step around this in code. + if (!isset($this)) { + $config = cache_config::instance(); + } else { + $config = $this; + } + $stores = $config->get_all_stores(); if (!array_key_exists($storename, $stores)) { // The store does not exist. diff --git a/cache/classes/helper.php b/cache/classes/helper.php index 049aca5dda8..873dbbe6279 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -414,12 +414,17 @@ class cache_helper { * Think twice before calling this method. It will purge **ALL** caches regardless of whether they have been used recently or * anything. This will involve full setup of the cache + the purge operation. On a site using caching heavily this WILL be * painful. + * + * @param bool $usewriter If set to true the cache_config_writer class is used. This class is special as it avoids + * it is still usable when caches have been disabled. + * Please use this option only if you really must. It's purpose is to allow the cache to be purged when it would be + * otherwise impossible. */ - public static function purge_all() { - $config = cache_config::instance(); - + public static function purge_all($usewriter = false) { + $factory = cache_factory::instance(); + $config = $factory->create_config_instance($usewriter); foreach ($config->get_all_stores() as $store) { - self::purge_store($store['name']); + self::purge_store($store['name'], $config); } } @@ -427,10 +432,13 @@ class cache_helper { * Purges a store given its name. * * @param string $storename + * @param cache_config $config * @return bool */ - public static function purge_store($storename) { - $config = cache_config::instance(); + public static function purge_store($storename, cache_config $config = null) { + if ($config === null) { + $config = cache_config::instance(); + } $stores = $config->get_all_stores(); if (!array_key_exists($storename, $stores)) { @@ -450,10 +458,10 @@ class cache_helper { foreach ($config->get_definitions_by_store($storename) as $id => $definition) { $definition = cache_definition::load($id, $definition); - $instance = new $class($store['name'], $store['configuration']); - $instance->initialise($definition); - $instance->purge(); - unset($instance); + $definitioninstance = clone($instance); + $definitioninstance->initialise($definition); + $definitioninstance->purge(); + unset($definitioninstance); } return true; diff --git a/cache/disabledlib.php b/cache/disabledlib.php index 1e95bbc466c..e65b766be6e 100644 --- a/cache/disabledlib.php +++ b/cache/disabledlib.php @@ -253,7 +253,7 @@ class cache_factory_disabled extends cache_factory { * Creates a cache config instance with the ability to write if required. * * @param bool $writer Unused. - * @return cache_config|cache_config_writer + * @return cache_config_disabled|cache_config_writer */ public function create_config_instance($writer = false) { // We are always going to use the cache_config_disabled class for all regular request. @@ -267,10 +267,13 @@ class cache_factory_disabled extends cache_factory { } if (!array_key_exists($class, $this->configs)) { self::set_state(self::STATE_INITIALISING); - if (!$writer) { + if ($class === 'cache_config_disabled') { $configuration = $class::create_default_configuration(); } else { $configuration = false; + if (!cache_config::config_file_exists()) { + cache_config_writer::create_default_configuration(true); + } } $this->configs[$class] = new $class; $this->configs[$class]->load($configuration); @@ -373,9 +376,10 @@ class cache_config_disabled extends cache_config_writer { /** * Creates the default configuration and saves it. * + * @param bool $forcesave Ignored because we are disabled! * @return array */ - public static function create_default_configuration() { + public static function create_default_configuration($forcesave = false) { global $CFG; // HACK ALERT. diff --git a/cache/locallib.php b/cache/locallib.php index bc4ce6b99f1..22fc71316f9 100644 --- a/cache/locallib.php +++ b/cache/locallib.php @@ -388,10 +388,11 @@ class cache_config_writer extends cache_config { * This function calls config_save, however it is safe to continue using it afterwards as this function should only ever * be called when there is no configuration file already. * + * @param bool $forcesave If set to true then we will forcefully save the default configuration file. * @return true|array Returns true if the default configuration was successfully created. * Returns a configuration array if it could not be saved. This is a bad situation. Check your error logs. */ - public static function create_default_configuration() { + public static function create_default_configuration($forcesave = false) { global $CFG; // HACK ALERT. @@ -433,7 +434,7 @@ class cache_config_writer extends cache_config { $factory = cache_factory::instance(); // We expect the cache to be initialising presently. If its not then something has gone wrong and likely // we are now in a loop. - if ($factory->get_state() !== cache_factory::STATE_INITIALISING) { + if (!$forcesave && $factory->get_state() !== cache_factory::STATE_INITIALISING) { return $writer->generate_configuration_array(); } $factory->set_state(cache_factory::STATE_SAVING); diff --git a/lib/upgradelib.php b/lib/upgradelib.php index ec59a74f553..2a94bc7d8de 100644 --- a/lib/upgradelib.php +++ b/lib/upgradelib.php @@ -1519,9 +1519,7 @@ function upgrade_core($version, $verbose) { try { // Reset caches before any output purge_all_caches(); - // Disable the use of cache stores here. We will reset the factory after we've performed the installation. - // This ensures that we don't permanently cache anything during installation. - cache_factory::disable_stores(); + cache_helper::purge_all(true); // Upgrade current language pack if we can upgrade_language_pack(); @@ -1552,8 +1550,6 @@ function upgrade_core($version, $verbose) { // Update core definitions. cache_helper::update_definitions(true); - // Reset the cache, this returns it to a normal operation state. - cache_factory::reset(); // Purge caches again, just to be sure we arn't holding onto old stuff now. purge_all_caches(); @@ -1582,10 +1578,6 @@ function upgrade_noncore($verbose) { // upgrade all plugins types try { - // Disable the use of cache stores here. - // We don't reset this, the site can live without proper caching for life of this request. - cache_factory::disable_stores(); - $plugintypes = get_plugin_types(); foreach ($plugintypes as $type=>$location) { upgrade_plugins($type, 'print_upgrade_part_start', 'print_upgrade_part_end', $verbose); From fa167c4ac3db1cba4e98bad38e4a84b2c6c482b7 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Thu, 9 May 2013 12:48:48 +1200 Subject: [PATCH 3/4] MDL-39526 cache: purge all stores after each plugin upgrade --- lib/upgradelib.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/upgradelib.php b/lib/upgradelib.php index 2a94bc7d8de..e257d2d8a6e 100644 --- a/lib/upgradelib.php +++ b/lib/upgradelib.php @@ -486,7 +486,7 @@ function upgrade_plugins($type, $startcallback, $endcallback, $verbose) { message_update_processors($plug); } upgrade_plugin_mnet_functions($component); - + cache_helper::purge_all(true); purge_all_caches(); $endcallback($component, true, $verbose); @@ -519,7 +519,7 @@ function upgrade_plugins($type, $startcallback, $endcallback, $verbose) { message_update_processors($plug); } upgrade_plugin_mnet_functions($component); - + cache_helper::purge_all(true); purge_all_caches(); $endcallback($component, false, $verbose); From 3993a37c20600b71a31b05f8b2493ddbced84a6f Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Thu, 9 May 2013 15:32:36 +1200 Subject: [PATCH 4/4] MDL-39526 cache: cleaned up disable_stores calls --- lib/moodlelib.php | 3 --- lib/upgradelib.php | 4 ---- 2 files changed, 7 deletions(-) diff --git a/lib/moodlelib.php b/lib/moodlelib.php index c37e32c5856..a2c508fec0f 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -1391,9 +1391,6 @@ function get_config($plugin, $name = NULL) { // install the database. $siteidentifier = $DB->get_field('config', 'value', array('name' => 'siteidentifier')); } catch (dml_exception $ex) { - // It's failed. We'll use this opportunity to disable cache stores so that we don't inadvertingly start using - // old caches. People should delete their moodledata dirs when reinstalling the database... but they don't. - cache_factory::disable_stores(); // Set siteidentifier to false. We don't want to trip this continually. $siteidentifier = false; throw $ex; diff --git a/lib/upgradelib.php b/lib/upgradelib.php index e257d2d8a6e..810ef563aab 100644 --- a/lib/upgradelib.php +++ b/lib/upgradelib.php @@ -1467,10 +1467,6 @@ function install_core($version, $verbose) { remove_dir($CFG->dataroot.'/muc', true); try { - // Disable the use of cache stores here. We will reset the factory after we've performed the installation. - // This ensures that we don't permanently cache anything during installation. - cache_factory::disable_stores(); - set_time_limit(600); print_upgrade_part_start('moodle', true, $verbose); // does not store upgrade running flag