Bug 37552 - Automatic renewals cronjob can die when an item scheduled for renewal is checked in
Summary: Automatic renewals cronjob can die when an item scheduled for renewal is chec...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Emily Lamancusa (emlam)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-08-01 21:35 UTC by Nick Clemens (kidclamp)
Modified: 2024-09-18 06:47 UTC (History)
8 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.09
Circulation function:


Attachments
Bug 37552: Wrap auto renewal attempt in eval to ensure script does not die (2.20 KB, patch)
2024-08-01 21:54 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37552: Wrap auto renewal attempt in eval to ensure script does not die (2.25 KB, patch)
2024-08-03 16:29 UTC, CJ Lynce
Details | Diff | Splinter Review
Bug 37552: Wrap auto renewal attempt in eval to ensure script does not die (2.30 KB, patch)
2024-08-14 15:17 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 37552: (24.05.x) Wrap auto renewal attempt in eval to ensure script does not die (1.22 KB, patch)
2024-09-09 13:48 UTC, Lucas Gass (lukeg)
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-01 21:35:20 UTC
We start the automatic_renewal cron by getting all the checkouts that might be renewed

We then loop through and try to renew each. In a large system this can take some time.

If during that time an item in the list is checked in the cronjob will die when it tries to update the checkout. In my testing it only happens for items that will not be renewed.

To recreate:
1 - Checkout an item to a patron and mark it for auto renewal
2 - Add 'sleep(10);' to the cronjob at line 194
3 - perl misc/cronjobs/automatic_renewals.pl -v -c
4 - Check the item in (quickly)
5 - The cron dies:
DBIx::Class::Row::update(): Can't update Koha::Schema::Result::Issue=HASH(0x586e1a674fb0): row not found at /kohadevbox/koha/Koha/Object.pm line 172
DBIx::Class::Row::update(): Can't update Koha::Schema::Result::Issue=HASH(0x586e1a674fb0): row not found at /kohadevbox/koha/Koha/Object.pm line 172
Comment 1 Nick Clemens (kidclamp) 2024-08-01 21:54:06 UTC
Created attachment 169966 [details] [review]
Bug 37552: Wrap auto renewal attempt in eval to ensure script does not die

When libraries have a lot of checkouts, or an AMH, checkins can happen while the cron is running.

This patch simply adds an eval around the auto renewal attempt in case of early check in or other errors.

You can verify cron completed by enabling cronjob log in system preferences and checking the action logs

To test:
1 - Add 'sleep(10);' to automatic_renewals.pl
2 - Set circulation rules to enable automatic renewals
3 - Issue an item to a patron
4 - perl misc/cronjobs/automatic_renewals.pl -v
5 - Confirm item would not be renewed
6 - perl misc/cronjobs/automatic_renewals.pl -v -c
7 - Quickly check in the item
8 - The cronjob dies
    DBIx::Class::Row::update(): Can't update Koha::Schema::Result::Issue=HASH(0x586e1a674fb0): row not found at /kohadevbox/koha/Koha/Object.pm line 172
9 - Apply patch
10 - Checkout the item again
11 - perl misc/cronjobs/automatic_renewals.pl -v -c
12 - Quickly checkin the item
13 - You get a warning, but the cron completes
Comment 2 CJ Lynce 2024-08-03 16:29:18 UTC
Created attachment 170043 [details] [review]
Bug 37552: Wrap auto renewal attempt in eval to ensure script does not die

When libraries have a lot of checkouts, or an AMH, checkins can happen while the cron is running.

This patch simply adds an eval around the auto renewal attempt in case of early check in or other errors.

You can verify cron completed by enabling cronjob log in system preferences and checking the action logs

To test:
1 - Add 'sleep(10);' to automatic_renewals.pl
2 - Set circulation rules to enable automatic renewals
3 - Issue an item to a patron
4 - perl misc/cronjobs/automatic_renewals.pl -v
5 - Confirm item would not be renewed
6 - perl misc/cronjobs/automatic_renewals.pl -v -c
7 - Quickly check in the item
8 - The cronjob dies
    DBIx::Class::Row::update(): Can't update Koha::Schema::Result::Issue=HASH(0x586e1a674fb0): row not found at /kohadevbox/koha/Koha/Object.pm line 172
9 - Apply patch
10 - Checkout the item again
11 - perl misc/cronjobs/automatic_renewals.pl -v -c
12 - Quickly checkin the item
13 - You get a warning, but the cron completes

Signed-off-by: CJ Lynce <cj.lynce@westlakelibrary.org>
Comment 3 CJ Lynce 2024-08-03 16:33:23 UTC
Confirmed that the existing script appears to die only with items that would not be renewed. The patch corrects this so that the script no longer dies, but produces an error and moves on, when an item is checked-in mid-processing of automatic renewals.
Comment 4 Emily Lamancusa (emlam) 2024-08-14 15:17:31 UTC
Created attachment 170320 [details] [review]
Bug 37552: Wrap auto renewal attempt in eval to ensure script does not die

When libraries have a lot of checkouts, or an AMH, checkins can happen while the cron is running.

This patch simply adds an eval around the auto renewal attempt in case of early check in or other errors.

You can verify cron completed by enabling cronjob log in system preferences and checking the action logs

To test:
1 - Add 'sleep(10);' to automatic_renewals.pl
2 - Set circulation rules to enable automatic renewals
3 - Issue an item to a patron
4 - perl misc/cronjobs/automatic_renewals.pl -v
5 - Confirm item would not be renewed
6 - perl misc/cronjobs/automatic_renewals.pl -v -c
7 - Quickly check in the item
8 - The cronjob dies
    DBIx::Class::Row::update(): Can't update Koha::Schema::Result::Issue=HASH(0x586e1a674fb0): row not found at /kohadevbox/koha/Koha/Object.pm line 172
9 - Apply patch
10 - Checkout the item again
11 - perl misc/cronjobs/automatic_renewals.pl -v -c
12 - Quickly checkin the item
13 - You get a warning, but the cron completes

Signed-off-by: CJ Lynce <cj.lynce@westlakelibrary.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 5 Emily Lamancusa (emlam) 2024-08-14 15:22:15 UTC
I squashed in a perltidy, but otherwise the code looks good and the QA script is happy. Passing QA
Comment 6 Katrin Fischer 2024-08-16 15:12:09 UTC
There are some terminology "glitches" in the script (issue vs. checkout and borrower vs. patron), but I think that's for another day. Thanks for the patch!
Comment 7 Katrin Fischer 2024-08-16 15:37:29 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 8 Lucas Gass (lukeg) 2024-09-09 13:48:06 UTC
Created attachment 171199 [details] [review]
Bug 37552: (24.05.x) Wrap auto renewal attempt in eval to ensure script does not die
Comment 9 Lucas Gass (lukeg) 2024-09-09 14:11:35 UTC
If someone wants to have a look at the 24.05.x specific patch I'd be happy to backport.
Comment 10 Lucas Gass (lukeg) 2024-09-09 14:18:08 UTC
Nick OK'd the 24.05.x patch.

Backported to 24.05.x for upcoming 24.05.04
Comment 11 Fridolin Somers 2024-09-17 14:08:03 UTC
Pushed to 23.11.x for 23.11.09
Comment 12 Wainui Witika-Park 2024-09-18 06:47:50 UTC
Not backporting to 23.05.x unless requested