Skip to content

Commit

Permalink
CRM-21109 only clear caches once on cli script, consolidate code
Browse files Browse the repository at this point in the history
This is a combination of the commits in
civicrm#10943

Bringing this in does not make any actual change for us. However, it makes it easier for us
to play with the following options

1) setting cache mode to FALSE on a per process basis.
Currently there are statics to try to ensure that caches are flushed
only once per process, but without this the add to group & remove from
group functions bypass them. From a script
CRM_Core_Config::setPermitCacheFlushMode(FALSE);
would prevent cache flushing in the session for group_contact_cache & acl caches until the
script ends or it is retoggled. Also acl caches were not previously subject to the statics.
2)We would consider flushing caches by cron rather than during script runtime. There
is a setting for that & we could extend to cover acl cache. Flushing by cron
permits (optionally) the use of TRUNCATE - which appears to be faster for everyone in the world but us :-)

Change-Id: Ib790939421d3b7ce5d2e258154a8015b8652234f
  • Loading branch information
eileenmcnaughton committed Feb 1, 2018
1 parent 2339807 commit eb24c31
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 59 deletions.
3 changes: 3 additions & 0 deletions CRM/ACL/BAO/Cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ public static function updateEntry($id) {
* Deletes all the cache entries.
*/
public static function resetCache() {
if (!CRM_Core_Config::isPermitCacheFlushMode()) {
return;
}
// reset any static caching
self::$_cache = NULL;

Expand Down
12 changes: 2 additions & 10 deletions CRM/Contact/BAO/Contact.php
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,7 @@ public static function &create(&$params, $fixAddress = TRUE, $invokeHooks = TRUE
//add website
CRM_Core_BAO_Website::create($params['website'], $contact->id, $skipDelete);

//get userID from session
$session = CRM_Core_Session::singleton();
$userID = $session->get('userID');
$userID = CRM_Core_Session::singleton()->get('userID');
// add notes
if (!empty($params['note'])) {
if (is_array($params['note'])) {
Expand Down Expand Up @@ -432,13 +430,7 @@ public static function &create(&$params, $fixAddress = TRUE, $invokeHooks = TRUE
'name'
);

if (!$config->doNotResetCache) {
// Note: doNotResetCache flag is currently set by import contact process and merging,
// since resetting and
// rebuilding cache could be expensive (for many contacts). We might come out with better
// approach in future.
CRM_Contact_BAO_Contact_Utils::clearContactCaches($contact->id);
}
CRM_Contact_BAO_Contact_Utils::clearContactCaches();

if ($invokeHooks) {
if ($isEdit) {
Expand Down
21 changes: 12 additions & 9 deletions CRM/Contact/BAO/Contact/Utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -897,18 +897,21 @@ public static function getAddressShareContactNames(&$addresses) {
* caches, but are backing off from this with every release. Compromise between ease of coding versus
* performance versus being accurate at that very instant
*
* @param $contactID
* The contactID that was edited / deleted.
* @param bool $isEmptyPrevNextTable
* Should the civicrm_prev_next table be cleared of any contact entries.
* This is currently done from import but not other places and would
* likely affect user experience in unexpected ways. Existing behaviour retained
* ... reluctantly.
*/
public static function clearContactCaches($contactID = NULL) {
// clear acl cache if any.
CRM_ACL_BAO_Cache::resetCache();

if (empty($contactID)) {
// also clear prev/next dedupe cache - if no contactID passed in
public static function clearContactCaches($isEmptyPrevNextTable = FALSE) {
if (!CRM_Core_Config::isPermitCacheFlushMode()) {
return;
}
if ($isEmptyPrevNextTable) {
CRM_Core_BAO_PrevNextCache::deleteItem();
}

// clear acl cache if any.
CRM_ACL_BAO_Cache::resetCache();
CRM_Contact_BAO_GroupContactCache::opportunisticCacheFlush();
}

Expand Down
26 changes: 2 additions & 24 deletions CRM/Contact/BAO/GroupContact.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,7 @@ public static function addContactsToGroup(
list($numContactsAdded, $numContactsNotAdded)
= self::bulkAddContactsToGroup($contactIds, $groupId, $method, $status, $tracking);

// also reset the acl cache
$config = CRM_Core_Config::singleton();
if (!$config->doNotResetCache) {
CRM_ACL_BAO_Cache::resetCache();
}

// reset the group contact cache for all group(s)
// if this group is being used as a smart group
CRM_Contact_BAO_GroupContactCache::opportunisticCacheFlush();
CRM_Contact_BAO_Contact_Utils::clearContactCaches();

CRM_Utils_Hook::post('create', 'GroupContact', $groupId, $contactIds);

Expand Down Expand Up @@ -245,21 +237,7 @@ public static function removeContactsFromGroup(
}
}

// also reset the acl cache
$config = CRM_Core_Config::singleton();
if (!$config->doNotResetCache) {
CRM_ACL_BAO_Cache::resetCache();
}

// reset the group contact cache for all group(s)
// if this group is being used as a smart group
// @todo consider what to do here - it feels like we should either
// 1) just invalidate the specific group's cache(& perhaps any parents) & let cron do it's thing or
// possibly clear this specific groups cache, or just call opportunisticCacheFlush() - which would have the
// same effect as the remove call. The reservation about that is that it is no more aggressive for the group that
// we know is altered than for all the others, or perhaps, more the point with it's parents & groups that use it in
// their criteria.
CRM_Contact_BAO_GroupContactCache::remove();
CRM_Contact_BAO_Contact_Utils::clearContactCaches();

CRM_Utils_Hook::post($op, 'GroupContact', $groupId, $contactIds);

Expand Down
16 changes: 9 additions & 7 deletions CRM/Contact/BAO/GroupContactCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -297,16 +297,18 @@ public static function updateCacheTime($groupID, $processed) {
* In fact it turned out there is little overlap between the code when group is passed in
* and group is not so it makes more sense as separate functions.
*
* @todo remove last call to this function from outside the class then make function protected,
* enforce groupID as an array & remove non group handling.
* @todo enforce groupID as an array & remove non group handling.
* Use flushCaches when no group id provided.
*
* @param int $groupIDs
* the groupID to delete cache entries, NULL for all groups.
* @param bool $onceOnly
* run the function exactly once for all groups.
*/
public static function remove($groupIDs = NULL, $onceOnly = TRUE) {
static $invoked = FALSE;
protected static function remove($groupIDs = NULL, $onceOnly = TRUE) {
if (!isset(Civi::$statics[__CLASS__]['remove_invoked'])) {
Civi::$statics[__CLASS__] = array('remove_invoked' => FALSE);
}

// typically this needs to happy only once per instance
// this is especially TRUE in import, where we don't need
Expand All @@ -315,14 +317,14 @@ public static function remove($groupIDs = NULL, $onceOnly = TRUE) {
// i.e. cache is reset for all groups
if (
$onceOnly &&
$invoked &&
Civi::$statics[__CLASS__]['remove_invoked'] &&
$groupIDs == NULL
) {
return;
}

if ($groupIDs == NULL) {
$invoked = TRUE;
Civi::$statics[__CLASS__]['remove_invoked'] = TRUE;
}
elseif (is_array($groupIDs)) {
foreach ($groupIDs as $gid) {
Expand Down Expand Up @@ -479,7 +481,7 @@ protected static function flushCaches() {
* @throws \CRM_Core_Exception
*/
protected static function getLockForRefresh() {
if (!isset(Civi::$statics[__CLASS__])) {
if (!isset(Civi::$statics[__CLASS__]['is_refresh_init'])) {
Civi::$statics[__CLASS__] = array('is_refresh_init' => FALSE);
}

Expand Down
2 changes: 1 addition & 1 deletion CRM/Contact/Form/Merge.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function preProcess() {

// get user info of main contact.
$config = CRM_Core_Config::singleton();
$config->doNotResetCache = 1;
CRM_Core_Config::setPermitCacheFlushMode(FALSE);

$mainUfId = CRM_Core_BAO_UFMatch::getUFId($this->_cid);
$mainUser = NULL;
Expand Down
2 changes: 1 addition & 1 deletion CRM/Contact/Import/Form/Preview.php
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ public function postProcess() {

// Clear all caches, forcing any searches to recheck the ACLs or group membership as the import
// may have changed it.
CRM_Contact_BAO_Contact_Utils::clearContactCaches();
CRM_Contact_BAO_Contact_Utils::clearContactCaches(TRUE);

// add all the necessary variables to the form
$importJob->setFormVariables($this);
Expand Down
7 changes: 3 additions & 4 deletions CRM/Contact/Import/Parser/Contact.php
Original file line number Diff line number Diff line change
Expand Up @@ -1691,11 +1691,10 @@ public function createContact(&$formatted, &$contactFields, $onDuplicate, $conta
$this->formatParams($formatted, $onDuplicate, (int) $contactId);
}

// pass doNotResetCache flag since resetting and rebuilding cache could be expensive.
$config = CRM_Core_Config::singleton();
$config->doNotResetCache = 1;
// Resetting and rebuilding cache could be expensive.
CRM_Core_Config::setPermitCacheFlushMode(FALSE);
$cid = CRM_Contact_BAO_Contact::createProfileContact($formatted, $contactFields, $contactId, NULL, NULL, $formatted['contact_type']);
$config->doNotResetCache = 0;
CRM_Core_Config::setPermitCacheFlushMode(TRUE);

$contact = array(
'contact_id' => $cid,
Expand Down
20 changes: 20 additions & 0 deletions CRM/Core/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -546,4 +546,24 @@ public function handleFirstRun() {
Civi::settings()->set('installed', 1);
}

/**
* Is the system permitted to flush caches at the moment.
*/
static public function isPermitCacheFlushMode() {
return !CRM_Core_Config::singleton()->doNotResetCache;
}

/**
* Set cache clearing to enabled or disabled.
*
* This might be enabled at the start of a long running process
* such as an import in order to delay clearing caches until the end.
*
* @param bool $enabled
* If true then caches can be cleared at this time.
*/
static public function setPermitCacheFlushMode($enabled) {
CRM_Core_Config::singleton()->doNotResetCache = $enabled ? 0 : 1;
}

}
4 changes: 1 addition & 3 deletions CRM/Dedupe/Merger.php
Original file line number Diff line number Diff line change
Expand Up @@ -762,9 +762,7 @@ public static function merge($dupePairs = array(), $cacheParams = array(), $mode
$resultStats = array('merged' => array(), 'skipped' => array());

// we don't want dupe caching to get reset after every-merge, and therefore set the
// doNotResetCache flag
$config = CRM_Core_Config::singleton();
$config->doNotResetCache = 1;
CRM_Core_Config::setPermitCacheFlushMode(FALSE);
$deletedContacts = array();

while (!empty($dupePairs)) {
Expand Down
3 changes: 3 additions & 0 deletions bin/cli.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public function _accessing_from_cli() {
public function callApi() {
require_once 'api/api.php';

CRM_Core_Config::setPermitCacheFlushMode(FALSE);
// CRM-9822 -'execute' action always goes thru Job api and always writes to log
if ($this->_action != 'execute' && $this->_joblog) {
require_once 'CRM/Core/JobManager.php';
Expand All @@ -111,6 +112,8 @@ public function callApi() {
$this->_params['auth'] = FALSE;
$result = civicrm_api($this->_entity, $this->_action, $this->_params);
}
CRM_Core_Config::setPermitCacheFlushMode(TRUE);
CRM_Contact_BAO_Contact_Utils::clearContactCaches();

if (!empty($result['is_error'])) {
$this->_log($result['error_message']);
Expand Down

0 comments on commit eb24c31

Please sign in to comment.