Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement on dedupe find #15153

Closed
wants to merge 1 commit into from

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 29, 2019

Overview

Avoids an unlimited query when retrieving duplicates with a searchLimit set

Before

Unlimited query runs

After

Limited query runs

Technical Details

When we have a searchLimit this limits the number of contacts that we look for duplicates for.

When we have a searchLimit this limits the number of contacts that we look for duplicates for
however this is being ignored early on and we are retrieving an unlimited set of contacts
matching the criteria - so a criteria of id > 10 might return the whole db
to check against rather than the intended searchLimit of 50.

This is accessible via the api & the dedupetools extension leverages this for a non-quickform
dedupe experience but it's hitting performance issues

Comments

@pfigel are you able to look at this - I'm just accessing it via Dedupe.getduplicates & seeing this

We have been running this in production for a while

@civibot
Copy link

civibot bot commented Aug 29, 2019

(Standard links)

@civibot civibot bot added the master label Aug 29, 2019
When we have a searchLimit this limits the number of contacts that we look for duplicates for
however this is being ignored early on and we are retrieving an unlimited set of contacts
matching the criteria - so a criteria of id > 10 might return the whole db
to check against rather than the intended searchLimit of 50.

This is accessible via the api & the dedupetools extension leverages this for a  non-quickform
dedupe  experience but it's hitting  performance issues
@eileenmcnaughton
Copy link
Contributor Author

test this please

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 29, 2019
Alternative to
civicrm#15158
and

civicrm#15153

fixing both the  inconsistency & performance & making code more legible
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 29, 2019
Alternative to
civicrm#15158
and

civicrm#15153

fixing both the  inconsistency & performance & making code more legible
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 29, 2019
Alternative to
civicrm#15158
and

civicrm#15153

fixing both the  inconsistency & performance & making code more legible
@eileenmcnaughton eileenmcnaughton changed the title Minor performance improvement on dedupe find Performance improvement on dedupe find Aug 29, 2019
@eileenmcnaughton
Copy link
Contributor Author

I've put up an alternative here #15160

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 29, 2019
Alternative to
civicrm#15158
and

civicrm#15153

fixing both the  inconsistency & performance & making code more legible
@eileenmcnaughton
Copy link
Contributor Author

tes this please

@eileenmcnaughton eileenmcnaughton deleted the dedupe2 branch September 2, 2019 20:57
wmfgerrit pushed a commit to wikimedia/wikimedia-fundraising-crm that referenced this pull request Sep 3, 2019
This is intended to address the test fail in https://gerrit.wikimedia.org/r/#/c/wikimedia/fundraising/crm/civicrm/+/533134/

We need to have the right permission for the test to pass once the 'supporting bug' is fixed, and it turns out
we didn't

Fix  inconsistencies in duplicate retrieval

I've submitted this upstream at
civicrm/civicrm-core#15160

Alternative to
civicrm/civicrm-core#15158
and

civicrm/civicrm-core#15153

fixing both the  inconsistency & performance & making code more legible

Bug: T228826
Change-Id: Ie636d13ac05117b3fbce4c07e463c76e27fb764b
magnolia61 pushed a commit to magnolia61/civicrm-core that referenced this pull request Oct 13, 2019
Alternative to
civicrm#15158
and

civicrm#15153

fixing both the  inconsistency & performance & making code more legible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant