[Bug 35165] New: Batch item deletions are run in a single transaction and cause locking issues
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Bug ID: 35165 Summary: Batch item deletions are run in a single transaction and cause locking issues Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org We recently saw a site perform a batch deletion of ~18k items (and subsequently 2k bibs) While running the jobs we suddenly started seeing all sorts of actions fail, most notably placing reserves, and errors in the logs like: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction at /usr/share/koha/lib/Koha/Object.pm line 170 I believe part of the problem is that the entire batch deletion is a single transaction. While we should make parts of this atomic, I don't think the entire job should succeed or fail on an error. I think we should commit each item, then each bib, then continue. -- 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=35165 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au, | |jonathan.druart+koha@gmail. | |com, | |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.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=35165 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=35165 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tgoatley@gmail.com --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- *** Bug 26077 has been marked as a duplicate of this bug. *** -- 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=35165 mathieu saby <mathsabypro@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathsabypro@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 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=35165 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 161219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161219&action=edit Bug 35165: Do not delete items in a transaction We are testing the deletion with safe_delete before removing, beyond that I think the intent was to delete all of the items, deleting some is better than not deleting any. To test: 1 - Apply patch, restart all 2 - Submit a batch deletion 3 - Confirm items are deleted -- 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=35165 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 161220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161220&action=edit Bug 35165: Tidy -- 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=35165 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | QA Contact|testopia@bugs.koha-communit |nick@bywatersolutions.com |y.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=35165 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Nick Clemens from comment #0)
While we should make parts of this atomic, I don't think the entire job should succeed or fail on an error. I think we should commit each item, then each bib, then continue.
Each item and bib are singular actions - I don't actually think we need any transactions at this level at all. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Loïc <loic.vassaux-artur@epitech.eu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |loic.vassaux-artur@epitech. | |eu --- Comment #5 from Loïc <loic.vassaux-artur@epitech.eu> --- In "Details of job" after the deletion we have "Progress: 0/4" instead of "Progress: 4/4" without patch. In the same page, in "Report" we have "No items deleted." instead of "4 item(s) deleted." without patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 --- Comment #6 from Loïc <loic.vassaux-artur@epitech.eu> --- The bug appears just if we select "Delete records if no items remain." -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Loïc from comment #5)
In "Details of job" after the deletion we have "Progress: 0/4" instead of "Progress: 4/4" without patch.
In the same page, in "Report" we have "No items deleted." instead of "4 item(s) deleted." without patch.
(In reply to Loïc from comment #6)
The bug appears just if we select "Delete records if no items remain."
I do not recreate this - did you restart all after applying the patch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> ---
While we should make parts of this atomic, I don't think the entire job should succeed or fail on an error.
I think it should. We should discuss that with other people. If something unexpected happened we should not commit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #8)
While we should make parts of this atomic, I don't think the entire job should succeed or fail on an error.
I think it should. We should discuss that with other people. If something unexpected happened we should not commit.
The current situation places an unknown limit on the number that can be deleted - too many and you lock your system, can't circulate, can't catalog - this is very bad -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrewfh@dubcolib.org --- Comment #10 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- (In reply to Jonathan Druart from comment #8)
I think it should.
Can you elaborate? Nick presents a clear impact on the end-user and what appears to be a reasonable solution. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161219|0 |1 is obsolete| | --- Comment #11 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 161368 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161368&action=edit Bug 35165: Do not delete items in a transaction We are testing the deletion with safe_delete before removing, beyond that I think the intent was to delete all of the items, deleting some is better than not deleting any. To test: 1 - Apply patch, restart all 2 - Submit a batch deletion 3 - Confirm items are deleted Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161220|0 |1 is obsolete| | --- Comment #12 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 161369 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161369&action=edit Bug 35165: Tidy Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #13 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- (In reply to Jonathan Druart from comment #8)
If something unexpected happened we should not commit.
There's no expectation from the user that the entire batch of items will or will not delete as a group. So long as the user can reliably see which records were successfully deleted and which were not, the success of the job as a whole should not require every item succeed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161369|0 |1 is obsolete| | --- Comment #14 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 161370 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161370&action=edit Bug 35165: Tidy Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This is how I implemented the background job at the beginning. If we decide to change how it should behave I am fine with that, but then we should have the same behaviour everywhere and make it clear (for end-users and devs). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- The problem is mainly with the "retry". Say you have a "delete bib from 1 to 10". The first job deletes 1 to 5 but fails for 6 to 10. If you retry later after you fixed the deletion problem for 6-10, the second job (retry) will report failures for deletion 1 to 5. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161368|0 |1 is obsolete| | --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161480 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161480&action=edit Bug 35165: Do not delete items in a transaction We are testing the deletion with safe_delete before removing, beyond that I think the intent was to delete all of the items, deleting some is better than not deleting any. To test: 1 - Apply patch, restart all 2 - Submit a batch deletion 3 - Confirm items are deleted Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|nick@bywatersolutions.com |m.de.rooy@rijksmuseum.nl --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- We should avoid separate tidy patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Apart from discussion, I dont see unit tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #18)
We should avoid separate tidy patches.
When we make a change that reindents a whole section, keeping the tidy patch separate makes rebasing far easier. Why should we avoid them? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35165 --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #20)
(In reply to Marcel de Rooy from comment #18)
We should avoid separate tidy patches.
When we make a change that reindents a whole section, keeping the tidy patch separate makes rebasing far easier. Why should we avoid them?
General rules always have exceptions :) -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org