The Rapido ILL plugin [1] has a simple `after_hold_action` hook that checks if the current hold is linked to an ILL request [2], and in the positive case, it inserts a row on a `task_queue` table for the action to be processed asynchronously by a separate daemon/worker. Even though the plugin itself is not doing any heavy action on the DB, this causes DB locks that are easily reproducible. This seems to be a core Koha issue as it makes the holds cancelling process (and probably others) rather fragile. To reproduce: 1. Clone Rapido ILL: $ mkdir -p ~/git/koha-plugins && cd ~/git/koha-plugins $ git clone https://github.com/bywatersolutions/koha-plugin-rapido-ill.git rapido-ill 2. Start a KTD instance: $ ktd --proxy --name rapido --single-plugin ~/git/koha-plugins/rapido-ill up -d $ ktd --name rapido --wait-ready 200 3. Install the plugin config $ ktd --name rapido --shell k$ cd /kohadevbox/plugins/rapido-ill/scripts k$ perl bootstrap_rapido_testing.pl k$ restart_all 4. Create a hold for a known patron 5. Run this: k$ perl -e 'use Carp::Always; use Koha::Holds; my $hold = Koha::Holds->find(1);$hold->cancel({cancellation_reason => "OVER_365"})' => FAIL: It takes a while! It eventually fails with a lock wait timeout!: ``` C4::Reserves::_FixPriority(): DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction at /kohadevbox/koha/Koha/Hold.pm line 933 ``` => FACTS: There's a hold, being cancelled. There is no ILL request on the system. The only thing that is happening is that the plugin hook is being called, and is doing this query: ``` MariaDB [koha_cuyahoga]> SELECT me.* FROM illrequests me JOIN illrequestattributes ON illrequestattributes.illrequest_id = me.illrequest_id WHERE illrequestattributes.type = 'hold_id' AND illrequestattributes.value = '1' AND me.backend = 'RapidoILL'; Empty set (0.001 sec) ``` Moving the plugin hook outside the transaction fixes it. [1] https://github.com/bywatersolutions/koha-plugin-rapido-ill [2] Linked means checking if it has an extended attribute with the hold_id value. No FK checks.
*** This bug has been marked as a duplicate of bug 40220 ***
(In reply to Lucas Gass (lukeg) from comment #1) > > *** This bug has been marked as a duplicate of bug 40220 ***
Created attachment 191435 [details] [review] Bug 41603: Move after_hold_action plugin hook outside transaction The after_hold_action plugin hook is currently called from within the hold cancellation transaction in Koha::Hold->cancel(). This creates a fragile architecture where any plugin performing database queries can cause lock timeouts and transaction failures. The issue manifests when plugins query tables with foreign key relationships to biblio records. Even lightweight queries on seemingly unrelated tables (like illrequests) can trigger lock conflicts due to foreign key constraints that require shared locks on referenced rows. Since the plugin hook executes while the transaction holds locks on reserves/biblio tables, any plugin database access creates potential for circular lock dependencies and timeouts. This patch moves the plugin hook call outside the transaction boundary, ensuring that: - Core hold operations complete without plugin interference - Plugins receive the same data but execute after transaction commit - Database lock conflicts are eliminated - Plugin failures cannot corrupt core hold cancellation logic The hook receives identical data through a stored reference to the cancelled hold, maintaining full backward compatibility while eliminating the transaction fragility. Test plan: 1. Install any plugin that implements after_hold_action hook with database queries (e.g., ILL plugins checking hold relationships) 2. Create a hold and attempt to cancel it 3. Without patch: Operation may timeout with "Lock wait timeout exceeded" 4. With patch: Operation completes immediately 5. Verify plugin hook still receives correct hold data Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Created attachment 191794 [details] [review] Bug 41603: Move after_hold_action plugin hook outside transaction The after_hold_action plugin hook is currently called from within the hold cancellation transaction in Koha::Hold->cancel(). This creates a fragile architecture where any plugin performing database queries can cause lock timeouts and transaction failures. The issue manifests when plugins query tables with foreign key relationships to biblio records. Even lightweight queries on seemingly unrelated tables (like illrequests) can trigger lock conflicts due to foreign key constraints that require shared locks on referenced rows. Since the plugin hook executes while the transaction holds locks on reserves/biblio tables, any plugin database access creates potential for circular lock dependencies and timeouts. This patch moves the plugin hook call outside the transaction boundary, ensuring that: - Core hold operations complete without plugin interference - Plugins receive the same data but execute after transaction commit - Database lock conflicts are eliminated - Plugin failures cannot corrupt core hold cancellation logic The hook receives identical data through a stored reference to the cancelled hold, maintaining full backward compatibility while eliminating the transaction fragility. Test plan: 1. Install any plugin that implements after_hold_action hook with database queries (e.g., ILL plugins checking hold relationships) 2. Create a hold and attempt to cancel it 3. Without patch: Operation may timeout with "Lock wait timeout exceeded" 4. With patch: Operation completes immediately 5. Verify plugin hook still receives correct hold data Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases I tested with the KitchenSink plugin, adding an after_hold_action routine, and was able to cancel a hold placed in the staff interface both before and after from the interface and using: echo "UPDATE reserves SET reservedate='2025-01-01'" | sudo koha-mysql kohadev; perl misc/cronjobs/holds/cancel_unfilled_holds.pl -d 35 -v -c https://github.com/bywatersolutions/koha-plugin-rapido-ill/releases When I added the RapidoPLugin, the interface worked, but the command line failed After the patch all methods succeeded. It also makes sense to only send the hold to the plugin after the transaction has succeeded, and not during
Created attachment 191797 [details] [review] Bug 41603: Move after_hold_action plugin hook outside transaction The after_hold_action plugin hook is currently called from within the hold cancellation transaction in Koha::Hold->cancel(). This creates a fragile architecture where any plugin performing database queries can cause lock timeouts and transaction failures. The issue manifests when plugins query tables with foreign key relationships to biblio records. Even lightweight queries on seemingly unrelated tables (like illrequests) can trigger lock conflicts due to foreign key constraints that require shared locks on referenced rows. Since the plugin hook executes while the transaction holds locks on reserves/biblio tables, any plugin database access creates potential for circular lock dependencies and timeouts. This patch moves the plugin hook call outside the transaction boundary, ensuring that: - Core hold operations complete without plugin interference - Plugins receive the same data but execute after transaction commit - Database lock conflicts are eliminated - Plugin failures cannot corrupt core hold cancellation logic The hook receives identical data through a stored reference to the cancelled hold, maintaining full backward compatibility while eliminating the transaction fragility. Test plan: 1. Install any plugin that implements after_hold_action hook with database queries (e.g., ILL plugins checking hold relationships) 2. Create a hold and attempt to cancel it 3. Without patch: Operation may timeout with "Lock wait timeout exceeded" 4. With patch: Operation completes immediately 5. Verify plugin hook still receives correct hold data Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Nice work everyone! Pushed to main for 26.05
please add release notes
Thanks all, pushed to 25.11.x
I am not backporting this as it does not apply cleanly to 25.05.x.