Bug 6801 - Patron details page slow to load when many checkouts present
Summary: Patron details page slow to load when many checkouts present
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) major (vote)
Assignee: Ian Walls
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-28 02:53 UTC by Ian Walls
Modified: 2013-12-05 19:53 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed Patch (2.32 KB, patch)
2011-08-31 18:46 UTC, Ian Walls
Details | Diff | Splinter Review
Second Proposed Patch (10.24 KB, patch)
2011-09-22 15:03 UTC, Ian Walls
Details | Diff | Splinter Review
Bug 6801: checkoverdues returns unnecessary fields, causing slowness (12.67 KB, patch)
2011-09-22 15:37 UTC, Liz Rea
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6801: checkoverdues returns unnecessary fields, causing slowness (12.74 KB, patch)
2011-09-26 20:15 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Walls 2011-08-28 02:53:57 UTC
checkoverdues is used solely by C4::Members::patronflags to determine if a borrower has any overdues; if so, the ODUE flag is added to their and the list of overdues returned.  This winds up being fed to GetMemberDetails, and some of the calls then use the overdue items... sometimes.

Removing the marc and marcxml blobs would greatly speed up the return of this function, which can cause slowness for patrons with many, many checkouts (or with relatives with many, many checkouts).
Comment 1 Ian Walls 2011-08-31 18:46:15 UTC Comment hidden (obsolete)
Comment 2 Liz Rea 2011-08-31 19:27:05 UTC
I'm sad to report that a problematic patron (one with 73 checkouts, all overdue) still takes over a minute to load, even with this patch.

*sadface*
Comment 3 Ian Walls 2011-08-31 22:46:59 UTC
More research has led to the REAL root of the problem Liz is experiencing; it's not so much that checkoverdues is inefficient, but that it's being called WAY too often now that Relatives Checkouts are in place (it's called once for every issue checked out to the person or to their relatives).

On this particular issue, however, looking at how the returned values of checkoverdues are actually used, we only need to return itemnumber and date_due from the issues table (or anything extra we'd like to show over a SIP2 connection, once that functionality is added).  This would greatly shorten the query time, which is never a bad thing.
Comment 4 Ian Walls 2011-09-22 11:28:40 UTC
Updating the bug report title to reflect the real problem; solution forthcoming.
Comment 5 Ian Walls 2011-09-22 15:03:37 UTC Comment hidden (obsolete)
Comment 6 Liz Rea 2011-09-22 15:37:16 UTC Comment hidden (obsolete)
Comment 7 Liz Rea 2011-09-22 15:38:19 UTC
Squashed these two together at Ian's request. They both get my sign off.

One other sign off might be nice, since Ian wrote this thing. ;)
Comment 8 Ian Walls 2011-09-22 15:48:33 UTC
Another set of eyes is most thoroughly appreciated; I'll refrain from pushing this through QA immediately until someone else has had the opportunity to test.
Comment 9 Katrin Fischer 2011-09-26 20:15:10 UTC
Created attachment 5598 [details] [review]
[SIGNED-OFF] Bug 6801: checkoverdues returns unnecessary fields, causing slowness

Explicitly specifies which fields to return in C4::Overdues::checkoverdues
SQL:  all of biblio, items, and issues, and everything in biblioitems
EXCEPT marc, marcxml and timestamp.

Bug 6801: member details page taking long time to load when many checkouts present

This patch removes the call to GetMemberDetails in build_issue_data; this heavy-weight
subroutine was being run for every single item a patron (or their relatives) have checked out.
Instead, the borrowers first name, surname and cardnumber are added to the GetPendingIssues query.
I believe this is reasonable since GetPendingIssues can now return issues for multiple borrowers.

Also corrects the $borrowernumber used for GetIssuesCharges and CanItemBeRenewed; was using the borrower whose
page we were on, NOT the borrower of that specific item (which would be different in the Relatives Checkouts tab).

Template calls to [% scope.borrowername %] are now broken up into [% scope.firstname %] [% scope.surname %].

Signed-off-by: Liz Rea <lrea@nekls.org>
On my test data, a patron with 180 checkouts (without this patch) would take more than a minute to bring back the circulation.pl and moremember.pl pages.
With this patch, the time is reduced to 5 or so seconds.

Big ups to Ian for tenaciously hunting this one down.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 10 Chris Cormack 2011-09-26 20:52:25 UTC
Pushed to master, with a borrower with 286 checkouts the load time dropped from 
108858ms to 4075ms.
This was after restarting mysql to get rid of any query caching. A dramatic improvement.
Comment 11 Jared Camins-Esakov 2012-05-23 12:27:44 UTC
Appears to be working fine. Closing.