[Bug 36672] New: Circulation rules are performing too many lookups
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Bug ID: 36672 Summary: Circulation rules are performing too many lookups Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: System Administration Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com See bug 36668 - it tackles the size of the circulation page but doesn't significantly increase load time. Profiling it seems we spend about 50% of the loading time on Koha::Template::Plugin::CirculationRules::Search Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name. In systems with large numbers of categories and item types this becomes very slow. In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page. Additionally, we don't need to use all category/item types, we can use the 'used_categories' array from bug 36447 to only attempt categories that we know have rules -- 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=36672 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- 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=36672 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36673 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 165685 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165685&action=edit Bug 36672: Circulation rules are performing too many lookups Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name. In systems with large numbers of categories and item types this becomes very slow. In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page. Test Plan: 1) Apply this patch 2) For each rule section, create, update and delete a rule 3) No change in behavior should be noted! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165685|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 165687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165687&action=edit Bug 36672: Circulation rules are performing too many lookups Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name. In systems with large numbers of categories and item types this becomes very slow. In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page. Test Plan: 1) Apply this patch 2) For each rule section, create and delete a rule 3) No change in behavior should be noted! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Catrina Berka <catrina@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |catrina@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36668 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Pedro Amorim <pedro.amorim@ptfs-europe.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=36672 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165687|0 |1 is obsolete| | --- Comment #3 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 166151 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166151&action=edit Bug 36672: Circulation rules are performing too many lookups Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name. In systems with large numbers of categories and item types this becomes very slow. In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page. Test Plan: 1) Apply this patch 2) For each rule section, create and delete a rule 3) No change in behavior should be noted! Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #4 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Testing notes: Used Kyle's script bug 36668 (8,368 entries in * table ) ----- http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl: Before patch: Time: 20.72s | 25.27s | 26.21s | 21.68s -> 23.47s avg After patch: Time: 17.24s | 22.31s | 19.96s | 16.80s -> 19.07s avg Result: ~20% performance improvement ----- ----- http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl?branch=FFL (with a single rule entry) Before patch: Time 5.94s | 5.72s | 5.86s | 5.67s After patch: Time: 1.88s | 1.68 | 1.71s | 1.65s Result: Lightning fast ----- Adding and removing rules all appears to be working as expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166151|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 167393 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167393&action=edit Bug 36672: Circulation rules are performing too many lookups Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name. In systems with large numbers of categories and item types this becomes very slow. In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page. Test Plan: 1) Apply this patch 2) For each rule section, create and delete a rule 3) No change in behavior should be noted! Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com CC| |martin.renvoize@ptfs-europe | |.com Status|Signed Off |Passed QA --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice performance improvement, thanks. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.11.00 |24.11.00,24.05.02 released in| | Status|Pushed to main |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #8 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 24.05.x for upcoming 24.05.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Andrii Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com --- Comment #9 from Andrii Nugged <nugged@gmail.com> --- IMO We have a bug when elements with "All" (*) don't shown for the user and even die with a 500 error: Because smart-rules.tt have this removed: - [% SET c = '*' UNLESS c.defined AND c != '' %] ... - [% SET i = '*' UNLESS i.defined AND i != '' %] this makes wrong logic in [% IF i == '*' %] <em>All</em> and [% IF c == '*' %] <em>All</em> If to return lines after "SET waiting_hold_cancellation..": [% SET waiting_hold_cancellation = all_rules.$c.$i.waiting_hold_cancellation %] + [% SET c = '*' UNLESS c.defined AND c != '' %] + [% SET i = '*' UNLESS i.defined AND i != '' %] [% IF ( waiting_hold_cancellation.defined && waiting_hold_cancellation != '' ) %] It works normally, ... but I just did the preliminary test, so I suspect it. I will test more. But maybe my insight will give the authors a hint, and this can be confirmed faster (I am now in the middle of the travel). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #10 from Fridolin Somers <fridolin.somers@biblibre.com> --- I prefer not to backport on oldstable 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Text to go in the| |This improves the release notes| |performance of the | |circulation and rules page | |by reducing the number of | |lookups. This should | |improve the page loading | |times (including when | |editing and saving) when a | |library has many categories | |and item types. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |37255 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37255 [Bug 37255] Creating default waiting hold cancellation policy for all patron categories and itemtypes breaks Koha -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |37263 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37263 [Bug 37263] Creating default article request fees is not working -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36672 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org