[Bug 20086] New: AddRenewal is not executed as a transaction and can results in partial success and doubled fines
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Bug ID: 20086 Summary: AddRenewal is not executed as a transaction and can results in partial success and doubled fines Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com I can't recreate on demand, however, I can confirm several reports in the wild. On a large/busy system, grep for "Deadlock" in the plack error logs. On several sites we are seeing occasional: DBD::mysql::st execute failed: Deadlock found when trying to get lock; try restarting transaction [for Statement "UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? WHERE borrowernumber=? AND itemnumber=?" with ParamValues: 0='2099-01-01 23:59', 1=2, 2='2099-01-01', 3="#####", 4="#####"] at /usr/share/koha/lib/C4/Circulation.pm line 2895. (dates and numbers altered for privacy) In this case the fines are closed, but when the issue update fails the librarian repeats the renewal and fines are then recreated (koha finds no open fine, so calculates a new one) Ideally the whole sub would be a transaction -- 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=20086 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 70879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70879&action=edit Bug 20086 - AddRenewal is not executed as a transaction and can results in partial success and doubled fines This patch starts a transaction and only commits if renewal and fines updates and charges are successful (partial in any cna be problematic) There is no feedback (as currently there is none either) but if part fails, all fails. I didn't include stats and notifications in the transaction, but we could. To test: 1 - Apply patch 2 - prove t/db_dependent/Circulation.t 3 - Attempt circs and renewals should be no difference 4 - If possible make part of transaction fail and ensure all fails -- 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=20086 Edie Discher <edischer@cityoffargo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |edischer@cityoffargo.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- What if we are already in a transaction? Several unit tests include a call to AddRenewal.. -- 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=20086 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #2)
What if we are already in a transaction? Several unit tests include a call to AddRenewal..
Should not be a problem for DBIx. Only the outer transaction will commit. Use of txn_do is recommended. -- 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=20086 Cheryl <clackman@fargolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |clackman@fargolibrary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=20086 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70879|0 |1 is obsolete| | --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94374 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94374&action=edit Bug 20086: Use txn_do -- 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=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94374|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94377&action=edit Bug 20086: Use txn_do Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=20086 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94378&action=edit Bug 20086: (follow-up) No need to call ->new on Koha::Database We should use the existing pattern of calling schema directly on Koha::Database rather than creating a new object. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- 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=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Patch complexity|--- |Trivial patch -- 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=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@bugs.koha-c |y.org |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_19_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=20086 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94377|0 |1 is obsolete| | Attachment #94378|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94421 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94421&action=edit Bug 20086: Execute AddRenewal in a transaction to avoid partial success and doubled fines This patch starts a transaction and only commits if renewal and fines updates and charges are successful (partial in any cna be problematic) There is no feedback (as currently there is none either) but if part fails, all fails. I didn't include stats and notifications in the transaction, but we could. (Edit JD: not true, they are included) To test: 1 - Apply patch 2 - prove t/db_dependent/Circulation.t 3 - Attempt circs and renewals should be no difference 4 - If possible make part of transaction fail and ensure all fails Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Amended: commit title and one indendation (return statement) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94422 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94422&action=edit Bug 20086: (follow-up) No need to call ->new on Koha::Database We should use the existing pattern of calling schema directly on Koha::Database rather than creating a new object. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |19.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=20086 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_19_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #10 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- There is a conflict for 19.05.x. Please provide dedicated patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Benjamin Daeuber <bdaeuber@cityoffargo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bdaeuber@cityoffargo.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frank.hansen@ub.lu.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Maria <mbugz2@yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mbugz2@yahoo.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 Björn Nylén <bjorn.nylen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bjorn.nylen@ub.lu.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26457 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 [Bug 26457] DB DeadLock when renewing checkout items -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #11 from David Cook <dcook@prosentient.com.au> --- According to Bug 26457, the same deadlocks are still occurring. Based on Didier's report, this whole sub transaction probably made it worse? Interesting that the problem happened before adding the (too) large transaction though. In Didier's report, the lock contention seems to be due to the "WHERE borrowernumber =" causing the issuesborridx index to be locked. That shouldn't be an issue for this bug report, since there wasn't a large transaction that would cause waiting for locks... I'm guessing that some of the fine logic is slow though, and since the renewals are processed asynchronously, doing a large volume of renewals at once could cause lock contention I suppose... Anyway, best to continue this at Bug 26457... -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org