Summary: | Patron details page slow to load when many checkouts present | ||
---|---|---|---|
Product: | Koha | Reporter: | Ian Walls <koha.sekjal> |
Component: | Architecture, internals, and plumbing | Assignee: | Ian Walls <koha.sekjal> |
Status: | CLOSED FIXED | QA Contact: | Bugs List <koha-bugs> |
Severity: | major | ||
Priority: | PATCH-Sent (DO NOT USE) | CC: | chris, wizzyrea |
Version: | 3.6 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Proposed Patch
Second Proposed Patch Bug 6801: checkoverdues returns unnecessary fields, causing slowness [SIGNED-OFF] Bug 6801: checkoverdues returns unnecessary fields, causing slowness |
Description
Ian Walls
2011-08-28 02:53:57 UTC
Created attachment 5244 [details] [review] Proposed Patch Removes biblioitems.marc and biblioitems.marcxml from the checkoverdues SQL query. 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* 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. Updating the bug report title to reflect the real problem; solution forthcoming. Created attachment 5541 [details] [review] Second Proposed Patch This patch extends the work done by the first: Adds borrower firstname, surname and cardnumber to GetPendingIssues() return value. Removes calls to GetMemberDetails in build_issue_data subroutines of members/moremember.pl and circ/circulation.pl Change templates to use "[% scope.firstname %] [% scope.surname %]" instead of [% scope.borrowername %] Changes borrowernumber source in build_issue_data()'s checks for GetIssuingCharges() and CanBookBeRenewed(). Instead of using the borrowernumber for the page, use the borrowernumber for that specific issue. In current usage, this will be no different, but in theory for any Relatives Checkouts making use of this data, the issuing charges and renewal permission could be incorrect. Created attachment 5544 [details] [review] 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. 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. ;) 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. 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> 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. Appears to be working fine. Closing. |