| Summary: | Combine multiple db updates in C4::ImportBatch::BatchCommitRecords for efficiency/avoiding possible deadlocks | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Kyle M Hall (khall) <kyle> |
| Component: | Tools | Assignee: | Kyle M Hall (khall) <kyle> |
| Status: | RESOLVED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | m.de.rooy |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33972 | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.02,22.11.08
|
|
| Circulation function: | |||
| Attachments: |
Bug 33987 - Combine multiple db updates one in BatchCommitRecords
Bug 33987 - Combine multiple db updates one in BatchCommitRecords Bug 33987 - Combine multiple db updates one in BatchCommitRecords Bug 33987 - Combine multiple db updates one in BatchCommitRecords Bug 33987: Combine multiple db updates one in BatchCommitRecords |
||
|
Description
Kyle M Hall (khall)
2023-06-12 17:39:25 UTC
Created attachment 152282 [details] [review] Bug 33987 - Combine multiple db updates one in BatchCommitRecords When replacing existing records BatchCommitRecords will the table import_records will be updated three times for three different fields by three different queries. Not only is this inefficient ( especially for large batches ), it seems that this is causing the dreaded "Lock wait timeout exceeded; try restarting transaction" error on some mysql/mariadb configurations. 1) Test plan 2) Download a marc record from Koha 3) Modify the title of that same bib in Koha 4) Stage the downloaded record and overlay the existing record 5) Verify the title has reverted to the original title from the downloaded record! Created attachment 152297 [details] [review] Bug 33987 - Combine multiple db updates one in BatchCommitRecords When replacing existing records BatchCommitRecords will the table import_records will be updated three times for three different fields by three different queries. Not only is this inefficient ( especially for large batches ), it seems that this is causing the dreaded "Lock wait timeout exceeded; try restarting transaction" error on some mysql/mariadb configurations. 1) Test plan 2) Download a marc record from Koha 3) Modify the title of that same bib in Koha 4) Stage the downloaded record and overlay the existing record 5) Verify the title has reverted to the original title from the downloaded record! Signed-off-by: Sam Lau <samalau@gmail.com> Comment on attachment 152297 [details] [review] Bug 33987 - Combine multiple db updates one in BatchCommitRecords Review of attachment 152297 [details] [review]: ----------------------------------------------------------------- ::: C4/ImportBatch.pm @@ +682,5 @@ > $query = "UPDATE import_auths SET matched_authid = ? WHERE import_record_id = ?"; > } > + # Combine xml update, SetImportRecordOverlayStatus, and SetImportRecordStatus updates into a single update for efficiency, especially in a transaction > + my $sth = $dbh->prepare_cached("UPDATE import_records SET marcxml_old = ?, status = ?, overlay_status = ? WHERE import_record_id = ?"); > + $sth->execute($oldxml, 'imported', 'match_applied',, $rowref->{'import_record_id'}); Is there a typo on this line? I see two commas in a row? Created attachment 152399 [details] [review] Bug 33987 - Combine multiple db updates one in BatchCommitRecords When replacing existing records BatchCommitRecords will the table import_records will be updated three times for three different fields by three different queries. Not only is this inefficient ( especially for large batches ), it seems that this is causing the dreaded "Lock wait timeout exceeded; try restarting transaction" error on some mysql/mariadb configurations. 1) Test plan 2) Download a marc record from Koha 3) Modify the title of that same bib in Koha 4) Stage the downloaded record and overlay the existing record 5) Verify the title has reverted to the original title from the downloaded record! Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to David Cook from comment #3) > Comment on attachment 152297 [details] [review] [review] > Bug 33987 - Combine multiple db updates one in BatchCommitRecords > > Review of attachment 152297 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/ImportBatch.pm > @@ +682,5 @@ > > $query = "UPDATE import_auths SET matched_authid = ? WHERE import_record_id = ?"; > > } > > + # Combine xml update, SetImportRecordOverlayStatus, and SetImportRecordStatus updates into a single update for efficiency, especially in a transaction > > + my $sth = $dbh->prepare_cached("UPDATE import_records SET marcxml_old = ?, status = ?, overlay_status = ? WHERE import_record_id = ?"); > > + $sth->execute($oldxml, 'imported', 'match_applied',, $rowref->{'import_record_id'}); > > Is there a typo on this line? I see two commas in a row? I removed the second comma. It didnt matter but it should not be there (In reply to Kyle M Hall from comment #0) > When replacing existing records BatchCommitRecords will the table > import_records will be updated three times for three different fields by > three different queries. Not only is this inefficient ( especially for large > batches ), it seems that this is causing the dreaded "Lock wait timeout > exceeded; try restarting transaction" error on some mysql/mariadb > configurations. Patch is fine. Would be nice if this would prevent a deadlock. But I dont think that this is the ultimate solution. We need to dig a bit deeper still probably. Continuing on 33972 Created attachment 152403 [details] [review] Bug 33987 - Combine multiple db updates one in BatchCommitRecords When replacing existing records BatchCommitRecords will the table import_records will be updated three times for three different fields by three different queries. Not only is this inefficient ( especially for large batches ), it seems that this is causing the dreaded "Lock wait timeout exceeded; try restarting transaction" error on some mysql/mariadb configurations. 1) Test plan 2) Download a marc record from Koha 3) Modify the title of that same bib in Koha 4) Stage the downloaded record and overlay the existing record 5) Verify the title has reverted to the original title from the downloaded record! Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 152406 [details] [review] Bug 33987: Combine multiple db updates one in BatchCommitRecords When replacing existing records BatchCommitRecords will the table import_records will be updated three times for three different fields by three different queries. Not only is this inefficient ( especially for large batches ), it seems that this is causing the dreaded "Lock wait timeout exceeded; try restarting transaction" error on some mysql/mariadb configurations. 1) Test plan 2) Download a marc record from Koha 3) Modify the title of that same bib in Koha 4) Stage the downloaded record and overlay the existing record 5) Verify the title has reverted to the original title from the downloaded record! Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Fixed the commit message ;) Pushed to master for 23.11. Nice work everyone, thanks! Thanks for all the hard work! Pushed to 23.05.x for the next release Nice work everyone! Pushed to 22.11.x for next release |