| Summary: | Plugin hook causing DB locks when cancelling holds | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | Plugin architecture | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | Needs documenting --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
| Severity: | critical | ||
| Priority: | P5 - low | CC: | Chip.Halvorsen, dbarden, kkrueger, Laura.escamilla, lisette, lthorrat, lucas, nick, tomascohen |
| Version: | Main | Keywords: | release-notes-needed |
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38384 | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
26.05.00,25.11.02
|
|
| Circulation function: | |||
| Attachments: |
Bug 41603: Move after_hold_action plugin hook outside transaction
Bug 41603: Move after_hold_action plugin hook outside transaction Bug 41603: Move after_hold_action plugin hook outside transaction |
||
|
Description
Tomás Cohen Arazi (tcohen)
2026-01-13 15:42:52 UTC
*** 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. |