Bug 37775

Summary: update_totalissues.pl uses $dbh->commit but does not use transactions
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Command-line UtilitiesAssignee: Nick Clemens (kidclamp) <nick>
Status: Pushed to main --- QA Contact: Martin Renvoize <martin.renvoize>
Severity: major    
Priority: P5 - low CC: martin.renvoize, robin
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:
Bug Depends on: 36474    
Bug Blocks:    
Attachments: Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option
Bug 37775: Spelling and tidy
Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option
Bug 37775: Spelling and tidy
Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option
Bug 37775: Spelling and tidy

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>
Comment 5 Martin Renvoize 2024-09-18 04:10:01 UTC
Created attachment 171648 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2024-09-18 04:10:03 UTC
Created attachment 171649 [details] [review]
Bug 37775: Spelling and tidy

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2024-09-19 11:35:50 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant