Bug 32990 - Possible deadlock in C4::ImportBatch::_update_batch_record_counts
Summary: Possible deadlock in C4::ImportBatch::_update_batch_record_counts
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-17 08:32 UTC by Marcel de Rooy
Modified: 2023-12-28 20:47 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06,22.05.14


Attachments
Bug 32990: Prevent deadlock in _update_batch_record_counts (2.53 KB, patch)
2023-02-17 08:40 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32990: Prevent deadlock in _update_batch_record_counts (2.54 KB, patch)
2023-03-06 13:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 32990: Prevent deadlock in _update_batch_record_counts (2.60 KB, patch)
2023-04-28 13:05 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2023-02-17 08:32:22 UTC
See 32558. Running this sub in parallel within multiple transactions creates a deadlock.

C4::ImportBatch::_update_batch_record_counts(): DBI Exception: DBD::mysql::st execute failed: Deadlock found when trying to get lock; try restarting transaction at /usr/share/koha/C4/ImportBatch.pm line 392
Comment 1 Marcel de Rooy 2023-02-17 08:40:38 UTC
Created attachment 146752 [details] [review]
Bug 32990: Prevent deadlock in _update_batch_record_counts

Resolves:
C4::ImportBatch::_update_batch_record_counts(): DBI Exception: DBD::mysql::st execute failed: Deadlock found when trying to get lock; try restarting transaction at /usr/share/koha/C4/ImportBatch.pm line 392

See also bug 32558.

Test plan:
If you apply 32558 first, run multiple processes that stage a marc import.
Without this patch, you can trigger the deadlock.
With this patch, it works.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Emily Lamancusa 2023-03-01 21:41:20 UTC
Hi Marcel, I tried to test this, but I haven't been able to reproduce the deadlock. Were you stopping the worker daemons, queueing up multiple MARC imports, and then manually restarting the worker as in the test plan for bug 32558? Anything else to keep in mind to make the deadlock more likely?
Comment 3 Marcel de Rooy 2023-03-02 06:40:22 UTC
(In reply to Emily Lamancusa from comment #2)
> Hi Marcel, I tried to test this, but I haven't been able to reproduce the
> deadlock. Were you stopping the worker daemons, queueing up multiple MARC
> imports, and then manually restarting the worker as in the test plan for bug
> 32558? Anything else to keep in mind to make the deadlock more likely?

Hi Emily. Thx for trying. Yes, I stopped and restarted later. My test plan was a bit too compact, i am afraid..
Comment 4 Kyle M Hall 2023-03-06 13:27:45 UTC
Created attachment 147785 [details] [review]
Bug 32990: Prevent deadlock in _update_batch_record_counts

Resolves:
C4::ImportBatch::_update_batch_record_counts(): DBI Exception: DBD::mysql::st execute failed: Deadlock found when trying to get lock; try restarting transaction at /usr/share/koha/C4/ImportBatch.pm line 392

See also bug 32558.

Test plan:
If you apply 32558 first, run multiple processes that stage a marc import.
Without this patch, you can trigger the deadlock.
With this patch, it works.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Nick Clemens 2023-04-28 11:25:58 UTC
This worked, however, the error I got was:

C4::ImportBatch::SetImportRecordMatches(): DBI Exception: DBD::mysql::st execute failed: Deadlock found when trying to get lock; try restarting transaction at /kohadevbox/koha/C4/ImportBatch.pm line 494


Does the statement in that routine need an update too?
Comment 6 Marcel de Rooy 2023-04-28 11:59:41 UTC
(In reply to Nick Clemens from comment #5)
> This worked, however, the error I got was:
> 
> C4::ImportBatch::SetImportRecordMatches(): DBI Exception: DBD::mysql::st
> execute failed: Deadlock found when trying to get lock; try restarting
> transaction at /kohadevbox/koha/C4/ImportBatch.pm line 494
> 
> 
> Does the statement in that routine need an update too?

Looks like it doesnt. You just hit the same problem in another part of the code execution in backgroundjob->process. It first does Stage and later calls FindDup.
Comment 7 Nick Clemens 2023-04-28 13:05:37 UTC
Created attachment 150369 [details] [review]
Bug 32990: Prevent deadlock in _update_batch_record_counts

Resolves:
C4::ImportBatch::_update_batch_record_counts(): DBI Exception: DBD::mysql::st execute failed: Deadlock found when trying to get lock; try restarting transaction at /usr/share/koha/C4/ImportBatch.pm line 392

See also bug 32558.

Test plan:
If you apply 32558 first, run multiple processes that stage a marc import.
Without this patch, you can trigger the deadlock.
With this patch, it works.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Tomás Cohen Arazi 2023-05-05 15:14:46 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 9 Martin Renvoize 2023-05-12 15:23:51 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 10 Katrin Fischer 2023-06-08 12:41:08 UTC
We have reports about import stuck at 0% on the mailing list, also with 22.11.06-03 - could this be related in any way? We've also seen some strange behaviour in 22.11.05.

https://lists.katipo.co.nz/pipermail/koha/2023-June/059555.html
Comment 11 Lucas Gass 2023-06-13 16:19:32 UTC
Backported to 22.05.x for upcoming 22.05.14
Comment 12 Danyon Sewell 2023-07-20 03:49:58 UTC
Does this need to be backported to 21.11.x?
Comment 13 Marcel de Rooy 2023-07-20 05:32:59 UTC
(In reply to Danyon Sewell from comment #12)
> Does this need to be backported to 21.11.x?

See the connection to bug 32558 (not in 21.11). I think it is not needed. But if it applies, should not harm either?