[Bug 41416] New: Poor performance when clicking 'Update hold(s)' on request.pl for records with many holds
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Bug ID: 41416 Summary: Poor performance when clicking 'Update hold(s)' on request.pl for records with many holds Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, lisette@bywatersolutions.com, tomascohen@gmail.com When updating holds priority or other fields on a record with many holds it can take a long time for the server to return. Looking at the code we call ModReserve for each hold on the page - ModReserve call _FixPriority for each hold - _FixPriority loops through every hold on the record an updates the priority to ensure correct ordering. The same outcome can be achieved in less time by calling _FixPriority a single time (per bib) after the loop -- 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=41416 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- 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=41416 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190383 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190383&action=edit Bug 41416: Add skip_fix_priority option to ModReserve This patch adds a new parameter to ModReserve: skip_fix_priority The page for altering priority ensures each hold has a unique rank, so doesn't allow for setting two holds to the same rank, so we don't need to reorder everything until the end (in case of some cancellations etc) This patch also makes FixPriority exported and updates the name as it is already called in a few places outside of C4:Reserve To test: 1 - Place many holds on a record, 50 - 150 https://github.com/kidclamp/handy-koha-script/blob/main/randhold.pl 2 - View the record in the staff interface: http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=312 3 - Click on the 'Holds' tab 4 - Adjust the priority of several holds using the dropdowns 5 - Open the network tab in the browser console (F12) to view the timing 6 - Click 'Update hold(s)' and note the time 7 - Apply patch, restart all 8 - Repeat 9 - Confirm timing is better and numbering updates correctly 10 - Sign off! Note: The script adds holds that may be marked as 'lowestPriority' these will always all group at the bottom of the record -- 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=41416 Katie Bliss <kebliss@dmpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kebliss@dmpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@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=41416 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 David Nind <david@davidnind.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=41416 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190383|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 190868 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190868&action=edit Bug 41416: Add skip_fix_priority option to ModReserve This patch adds a new parameter to ModReserve: skip_fix_priority The page for altering priority ensures each hold has a unique rank, so doesn't allow for setting two holds to the same rank, so we don't need to reorder everything until the end (in case of some cancellations etc) This patch also makes FixPriority exported and updates the name as it is already called in a few places outside of C4:Reserve To test: 1 - Place many holds on a record, 50 - 150 https://github.com/kidclamp/handy-koha-script/blob/main/randhold.pl 2 - View the record in the staff interface: http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=312 3 - Click on the 'Holds' tab 4 - Adjust the priority of several holds using the dropdowns 5 - Open the network tab in the browser console (F12) to view the timing 6 - Click 'Update hold(s)' and note the time 7 - Apply patch, restart all 8 - Repeat 9 - Confirm timing is better and numbering updates correctly 10 - Sign off! Note: The script adds holds that may be marked as 'lowestPriority' these will always all group at the bottom of the record Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Placing lots of holds: perl randhold.pl -b 312 -n 150 2. Before the patch, even just changing the priority of one hold (with the tick box selected) timed out after 5 mins with a server error: Proxy Error The proxy server received an invalid response from an upstream server. The proxy server could not handle the request 3. After the patch, the update worked and didn't timeout - 5 to 10 seconds for around 10 priority changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bibliothek@th-wildau.de --- Comment #4 from Jan Kissig <bibliothek@th-wildau.de> --- I wanted to take a look at this one and followed the test plan. In my case the changing of the priority did not work as expected neither before nor after applying the patch so it should not be related to this patch. I used a slightly modified version of your randhold.pl (https://gitlab.com/-/snippets/4923530) in order to have only readable patron names and only used a short amount of holds: - perl randhold.pl -b 262 -n 5 - the behavior is a bit obscure but take f.e. one patron and move its prio step by step down and check if the prio changes as expected and then move it up. I alway get stuck with one patron that would not get a higher prio. Maybe someone can confirm this. Again, this behavior does also occur before applying the patch and therefore should not interfere with it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 --- Comment #5 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to Jan Kissig from comment #4)
I wanted to take a look at this one and followed the test plan. In my case the changing of the priority did not work as expected neither before nor after applying the patch so it should not be related to this patch.
I used a slightly modified version of your randhold.pl (https://gitlab.com/-/snippets/4923530) in order to have only readable patron names and only used a short amount of holds:
- perl randhold.pl -b 262 -n 5 - the behavior is a bit obscure but take f.e. one patron and move its prio step by step down and check if the prio changes as expected and then move it up. I alway get stuck with one patron that would not get a higher prio.
Maybe someone can confirm this.
Again, this behavior does also occur before applying the patch and therefore should not interfere with it.
Can you confirm the 'stuck' holds aren't marked as 'lowestPriority'? These will always get sorted to the bottom before and after -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23269 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 --- Comment #6 from Jan Kissig <bibliothek@th-wildau.de> --- (In reply to Nick Clemens (kidclamp) from comment #5)
(In reply to Jan Kissig from comment #4)
I wanted to take a look at this one and followed the test plan. In my case the changing of the priority did not work as expected neither before nor after applying the patch so it should not be related to this patch.
I used a slightly modified version of your randhold.pl (https://gitlab.com/-/snippets/4923530) in order to have only readable patron names and only used a short amount of holds:
- perl randhold.pl -b 262 -n 5 - the behavior is a bit obscure but take f.e. one patron and move its prio step by step down and check if the prio changes as expected and then move it up. I alway get stuck with one patron that would not get a higher prio.
Maybe someone can confirm this.
Again, this behavior does also occur before applying the patch and therefore should not interfere with it.
Can you confirm the 'stuck' holds aren't marked as 'lowestPriority'? These will always get sorted to the bottom before and after
You were absolutely right. Some of the holds were randomly marked with the underlined double arrow (lowestPriority). Without any lowestPriority item the changing of priorities worked perfectly. So I learned something new from Koha today and maybe I shouldn't test features I seldom use ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190868|0 |1 is obsolete| | --- Comment #7 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 191481 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191481&action=edit Bug 41416: Add skip_fix_priority option to ModReserve This patch adds a new parameter to ModReserve: skip_fix_priority The page for altering priority ensures each hold has a unique rank, so doesn't allow for setting two holds to the same rank, so we don't need to reorder everything until the end (in case of some cancellations etc) This patch also makes FixPriority exported and updates the name as it is already called in a few places outside of C4:Reserve To test: 1 - Place many holds on a record, 50 - 150 https://github.com/kidclamp/handy-koha-script/blob/main/randhold.pl 2 - View the record in the staff interface: http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=312 3 - Click on the 'Holds' tab 4 - Adjust the priority of several holds using the dropdowns 5 - Open the network tab in the browser console (F12) to view the timing 6 - Click 'Update hold(s)' and note the time 7 - Apply patch, restart all 8 - Repeat 9 - Confirm timing is better and numbering updates correctly 10 - Sign off! Note: The script adds holds that may be marked as 'lowestPriority' these will always all group at the bottom of the record Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Jan Kissig <bibliothek@th-wildau.de> 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=41416 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 --- Comment #8 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Michelle Spinney <mspinney@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mspinney@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00 |26.05.00,25.11.02 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 --- Comment #9 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |Laura.escamilla@bywatersolu | |tions.com --- Comment #10 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- I am not backporting this as it does not apply cleanly to 25.05.x and would require additional adjustments specific to that branch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41416 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43017 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43017 [Bug 43017] [OMNIBUS] Interface optimization -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org