dev/core#367: Query optimization for A-Z pager by adding indices to t…

[twomice]

…emp table.

Overview

As described at https://lab.civicrm.org/dev/core/issues/367, sites with large numbers of "spouse" relationships (or other reciprocally named relationship types) can encounter WSOD and/or minutes-long wait times when performing an Advanced Search on that relationship type with a "target contact is in group" criteria.

Before

As described in the ticket repro steps, a particular set of criteria for Advanced Search, performed on a sufficiently large contact base, results in either a minutes-long wait for search results, or a fatal error "Sorry, due to an error, we are unable to fulfill your request at the moment. You may want to contact your administrator or service provider with more details about what action you were performing when this occurred.
DB Error: unknown error"

After

On the same site with the same search criteria, wait time is reduced to mere seconds.

Technical Details

Just adding a couple of indices and matching column definitions (e,g, unsigned int 10) for columns that need them in the temporary table that's used to flatten relationships before searching.

Comments

Kinda hard to reproduce considering you need a fairly large contact base. Also not sure how we could test for 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]

@twomice don't you need to have a column for the civicrm_relationship_id as well?)

[twomice]

I guess not. I mean, it's been working just fine without that column so far.

[eileenmcnaughton]

So I'm OK with the testing @twomice has done to cover the performance side - given this is logical.

Was there a unit test from last time that actually covers that code @twomice ?

[twomice]

@eileenmcnaughton If by "last time" you mean https://issues.civicrm.org/jira/browse/CRM-21811 and its PR #11732, yes there was a unit test in that PR, and that test covers the search criteria mentioned here; worth noting though, that test will never really hit this issue on a normal jenkins test site, which just won't have enough contacts to hit the performance limitations.

[eileenmcnaughton]

@twomice right - so there are 2 forms of testing

1. it doesnt break anything
2. it performs well

I'm Ok to take your testing on 2 since it also looks like the right approach.

If jenkins is doing 1 I'm happy to merge

[twomice]

Thanks @eileenmcnaughton !