(dev/core#284) System::flushCache - Reproduce legacy cache behavior. Improve test performance. #12590
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
---------------------------------------- `CRM_Utils_System::flushCache()` calls `CRM_Utils_Cache::singleton()->flush()`. In `5.3`, this triggered a cascading effect; in development of `5.4.alpha`, some of the cascades were overzealous and we revised to get tighter control over cascading. With an aim to being conservative and reproducing old behavior, I previously patched `5.4.alpha` to add several extra flushes and simulate the old cascades. However, it wasn't really as conservative as hoped -- because the "old behavior" depended on the environment. This patch brings us closer the "old behavior". See also: https://lab.civicrm.org/dev/core/issues/284 Before (Behavior in version <=`5.3`) ---------------------------------------- On systems with memory-backed caches, `flushCache()` had an aggressive cascading side-effect where several named caches (`settings`, etc) were also flushed. On systems with a default configuration (SQL+ArrayCache), `flushCache()` had a very limited cascading effect -- it *only cleared the in-process ArrayCache*. The bulk of the cache content was preserved in SQL. Before (Behavior in version ~= `5.4.alpha`) ---------------------------------------- To simulate the cascading effect, `flushCache()` explicitly flushes a half-dozen individual caches. (These half-dozen are chosen to match the old cascade list and exclude some new things which would problematic.) On systems with memory-backed caches, this reproduces the aggressive cascading effect. On systems with a default configuration (SQL+ArrayCache), this amplifies the flushing -- because it also destroys the underlying SQL caches. This has the side-effect of significantly degrading performance of the test suite. After (Behavior with patch) ---------------------------------------- `CRM_Utils_System::flushCache` calls `CRM_Utils_Cache::singleton()->flush()`. To simulate the cascading effect, `flushCache()` explicitly flushes a half-dozen individual caches... *but only on memory-backed* systems. On systems with memory-backed caches, this reproduces the aggressive cascading effect. On systems with a default configuration (SQL+ArrayCache), this is closer to the old behavior. The bulk of the cache remains available in SQL. Based on local spot-checking, this restores performance of the test suite. Comments ---------------------------------------- Deep down, I don't really believe the cascading effect is a good thing. At some point, I'd rather just remove these bits. But in absence of a crystal ball to predict the side-effects of that, I think it's good to find a better approximation of the old behavior.
|
(Standard links)
|
totten
changed the title
|
test this please |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
CRM_Utils_System::flushCache()callsCRM_Utils_Cache::singleton()->flush(). The significance of this changed during development of 5.4.alpha. With an aim to being conservative and reproducing old behavior, I previously patched 5.4.alpha to add several extra flushes. However, it wasn't really asconservative as hoped -- because the "old behavior" depended on the environment. This patch brings us closer the "old behavior".
See also: https://lab.civicrm.org/dev/core/issues/284
Before (Behavior in version <=5.3)
On systems with memory-backed caches, this had an aggressive cascading side-effect where several named caches (
settings, etc) were also flushed.On systems with a default configuration (SQL+ArrayCache), there was a very limited cascading effect -- it only cleared the in-process ArrayCache. The bulk of the cache content was preserved in SQL.
Before (Behavior in version ~= 5.4.alpha)
CRM_Utils_System::flushCache()callsCRM_Utils_Cache::singleton()->flush().To simulate the cascading effect,
flushCache()explicitly flushes a half-dozen individual caches. (These half-dozen are chosen to match the old cascade list and exclude some new things which would problematic.)On systems with memory-backed caches, this reproduces the aggressive cascading effect.
On systems with with a default configuration (SQL+ArrayCache), this amplifies the flushing -- because it also destroys the underlying SQL caches.
This has the side-effect of significantly degrading performance of the test suite.
After (Behavior with patch)
CRM_Utils_System::flushCachecallsCRM_Utils_Cache::singleton()->flush().To simulate the cascading effect,
flushCache()explicitly flushes a half-dozen individual caches... but only on memory-backed systems.On systems with memory-backed caches, this reproduces the aggressive cascading effect.
On systems with with a default configuration (SQL+ArrayCache), this is closer to the old behavior. The bulk of the cache remains available in SQL.
Based on local spot-checking, this restores performance of the test suite.