dev/core#1862 Skip cache tables during merge, rely on cache management processes

[eileenmcnaughton]

Overview

Fixes a dedupe regression on cache tables by not merging cache tables & relying on 'normal cache management'

Before

Potential for reproducible fatal error when merging contacts that are both in the same smart group.

• replicated in attached test

After

Problematic queries are not run -- this also reduces the total number of potentially locking queries

Technical Details

We later call Contact.create on both contacts so that should manage the caches for this update 'as well as for any other update'
and solve the bug + reduce locking queries

Comments

Thanks to @JKingsnorth for reporting this

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

This looks good to me merge on pass

[seamuslee001]

Jenkins re test this please

[JKingsnorth]

@eileenmcnaughton Thanks very much for posting this.

I'll run through a full test of this now.

[JKingsnorth]

@eileenmcnaughton I've found a related issue which I've documented on the GitLab issue: https://lab.civicrm.org/dev/core/-/issues/1862

[JKingsnorth]

@eileenmcnaughton I agree with the principle of this. Although the smart group lists will no longer be 'immediately' updated, this isn't a problem. Letting the cache-rebuild fix these up, the next time the caches are re-built, makes sense.

[eileenmcnaughton]

@JKingsnorth I think it might happen sooner since there is still stuff in contact.create for it

[JKingsnorth]

@eileenmcnaughton in my test, it did not. But we are using deterministic cache flush, I didn't check with opportunistic.

[eileenmcnaughton]

@JKingsnorth right - so you have opted out of aggressive cache flushing - that makes sense

[JKingsnorth]

@eileenmcnaughton For completeness, I'll test with 'opportunistic' mode tomorrow.

[eileenmcnaughton]

Thanks @JKingsnorth

[JKingsnorth]

(TLDR - it's all good.)

If you merge contacts, smart group associations are not carried across like they used to be, which is expected.

If you are using Civi's defaults of 'opportunistic' cache flushing, and smart group timeout of '5' then the merged contact is not in any new smart groups it would be eligible for (expected). Likewise if you are using 'deterministic' cache flushing.

If you use 'opportunistic' cache flushing, and set your smart group timeout set to '0' (which would be madness) then the merged contact is added to any smart groups they 'become' eligible for. This means that the merge process does an opportunistic flush of the cache, which is what you thought.

[eileenmcnaughton]

@JKingsnorth funnily enough we actually do set smart group timeout to 0 - to be honest I'm not sure if it's better or worse but I tried it out after some deadlocks & we never turned it back on. I know @andrew-cormick-dockery at one point disabled it for AUG & I think was positive about the impact