[Bug 34678] New: Real-time holds queue updates can cause the holds queue builder cron to fail
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Bug ID: 34678 Summary: Real-time holds queue updates can cause the holds queue builder cron to fail Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: emily.lamancusa@montgomerycountymd.gov QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com In bug 34656, it came to light that, if a real-time holds queue update and the holds queue builder cron try to update the same bib at the same time, it can cause the cron to fail. Specifically, if a real-time update adds a particular item to hold_fill_targets, and the holds queue builder tries to add the same item afterwards, the cron will fail with an error like: C4::HoldsQueue::AddToHoldTargetMap(): DBI Exception: DBD::mysql::st execute failed: Duplicate entry '464273' for key 'PRIMARY' at /usr/share/koha/lib/C4/HoldsQueue.pm line 1015 The biggest issue here is that any remaining holds that the cron has not yet processed will be left out of the holds queue until something else triggers them to update. (in our system, the issue in bug 34656 was occurring regularly, and typically left over 1,000 holds out of the holds queue!) If the holds queue builder can be forced to run while RealTimeHoldsQueue is enabled, it needs a way to avoid crashing if a real-time update happens to add a bib back into the queue before the queue builder gets to it. -- 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=34678 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=34678 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 155594 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155594&action=edit Bug 34678: Unit test -- 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=34678 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 155595 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155595&action=edit Bug 34678: Allow new entries to overwrite hold_fill_targets When using background jobs, there is a possibility of a race condition where two jobs will be updating the holds queue for the same biblio. We should try to minimize those cases (see bug 34596) In the meantime though, we should prevent jobs possibly dying, and allow the most recent update to succeed. There is a possibility two updates wil assign different items to the same reserve, and that a reserve could end up in the queue twice, however, whichever one is filled first will delete both entries. as filling the hold deletes by reserve id (see bug 24359) To test: 1 - prove -v t/db_dependent/Reserves.t 2 - It fails 3 - Apply patch 4 - t/db_dependent/Reserves.t 5 - It succeeds! -- 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=34678 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Real-time holds queue |Concurrent changes to the |updates can cause the holds |holds can fail due to |queue builder cron to fail |primary key constraints CC| |nick@bywatersolutions.com -- 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=34678 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | CC| |lucas@bywatersolutions.com -- 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=34678 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155594|0 |1 is obsolete| | --- Comment #3 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 155628 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155628&action=edit Bug 34678: Unit test Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155595|0 |1 is obsolete| | --- Comment #4 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 155629 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155629&action=edit Bug 34678: Allow new entries to overwrite hold_fill_targets When using background jobs, there is a possibility of a race condition where two jobs will be updating the holds queue for the same biblio. We should try to minimize those cases (see bug 34596) In the meantime though, we should prevent jobs possibly dying, and allow the most recent update to succeed. There is a possibility two updates wil assign different items to the same reserve, and that a reserve could end up in the queue twice, however, whichever one is filled first will delete both entries. as filling the hold deletes by reserve id (see bug 24359) To test: 1 - prove -v t/db_dependent/Reserves.t 2 - It fails 3 - Apply patch 4 - t/db_dependent/Reserves.t 5 - It succeeds! Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion CC| |m.de.rooy@rijksmuseum.nl --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Should we use a MySQL extension like REPLACE ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au, | |fridolin.somers@biblibre.co | |m, | |jonathan.druart+koha@gmail. | |com, | |julian.maurice@biblibre.com | |, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #5)
Should we use a MySQL extension like REPLACE ?
It appears that MERGE is the closest SQL standard keyword with the same functionality. However, MySQL/MariaDB doesn't support it. There is also no DBIC alternative to use as universal fix. The alternative would be to run a delete query with the insert as a transaction. That feels rather clunky and inefficient to simply avoid a specific use of this keyword. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Maybe it is time to discuss (again) if we're gonna take advantage of the supported DB engine features. This discussion should take flame somewhere else I think. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Well, the consensus is/was to avoid MySQLisms... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #8)
Well, the consensus is/was to avoid MySQLisms...
I that case I think the atomic delete + commit is the best course of action! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #9)
(In reply to Jonathan Druart from comment #8)
Well, the consensus is/was to avoid MySQLisms...
I that case I think the atomic delete + commit is the best course of action!
OK, go ahead. Discussion over? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34596 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155628|0 |1 is obsolete| | Attachment #155629|0 |1 is obsolete| | --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157913 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157913&action=edit Bug 34678: Unit test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157914 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157914&action=edit Bug 34678: Allow new entries to overwrite hold_fill_targets When using background jobs, there is a possibility of a race condition where two jobs will be updating the holds queue for the same biblio. We should try to minimize those cases (see bug 34596) In the meantime though, we should prevent jobs possibly dying, and allow the most recent update to succeed. There is a possibility two updates wil assign different items to the same reserve, and that a reserve could end up in the queue twice, however, whichever one is filled first will delete both entries. as filling the hold deletes by reserve id (see bug 24359) This patch adds a transaction to delete and then inset the new row To test: 1 - prove -v t/db_dependent/Reserves.t 2 - It fails 3 - Apply patch 4 - t/db_dependent/Reserves.t 5 - It succeeds! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157913|0 |1 is obsolete| | --- Comment #13 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 157950 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157950&action=edit Bug 34678: Unit test Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157914|0 |1 is obsolete| | --- Comment #14 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 157951 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157951&action=edit Bug 34678: Allow new entries to overwrite hold_fill_targets When using background jobs, there is a possibility of a race condition where two jobs will be updating the holds queue for the same biblio. We should try to minimize those cases (see bug 34596) In the meantime though, we should prevent jobs possibly dying, and allow the most recent update to succeed. There is a possibility two updates wil assign different items to the same reserve, and that a reserve could end up in the queue twice, however, whichever one is filled first will delete both entries. as filling the hold deletes by reserve id (see bug 24359) This patch adds a transaction to delete and then inset the new row To test: 1 - prove -v t/db_dependent/Reserves.t 2 - It fails 3 - Apply patch 4 - t/db_dependent/Reserves.t 5 - It succeeds! Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> 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=34678 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- =head2 AddToHoldTargetMap =cut Hmm. Great documentation :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=34678 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157950|0 |1 is obsolete| | Attachment #157951|0 |1 is obsolete| | --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 157970 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157970&action=edit Bug 34678: Unit test Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 157971 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157971&action=edit Bug 34678: Allow new entries to overwrite hold_fill_targets When using background jobs, there is a possibility of a race condition where two jobs will be updating the holds queue for the same biblio. We should try to minimize those cases (see bug 34596) In the meantime though, we should prevent jobs possibly dying, and allow the most recent update to succeed. There is a possibility two updates wil assign different items to the same reserve, and that a reserve could end up in the queue twice, however, whichever one is filled first will delete both entries. as filling the hold deletes by reserve id (see bug 24359) This patch adds a transaction to delete and then inset the new row To test: 1 - prove -v t/db_dependent/Reserves.t 2 - It fails 3 - Apply patch 4 - t/db_dependent/Reserves.t 5 - It succeeds! Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00 |23.11.00,23.05.06 released in| | Status|Pushed to master |Pushed to stable --- Comment #20 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|23.11.00,23.05.06 |23.11.00,23.05.06,22.11.12 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678 --- Comment #21 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org