CiviCRM Core issueshttps://lab.civicrm.org/dev/core/-/issues2020-09-09T05:24:30Zhttps://lab.civicrm.org/dev/core/-/issues/635Implement reconnect/replay-on-write for database connections2020-09-09T05:24:30ZtottenImplement reconnect/replay-on-write for database connectionsCurrently, CiviCRM always connects to a singular DSN. For epic:ro-db, we seek compatibility with a split DB architecture in which one routes MySQL requests to (a) read-only slave DBs and/or (b) read-write master DB. This issue specifical...Currently, CiviCRM always connects to a singular DSN. For epic:ro-db, we seek compatibility with a split DB architecture in which one routes MySQL requests to (a) read-only slave DBs and/or (b) read-write master DB. This issue specifically proposes a "reconnect-on-write" or "replay-on-write" (RPOW) mechanism as a general, global baseline.
## Technical Overview
One possible technique is to sprinkle flags/hints into the application to indicate which use-cases should be served by slave/rodb or by master/rwdb. *Some* of this sprinkling is likely happen, but we have a large, open-ended application (with several built-in subsystems and several third-party extensions/addons). Auditing all of these is somewhat daunting task.
RPOW aims to provide a *generic baseline* that relies primarily on MySQL semantics (and doesn't require auditing every use-case carefully). The general idea is:
1. Connect optimistically to the read-only slave (expecting a read-only use-case). We can continue using the RODB as long as requests are read-oriented (e.g. `SELECT`).
2. If there is an actual write operation (e.g. `UPDATE`), then reconnect to the read-write master.
Dynamically switching to the read-write master is not quite as simple as it sounds:
* Some SQL statements (eg `SET @foo` and `CREATE TEMPORARY TABLE`) can be legitimately used in a read-oriented operation (e.g. advanced querying/reporting) -- but they may also be prelude to a write-operation (e.g. building a temp-table with a list of targets and then updating each one). We allow these to execute on the RODB -- but, when/if we reconnect to RWDB, then we *replay* those statements.
* To support replay, we must be able to classify any SQL statement into one of three buckets:
* `READ` (Ex: `SELECT * FROM foo`): The SQL statement has no side-effects; it simply reads data.
* `BUFFER` (Ex: `SET @user_id = 123`): The SQL statement has no long-term, persistent side-effects; it can, however, have temporary side-effects during the present MySQL session.
* `WRITE` (Ex: `TRUNCATE foo`): The SQL statement has long-term, persistent side-effects and must be executed on the master. (Generally, if we can't demonstrate that something is `READ` or `BUFFER`, then we assume it is `WRITE`.)
* The MySQL query language has interesting edge-cases (e.g. `SELECT @foo := id FROM bar FOR UPDATE`) that should be handled correctly.
* We don't know anything about the delay in sync'ing between RODB and RWDB. After making a write, we'll continue sending all requests (reads or writes) to RWDB for some *period of time*. (Ex: After updating a contact in RWDB, the user's browser gets a cookie -- and, for the next 60 seconds, any additional reads should hit RWDB.)
## Limitations and Assumptions
* RPOW makes sense if user's are primarily reading from MySQL. Stock CiviCRM relies extensively on MySQL for caching and session-state, which leads to frequent writes. However, if you use the Redis integration for caches/sessions/prevnext, then this is significantly reduced.
* RPOW aims to *mitigate/reduce* the need for sprinkling use-case specific hints. However, there may still be scenarios where one wants to sprinkle hints. In particular: the contact-edit screen uses optimistic-locking (which reads the last-modified timestamp before authorizing updates); for correct oplocking, there should be a hint that any POST requests to the contact-edit screen need the RWDB.
## Relevant Tasks / Patches / Subtasks
The following are types of patches / subtasks we may expect:
* Adding a new DB driver -- an admin can opt-in to using RPOW behavior by setting `CIVICRM_DSN` to a special value (and registering DSNs for both RODBs and RWDB).
* Adding a unit-test to ensure correct classification of a range of SQL examples.
* Maintaining a cookie or session-variable to indicate that a user needs to be temporarily directed to RWDB by default.
* Updating existing use-cases to reduce gratuitous writes or to send hints about specific write-oriented use-cases.
NOTE: I've currently got a draft project in https://github.com/totten/rpow ; however, I'm filing this issue here because some of this work will need to come back into core.
## Pull Requests
* [#13394 - Reduce unnecessary SQL writes](https://github.com/civicrm/civicrm-core/pull/13394) (m)
* [#13500 - (REF) Add CRM_Utils_Cache::nack(). Use it for NaiveHasTrait](https://github.com/civicrm/civicrm-core/pull/13500) (m)
* [#13496 - Implement local array-cache for use with Redis/Memcache](https://github.com/civicrm/civicrm-core/pull/13496) (m)
* [#13489 - Deprecate CRM_Core_BAO_Cache for I/O. Optionally redirect I/O to Redis or Memcache. #13489 ](https://github.com/civicrm/civicrm-core/pull/13489) (m)
* [#13514 - CRM_Utils_Cache::nack() - Fix format](https://github.com/civicrm/civicrm-core/pull/13514) (m)tottentottenhttps://lab.civicrm.org/dev/core/-/issues/284Aggressive cache clearing significantly increases test time2018-08-01T19:25:15ZtottenAggressive cache clearing significantly increases test timeThe test suites are significantly slower in Civi 5.4.beta1/5.5.alpha1 than in 5.3. (*Ballpark: +1hr*.) One might speculate that this stems from high contention for CI resources, additional tests, and/or generic changes in the framework/p...The test suites are significantly slower in Civi 5.4.beta1/5.5.alpha1 than in 5.3. (*Ballpark: +1hr*.) One might speculate that this stems from high contention for CI resources, additional tests, and/or generic changes in the framework/process. While the first two may also be factors, I've [run the test suites in isolation](https://github.com/totten/civi-test-bench-18) and found a clear [across-the-board slowdown (Jul 24 dataset)](https://docs.google.com/spreadsheets/d/1ktf5cd3LMrk_Keg4kgDIEr4-rL3ZIbDDzGHCI09-mOA/edit?usp=sharing) causing many test runtimes to grow by double (or more). For example, when running `api_v3_SyntaxConformanceTest` on an isolated system (no CPU/RAM contention), the time increased from 251s (v5.3) to 936s (v5.4.beta1) even while the #test-functions and #entities remained flat.
Focusing on one specific example (`api_v3_UtilsTest`; increased from ~1s to ~7s), I used `git bisect` to track the cause to 5aac553c8b88be9d7fb8ab568a923a3301fea4b2 from [12331](https://github.com/civicrm/civicrm-core/pull/12331) and dev/core#174 -- in particular, the biggest impact comes from adding `CRM_Extension_System::singleton()->getCache()->flush()` (+~5s), although there's also observable impact from `Civi::cache('settings')->flush();` (+~1s).
In discussing 12331, one of the major concerns was "How do we make `CRM_Utils_System::flushCache()` behave the same as before?" 5aac553 tried to do this, but it actually was a bit more aggressive. To see this, it helps to fully describe the behavior -- in each configuration, what's the impact on each cache?
* __(OLD) Behavior circa 5.3.x__ -- `CRM_Utils_System::flushCache()` calls `CRM_Utils_Cache::singleton()->flush()` which has a cascading effect on other caches.
* __(DEFAULT) Default configuration using ArrayCache+SQL table__
* `CRM_Utils_Cache` - Destroys the general `ArrayCache`, which is used as the front-cache for several others.
* `CRM_Core_BAO_Cache`: Due to `CRM_Utils_Cache` flush, the front-caches are flushed, but the underlying SQL caches are preserved.
* `settings`: Same as `CRM_Core_BAO_Cache`. (Flush ArrayCache at front, but keep underlying SQL cache.)
* `js_strings`: Same as `CRM_Core_BAO_Cache`. (Flush ArrayCache at front, but keep underlying SQL cache.)
* `community_messages`: Same as `CRM_Core_BAO_Cache`. (Flush ArrayCache at front, but keep underlying SQL cache.)
* `CRM_Extension_System::$cache`: Same as `CRM_Core_BAO_Cache`. (Flush ArrayCache at front, but keep underlying SQL cache.)
* `CiviCxnHttp`: Same as `CRM_Core_BAO_Cache`. (Flush ArrayCache at front, but keep underlying SQL.)
* __(MEM) Alternative Memcache/Redis configuration__
* `CRM_Utils_Cache` - Flushes *everything* in memory-backed cache.
* `CRM_Core_BAO_Cache`: Destroys the memory-backed front-cache but preserves the underlying SQL cache.
* `settings`: Completely flushed. (Due to `CRM_Utils_Cache` flush.)
* `js_strings`: Completely flushed. (Due to `CRM_Utils_Cache` flush.)
* `community_messages`: Completely flushed. (Due to `CRM_Utils_Cache` flush.)
* `CRM_Extension_System::$cache`: Completely flushed. (Due to `CRM_Utils_Cache` flush.)
* `CiviCxnHttp`: Same as `CRM_Core_BAO_Cache`. (Flush memory-backed front-cache, but keep underlying SQL cache.)
* __(NEW) Behavior circa 5.4.beta1/5.5.alpha1__ -- `CRM_Utils_System::flushCache()` calls several individual flush functions
* __(DEFAULT) Default configuration using ArrayCache+SQL table__
* `CRM_Core_BAO_Cache`: Due to `CRM_Utils_Cache` flush, the front-caches are flushed, but the underlying SQL caches are preserved.
* `settings`: Completely flushed. (Due to explicit call.)
* `js_strings`: Completely flushed. (Due to explicit call.)
* `community_messages`: Completely flushed. (Due to explicit call.)
* `CRM_Extension_System::$cache`: Completely flushed. (Due to explicit call.)
* `CiviCxnHttp`: Completely flushed. (Due to explicit call.)
* __(MEM) Alternative Memcache/Redis configuration__
* `CRM_Core_BAO_Cache`: Due to `CRM_Utils_Cache` flush, the front-caches are flushed, but the underlying SQL caches are preserved.
* `settings`: Completely flushed. (Due to explicit call.)
* `js_strings`: Completely flushed. (Due to explicit call.)
* `community_messages`: Completely flushed. (Due to explicit call.)
* `CRM_Extension_System::$cache`: Completely flushed. (Due to explicit call.)
* `CiviCxnHttp`: Completely flushed. (Due to explicit call.)
With that data, how we would describe both the original intent of 5aac553 ... and it's unintended consequence in this issue?
* During the development of 5.4.alpha1, we looked at the behavior of OLD-MEM and intended to keep the behavior the same. Thus, we have NEW-MEM which is basically the same as OLD-MEM.
* NEW-DEFAULT, NEW-MEM, and OLD-MEM are basically the same. But NEW-DEFAULT and OLD-DEFAULT are different.
* In running unit-tests, one flushes the caches frequently. The behavior of OLD-DEFAULT (where it *only* flushes front-caches... but generally preserves underlying caches) performs better for unit-tests. The behavior of NEW-DEFAULT/NEW-MEM/OLD-MEM does not perform as well.https://lab.civicrm.org/dev/core/-/issues/217Allow replacement of PrevNextCache implementation (for search screens)2019-01-10T10:07:11ZtottenAllow replacement of PrevNextCache implementation (for search screens)This is a major sub-task of dev/core#174 (*Consistently use swappable cache interfaces*).
# Background
The PrevNextCache is used to temporarily save the results of a query. It is specifically used in the following scenarios:
* Contact...This is a major sub-task of dev/core#174 (*Consistently use swappable cache interfaces*).
# Background
The PrevNextCache is used to temporarily save the results of a query. It is specifically used in the following scenarios:
* Contact searches (Related -- other derived searches and the "View Contact" result screen)
* Dedupe/merge
For this issue, our aim is specifically to allow the contact-searches to retain their results via Redis/Memcache (instead of MySQL).
# Touch Points
We want to make it possible to swap the prev-next implementation, but the functionality of `CRM_Core_BAO_PrevNextCache` is a bit more sophisticated than `CRM_Utils_Cache_Interface`. So, first, we need to understand the places which touch the prev-next cache (*before patching*).
__Q1: Where does it `INSERT` into `civicrm_prevnext_cache`?__
* `CRM_Contact_Selector::fillupPrevNextCache`
* `CRM_Campaign_Selector_Search::buildPrevNextCache`
__Q2: Where does it `SELECT` from `civicrm_prevnext_cache`?__
* `CRM_Contact_BAO_Query::getCachedContacts`
__Q3: What are all the functions supported by the BAO? When are they relevant__
* (Group A) Relevant to contact searches (and maybe also dedupe/merge)
* `public static function markSelection($cacheKey, $action = 'unselect', $cIds = NULL, $entity_table = 'civicrm_contact') {`
* `public static function getSelection($cacheKey, $action = 'get', $entity_table = 'civicrm_contact') {`
* `public static function getPositions($cacheKey, $id1, $id2, &$mergeId = NULL, $join = NULL, $where = NULL, $flip = FALSE) {`
* `public static function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = 'civicrm_contact') {`
* `public static function getSelectedContacts() {`
* `public static function getCount($cacheKey, $join = NULL, $where = NULL, $op = "=", `$params = array()) {
* (Group B) Only relevant to dedupe/merge
* `public static function flipPair(array $prevNextId, $onlySelected) {`
* `public static function deletePair($id1, $id2, $cacheKey = NULL, $isViceVersa = FALSE, $entityTable = 'civicrm_contact') {`
* `public static function markConflict($id1, $id2, $cacheKey, $conflicts) {`
* `public static function setItem($values) {`
* `public static function refillCache($rgid, $gid, $cacheKeyString, $criteria, $checkPermissions, $searchLimit = 0) {`
* `public static function retrieve($cacheKey, $join = NULL, $whereClause = NULL, $offset = 0, $rowCount = 0, $select = array(),...)`
* (Group C) Special, internal, or unused
* `public static function cleanupCache() {`
* `public static function is_serialized($string) {`
* `public static function buildSelectedContactPager(&$form, &$params) {`
# General Approach
* Define a service `Civi::service('prevnext')` which satisfies an interface `CRM_Core_PrevNextCache_Interface`.
* Create two implementations of the service:
* `CRM_Core_PrevNextCache_Sql` (which is pretty thin -- it generally delegates to the existing BAO code)
* ~~`CRM_Core_PrevNextCache_Memory` (which uses a memory-backed cache)~~
* `CRM_Core_PrevNextCache_Redis` (which uses Redis for a memory-backed cache)
* The scope of the interface would include:
* Functionality needed by the inserters (`CRM_Contact_Selector::fillupPrevNextCache` and `CRM_Campaign_Selector_Search::buildPrevNextCache`)
* Functionality needed by the reader (`CRM_Contact_BAO_Query::getCachedContacts`)
* Functionality from the BAO that's relevant to contact-searches (eg `markSelection` but not `deletePair`)
* In the contact-search use-cases, change to use the service instead of the BAO.
* Ex: `CRM_Core_BAO_PrevNextCache::markSelection(...)` => `Civi::service('prevnext')->markSelection(...)`
*Note*: I initially considered a different relationship (where `CRM_Core_BAO_PrevNextCache` remains the canonical/front-facing interface and `CRM_Core_PrevNextCache_Interface` remains hidden behind the BAO). However, given that dedupe/merge needs to continue using the existing SQL implementation, it felt like that would get messier; it seemed cleaner to split them apart more clearly ("use-case (1) always invokes code (A), and use-case (2) always invokes code (B)" -- i.e. "all contact-search code invokes `Civi::service('prevnext')`, and all dedupe code invokes PrevNext BAO").
![Screen_Shot_2018-06-28_at_6.55.29_PM](/uploads/7fc6852ecefe6e5638e0b3a21fb37e44/Screen_Shot_2018-06-28_at_6.55.29_PM.png)
# PR List
* [12377 - General/overall patchset](https://github.com/civicrm/civicrm-core/pull/12377)
* [12438 - CRM_Contact_Selector::getRows() - Use generator instead of DAO loop](https://github.com/civicrm/civicrm-core/pull/12438)
* [12528 - Add skeletal PrevNextCache service ](https://github.com/civicrm/civicrm-core/pull/12528)
* [12543 - PrevNext - Probe for best available implementation (memory-backed or SQL-backed)](https://github.com/civicrm/civicrm-core/pull/12543)
* [12544 - CRM_Utils_Cache_Redis::connect() - Allow pooling connections](https://github.com/civicrm/civicrm-core/pull/12544)
* [12545 - PrevNext - Define and use fillWithSql()/fillWithArray()](https://github.com/civicrm/civicrm-core/pull/12545)
* [12556 - Migrate selection methods](https://github.com/civicrm/civicrm-core/pull/12556)
* [12558 - Allow swapping getPositions (etal) for contact-search](https://github.com/civicrm/civicrm-core/pull/12558)
* [12663 - Use more consistent cache-keys while adjusting filters](https://github.com/civicrm/civicrm-core/pull/12663)
* [12664 - Remove references to entity_table and entity_id2 from service. Add test.](https://github.com/civicrm/civicrm-core/pull/12664)
* [12665 - Implement Redis. Decouple Query::getCachedContacts()](https://github.com/civicrm/civicrm-core/pull/12665)
# Test Scenarios
The prev-next cache has evolved a number of conditionals to handle edge-cases in which it's used. We should be testing these in a targeted/incremental way during development. Once we have a fairly complete PR, we should do a final/comprehensive pass through these use-cases.
* __Quick Search Box / Advanced Search / Search Builder__: This is the main section of the code that we're targetting for revision.
* Execute a search with multiple pages of results
* Execute a search with one page of results
* Drill-down and view a contact in the search results. Navigate using "Previous" button, "Next" button, and the breadcrumb "Search Results"
* [CRM-9096](https://issues.civicrm.org/jira/browse/CRM-9096) Create a profile. Mark some fields as visible/enabled for listings. Run an "Advanced Search" and use this profile. Re-sort results.
* Select a handful of contacts on different pages. Execute a task (e.g. "Export") and ensure that it has the appropriate contacts.
* Select all contacts. Execute a task (e.g. "Export") and ensure that it has the appropriate contacts.
* __Custom Search__: Most of the code is shared with above, but a few lines are different.
* Run a few custom searches. Use various sort/filter options.
* __CiviCampaign__: This component has logic to fill the cache directly -- which doesn't use the same code-paths.
* Create a survey. Reserve respondents. Interview. Release. (When browsing respondents, try to sort.)
* __CiviRules__: This component uses some of the code from contact-search, and there's a comment about special case.
* Browse the list of available rules.
* __Dedupe/Merge__: In principle, this code isn't supposed to be touched/changed by our PR, and there is test coverage over this functionality. OTOH, if one made a mistake (inappropriately changing shared code), there could be a regression. So do a couple tests with this anyway.5.9tottentottenhttps://lab.civicrm.org/dev/core/-/issues/183Temporary tables should follow consistent naming convention2020-08-31T02:03:19ZtottenTemporary tables should follow consistent naming convention#tldr
- check https://lab.civicrm.org/dev/core/issues/183#note_5717 for how to use the new syntax
# Context
For `epic:ro-db`, we aim to allow CiviCRM PHP workers to connect to read-only MySQL slaves and run searches+reports. These of...#tldr
- check https://lab.civicrm.org/dev/core/issues/183#note_5717 for how to use the new syntax
# Context
For `epic:ro-db`, we aim to allow CiviCRM PHP workers to connect to read-only MySQL slaves and run searches+reports. These often require temporary tables.
By default, if you have replicated MySQL configuration, MySQL replicates TEMPORARY tables. However, there isn't much point in doing this because TEMPORARY tables are session-scoped. (If there's a fault, you can't recover access to them.) One can disable replication by setting an option like `--replicate-wild-ignore-table=<wildcard>`. See also:
https://dev.mysql.com/doc/refman/5.7/en/replication-features-temptables.html
> A recommended practice when using statement-based or mixed-format replication is to designate a prefix for exclusive use in naming temporary tables that you do not want replicated...
If you search the CiviCRM source tree for temp tables, you can find a variety of table name formats... such as `civicrm_temp_exampledata` or `civicrm_exampledata_temp_{$rand}` or `X_exampledata` or just `exampledata`. Fortunately, these are amenable to change: they’re easy to find with “grep”, and (by virtue of being temporary tables) they’re only referenced a couple times. However, there are a large number of them, and (after renaming a temp-table) you should `r-run` the affected code to ensure it works.
Note: The benefit of the change will likely accrue on systems which use a replicated master/slave topology on MySQL v5.6+. However, we don't need to test that specific configuration. Our focus here is just on the naming.
# Goal
Enforce a common naming convention for temporary tables:
* `civicrm_tmp_e_%` matches any true temp table (i.e. "temporary-during-this-connection"; CREATE TEMPORARY TABLE). The "e" stands for "ephemeral".
* `civicrm_tmp_d_%` matches any quasi-temp table (i.e. "temporary-for-a-few-minutes-or-hours"; CREATE TABLE). The "d" stands for "durable".
* `civicrm_tmp_%` matches any temp table, whether ephemeral or durable.
This arrangement will give the MySQL DBA some decent ways to scope their policies.
To generate a table name, one can hard-code the prefix or use a helper. Following [12311](https://github.com/civicrm/civicrm-core/pull/12311), you can use the helper `CRM_Utils_SQL_TempTable`, e.g.
![Screen_Shot_2018-06-15_at_10.24.08_AM](/uploads/c7b670a73c0d1eafd464a1e65ddde0bd/Screen_Shot_2018-06-15_at_10.24.08_AM.png)
The general goal is update calls to `createTempTableName()` (or to manually-constructed table-names) with the newer convention, and then re-test anything that have used it.
Changing the prefix on temp tables should usually be pretty safe/portable, provided that we do some basic searching/testing after each item is renamed, i.e.
* Search code-base for anything that might have used the temporary-table name.
* Run a use-case or two that involves actually creating the temporary table.
# Audit list
Just to help track progress, here's a list of files with tell-tale signs that they may be using temporary (ephemeral or durable) tables. Here's the process for auditing each file:
1. Search for the file for any references to the target phrase (`CREATE TEMPORARY TABLE` or `CREATE TABLE` or `createTempTableName`).
2. Determine if the file is creating a temporary (ephemeral or durable) table. Check if the table name matches either `civicrm_tmp_e_%` or `civicrm_tmp_d_%`.
1. If it doesn't really create a temp-table, we're done.
2. If it creates a compliant temp-table, we're done.
3. If it creates a non-compliant temp-table, then we need to patch.
3. Figure out use-case(s) which hit that temp-table.
1. To see if you've found a real use-case, it helps to hack the code in question and provoke an error. (eg `throw new Exception('Oooga booga')`)
2. (NB: we're being optimistic that if one use-case works for the table that they all will. We're expecting failures to come out in the wash.)
4. Change the temp-table name. Search+update anything that references the same temp table. Verify the use-case still works as before.
5. Submit the patch. (If it's a new PR, include the link.)
6. Check the item as done. (The PR doesn't have to be merged -- we just to ensure that this item isn't re-examined.)
Files:
(note some of these have been fixed without being checked off)
* Files matching `CREATE TEMPORARY TABLE`
* [x] `CRM/Activity/BAO/Activity.php`
* [x] `CRM/Campaign/BAO/Query.php`
* [x] `CRM/Campaign/Form/Task/Interview.php`
* [x] `CRM/Contact/BAO/GroupContactCache.php`
* [x] `CRM/Contact/BAO/Query.php`
* [x] `CRM/Contact/Form/Search/Custom/ContribSYBNT.php`
* [x] `CRM/Contact/Form/Search/Custom/DateAdded.php`
* [x] `CRM/Contact/Form/Search/Custom/FullText.php`
* [x] `CRM/Contact/Form/Search/Custom/Group.php`
* [x] `CRM/Contact/Form/Search/Custom/PriceSet.php`
* [x] `CRM/Contact/Form/Search/Custom/RandomSegment.php`
* [x] `CRM/Contact/Import/Form/DataSource.php`
* [x] `CRM/Contribute/BAO/Query.php`
* [x] `CRM/Dedupe/BAO/QueryBuilder/IndividualUnsupervised.php`
* [x] `CRM/Dedupe/BAO/RuleGroup.php`
* [x] `CRM/Logging/ReportSummary.php`
* [x] `CRM/Mailing/BAO/Mailing.php`
* [x] `CRM/Mailing/BAO/Recipients.php`
* [x] `CRM/Mailing/Event/BAO/Delivered.php`
* [x] `CRM/Report/Form/Activity.php`
* [x] `CRM/Report/Form/ActivitySummary.php`
* [x] `CRM/Report/Form/Contribute/Bookkeeping.php`
* [x] `CRM/Report/Form/Contribute/Detail.php`
* [x] `CRM/Report/Form/Contribute/Lybunt.php`
* [x] `CRM/Report/Form/Contribute/Repeat.php`
* [x] `CRM/Report/Form/Member/ContributionDetail.php`
* [x] `CRM/Report/Form.php`
* [x] `CRM/Upgrade/Incremental/php/FourThree.php`
* [x] `CRM/Upgrade/Incremental/php/FourTwo.php`
* [x] `CRM/Utils/SQL/TempTable.php`
* [x] `Civi/Install/Requirements.php`
* [x] `tests/phpunit/api/v3/MailingTest.php`
* [x] `install/index.php`
* Files matching `CREATE TABLE`
* [x] `CRM/Contact/Form/Task.php`
* [x] `CRM/Contact/Import/Form/DataSource.php`
* [x] `CRM/Contact/Import/ImportJob.php`
* [x] `CRM/Core/BAO/SchemaHandler.php`
* [x] `CRM/Core/DAO.php`
* [x] `CRM/Export/BAO/Export.php`
* [x] `CRM/Import/DataSource/CSV.php`
* [x] `CRM/Logging/Schema.php`
* [x] `CRM/Upgrade/Incremental/php/FourFour.php`
* [x] `CRM/Upgrade/Incremental/php/FourSeven.php`
* [x] `CRM/Upgrade/Incremental/sql/4.2.alpha1.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.2.beta1.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.2.beta6.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.3.alpha1.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.4.alpha1.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.5.alpha1.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.6.3.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.6.5.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.6.alpha1.mysql.tpl`
* [x] `CRM/Upgrade/Incremental/sql/4.7.alpha1.mysql.tpl`
* [x] `CRM/Utils/SQL/TempTable.php`
* [x] `CRM/Utils/String.php`
* [x] `Civi/Core/InstallationCanary.php`
* [x] `Civi/Install/Requirements.php`
* [x] `Civi/Test/CiviEnvBuilder.php`
* [x] `tests/phpunit/api/v3/CustomApiTest.php`
* [x] `tests/phpunit/api/v3/dataset/uf_group_contact_activity_26.xml`
* [x] `tests/phpunit/api/v3/LoggingTest.php`
* [x] `tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php`
* [x] `tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php`
* [x] `tests/phpunit/CRM/Core/DAOTest.php`
* [x] `tests/phpunit/CRM/Logging/LoggingTest.php`
* [x] `tests/phpunit/CRM/Logging/SchemaTest.php`
* [x] `tests/phpunit/CRM/Utils/QueryFormatterTest.php`
* [x] `install/index.php`
* Files matching `createTempTableName`
* [x] `CRM/Campaign/BAO/Query.php`
* [x] `CRM/Campaign/Form/Task/Interview.php`
* [x] `CRM/Contact/BAO/Query.php`
* [x] `CRM/Core/DAO.php`
* [x] `CRM/Report/Form/ActivitySummary.php`
* [x] `CRM/Report/Form/Contribute/Bookkeeping.php`
* [x] `CRM/Upgrade/Incremental/php/FourThree.php`
* [x] `CRM/Upgrade/Incremental/php/FourTwo.php`
* [x] `tests/phpunit/CRM/Core/DAOTest.php`
# Audit Mentality
The deliverables are patches -- hopefully mostly simple patches -- but this is a bit misleading. If we just wanted simple patches to some files, there are faster ways to edit the files. The main work in here is the audit -- ie when we look at a file, figure out how it's used. That's why we have to do searching+testing as part of the update.tottentottenhttps://lab.civicrm.org/dev/core/-/issues/179Redis driver - Allow anonymous connections2018-06-15T21:04:39ZtottenRedis driver - Allow anonymous connectionsThe Redis driver *always* sends a password. That's well and good in production. However, if you're doing local development, the dev instance of redis may be configured to allow local-only, unauthenticated connections -- for which no pass...The Redis driver *always* sends a password. That's well and good in production. However, if you're doing local development, the dev instance of redis may be configured to allow local-only, unauthenticated connections -- for which no password is available.https://lab.civicrm.org/dev/core/-/issues/178Redis driver - Error messages are invisible2018-06-14T20:51:19ZtottenRedis driver - Error messages are invisibleIf the Redis server returns an error message (e.g. due to malformed requests or credentials), the substance of this message is invisible to the site administrator.If the Redis server returns an error message (e.g. due to malformed requests or credentials), the substance of this message is invisible to the site administrator.https://lab.civicrm.org/dev/core/-/issues/177Redis driver - Reports incorrect value for cache-miss2018-06-17T23:17:42ZtottenRedis driver - Reports incorrect value for cache-miss`CRM_Utils_Cache_Interface::get()` specifies that the return value for a non-existent cache-key is `NULL`. But `CRM_Utils_Cache_Redis::get()` returns `FALSE`. This makes it impossible to detect a cache-miss.`CRM_Utils_Cache_Interface::get()` specifies that the return value for a non-existent cache-key is `NULL`. But `CRM_Utils_Cache_Redis::get()` returns `FALSE`. This makes it impossible to detect a cache-miss.5.4.0https://lab.civicrm.org/dev/core/-/issues/174Consistently use swappable cache interfaces2023-12-20T06:26:25ZtottenConsistently use swappable cache interfaces# Background
CiviCRM caches were originally (and are primarily) stored in the table `civicrm_cache`. This design is great for small sites (easy to deploy on server with less software to administer). However, for larger sites that get va...# Background
CiviCRM caches were originally (and are primarily) stored in the table `civicrm_cache`. This design is great for small sites (easy to deploy on server with less software to administer). However, for larger sites that get value from using multiple MySQL servers, it's not so great: cache data (by its nature) is frequently written+read, but it's expensive to write cache data when there are multiple MySQL servers... and the overhead doesn't give us much benefit because the data is fundamentally expendable. A lighter system (like Memcache/Redis) would be more generally appropriate.
You might recall that CiviCRM's cache subsystem can optionally incorporate Memcache/Redis (since roughly 4.3ish?). This appears as `CRM_Utils_Cache_*`, and it started some great improvements (e.g. using faster cache providers; and swapping cache providers). Notably, the functionality uses Memcache/Redis as an *extra cache tier* (i.e. when reading a cache, it first checks memcache; then it falls back to checking `civicrm_cache` in MySQL). This helps *read performance* (because your cache-hit usually returns faster), but it doesn't help *write performance* (because you need to propagate cache-writes to every tier). Using *only* Memcache/Redis (as a single tier -- without MySQL) would be even better for performance.
I don't believe that the status quo reflects a technical necessity. (If you can handle deployment/configuration of Memcache/Redis a front-tier... then you can probably handle it as the main tier.) Rather, it reflects a development practicality: *the app was originally written to only use `civicrm_cache`*. Consequently, there are a handful of oddball use-cases/code-paths which directly access `civicrm_cache` or `CRM_Core_BAO_Cache`, and it would've been harder to do anything if it had required finding+fixing all of them.
# Scope
The general scope of this ticket is to hunt down the oddball use-cases/code-paths which directly reference `civicrm_cache` or `CRM_Core_BAO_Cache`. I expect the project to produce a handful of distinct PRs. To avoid redundantly documenting this, I'm aiming for each PR to have a more concrete discussion about the use-cases/code-paths being cleaned up.
# PR List
* [12330 - CRM_Utils_Cache_Redis::flush() - Respect prefixes](https://github.com/civicrm/civicrm-core/pull/12330) (m)
* [12331 - CRM_Utils_Cache - Always use a prefix. Standardize delimiter](https://github.com/civicrm/civicrm-core/pull/12331) (m)
* [12336 - systemStatusCheckResult - Migrate from settings to cache](https://github.com/civicrm/civicrm-core/pull/12336) (m)
* [12342 - Caching - Comply with PSR-16 interfaces](https://github.com/civicrm/civicrm-core/pull/12342) (m)
* [215 - Import PSR-16 compliance test](https://github.com/civicrm/civicrm-packages/pull/215) (m)
* [12348 - Cache-keys for CRM_Utils_Cache_* should avoid reserved chars](https://github.com/civicrm/civicrm-core/pull/12348) (m)
* ~~[12350 - civicrm_cache - Allow storage of binary data](https://github.com/civicrm/civicrm-core/pull/12350)~~
* [12354 - civicrm_cache - Allow storage of binary data](https://github.com/civicrm/civicrm-core/pull/12354) (m)
* [~~12360~~ - Full PSR-16 compliance for ...](https://github.com/civicrm/civicrm-core/pull/12360) (*depends: ~~12342, 12348, 12354~~*). Cherry-picked to form smaller PRs:
* [12376 - Add various utilities to support PSR-16](https://github.com/civicrm/civicrm-core/pull/12376) (m)
* [12378 - ArrayCache, Redis](https://github.com/civicrm/civicrm-core/pull/12378) (m)
* [12379 - SqlGroup](https://github.com/civicrm/civicrm-core/pull/12379) (m)
* [12380 - Memcache(d)](https://github.com/civicrm/civicrm-core/pull/12380) (m)
* [12381 - APCcache](https://github.com/civicrm/civicrm-core/pull/12381) (m)
* [12389 - Add test to prevent hidden interactions among caches](https://github.com/civicrm/civicrm-core/pull/12389) (m)
* [12362 - Forms/Sessions - Store state via Civi::cache('session')](https://github.com/civicrm/civicrm-core/pull/12362) (*depends: 12360*)
* [12368 - ~~civicrm_cache - Finish transition from DATETIME to TIMESTAMP](https://github.com/civicrm/civicrm-core/pull/12368)~~ (m)
See also
--------
* dev/core#217 - Allow replacement of PrevNextCache implementation (for search screens)
* dev/core#284 - Aggressive cache clearing significantly increases test time5.9tottentotten