From a6a8a987d8166127ffcdb938553a8ebfd3232574 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 4 May 2023 19:28:01 +0000 Subject: [PATCH] Bug 29507: Speed up auto renew cronjob via parallel processing The cron can take a very long time to run on systems with many issues. For example, a partner with ~250k auto_renew issues is taking about 9 hours to run. If we run that same number of issues in 5 parallel chunks ( splitting the number of issues as evenly as possible ), it could take under 2 hours. Test Plan: 1) Generate a number of issues marked for auto_renew 2) Run the automatic_renewals.pl, use the `time` utility to track how much time it took to run 3) Set parallel_loops to 10 in auto_renew_cronjob section of config in koha-conf 4) Repeat step 2, note the improvement in speed 5) Experiment with other values --- debian/templates/koha-conf-site.xml.in | 4 + etc/koha-conf.xml | 4 + misc/cronjobs/automatic_renewals.pl | 320 ++++++++++++++----------- 3 files changed, 190 insertions(+), 138 deletions(-) diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in index 8733dc6690..079a09a621 100644 --- a/debian/templates/koha-conf-site.xml.in +++ b/debian/templates/koha-conf-site.xml.in @@ -459,6 +459,10 @@ __END_SRU_PUBLICSERVER__ __MESSAGE_BROKER_VHOST__ + + 1 + + 1 diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml index 829ffd3e7d..7ff10dc511 100644 --- a/etc/koha-conf.xml +++ b/etc/koha-conf.xml @@ -271,6 +271,10 @@ + + 1 + + 1 diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index 16006d5989..b8dc58d26d 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -76,6 +76,7 @@ chosen 'Digests only' on the advance messages. use Modern::Perl; use Pod::Usage qw( pod2usage ); +use Parallel::ForkManager; use Getopt::Long qw( GetOptions ); use Koha::Script -cron; @@ -135,7 +136,7 @@ $verbose = 1 unless $verbose or $confirm; print "Test run only\n" unless $confirm; print "getting auto renewals\n" if $verbose; -my $auto_renews = Koha::Checkouts->search( +my @auto_renews = Koha::Checkouts->search( { auto_renew => 1, 'patron.autorenew_checkouts' => 1, @@ -143,173 +144,216 @@ my $auto_renews = Koha::Checkouts->search( { join => ['patron','item'] } -); -print "found " . $auto_renews->count . " auto renewals\n" if $verbose; +)->as_list; +print "found " . scalar @auto_renews . " auto renewals\n" if $verbose; my $renew_digest = {}; my %report; my @item_renewal_ids; -while ( my $auto_renew = $auto_renews->next ) { - print "examining item '" . $auto_renew->itemnumber . "' to auto renew\n" if $verbose; - - my ( $borrower_preferences, $wants_messages, $wants_digest ) = ( undef, 0, 0 ); - if ( $send_notices_pref eq 'preferences' ){ - $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( - { - borrowernumber => $auto_renew->borrowernumber, - message_name => 'auto_renewals' - } - ); - $wants_messages = 1 - if $borrower_preferences - && $borrower_preferences->{transports}; - $wants_digest = 1 - if $wants_messages - && $borrower_preferences->{wants_digest}; - } else { # Preference is never or cron - $wants_messages = $send_notices; - $wants_digest = 0; - } - # CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script - my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->patron, $auto_renew, undef, 1 ); - my $updated; - if ( $error eq 'auto_renew' ) { - $updated = 1; - if ($verbose) { - say sprintf "Issue id: %s for borrower: %s and item: %s %s be renewed.", - $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, - $confirm ? 'will' : 'would'; +my $cron_options = C4::Context->config('auto_renew_cronjob'); +my $loops = $cron_options ? $cron_options->{parallel_loops_count} // 1 : 1; + +# Split the list of issues into chunks to run in parallel +my @chunks; +if ( $loops > 1 ) { + my $i = 0; + my $borrowernumber = 0; + while (@auto_renews) { + my $auto_renew = pop(@auto_renews); + if ( $borrowernumber != $auto_renew->borrowernumber ) { + $i++ if $borrowernumber; + $borrowernumber = $auto_renew->borrowernumber; } - if ($confirm) { - my $date_due = AddRenewal( + $i = 0 if $i >= $loops; + push( @{ $chunks[$i] }, $auto_renew ); + } + my $pm = Parallel::ForkManager->new($loops); + DATA_LOOP: + foreach my $chunk (@chunks) { + my $pid = $pm->start and next DATA_LOOP; + _ProcessRenewals($chunk); + $pm->finish; + } + $pm->wait_all_children; +} +else { + _ProcessRenewals( \@auto_renews ); +} + +cronlogaction({ action => 'End', info => "COMPLETED" }); + +=head1 METHODS + +=head2 _ProcessRenewals + + Internal method to process the queue in chunks + +=cut + +sub _ProcessRenewals { + + my $auto_renew_issues = shift; + + foreach my $auto_renew (@$auto_renew_issues) { + print "examining item '" . $auto_renew->itemnumber . "' to auto renew\n" if $verbose; + + my ( $borrower_preferences, $wants_messages, $wants_digest ) = ( undef, 0, 0 ); + if ( $send_notices_pref eq 'preferences' ){ + $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $auto_renew->borrowernumber, - itemnumber => $auto_renew->itemnumber, - branch => $auto_renew->branchcode, - seen => 0, - automatic => 1, + message_name => 'auto_renewals' } ); - push @item_renewal_ids, $auto_renew->itemnumber; - $auto_renew->auto_renew_error(undef)->store; + $wants_messages = 1 + if $borrower_preferences + && $borrower_preferences->{transports}; + $wants_digest = 1 + if $wants_messages + && $borrower_preferences->{wants_digest}; + } else { # Preference is never or cron + $wants_messages = $send_notices; + $wants_digest = 0; } - push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew - if ($wants_messages) && !$wants_digest; - } elsif ( $error eq 'too_many' - || $error eq 'on_reserve' - || $error eq 'restriction' - || $error eq 'overdue' - || $error eq 'too_unseen' - || $error eq 'auto_account_expired' - || $error eq 'auto_too_late' - || $error eq 'auto_too_much_oweing' - || $error eq 'auto_too_soon' - || $error eq 'too_soon' - || $error eq 'item_denied_renewal' - || $error eq 'item_issued_to_other_patron' ) - { - if ($verbose) { - say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)", - $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, - $confirm ? 'will' : 'would', $error; - } - $updated = 1 if ( !$auto_renew->auto_renew_error || $error ne $auto_renew->auto_renew_error ); - if ($updated) { - $auto_renew->auto_renew_error($error)->store if $confirm; + + # CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script + my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->patron, $auto_renew, undef, 1 ); + my $updated; + if ( $error eq 'auto_renew' ) { + $updated = 1; + if ($verbose) { + say sprintf "Issue id: %s for borrower: %s and item: %s %s be renewed.", + $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, + $confirm ? 'will' : 'would'; + } + if ($confirm) { + my $date_due = AddRenewal( + { + borrowernumber => $auto_renew->borrowernumber, + itemnumber => $auto_renew->itemnumber, + branch => $auto_renew->branchcode, + seen => 0, + automatic => 1, + } + ); + push @item_renewal_ids, $auto_renew->itemnumber; + $auto_renew->auto_renew_error(undef)->store; + } push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew - if $error ne 'auto_too_soon' && ( $wants_messages && !$wants_digest ); # Do not notify if it's too soon + if ($wants_messages) && !$wants_digest; + } elsif ( $error eq 'too_many' + || $error eq 'on_reserve' + || $error eq 'restriction' + || $error eq 'overdue' + || $error eq 'too_unseen' + || $error eq 'auto_account_expired' + || $error eq 'auto_too_late' + || $error eq 'auto_too_much_oweing' + || $error eq 'auto_too_soon' + || $error eq 'too_soon' + || $error eq 'item_denied_renewal' + || $error eq 'item_issued_to_other_patron' ) + { + if ($verbose) { + say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)", + $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, + $confirm ? 'will' : 'would', $error; + } + $updated = 1 if ( !$auto_renew->auto_renew_error || $error ne $auto_renew->auto_renew_error ); + if ($updated) { + $auto_renew->auto_renew_error($error)->store if $confirm; + push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew + if $error ne 'auto_too_soon' && ( $wants_messages && !$wants_digest ); # Do not notify if it's too soon + } } - } - if ( $wants_digest ) { - # cache this one to process after we've run through all of the items. - if ($digest_per_branch) { - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{success}++ if $error eq 'auto_renew'; - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++ unless $error eq 'auto_renew' || $error eq 'auto_too_soon'; - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{results}->{$error}++ ; - push @{$renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{issues}}, $auto_renew->itemnumber; - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && $error ne 'auto_too_soon'; - } else { - $renew_digest->{ $auto_renew->borrowernumber }->{success} ++ if $error eq 'auto_renew'; - $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $error eq 'auto_renew' || $error eq 'auto_too_soon'; - $renew_digest->{ $auto_renew->borrowernumber }->{results}->{$error}++ ; - $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && $error ne 'auto_too_soon'; - push @{$renew_digest->{ $auto_renew->borrowernumber }->{issues}}, $auto_renew->itemnumber; + if ( $wants_digest ) { + # cache this one to process after we've run through all of the items. + if ($digest_per_branch) { + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{success}++ if $error eq 'auto_renew'; + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++ unless $error eq 'auto_renew' || $error eq 'auto_too_soon'; + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{results}->{$error}++ ; + push @{$renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{issues}}, $auto_renew->itemnumber; + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && $error ne 'auto_too_soon'; + } else { + $renew_digest->{ $auto_renew->borrowernumber }->{success} ++ if $error eq 'auto_renew'; + $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $error eq 'auto_renew' || $error eq 'auto_too_soon'; + $renew_digest->{ $auto_renew->borrowernumber }->{results}->{$error}++ ; + $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && $error ne 'auto_too_soon'; + push @{$renew_digest->{ $auto_renew->borrowernumber }->{issues}}, $auto_renew->itemnumber; + } } - } -} + } -if( @item_renewal_ids ){ - my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); - $indexer->index_records( \@item_renewal_ids, "specialUpdate", "biblioserver" ); -} + if( @item_renewal_ids ){ + my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); + $indexer->index_records( \@item_renewal_ids, "specialUpdate", "biblioserver" ); + } -if ( $send_notices && $confirm ) { - for my $borrowernumber ( keys %report ) { - my $patron = Koha::Patrons->find($borrowernumber); - my $borrower_preferences = - C4::Members::Messaging::GetMessagingPreferences( - { - borrowernumber => $borrowernumber, - message_name => 'auto_renewals' - } - ); - for my $issue ( @{ $report{$borrowernumber} } ) { - my $item = $issue->item; - # Force sending of email and only email if pref is set to "cron" - my @transports = $send_notices_pref eq 'preferences' ? keys %{ $borrower_preferences->{'transports'} } : 'email'; - foreach my $transport ( @transports ) { - my $letter = C4::Letters::GetPreparedLetter ( - module => 'circulation', - letter_code => 'AUTO_RENEWALS', - tables => { - borrowers => $patron->borrowernumber, - issues => $issue->itemnumber, - items => $issue->itemnumber, - biblio => $item->biblionumber, - }, - lang => $patron->lang, - message_transport_type => $transport, + if ( $send_notices && $confirm ) { + for my $borrowernumber ( keys %report ) { + my $patron = Koha::Patrons->find($borrowernumber); + my $borrower_preferences = + C4::Members::Messaging::GetMessagingPreferences( + { + borrowernumber => $borrowernumber, + message_name => 'auto_renewals' + } ); - - if ($letter) { - my $library = $patron->library; - my $admin_email_address = $library->from_email_address; - - C4::Letters::EnqueueLetter( - { - letter => $letter, - borrowernumber => $borrowernumber, - from_address => $admin_email_address, - message_transport_type => $transport - } + for my $issue ( @{ $report{$borrowernumber} } ) { + my $item = $issue->item; + # Force sending of email and only email if pref is set to "cron" + my @transports = $send_notices_pref eq 'preferences' ? keys %{ $borrower_preferences->{'transports'} } : 'email'; + foreach my $transport ( @transports ) { + my $letter = C4::Letters::GetPreparedLetter ( + module => 'circulation', + letter_code => 'AUTO_RENEWALS', + tables => { + borrowers => $patron->borrowernumber, + issues => $issue->itemnumber, + items => $issue->itemnumber, + biblio => $item->biblionumber, + }, + lang => $patron->lang, + message_transport_type => $transport, ); + + if ($letter) { + my $library = $patron->library; + my $admin_email_address = $library->from_email_address; + + C4::Letters::EnqueueLetter( + { + letter => $letter, + borrowernumber => $borrowernumber, + from_address => $admin_email_address, + message_transport_type => $transport + } + ); + } } } } - } - if ($digest_per_branch) { - while (my ($branchcode, $digests) = each %$renew_digest) { + if ($digest_per_branch) { + while (my ($branchcode, $digests) = each %$renew_digest) { + send_digests({ + digests => $digests, + branchcode => $branchcode, + letter_code => 'AUTO_RENEWALS_DGST', + }); + } + } else { send_digests({ - digests => $digests, - branchcode => $branchcode, + digests => $renew_digest, letter_code => 'AUTO_RENEWALS_DGST', }); } - } else { - send_digests({ - digests => $renew_digest, - letter_code => 'AUTO_RENEWALS_DGST', - }); } -} -cronlogaction({ action => 'End', info => "COMPLETED" }); - -=head1 METHODS +} =head2 send_digests -- 2.30.2