[Bug 26457] New: DB DeadLock when renewing checkout items
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Bug ID: 26457 Summary: DB DeadLock when renewing checkout items Change sponsored?: --- Product: Koha Version: 19.11 Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: didier.gautheron@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Hi, Since it's in a transaction ( https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 ) we are seeing deadlock when renewing checkout and items aren't renewed randomly. plack error 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 P SHOW ENGINE INNODB STATUS; ----------------------- LATEST DETECTED DEADLOCK ------------------------ 2020-09-14 11:09:55 0x7f69b85c0700 *** (1) TRANSACTION: TRANSACTION 19363996, ACTIVE 1 sec fetching rows mysql tables in use 4, locked 4 LOCK WAIT 7 lock struct(s), heap size 1128, 8 row lock(s) MySQL thread id 22449, OS thread handle 140092040718080, query id 38767706 localhost kohaadmin Updating UPDATE issues SET date_due = '2020-09-18 23:59', renewals = '1', lastreneweddate = '2020-09-14T11:09:54' WHERE borrowernumber='93697' AND itemnumber='999735' *** (1) WAITING FOR THIS LOCK TO BE GRANTED: RECORD LOCKS space id 476 page no 87 n bits 248 index PRIMARY of table `koha`.`issues` trx id 19363996 lock_mode X locks rec but not gap waiting Record lock, heap no 109 PHYSICAL RECORD: n_fields 18; compact format; info bits 0 0: len 4; hex 802713f6; asc ' ;; 1: len 6; hex 000000000000; asc ;; 2: len 7; hex 80000000000000; asc ;; 3: len 4; hex 80016e01; asc n ;; 4: len 4; hex 8008e784; asc ;; 5: len 5; hex 99a7397ec0; asc 9~ ;; 6: len 6; hex 43454e545245; asc CENTRE;; 7: SQL NULL; 8: SQL NULL; 9: len 1; hex 80; asc ;; 10: len 1; hex 80; asc ;; 11: SQL NULL; 12: len 4; hex 5f326b5e; asc _2k^;; 13: len 5; hex 99a6e20000; asc ;; 14: len 4; hex 80000000; asc ;; 15: SQL NULL; 16: SQL NULL; 17: SQL NULL; *** (2) TRANSACTION: TRANSACTION 19363999, ACTIVE 1 sec starting index read mysql tables in use 4, locked 4 4 lock struct(s), heap size 1128, 3 row lock(s) MySQL thread id 22450, OS thread handle 140092041332480, query id 38767702 localhost kohaadmin Updating UPDATE issues SET date_due = '2020-09-18 23:59', renewals = '1', lastreneweddate = '2020-09-14T11:09:54' WHERE borrowernumber='93697' AND itemnumber='583556' *** (2) HOLDS THE LOCK(S): RECORD LOCKS space id 476 page no 87 n bits 248 index PRIMARY of table `koha`.`issues` trx id 19363999 lock_mode X locks rec but not gap Record lock, heap no 109 PHYSICAL RECORD: n_fields 18; compact format; info bits 0 0: len 4; hex 802713f6; asc ' ;; 1: len 6; hex 000000000000; asc ;; 2: len 7; hex 80000000000000; asc ;; 3: len 4; hex 80016e01; asc n ;; 4: len 4; hex 8008e784; asc ;; 5: len 5; hex 99a7397ec0; asc 9~ ;; 6: len 6; hex 43454e545245; asc CENTRE;; 7: SQL NULL; 8: SQL NULL; 9: len 1; hex 80; asc ;; 10: len 1; hex 80; asc ;; 11: SQL NULL; 12: len 4; hex 5f326b5e; asc _2k^;; 13: len 5; hex 99a6e20000; asc ;; 14: len 4; hex 80000000; asc ;; 15: SQL NULL; 16: SQL NULL; 17: SQL NULL; *** (2) WAITING FOR THIS LOCK TO BE GRANTED: RECORD LOCKS space id 476 page no 425 n bits 1168 index issuesborridx of table `koha`.`issues` trx id 19363999 lock_mode X waiting Record lock, heap no 52 PHYSICAL RECORD: n_fields 2; compact format; info bits 0 0: len 4; hex 80016e01; asc n ;; 1: len 4; hex 8026df40; asc & @;; *** WE ROLL BACK TRANSACTION (2) -- 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=26457 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #1 from David Cook <dcook@prosentient.com.au> --- I'm more familiar with PostgreSQL deadlocks than MySQL deadlocks, but it looks to me like 2 separate Koha processes are trying to add renewals for different items for the same borrower, and perhaps the lock contention is over the issuesborridx index. Looking at Bug 20086, I see that those are unnecessarily large transactions. It's better to keep transactions as small as possible, especially to avoid scenarios like this. Looking at koha-tmpl/intranet-tmpl/prog/js/checkouts.js, I see that the renewals are POSTed to "/cgi-bin/koha/svc/renew" asynchronously, so that's how there would be multiple AddRenewal transactions running for the same borrower at the same time... Looking at https://dev.mysql.com/doc/refman/8.0/en/innodb-locking.html#innodb-record-lo..., I think my theory is correct about the issue being contention over the issuesborridx. Now, I haven't noticed this / haven't gotten any reports of this, but I don't think we really use fines, so this report is interesting. I'm guessing that your renewals must be processed *very* slowly, and that your 2nd transaction is rolled back, because your 1st transaction is taking too long to release a lock on issuesborridx. I'd say the solution is to redo the transaction handling for C4::Circulation::AddRenewal. Didier, do you been able to reproduce this issue in koha-testing-docker? That would help in fixing the problem. -- 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=26457 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20086 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20086 [Bug 20086] AddRenewal is not executed as a transaction and can results in partial success and doubled fines -- 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=26457 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- I suppose one option would be to do 1 AJAX request with all the information, rather than 1 AJAX request for *each* issue. That would probably cause a performance degradation overall though. I suppose one obvious fix would be changing from UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? WHERE borrowernumber=? AND itemnumber=?" to: UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? WHERE issue_id=?" -- 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=26457 --- Comment #3 from didier <didier.gautheron@biblibre.com> --- (In reply to David Cook from comment #1)
I'm more familiar with PostgreSQL deadlocks than MySQL deadlocks, but it looks to me like 2 separate Koha processes are trying to add renewals for different items for the same borrower, and perhaps the lock contention is over the issuesborridx index.
Looking at Bug 20086, I see that those are unnecessarily large transactions. It's better to keep transactions as small as possible, especially to avoid scenarios like this.
Looking at koha-tmpl/intranet-tmpl/prog/js/checkouts.js, I see that the renewals are POSTed to "/cgi-bin/koha/svc/renew" asynchronously, so that's how there would be multiple AddRenewal transactionsissuesborridx running for the same borrower at the same time... It's also our analysis, there's redundant indexes on issues which may on may not compound the problem. PRIMARY KEY (`issue_id`), UNIQUE KEY `itemnumber` (`itemnumber`), KEY `issuesborridx` (`borrowernumber`), KEY `itemnumber_idx` (`itemnumber`), KEY `branchcode_idx` (`branchcode`), KEY `bordate` (`borrowernumber`,`timestamp`),
itemnumber_idx and issuesborridx are redundant.
Looking at https://dev.mysql.com/doc/refman/8.0/en/innodb-locking.html#innodb-record- locks, I think my theory is correct about the issue being contention over the issuesborridx.
Now, I haven't noticed this / haven't gotten any reports of this, but I don't think we really use fines, so this report is interesting.
I'm guessing that your renewals must be processed *very* slowly, and that your 2nd transaction is rolled back, because your 1st transaction is taking too long to release a lock on issuesborridx.
I'd say the solution is to redo the transaction handling for C4::Circulation::AddRenewal.
I'm not sure block to redo is idempotent...
Didier, do you been able to reproduce this issue in koha-testing-docker? That would help in fixing the problem.
I'll try, maybe adding a fat sleep(5) in transaction block. -- 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=26457 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to didier from comment #3)
It's also our analysis, there's redundant indexes on issues which may on may not compound the problem. PRIMARY KEY (`issue_id`), UNIQUE KEY `itemnumber` (`itemnumber`), KEY `issuesborridx` (`borrowernumber`), KEY `itemnumber_idx` (`itemnumber`), KEY `branchcode_idx` (`branchcode`), KEY `bordate` (`borrowernumber`,`timestamp`),
itemnumber_idx and issuesborridx are redundant.
I doubt that the indexes compound the problem, but I suppose more investigation would be needed. I see that itemnumber_idx is redundant, but how is issuesborridx redundant? It's not the same as bordate.
I'll try, maybe adding a fat sleep(5) in transaction block.
Certainly worth experimenting. -- 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=26457 --- Comment #5 from didier <didier.gautheron@biblibre.com> --- (In reply to David Cook from comment #4)
I see that itemnumber_idx is redundant, but how is issuesborridx redundant? It's not the same as bordate.
bordate is a composed index with borrowernumber first and mysql is able to use it for search on borrowernumber only. explain select * from issues where borrowernumber = 3; +------+-------------+--------+------+-----------------------+---------------+---------+-------+------+-------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+--------+------+-----------------------+---------------+---------+-------+------+-------+ | 1 | SIMPLE | issues | ref | issuesborridx,bordate | issuesborridx | 5 | const | 1 | | +------+-------------+--------+------+-----------------------+---------------+---------+-------+------+-------+ drop index issuesborridx on issues; explain select * from issues where borrowernumber = 3; +------+-------------+--------+------+---------------+---------+---------+-------+------+-------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+--------+------+---------------+---------+---------+-------+------+-------+ | 1 | SIMPLE | issues | ref | bordate | bordate | 5 | const | 1 | | +------+-------------+--------+------+---------------+---------+---------+-------+------+-------+
I'll try, maybe adding a fat sleep(5) in transaction block.
Certainly worth experimenting.
I did it but was unable to reproduce the deadlock even with a sleep(5) and 8 concurrent plack workers. -- 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=26457 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to didier from comment #5)
(In reply to David Cook from comment #4)
I see that itemnumber_idx is redundant, but how is issuesborridx redundant? It's not the same as bordate.
bordate is a composed index with borrowernumber first and mysql is able to use it for search on borrowernumber only.
That's interesting! I've had mixed experiences (with PostgreSQL) with composite indexes being used when searching for only part of the index. I hadn't thought about column order, but maybe that explains some of my experiences. Food for thought!
I'll try, maybe adding a fat sleep(5) in transaction block.
Certainly worth experimenting. I did it but was unable to reproduce the deadlock even with a sleep(5) and 8 concurrent plack workers.
How many renewals did you try to do? I figure if you have maybe 32+ renewals and the browser does about 8 requests in parallel, maybe you'd be able to reproduce it with that. -- 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=26457 --- Comment #7 from didier <didier.gautheron@biblibre.com> --- (In reply to David Cook from comment #6)
How many renewals did you try to do? I figure if you have maybe 32+ renewals and the browser does about 8 requests in parallel, maybe you'd be able to reproduce it with that.
8 or 9 so there was 8 requests in parallel. -- 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=26457 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to didier from comment #7)
8 or 9 so there was 8 requests in parallel.
Hmm and did they have all fine generation? I keep thinking that has to be part of it, as I haven't noticed any deadlock issues, but we don't use fines typically. -- 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=26457 --- Comment #9 from didier <didier.gautheron@biblibre.com> --- (In reply to David Cook from comment #8)
(In reply to didier from comment #7)
8 or 9 so there was 8 requests in parallel.
Hmm and did they have all fine generation? I keep thinking that has to be part of it, as I haven't noticed any deadlock issues, but we don't use fines typically.
As far I can tell they don't fine overdue. -- 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=26457 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- If you want to fix that you should follow what is done in C4::Circulation::SendCirculationAlert -- 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=26457 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|19.11 |master -- 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=26457 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #10)
If you want to fix that you should follow what is done in C4::Circulation::SendCirculationAlert
Reading the other comments, maybe this is not needed. -- 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=26457 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I've tried hard to recreate the problem but failed (ktd). 40 plack workers, limit 50 renewals I created a bunch of checkouts for a given patron: use Koha::Items; use C4::Circulation; use t::lib::Mocks; my $items = Koha::Items->search(); my $patron = Koha::Patrons->find(51); t::lib::Mocks::mock_userenv({ patron => $patron }); while ( my $item = $items->next ) { C4::Circulation::AddIssue($patron->unblessed, $item->barcode); } Then opened 5 tabs /cgi-bin/koha/circ/circulation.pl?borrowernumber=51 And click "Renewal all" as soon as I could. Everything got renewed correctly. -- 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=26457 --- Comment #13 from didier <didier.gautheron@biblibre.com> --- Hi, Thanks for looking at it. We have a second library with the same issue, but they have only issuesborridx. we are using nginx and mariadb mysql --version mysql Ver 15.1 Distrib 10.4.13-MariaDB, for debian-linux-gnu (x86_64) using readline 5.2 -- 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=26457 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26208 --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Investigating this, as near as I can tell it only occurs during 'Renew all' when too many processes hit the server at once. I cannot recreate locally, but can verify on larger active koha instances The renewals silently fail and the user is never notified leading to problems later. We have other issues from the multiple concurrent renewals, see bug 26208 I attached a patch there to perform the actions serially which I believe will mitigate this issue -- 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=26457 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- It would be good to have more data from real life examples. -- 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=26457 --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to David Cook from comment #15)
It would be good to have more data from real life examples.
From the access logs, there are three requests with the same timestamp: somesite.bywatersolutions.com:80 IP.IP.IP.IP - - [30/Nov/2020:16:02:14 -0700] "POST /cgi-bin/koha/svc/renew HTTP/1.1" 200 349 "https://staff.somesite.bywatersolutions.com/cgi-bin/koha/circ/circulation.pl..." "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.198 Safari/537.36" somesite.bywatersolutions.com:80 IP.IP.IP.IP - - [30/Nov/2020:16:02:14 -0700] "POST /cgi-bin/koha/svc/renew HTTP/1.1" 200 350 "https://staff.somesite.bywatersolutions.com/cgi-bin/koha/circ/circulation.pl..." "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.198 Safari/537.36" somesite.bywatersolutions.com:80 IP.IP.IP.IP - - [30/Nov/2020:16:02:14 -0700] "POST /cgi-bin/koha/svc/renew HTTP/1.1" 200 349 "https://staff.somesite.bywatersolutions.com/cgi-bin/koha/circ/circulation.pl..." "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.198 Safari/537.36"
In the plack log: 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='2020-12-30 23:59', 1=1, 2=DateTime=HASH(0x5561e7d4fb08), 3="borrowernumber", 4="itemnumber"] at /usr/share/koha/lib/C4/Circulation.pm line 2964. In the action_logs: *************************** 15. row *************************** action_id: 10050103 timestamp: 2020-11-30 16:02:14 user: staff_borrowernumber module: CIRCULATION action: RENEWAL object: borrowernumber info: itemnumber interface: intranet Statistics: *************************** 19. row *************************** datetime: 2020-11-30 16:02:14 branch: BRANCH value: 0.0000 type: renew other: itemnumber: itemnumber itemtype: BOOK location: ADULT borrowernumber: borrowernumber ccode: FICTION Issues table: *************************** 1. row *************************** Table: issues Create Table: CREATE TABLE `issues` ( `issue_id` int(11) NOT NULL AUTO_INCREMENT, `borrowernumber` int(11) NOT NULL, `itemnumber` int(11) NOT NULL, `date_due` datetime DEFAULT NULL, `branchcode` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL, `returndate` datetime DEFAULT NULL, `lastreneweddate` datetime DEFAULT NULL, `renewals` tinyint(4) NOT NULL DEFAULT '0', `auto_renew` tinyint(1) DEFAULT '0', `auto_renew_error` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL, `timestamp` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, `issuedate` datetime DEFAULT NULL, `onsite_checkout` int(1) NOT NULL DEFAULT '0', `note` longtext COLLATE utf8mb4_unicode_ci, `notedate` datetime DEFAULT NULL, `noteseen` int(1) DEFAULT NULL, PRIMARY KEY (`issue_id`), KEY `issuesborridx` (`borrowernumber`), KEY `bordate` (`borrowernumber`,`timestamp`), KEY `itemnumber_idx` (`itemnumber`), KEY `branchcode_idx` (`branchcode`), CONSTRAINT `issues_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON UPDATE CASCADE, CONSTRAINT `issues_ibfk_2` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON UPDATE CASCADE ) ENGINE=InnoDB AUTO_INCREMENT=increment DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci Server version: 10.1.47-MariaDB-0+deb9u1 Debian 9.13 Koha 19.1108000 -- 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=26457 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=26457 --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114184 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114184&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Confirm that 'Renew all' feature continues to work as expected -- 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=26457 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114184|0 |1 is obsolete| | --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114185&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Confirm that 'Renew all' feature continues to work as expected -- 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=26457 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114185|0 |1 is obsolete| | --- Comment #19 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114186 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114186&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Confirm that 'Renew all' feature continues to work as expected -- 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=26457 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114186|0 |1 is obsolete| | --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114187 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114187&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Confirm that 'Renew all' feature continues to work as expected -- 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=26457 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114187|0 |1 is obsolete| | --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114190&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Confirm that 'Renew all' feature continues to work as expected -- 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=26457 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Could you provide a 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=26457 --- Comment #23 from David Cook <dcook@prosentient.com.au> --- Thanks for the info, Nick. And yeah I think throwing an exception is a good immediate improvement. Looking at Bug 20086, I think that we're trying to do way too much in 1 database transaction. I've seen this on other systems with a lot of concurrent activity, and debugging these deadlocks is a struggle. The AddRenewal function is a bit of a mess in general... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #24 from Victor Grousset/tuxayo <victor@tuxayo.net> --- After applying the patch , while restarting the services, I have a compilation issue. root@kohadevbox:koha((6982680ae9...))$ restart_all Restarting Apache httpd web server: apache2. Restarting Koha ILS: koha-commonStopping Plack daemon for kohadev:. Stopping Z39.50/SRU daemon for kohadev:. Stopping Koha worker daemon for kohadev:. Stopping Koha indexing daemon for kohadev:. Starting Plack daemon for kohadev:. Starting Z39.50/SRU daemon for kohadev:syntax error at /kohadevbox/koha/C4/Circulation.pm line 3091, near "->errstr;" Type of arg 1 to List::MoreUtils::any must be block or sub {} (not reference constructor) at /kohadevbox/koha/C4/Circulation.pm line 4397, near "} )" Type of arg 1 to List::MoreUtils::any must be block or sub {} (not reference constructor) at /kohadevbox/koha/C4/Circulation.pm line 4400, near "}) "
2 - Confirm that 'Renew all' feature continues to work as expected
Is testing the "Renew all" button in a patron's checkout list enough? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114190|0 |1 is obsolete| | --- Comment #25 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114669 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114669&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Checkout several items to a patron 3 - Confirm that 'Renew all' feature continues to work as expected and all items are renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #26 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #22)
Could you provide a test?
I have tried, but it seems it requires a lot of mocking and I have not been successful, help would be appreciated :-) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #27 from David Cook <dcook@prosentient.com.au> --- (In reply to Nick Clemens from comment #26)
(In reply to Jonathan Druart from comment #22)
Could you provide a test?
I have tried, but it seems it requires a lot of mocking and I have not been successful, help would be appreciated :-)
Couldn't you add a unit test for AddRenewal that just puts in junk data to trigger the exception? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #28 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114697&action=edit Bug 26457: Unit test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #29 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to David Cook from comment #27)
(In reply to Nick Clemens from comment #26)
(In reply to Jonathan Druart from comment #22)
Could you provide a test?
I have tried, but it seems it requires a lot of mocking and I have not been successful, help would be appreciated :-)
Couldn't you add a unit test for AddRenewal that just puts in junk data to trigger the exception?
Indeed I can! Turns out we pass 'lastreneweddate' right through, so adding junk data in the call lets us trigger the exception -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 David Nind <david@davidnind.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=26457 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114669|0 |1 is obsolete| | --- Comment #30 from David Nind <david@davidnind.com> --- Created attachment 117227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117227&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Checkout several items to a patron 3 - Confirm that 'Renew all' feature continues to work as expected and all items are renewed Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114697|0 |1 is obsolete| | --- Comment #31 from David Nind <david@davidnind.com> --- Created attachment 117228 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117228&action=edit Bug 26457: Unit test Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #32 from David Nind <david@davidnind.com> --- Tested by: - Following test plan (checking out 11 items to a patron, renewing all items 5 times) - Running prove t/db_dependent/Circulation.t (all tests pass) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=26457 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=26457 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #33 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QAing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117227|0 |1 is obsolete| | --- Comment #34 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 117371 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117371&action=edit Bug 26457: Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Checkout several items to a patron 3 - Confirm that 'Renew all' feature continues to work as expected and all items are renewed Signed-off-by: David Nind <david@davidnind.com> 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=26457 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117228|0 |1 is obsolete| | --- Comment #35 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 117372 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117372&action=edit Bug 26457: Unit test Signed-off-by: David Nind <david@davidnind.com> 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=26457 --- Comment #36 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 117373 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117373&action=edit Bug 26457: (QA follow-up) Switch to PK index in UPDATE on issues The deadlock reports tell us that multiple transactions are waiting for a X lock on a record but using a secondary index on borrowernumber and itemnumber. Since we have the issue_id at hand already, we should use that and benefit from the clustered index (on PK) instead of using a secondary index. 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=26457 --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Just for completeness, from sql man: To avoid deadlocks when performing multiple concurrent write operations on a single InnoDB table, acquire necessary locks at the start of the transaction by issuing a SELECT ... FOR UPDATE statement for each group of rows expected to be modified, even if the data change statements come later in the transaction. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #38 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=26457 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00 |21.05.00,20.11.04 released in| | Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m --- Comment #39 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #40 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Doesn't apply cleanly to 20.05. Thought I'd cleared merge errors, but ended up with an error when trying to restart services: Global symbol "$unseen_renewals" requires explicit package name (did you forget to declare "my $unseen_renewals"?) at /kohadevbox/koha/C4/Circulation.pm line 2977. Type of arg 1 to List::MoreUtils::any must be block or sub {} (not reference constructor) at /kohadevbox/koha/C4/Circulation.pm line 4213, near "} )" Type of arg 1 to List::MoreUtils::any must be block or sub {} (not reference constructor) at /kohadevbox/koha/C4/Circulation.pm line 4216, near "}) " Please rebase for backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #41 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 117876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117876&action=edit Bug 26457: [20.05.x] Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Checkout several items to a patron 3 - Confirm that 'Renew all' feature continues to work as expected and all items are renewed Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 26457: Unit test Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 26457: (QA follow-up) Switch to PK index in UPDATE on issues The deadlock reports tell us that multiple transactions are waiting for a X lock on a record but using a secondary index on borrowernumber and itemnumber. Since we have the issue_id at hand already, we should use that and benefit from the clustered index (on PK) instead of using a secondary index. 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=26457 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|21.05.00,20.11.04 |21.05.00,20.11.04,20.05.10 released in| | --- Comment #42 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED --- Comment #43 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Issue when trying to backport to 19.11.x, failing test. kohadev-koha@be9bfe372adf:/kohadevbox/koha$ prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 40/47 DBD::mysql::st execute failed: Incorrect datetime value: 'HASH(0x562f3aaf8048)' for column 'lastreneweddate' at row 1 [for Statement "UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? WHERE issue_id = ?" with ParamValues: 0='2021-04-12 23:59', 1=1, 2=HASH(0x562f3aaf8048), 3=264] at /kohadevbox/koha/C4/Circulation.pm line 2969. # No tests run! t/db_dependent/Circulation.t .. 42/47 # Failed test 'No tests run for subtest "AddRenewal and AddIssuingCharge tests"' # at t/db_dependent/Circulation.t line 3201. Update of issue# 264 failed with error: Incorrect datetime value: 'HASH(0x562f3aaf8048)' for column 'lastreneweddate' at row 1# Looks like your test exited with 255 just after 42. The conflicts weren't tricky to solve and I don't see how the patch affects lastreneweddate. So help needed for backporting to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #44 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 118647 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118647&action=edit Bug 26457: [19.11.x] Throw exception if update of issues table fails While this won't prevent the deadlock, it should catch the case where a deadlock causes the DB update to fail and provide feedback to the user and rollback the transaction I don't know how to trigger the deadlock, I can only confirm that we see it, and that this should catch it. To test: 1 - Apply patches 2 - Checkout several items to a patron 3 - Confirm that 'Renew all' feature continues to work as expected and all items are renewed Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 26457: Unit test Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 26457: (QA follow-up) Switch to PK index in UPDATE on issues The deadlock reports tell us that multiple transactions are waiting for a X lock on a record but using a secondary index on borrowernumber and itemnumber. Since we have the issue_id at hand already, we should use that and benefit from the clustered index (on PK) instead of using a secondary index. 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=26457 --- Comment #45 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I had a try with the conflict on 19.11, can you test this patch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 --- Comment #46 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Thanks a lot Jonathan, it works! :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00,20.11.04,20.05.10 |21.05.00,20.11.04,20.05.10, released in| |19.11.16 --- Comment #47 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Backported: Pushed to 19.11.x branch for 19.11.16 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29650 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26457 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |vvishnuaejith@gmail.com --- Comment #48 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 29650 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org