CiviCRM Community Forums (archive)

The current implementation of GroupContactCache attempts to create some illusion of consistency but actually only does if you assume a single user accessing Civi. This is the fundamental problem behind CRM-12466 and the reason why a call to loadAll with no arguments is made to try to ensure consistency.

In GroupContactCache::add the following workflow occurs:

a. remove all group cache entries for all groups being refreshed
b. for each group being refreshed refill this group

So any queries that attempt to use the GroupContactCache between a and b utilising a group that is being refreshed will get an inconsistent view of the cache. You stated earlier that you expect the cache behaviour to be consistent but if someone executes a search between a and b above then they will get an inconsistent view of the cache. You could reformulate the add function to do:

for each group:
   Lock cache table
   Remove all cache entries for group
   Refresh group entries
   end lock

which ensures that you have a consistent view of the groups. Note that you can't use row level locking because you are removing the rows here, which is unfortunate because locking the table for update is not really scalable.

Also in Contact_BAO_Query::addGroupContactCache there is a call to GroupContactCache::load($smartGroupId) which obviously populates the cache table at the point that the sql query is being constructed but doesn't do anything to ensure that it will still be valid at the point the query is actually executed.

Contact::delete calls Contact_BAO_query::remove. Unless smartGroupTimeout is set to 0 this will only refresh the groups that have timed out. This potentially leaves contact ids to the deleted contact in the GroupContact cache. Contact::delete should do something like DELETE from group_contact_cache WHERE contact_id=deleted_contact_id instead and then let the normal timeout mechanism refresh the smart groups (relationships mean that this might cause other members to be removed / added to groups) GroupContact::addContactToGroup has exactly the same issue.

In fact every call to GroupContactCache::remove() has similar issues!

Now the real question comes down to; should we actually be providing immediate consistency within the GroupContactCache? We have the smartGroupCache timeout variable which controls how frequently the cache gets refreshed, so given how difficult it is to actually provide consistency maybe just working to within the cache timeout is reasonable?

1. Likewise I don't see it but think something is there as I'm getting lots of timeouts due to the group_contact_cache table being locked.

2. The problem with incremental updates is its really hard to do it right; smart groups can encode complex relationships between groups and contacts so it becomes tricky to work out what invalidates what.

3. The real issue is about bounding the runtime; I don't want any of my page loads to return a 504 which is what is currently happening. Ideally I want a consistent view but practically an inconsistent view is probably ok. Basically what I currently have deployed is the 'dirty' option. I've removed the rebuild from the group tab and have had to change my cron frequency from every minute to every 15 minutes as the default is for the group cache to get rebuilt on every run, every hour is too infrequent as I have to run with a limit of 10 and I have over 500 groups. I had to go to 15 minutes because with 1 minute I end up with lock contention on the table which impacts page loads giving frequent 504's.