[Bug 30004] New: Prevent TooMany from executing too many SQL queries
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Bug ID: 30004 Summary: Prevent TooMany from executing too many SQL queries Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: julian.maurice@biblibre.com QA Contact: testopia@bugs.koha-community.org If a maximum number of checkouts allowed is defined in circulation rules, C4::Circulation::TooMany will loop over all patron's checkouts. When a patron has several hundreds of checkouts, it can really slow down the checkout process by several seconds (or even tens of seconds) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 130106 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130106&action=edit Bug 30004: Prevent TooMany from executing too many SQL queries If a maximum number of checkouts allowed is defined in circulation rules, C4::Circulation::TooMany will loop over all patron's checkouts. When a patron has several hundreds of checkouts, it can really slow down the checkout process by several seconds (or even tens of seconds) This patch does two things: - Always prefetch item data so that `$c->item` does not execute an additional SQL query at every iteration of the loop. Item data is always needed at the first line of the loop, so there is really no downside for doing this. - Build the `@types` array only once, out of the checkouts loop. Since it does not depend at all on patron's checkouts data, it does not make sense to build it inside the loop. Test plan: 1. Before applying the patch, create a patron with a lot of checkouts. I tested with 1000 checkouts, but the slowness should be noticeable with less. 2. Make sure you have a circulation rule (one that apply to your patron and the item(s) you will check out for testing) with a maximum number of checkouts allowed 3. Check out an item for the patron with a lot of checkouts. Measure the time it takes. 4. Apply the patch 5. Check out another item (or check in and then check out the same item used in step 3). Measure the time it takes and compare it to step 3. It should be faster now. 6. Run `prove t/db_dependent/Circulation/TooMany.t` -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 --- Comment #2 from Julian Maurice <julian.maurice@biblibre.com> --- This problem affects at least versions 20.11, 21.05, 21.11, and master -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |julian.maurice@biblibre.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Severity|enhancement |major Blocks| |7923 --- Comment #3 from Fridolin Somers <fridolin.somers@biblibre.com> --- Performance issue so i change from enhancement to major bug Good catch Julian ;) Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7923 [Bug 7923] Performance omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- Is there an easy way to generate 1,000 checkouts for a patron? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to David Nind from comment #4)
Is there an easy way to generate 1,000 checkouts for a patron?
You can create multiple items at once by clicking on "Add multiple copies" button when creating an item. For checkouts, there is the "batch check out" feature. (Maybe do not create 1000 checkouts at once as it can take a lot of time, but by batches of 100 it should be doable) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130106|0 |1 is obsolete| | --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 131066 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131066&action=edit Bug 30004: Prevent TooMany from executing too many SQL queries If a maximum number of checkouts allowed is defined in circulation rules, C4::Circulation::TooMany will loop over all patron's checkouts. When a patron has several hundreds of checkouts, it can really slow down the checkout process by several seconds (or even tens of seconds) This patch does two things: - Always prefetch item data so that `$c->item` does not execute an additional SQL query at every iteration of the loop. Item data is always needed at the first line of the loop, so there is really no downside for doing this. - Build the `@types` array only once, out of the checkouts loop. Since it does not depend at all on patron's checkouts data, it does not make sense to build it inside the loop. Test plan: 1. Before applying the patch, create a patron with a lot of checkouts. I tested with 1000 checkouts, but the slowness should be noticeable with less. 2. Make sure you have a circulation rule (one that apply to your patron and the item(s) you will check out for testing) with a maximum number of checkouts allowed 3. Check out an item for the patron with a lot of checkouts. Measure the time it takes. 4. Apply the patch 5. Check out another item (or check in and then check out the same item used in step 3). Measure the time it takes and compare it to step 3. It should be faster now. 6. Run `prove t/db_dependent/Circulation/TooMany.t` Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30167 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Compared a call to CanBookBeIssued in a pl script, with 960 checkouts: Without patch: 4.3s With patch : 1.7s -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131066|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131972&action=edit Bug 30004: Prevent TooMany from executing too many SQL queries If a maximum number of checkouts allowed is defined in circulation rules, C4::Circulation::TooMany will loop over all patron's checkouts. When a patron has several hundreds of checkouts, it can really slow down the checkout process by several seconds (or even tens of seconds) This patch does two things: - Always prefetch item data so that `$c->item` does not execute an additional SQL query at every iteration of the loop. Item data is always needed at the first line of the loop, so there is really no downside for doing this. - Build the `@types` array only once, out of the checkouts loop. Since it does not depend at all on patron's checkouts data, it does not make sense to build it inside the loop. Test plan: 1. Before applying the patch, create a patron with a lot of checkouts. I tested with 1000 checkouts, but the slowness should be noticeable with less. 2. Make sure you have a circulation rule (one that apply to your patron and the item(s) you will check out for testing) with a maximum number of checkouts allowed 3. Check out an item for the patron with a lot of checkouts. Measure the time it takes. 4. Apply the patch 5. Check out another item (or check in and then check out the same item used in step 3). Measure the time it takes and compare it to step 3. It should be faster now. 6. Run `prove t/db_dependent/Circulation/TooMany.t` Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Nind from comment #4)
Is there an easy way to generate 1,000 checkouts for a patron?
The following code will checkout all the items from the DB to the patron with borrowernumber=51 use Modern::Perl; use C4::Circulation qw( AddIssue ); use Koha::Items; use Koha::Patrons; use t::lib::Mocks; my $patron = Koha::Patrons->find(51); t::lib::Mocks::mock_userenv({patron => $patron}); my $items = Koha::Items->search; my $patron_unblessed = $patron->unblessed; while ( my $i = $items->next ) { AddIssue($patron_unblessed, $i->barcode); } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 --- Comment #10 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|22.05.00 |22.05.00,21.11.05 released in| | CC| |kyle@bywatersolutions.com --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.11.x for 21.11.05 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org