search

progressivetech / net.ourpowerbase.sumfields

The Summary Fields extension creates read-only custom data fields that extend Contact and are automatically populated with up-to-date totals such as total lifetime contributions, last contribution amount, last attended event, etc.
Other
8 stars 29 forks source link

performance improvements #11

Closed lcdservices closed 7 years ago

lcdservices commented 8 years ago

this is a very useful extension and I'd like to see it continue to grow and be refined in functionality.

per issue #7 we also have seen some inconsistencies relying on the triggers which necessitates running the update across the entire db. unfortunately, for a DB of moderate size (~72k contacts), the cron job takes incredibly long to complete -- sometimes so long we hit DB issues and the query crashes (it crashed after running for about 10hrs yesterday).

the contribution sql query is the main culprit. you've structured it so that it creates one very large insert query in which each column to be inserted is it's own calculation query (e.g. finding the sum or count of contributions for a specific time period). independently, each subquery can complete in a reasonable period of time (some longer than others); but when combined into a single massive query, it gets completely bogged down.

I did a quick and dirty refactor of the temp table insertion query so that we put a primary key index on contact_id, construct an array with each subquery, and then cycle through and process each subquery using an "insert... on duplicate key update" statement. this seems to improve the update performance considerably.

I had an immediately deadline I needed to get these updates processed for, so my fix was truly "quick and dirty" (I short-circuited your code and hard coded the list of queries we needed). but I think this would be a worthwhile improvement to the extension for you to consider.

colemanw commented 7 years ago

@lcdservices I pushed in some changes like the ones you described, here: https://github.com/progressivetech/net.ourpowerbase.sumfields/commit/4c859b35a36c1aedd7f63e520c8657e6e5eaa9dc#diff-ce496e47394f743bc436be959b9bba7eR463

Is that what you had in mind? If so we can close this issue.

lcdservices commented 7 years ago

yes, that's basically the gist of what I had done. has this been included in a recent release? I didn't implement and test the performance -- I only reviewed the code.

colemanw commented 7 years ago

Hasn't been released yet but the changes have been merged to master.

jmcclelland commented 7 years ago

Oops. Lack of new version was an oversite. I just tagged it, new version should be out tomorrow.