(dev/core#635) Reduce unnecessary SQL writes #13394
Merged
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
The selector was set to specifically store the size of the result set in the SQL-based cache. For epic:ro-db, we should be able to store these in a non-SQL location. The `long` cache suffices.
This line of code creates an extra record to facilitate clearing the SQL-based prevnext cache. However, with Redis-based prevnext cache, it's not needed because the Redis implementation simply uses a TTL -- and (on Redis deployments) this lines to gratuitous SQL writes.
|
(Standard links)
|
totten
commented
Jan 7, 2019
| CRM_Core_BAO_Cache::setItem($cacheKey, 'CiviCRM Search PrevNextCache', $cacheKey); | ||
| if (Civi::service('prevnext') instanceof CRM_Core_PrevNextCache_Sql) { | ||
| // SQL-backed prevnext cache uses an extra record for pruning the cache. | ||
| CRM_Core_BAO_Cache::setItem($cacheKey, 'CiviCRM Search PrevNextCache', $cacheKey); |
There was a problem hiding this comment.
To see that this is only relevant to SQL-based prevnext cache, one can follow these steps:
- Grep for "CiviCRM Search PrevNextCache"
- Determine the only party reading this record is
CRM_Core_BAO_PrevNextCache::cleanupCache() - Observe that this is a cleanup function focused on
civicrm_cachejoined withcivicrm_prevnext_cache. Per dev/core#217, the search screens (CRM_Contact_Selector) only puts data intocivicrm_prevnext_cacheif we're using the SQL-based driver.
|
Agreed. |
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
To meaningfully use replay-on-write strategy, one should avoid unnecessary SQL writes.
Before
CRM_Contact_Selectorstores the result-count in the SQL-backed cache (civicrm_cache) regardless of the configuration.CRM_Contact_SelectorandCRM_Campaign_Selector_Searchadd a cache-record tocivicrm_cacheto facilitate cleanup ofcivicrm_prevnext_cache.After
CRM_Contact_Selectorstores the result-count in thelongcache, which could be Redis/Memcache or SQL (depending on the local configuration).CRM_Contact_SelectorandCRM_Campaign_Selector_Searchadd the record... but only ifcivicrm_prevnext_cacheis actually being used.