Bug 41416

Summary: Poor performance when clicking 'Update hold(s)' on request.pl for records with many holds
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Hold requestsAssignee: Nick Clemens (kidclamp) <nick>
Status: Pushed to main --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrew, bibliothek, david, dcook, gmcharlt, kebliss, lisette, mspinney, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23269
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
26.05.00
Circulation function:
Attachments: Bug 41416: Add skip_fix_priority option to ModReserve
Bug 41416: Add skip_fix_priority option to ModReserve
Bug 41416: Add skip_fix_priority option to ModReserve

Description Nick Clemens (kidclamp) 2025-12-09 18:58:50 UTC
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
Comment 1 Nick Clemens (kidclamp) 2025-12-09 19:07:36 UTC
Created attachment 190383 [details] [review]
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
Comment 2 David Nind 2026-01-03 20:27:05 UTC
Created attachment 190868 [details] [review]
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>
Comment 3 David Nind 2026-01-03 20:30:09 UTC
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.
Comment 4 Jan Kissig 2026-01-12 11:17:46 UTC
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.
Comment 5 Nick Clemens (kidclamp) 2026-01-13 14:51:12 UTC
(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
Comment 6 Jan Kissig 2026-01-13 17:11:37 UTC
(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 ;)
Comment 7 Jan Kissig 2026-01-15 06:28:15 UTC
Created attachment 191481 [details] [review]
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>
Comment 8 Lucas Gass (lukeg) 2026-01-16 21:11:10 UTC
Nice work everyone!

Pushed to main for 26.05