From cc7b1cd99a121008d7b2cdfab2fca4a1dc682bd5 Mon Sep 17 00:00:00 2001 From: eileenmcnaugton Date: Mon, 16 May 2016 13:44:58 +1200 Subject: [PATCH] CRM-16642 Switch to opportunisticCacheRefresh calls. There is one call left to the function called 'remove' which needs a little more consideration --- CRM/Activity/BAO/Activity.php | 3 +- CRM/Contact/BAO/Contact.php | 5 +- CRM/Contact/BAO/Contact/Utils.php | 3 +- CRM/Contact/BAO/GroupContact.php | 8 +++- CRM/Contact/BAO/GroupContactCache.php | 56 ++++++++++++++++------- CRM/Contact/Form/CustomData.php | 3 +- CRM/Contact/Form/Inline/CustomData.php | 3 +- CRM/Contact/Page/AJAX.php | 3 +- CRM/Contribute/BAO/Contribution.php | 3 +- CRM/Core/BAO/EntityTag.php | 8 +--- CRM/Event/BAO/Participant.php | 3 +- CRM/Member/BAO/Membership.php | 2 +- CRM/Upgrade/Incremental/php/FourSeven.php | 3 +- 13 files changed, 61 insertions(+), 42 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index ea3077a2e6..d4027444e5 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -584,8 +584,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { } } - // reset the group contact cache since smart groups might be affected due to this - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); if (!empty($params['id'])) { CRM_Utils_Hook::post('edit', 'Activity', $activity->id, $activity); diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index 309d552023..0e9dc2b34b 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -935,7 +935,7 @@ WHERE civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer'); CRM_Core_DAO::executeQuery('DELETE FROM civicrm_acl_contact_cache WHERE contact_id = %1', array(1 => array($contactID, 'Integer'))); } else { - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); } } @@ -1932,8 +1932,7 @@ ORDER BY civicrm_email.is_primary DESC"; CRM_Contact_BAO_GroupContact::addContactsToGroup($contactIds, $addToGroupID); } - // reset the group contact cache for this group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); if ($editHook) { CRM_Utils_Hook::post('edit', 'Profile', $contactID, $params); diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php index 5324795647..5eaf2fa867 100644 --- a/CRM/Contact/BAO/Contact/Utils.php +++ b/CRM/Contact/BAO/Contact/Utils.php @@ -910,8 +910,7 @@ Group By componentId"; CRM_Core_BAO_PrevNextCache::deleteItem(); } - // reset the group contact cache for this group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); } /** diff --git a/CRM/Contact/BAO/GroupContact.php b/CRM/Contact/BAO/GroupContact.php index fabed592d1..654b451380 100644 --- a/CRM/Contact/BAO/GroupContact.php +++ b/CRM/Contact/BAO/GroupContact.php @@ -148,7 +148,7 @@ class CRM_Contact_BAO_GroupContact extends CRM_Contact_DAO_GroupContact { // reset the group contact cache for all group(s) // if this group is being used as a smart group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); CRM_Utils_Hook::post('create', 'GroupContact', $groupId, $contactIds); @@ -250,6 +250,12 @@ class CRM_Contact_BAO_GroupContact extends CRM_Contact_DAO_GroupContact { // 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 opportunisticCacheRefresh() - 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_Utils_Hook::post($op, 'GroupContact', $groupId, $contactIds); diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php index 8cbea8e0e8..febcee0e97 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -290,6 +290,12 @@ WHERE id IN ( $groupIDs ) * cache date, i.e. the removal is not done if the group was recently * loaded into the cache. * + * 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. + * * @param int $groupID * the groupID to delete cache entries, NULL for all groups. * @param bool $onceOnly @@ -411,12 +417,23 @@ WHERE id = %1 * clear. */ protected static function refreshCaches() { - if (self::isRefreshAlreadyInitiated()) { + try { + $lock = self::getLockForRefresh(); + } + catch (CRM_Core_Exception $e) { + // Someone else is kindly doing the refresh for us right now. return; } - $params = array(1 => array(self::getCacheInvalidDateTime(), 'String')); - + // @todo this is consistent with previous behaviour but as the first query could take several seconds the second + // could become inaccurate. It seems to make more sense to fetch them first & delete from an array (which would + // also reduce joins). If we do this we should also consider how best to iterate the groups. If we do them one at + // a time we could call a hook, allowing people to manage the frequency on their groups, or possibly custom searches + // might do that too. However, for 2000 groups that's 2000 iterations. If we do all once we potentially create a + // slow query. It's worth noting the speed issue generally relates to the size of the group but if one slow group + // is in a query with 500 fast ones all 500 get locked. One approach might be to calculate group size or the + // number of groups & then process all at once or many query runs depending on what is found. Of course those + // preliminary queries would need speed testing. CRM_Core_DAO::executeQuery( " DELETE gc @@ -427,15 +444,18 @@ WHERE id = %1 $params ); + // Clear these out without resetting them because we are not building caches here, only clearing them, + // so the state is 'as if they had never been built'. CRM_Core_DAO::executeQuery( " UPDATE civicrm_group g - SET cache_date = null, - refresh_date = NOW() + SET cache_date = NULL, + refresh_date = NULL WHERE g.cache_date <= %1 ", $params ); + $lock->release(); } /** @@ -446,16 +466,24 @@ WHERE id = %1 * 2) a mysql lock. This works fine as long as CiviMail is not running, or if mysql is version 5.7+ * * Where these 2 locks fail we get 2 processes running at the same time, but we have at least minimised that. + * + * @return \Civi\Core\Lock\LockInterface + * @throws \CRM_Core_Exception */ - protected static function isRefreshAlreadyInitiated() { - static $invoked = FALSE; - if ($invoked) { - return TRUE; + protected static function getLockForRefresh() { + if (!isset(Civi::$statics[__CLASS__])) { + Civi::$statics[__CLASS__] = array('is_refresh_init' => FALSE); + } + + if (Civi::$statics[__CLASS__]['is_refresh_init']) { + throw new CRM_Core_Exception('A refresh has already run in this process'); } $lock = Civi::lockManager()->acquire('data.core.group.refresh'); - if (!$lock->isAcquired()) { - return TRUE; + if ($lock->isAcquired()) { + Civi::$statics[__CLASS__]['is_refresh_init'] = TRUE; + return $lock; } + throw new CRM_Core_Exception('Mysql lock unavailable'); } /** @@ -463,11 +491,9 @@ WHERE id = %1 * * Sites that do not run the smart group clearing cron job should refresh the caches under an opportunistic mode, akin * to a poor man's cron. The user session will be forced to wait on this so it is less desirable. - * - * @return bool */ public static function opportunisticCacheRefresh() { - if (Civi::settings()->get('contact_smart_group_display') == 'opportunistic') { + if (Civi::settings()->get('smart_group_cache_refresh_mode') == 'opportunistic') { self::refreshCaches(); } } @@ -476,8 +502,6 @@ WHERE id = %1 * Do a forced cache refresh. * * This function is appropriate to be called by system jobs & non-user sessions. - * - * @return bool */ public static function deterministicCacheRefresh() { if (self::smartGroupCacheTimeout() == 0) { diff --git a/CRM/Contact/Form/CustomData.php b/CRM/Contact/Form/CustomData.php index 24feb13a81..187d5fdbb6 100644 --- a/CRM/Contact/Form/CustomData.php +++ b/CRM/Contact/Form/CustomData.php @@ -309,8 +309,7 @@ class CRM_Contact_Form_CustomData extends CRM_Core_Form { $this->ajaxResponse += CRM_Contact_Form_Inline::renderFooter($this->_tableID); } - // reset the group contact cache for this group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); } } diff --git a/CRM/Contact/Form/Inline/CustomData.php b/CRM/Contact/Form/Inline/CustomData.php index 4570b75cc8..a21e5e1497 100644 --- a/CRM/Contact/Form/Inline/CustomData.php +++ b/CRM/Contact/Form/Inline/CustomData.php @@ -97,8 +97,7 @@ class CRM_Contact_Form_Inline_CustomData extends CRM_Contact_Form_Inline { $this->log(); - // reset the group contact cache for this group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); $this->response(); } diff --git a/CRM/Contact/Page/AJAX.php b/CRM/Contact/Page/AJAX.php index c4a27de65b..00aa2051ee 100644 --- a/CRM/Contact/Page/AJAX.php +++ b/CRM/Contact/Page/AJAX.php @@ -294,8 +294,7 @@ class CRM_Contact_Page_AJAX { echo CRM_Contact_BAO_Contact::getCountComponent('custom_' . $customGroupID, $contactId); } - // reset the group contact cache for this group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); CRM_Utils_System::civiExit(); } diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 1384065471..94a3388526 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -212,8 +212,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { ); } - // reset the group contact cache for this group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); if ($contributionID) { CRM_Utils_Hook::post('edit', 'Contribution', $contribution->id, $contribution); diff --git a/CRM/Core/BAO/EntityTag.php b/CRM/Core/BAO/EntityTag.php index 10182decce..f3bed639a7 100644 --- a/CRM/Core/BAO/EntityTag.php +++ b/CRM/Core/BAO/EntityTag.php @@ -168,9 +168,7 @@ class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag { $object = array($entityIdsAdded, $entityTable); CRM_Utils_Hook::post('create', 'EntityTag', $tagId, $object); - // reset the group contact cache for all groups - // if tags are being used in a smart group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); return array(count($entityIds), $numEntitiesAdded, $numEntitiesNotAdded); } @@ -244,9 +242,7 @@ class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag { $object = array($entityIdsRemoved, $entityTable); CRM_Utils_Hook::post('delete', 'EntityTag', $tagId, $object); - // reset the group contact cache for all groups - // if tags are being used in a smart group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); return array(count($entityIds), $numEntitiesRemoved, $numEntitiesNotRemoved); } diff --git a/CRM/Event/BAO/Participant.php b/CRM/Event/BAO/Participant.php index 60ccb75cd7..d0b2b0f5d7 100644 --- a/CRM/Event/BAO/Participant.php +++ b/CRM/Event/BAO/Participant.php @@ -138,8 +138,7 @@ class CRM_Event_BAO_Participant extends CRM_Event_DAO_Participant { $session = CRM_Core_Session::singleton(); - // reset the group contact cache for this group - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); if (!empty($params['id'])) { CRM_Utils_Hook::post('edit', 'Participant', $participantBAO->id, $participantBAO); diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index 52e79207db..0e4333d5c7 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -137,7 +137,7 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership { CRM_Member_BAO_MembershipLog::add($membershipLog, CRM_Core_DAO::$_nullArray); // reset the group contact cache since smart groups might be affected due to this - CRM_Contact_BAO_GroupContactCache::remove(); + CRM_Contact_BAO_GroupContactCache::opportunisticCacheRefresh(); if ($id) { if ($membership->status_id != $oldStatus) { diff --git a/CRM/Upgrade/Incremental/php/FourSeven.php b/CRM/Upgrade/Incremental/php/FourSeven.php index c4be97e4a3..0da96b8ed3 100644 --- a/CRM/Upgrade/Incremental/php/FourSeven.php +++ b/CRM/Upgrade/Incremental/php/FourSeven.php @@ -208,7 +208,7 @@ class CRM_Upgrade_Incremental_php_FourSeven extends CRM_Upgrade_Incremental_Base public function upgrade_4_7_8($rev) { $this->addTask(ts('Upgrade DB to %1: SQL', array(1 => $rev)), 'runSql', $rev); $this->addTask('Upgrade mailing foreign key constraints', 'upgradeMailingFKs'); - $this->addSmartGroupRefreshOptions(); + $this->addTask('Add Smartgroup refresh options', 'addSmartGroupRefreshOptions'); } /* @@ -723,6 +723,7 @@ FROM `civicrm_dashboard_contact` JOIN `civicrm_contact` WHERE civicrm_dashboard_ 'filter' => 1, 'is_reserved' => 1, )); + return TRUE; } } -- 2.25.1