[Bug 27707] New: Renewing doesn't work when renewal notices are enabled
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Bug ID: 27707 Summary: Renewing doesn't work when renewal notices are enabled 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: joonas.kylmala@helsinki.fi QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Depends on: 26639 Renewing loans doesn't work for patrons who have enabled renewal notices to be sent to them. To reproduce: 1) Enable RenewalSendNotice 2) Add some email address to patron and select the email box from the message preference "Item checkout and renewal" in order to receive renewal emails. 3) Checkout 4 items to the patron (maybe 1 is enough not quite sure) 4) Try to renew the loans in intranet and notice the progress icon circles forever and in the plack-error.log you have following error:
DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] at /usr/share/koha/lib/C4/Circulation.pm line 3590
This bug seems to have started happening after bug 26639 was merged, i.e. AUTO SAVEPOINT was enabled. I have found a workaround and possibly it is also a reasonable solution: In C4/Circulation.pm function SendCirculationAlert there is this comment:
# LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables.
So basically if that comment is true (I have not verified) then it means that the transaction that has been put around the "LOCK TABLE message_queue READ" stuff is doing nothing. And here based on the error message we got in plack-error.log the problem is that we are trying to commit a non-existent transaction because as per the comment in the code calling LOCK TABLE already commited the transaction so there is no transaction left for us to commit manually in the end. To summarise we need to remove the two lines I think: $schema->storage->txn_begin; $schema->storage->txn_commit; Also why I think the above solution is reasonable is because otherwise we need to rethought the design here totally on how the digest messages are sent and maybe add a new is_digest db column and do the logic in the message sending script instead of "merging db rows". For more background on why the table locking was introduced see bug 15854. PS. I think this bug might affect check-in and checkouts, not totally sure. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26639 [Bug 26639] Turn auto_savepoint ON -- 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=27707 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26208 CC| |jonathan.druart@bugs.koha-c | |ommunity.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=27707 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I did some testing and I confirm that the txn is not needed here. use Koha::Database; my $schema = Koha::Database->new->schema; $schema->txn_do(sub { $schema->txn_do( sub { my $dbh = $schema->storage->dbh; say "getting locks"; $dbh->do(q|LOCK TABLE message_queue READ|); $dbh->do(q|LOCK TABLE message_queue WRITE|); say "locked!"; sleep 10; say "unlocking"; $dbh->do(q|UNLOCK TABLES|); say "unlocked!"; }); }); Start several parallel processes. Remove the nested txn_do, try again. -- 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=27707 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |15854 Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15854 [Bug 15854] Race condition for sending renewal/check-in notices -- 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=27707 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=27707 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 116906 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116906&action=edit Bug 27707: (bug 26639 follow-up) Fix renewals when RenewalSendNotice is set Since bug 26639 we have auto savepoint enabled and the LOCK TABLE query in C4::Circulation::SendCirculationAlert is not correctly handled.
From the MySQL doc that is copied few lines before, "LOCK TABLE will commit any transactions", but here we don't have a savepoint and the release for a non-existent savepoint will throw a DBI exception.
This patch removes the unecessary transaction and prevent the following error when a renewal is done:
DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] at /usr/share/koha/lib/C4/Circulation.pm line 3590
Test plan: 1. Enable RenewalSendNotice 2. Add some email address to patron and select the email box from the message preference "Item checkout and renewal" in order to receive renewal emails. 3. Check 1 item out to a patron 4. Renew it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Andrew Nugged <nugged@gmail.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=27707 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116906|0 |1 is obsolete| | --- Comment #3 from Andrew Nugged <nugged@gmail.com> --- Created attachment 116909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116909&action=edit Bug 27707: (bug 26639 follow-up) Fix renewals when RenewalSendNotice is set Since bug 26639 we have auto savepoint enabled and the LOCK TABLE query in C4::Circulation::SendCirculationAlert is not correctly handled.
From the MySQL doc that is copied few lines before, "LOCK TABLE will commit any transactions", but here we don't have a savepoint and the release for a non-existent savepoint will throw a DBI exception.
This patch removes the unecessary transaction and prevent the following error when a renewal is done:
DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] at /usr/share/koha/lib/C4/Circulation.pm line 3590
Test plan: 1. Enable RenewalSendNotice 2. Add some email address to patron and select the email box from the message preference "Item checkout and renewal" in order to receive renewal emails. 3. Check 1 item out to a patron 4. Renew it Signed-off-by: Andrew Nugged <nugged@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 --- Comment #4 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- I found the MySQL documentation where the LOCK Tables behaviour with transactions is explained: https://dev.mysql.com/doc/refman/8.0/en/lock-tables.html -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |joonas.kylmala@helsinki.fi |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Joonas Kylmälä <joonas.kylmala@helsinki.fi> 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=27707 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116909|0 |1 is obsolete| | --- Comment #5 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 116910 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116910&action=edit Bug 27707: (bug 26639 follow-up) Fix renewals when RenewalSendNotice is set Since bug 26639 we have auto savepoint enabled and the LOCK TABLE query in C4::Circulation::SendCirculationAlert is not correctly handled.
From the MySQL doc that is copied few lines before, "LOCK TABLE will commit any transactions", but here we don't have a savepoint and the release for a non-existent savepoint will throw a DBI exception.
This patch removes the unecessary transaction and prevent the following error when a renewal is done:
DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] at /usr/share/koha/lib/C4/Circulation.pm line 3590
Test plan: 1. Enable RenewalSendNotice 2. Add some email address to patron and select the email box from the message preference "Item checkout and renewal" in order to receive renewal emails. 3. Check 1 item out to a patron 4. Renew it Signed-off-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, | |fridolin.somers@biblibre.co | |m, victor@tuxayo.net --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- @RMaints: you all need this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.05.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=27707 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|21.05.00 |21.05.00,20.11.03 released in| | --- Comment #8 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 --- Comment #9 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Testing this for backport on 20.05, I am able to renew without a problem with RenewalSendNotice on and the patch not applied. I get a warning in the plack-intranet-error.log: [2021/02/22 20:41:57] [WARN] DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] at /usr/share/perl5/DBIx/Class/Storage/DBI/mysql.pm line 152. That warning persists after I've applied the patch. Again, I get a successful renewal and renewal notice regardless of whether or not this patch is present. I'm inclined to skip backport, since I can't recreate the issue. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00,20.11.03 |21.05.00,20.11.03,20.05.09 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #10 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.09 As mentioned, I was unable to reproduce this error on my Koha Testing Docker running MariaDB 10.1.48. However, the patch applied fine and didn't break anything. I'm backporting on the assumption that this may impact 20.05 users with a different database version. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Pushed to oldoldstable Version(s)|21.05.00,20.11.03,20.05.09 |21.05.00,20.11.03,20.05.09, released in| |19.11.15 --- Comment #11 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Backported: Pushed to 19.11.x branch for 19.11.15 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 --- Comment #12 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Still problems with renewals, now the error is just: Waiting for table metadata lock | LOCK TABLE message_queue READ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 --- Comment #13 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Joonas Kylmälä from comment #12)
Still problems with renewals, now the error is just:
Waiting for table metadata lock | LOCK TABLE message_queue READ
The locked table seems to have been caused by Bug 28230 so continuing discussion relating to this there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27707 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28158 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org