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: Pushed to oldstable
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: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 36474
Blocks:
  Show dependency treegraph
 
Reported: 2024-08-29 12:41 UTC by Nick Clemens (kidclamp)
Modified: 2024-11-04 00:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.04,23.11.10
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
Bug 37775: Remove delayed commits from update_totalissues.pl and add progress option (3.59 KB, patch)
2024-09-18 04:10 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37775: Spelling and tidy (4.58 KB, patch)
2024-09-18 04:10 UTC, Martin Renvoize (ashimema)
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>
Comment 5 Martin Renvoize (ashimema) 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 (ashimema) 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 (ashimema) 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
Comment 8 Lucas Gass (lukeg) 2024-09-24 15:26:31 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 9 Fridolin Somers 2024-10-21 12:57:31 UTC
Pushed to 23.11.x for 23.11.10
Comment 10 Wainui Witika-Park 2024-11-04 00:02:30 UTC
Not backporting to 23.05.x unless requested