[Bug 41603] New: Plugin hook causing DB locks when cancelling holds
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Bug ID: 41603 Summary: Plugin hook causing DB locks when cancelling holds Initiative type: --- Sponsorship --- status: Product: Koha Version: 24.11 Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Plugin architecture Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org 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. -- 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=41603 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|24.11 |Main CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.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=41603 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=38384 CC| |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=41603 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisette@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=41603 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |DUPLICATE CC| |lucas@bywatersolutions.com --- Comment #1 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- *** This bug has been marked as a duplicate of bug 40220 *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|DUPLICATE |--- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |ASSIGNED --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Lucas Gass (lukeg) from comment #1)
*** This bug has been marked as a duplicate of bug 40220 ***
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 191435 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191435&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Daniel B. <dbarden@cuyahogalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dbarden@cuyahogalibrary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 lthorrat@cuyahogalibrary.org <lthorrat@cuyahogalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lthorrat@cuyahogalibrary.or | |g -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Chip Halvorsen <Chip.Halvorsen@WestlakeLibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Chip.Halvorsen@WestlakeLibr | |ary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Kristi Krueger <kkrueger@cuyahogalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kkrueger@cuyahogalibrary.or | |g -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Nick Clemens (kidclamp) <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=41603 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #191435|0 |1 is obsolete| | --- Comment #4 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 191794 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191794&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 --- Comment #5 from Nick Clemens (kidclamp) <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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk 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=41603 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #191794|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 191797 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191797&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 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=41603 --- Comment #7 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=41603 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed --- Comment #8 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- please add release notes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 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=41603 --- 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=41603 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41603 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED --- Comment #11 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Seems like an architecture change with no impact for the end user. Nothing to add or edit in the Koha manual. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org