From 118eb8309d743007397bc3169e5170812eb8dea9 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 29 Mar 2018 17:36:56 -0700 Subject: [PATCH 1/4] (dev/core#174) CRM_Utils_Cache - Always use a prefix. Standardize delimiter. "Prefixes" are a way to have one cache-server (e.g. one instance of redis or memcached) which stores several different data-sets. `CRM_Utils_Cache` uses prefixes in a couple ways: * (1) General site prefix (controlled via `civicrm.settings.php`) * (1a) If you have a single-site deployment, then the general prefix is blank. * (1b) If you have a multi-site deployment, then each site should use a different prefix (`mysite_1`, `mysite_2`, etc). * (2) Within a given deployment, prefixes may indicate different logical data-sets. * (2a) `Civi::cache()` or `Civi::cache('default')` or `CRM_Utils_Cache::singleton()` are the `default` data-set. * (2b) `CRM_Utils_Cache::create()` can instantiate new, special-purpose data-sets. For example, this is used for `Civi::cache('js_strings')`. This patch addresses two issues: * (Functional) Flushing the 'default' cache would likely flush all other caches because the 'default' cache didn't have a distinctive prefix. (This was observed Redis. Theoretically, the bug would apply to some-but-not-all cache backends.) * (Aesthetic) The full cache paths don't look consistent because they don't have a standard dlimiter. To fully understand, it helps to see example cache keys produced in a few configurations before and after the patch. See also: https://lab.civicrm.org/dev/core/issues/174 Before ----------------------------- | |Deployment Type|Logical Cache |Combined Cache Prefix |Example Cache Item (`foobar`)| |-|-|---------------|-------|-----------------| |1a,2a|Single-site|`default` |(empty-string)|`foobar`| |1a,2b|Single-site| `js_strings` |`_js_strings`|`_js_stringsfoobar`| |1b,2a|Multi-site |`default` |`mysite_1_`|`mysite_1_foobar`| |1b,2b|Multi-site |`js_strings` |`mysite_1_js_strings`|`mysite_1_js_stringsfoobar`| * If you have a single-site deployment and try to flush `default`, you inadvertently flush `js_strings` because everything matches the empty-string prefix. * If you have a multi-site deployment and try to flush `default`, you inadvertently flush `js_strings` because the prefix overlaps. * The three parts of the key (deployment ID, logical cache, and cache item) are not necessarily separated. After ----------------------------- | |Deployment Type|Logical Cache |Combined Cache Prefix |Example Cache Item (`foobar`)| |-|-|---------------|-------|-----------------| |1a,2a|Single-site|`default` |`/default/`|`/default/foobar`| |1a,2b|Single-site|`js_strings` |`/js_strings/`|`/js_strings/foobar`| |1b,2a|Multi-site |`default` |`mysite_1/default/`|`mysite_1/default/foobar`| |1b,2b|Multi-site |`js_strings` |`mysite_1/js_strings/`|`mysite_1/js_strings/foobar`| * If you have a single-site deployment and try to flush `default`, you only flush `default` because the prefixes are distinct. * If you have a multi-site deployment and try to flush `default`, you only flush `default` because the prefixes are distinct. * The three parts of the key (deployment ID, logical cache, and cache item) are always separated by `/`. Comments -------- When developing this patch, I found it helpful to: * Enable Redis driver * Open `redis-cli` and view the list of cache items with `keys *`. --- CRM/Utils/Cache.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CRM/Utils/Cache.php b/CRM/Utils/Cache.php index 00a92888fc8b..13aeec68ee36 100644 --- a/CRM/Utils/Cache.php +++ b/CRM/Utils/Cache.php @@ -35,6 +35,9 @@ * Cache is an empty base object, we'll modify the scheme when we have different caching schemes */ class CRM_Utils_Cache { + + const DELIMITER = '/'; + /** * (Quasi-Private) Treat this as private. It is marked public to facilitate testing. * @@ -83,6 +86,7 @@ public static function &singleton() { // a generic method for utilizing any of the available db caches. $dbCacheClass = 'CRM_Utils_Cache_' . $className; $settings = self::getCacheSettings($className); + $settings['prefix'] = CRM_Utils_Array::value('prefix', $settings, '') . self::DELIMITER . 'default' . self::DELIMITER; self::$_singleton = new $dbCacheClass($settings); } return self::$_singleton; @@ -186,7 +190,7 @@ public static function create($params = array()) { if (defined('CIVICRM_DB_CACHE_CLASS') && in_array(CIVICRM_DB_CACHE_CLASS, array('Memcache', 'Memcached', 'Redis'))) { $dbCacheClass = 'CRM_Utils_Cache_' . CIVICRM_DB_CACHE_CLASS; $settings = self::getCacheSettings(CIVICRM_DB_CACHE_CLASS); - $settings['prefix'] = $settings['prefix'] . '_' . $params['name']; + $settings['prefix'] = CRM_Utils_Array::value('prefix', $settings, '') . self::DELIMITER . $params['name'] . self::DELIMITER; return new $dbCacheClass($settings); } break; From 8e2819ad181756dd3be3688ab1ba40e5c4f16cc9 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 18 Jun 2018 14:57:56 -0700 Subject: [PATCH 2/4] CRM_Cxn_CiviCxnHttp - Expose getCache() function --- CRM/Cxn/CiviCxnHttp.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CRM/Cxn/CiviCxnHttp.php b/CRM/Cxn/CiviCxnHttp.php index f2b2c6357c8c..e5790bad453c 100644 --- a/CRM/Cxn/CiviCxnHttp.php +++ b/CRM/Cxn/CiviCxnHttp.php @@ -106,4 +106,11 @@ protected function createStreamOpts($verb, $url, $blob, $headers) { return $result; } + /** + * @return \CRM_Utils_Cache_Interface|null + */ + public function getCache() { + return $this->cache; + } + } From 5aac553c8b88be9d7fb8ab568a923a3301fea4b2 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 18 Jun 2018 14:58:10 -0700 Subject: [PATCH 3/4] (dev/core#174) System::flushCache() - Continue existing behavior aftering fixing prefixes The preceding update to `CRM_Utils_Cache` meant that `CRM_Utils_Cache::singleton()->flush()` (aka `Civi::cache()->flush()`) would flush only the *default* cache. This revision ensures that a general system-flush still hits the same caches. However, we can now define *other* caches which *won't* be hit by system-flush. --- CRM/Utils/System.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/CRM/Utils/System.php b/CRM/Utils/System.php index e2d72f530324..e88076c21735 100644 --- a/CRM/Utils/System.php +++ b/CRM/Utils/System.php @@ -1416,8 +1416,14 @@ public static function civiExit($status = 0) { public static function flushCache() { // flush out all cache entries so we can reload new data // a bit aggressive, but livable for now - $cache = CRM_Utils_Cache::singleton(); - $cache->flush(); + CRM_Utils_Cache::singleton()->flush(); + if (Civi\Core\Container::isContainerBooted()) { + Civi::cache('settings')->flush(); + Civi::cache('js_strings')->flush(); + Civi::cache('community_messages')->flush(); + CRM_Extension_System::singleton()->getCache()->flush(); + CRM_Cxn_CiviCxnHttp::singleton()->getCache()->flush(); + } // also reset the various static memory caches From 1d521e775fd420d05c17322c7613b626be256fac Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 29 Mar 2018 18:57:26 -0700 Subject: [PATCH 4/4] CRM_Utils_Cache_SqlGroup - Don't encourage infinite recursion The `deleteGroup(...$clearAll...)` option is heavy-handed and leads to call-paths that hard to grok. In the current design of CRM_Core_BAO_Cache (with multi-tier caching), one does need to clear these things. But let's not clear everything under the sun... --- CRM/Utils/Cache/SqlGroup.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CRM/Utils/Cache/SqlGroup.php b/CRM/Utils/Cache/SqlGroup.php index 6f3f45f79274..0bb8e98e277a 100644 --- a/CRM/Utils/Cache/SqlGroup.php +++ b/CRM/Utils/Cache/SqlGroup.php @@ -121,12 +121,16 @@ public function getFromFrontCache($key, $default = NULL) { * @param string $key */ public function delete($key) { - CRM_Core_BAO_Cache::deleteGroup($this->group, $key); + CRM_Core_BAO_Cache::deleteGroup($this->group, $key, FALSE); + CRM_Core_BAO_Cache::$_cache = NULL; // FIXME: remove multitier cache + CRM_Utils_Cache::singleton()->flush(); // FIXME: remove multitier cache unset($this->frontCache[$key]); } public function flush() { - CRM_Core_BAO_Cache::deleteGroup($this->group); + CRM_Core_BAO_Cache::deleteGroup($this->group, NULL, FALSE); + CRM_Core_BAO_Cache::$_cache = NULL; // FIXME: remove multitier cache + CRM_Utils_Cache::singleton()->flush(); // FIXME: remove multitier cache $this->frontCache = array(); }