Bug 37775 - update_totalissues.pl uses $dbh->commit but does not use transactions
Summary: update_totalissues.pl uses $dbh->commit but does not use transactions
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on: 36474
Blocks:
  Show dependency treegraph
 
Reported: 2024-08-29 12:41 UTC by Nick Clemens (kidclamp)
Modified: 2024-09-13 17:58 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option (3.48 KB, patch)
2024-08-30 12:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37775: Spelling and tidy (4.47 KB, patch)
2024-08-30 12:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option (3.53 KB, patch)
2024-09-13 17:58 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37775: Spelling and tidy (4.52 KB, patch)
2024-09-13 17:58 UTC, Brendan Lawlor
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-08-29 12:41:17 UTC
The script disables AutoCommit and calls commit on the dbh directly. As this dbh is shared I believe it closes all current transactions. We have seen in the logs a warning when the cron is running during library operating hours:
DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_3 does not exist at /usr/share/koha/lib/Koha/Object.pm line 170

I believe if we resolve bug 36474, effectively reducing the scope fo the job from "all records in the DB" to "only updated records" - we can remove the need for delayed commits here.
Comment 1 Nick Clemens (kidclamp) 2024-08-30 12:52:33 UTC
Created attachment 170908 [details] [review]
Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option

This patch removes setting AutoCommit to 0 and commiting only every X records.
Instead we commit as we go and report progress using a parameter.

Bug 36474 reduced the numebr of changes that are being committed, so this should be a reasonable change. The
use of commits without transactions was causing problems if the library was active while the script ran.

To test:
1 - perl misc/cronjobs/update_totalissues.pl -c
2 - Script runs, but with unknown parameter
3 - perl misc/cronjobs/update_totalissues.pl -p 10
4 - Script runs and reports every 10 records
5 - per; misc/cronjobs/update_totalissues.pl
6 - Script runs and reports every 100 records by default
Comment 2 Nick Clemens (kidclamp) 2024-08-30 12:52:35 UTC
Created attachment 170909 [details] [review]
Bug 37775: Spelling and tidy
Comment 3 Brendan Lawlor 2024-09-13 17:58:27 UTC
Created attachment 171490 [details] [review]
Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option

This patch removes setting AutoCommit to 0 and commiting only every X records.
Instead we commit as we go and report progress using a parameter.

Bug 36474 reduced the numebr of changes that are being committed, so this should be a reasonable change. The
use of commits without transactions was causing problems if the library was active while the script ran.

To test:
1 - perl misc/cronjobs/update_totalissues.pl -c
2 - Script runs, but with unknown parameter
3 - perl misc/cronjobs/update_totalissues.pl -p 10
4 - Script runs and reports every 10 records
5 - per; misc/cronjobs/update_totalissues.pl
6 - Script runs and reports every 100 records by default

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 4 Brendan Lawlor 2024-09-13 17:58:29 UTC
Created attachment 171491 [details] [review]
Bug 37775: Spelling and tidy

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>