Attachment 123688 Details for Bug 28833 – Bug 28833: Speed up holds queue builder via parallel processing

[patch] Bug 28833: Speed up holds queue builder via parallel processing

Bug-28833-Speed-up-holds-queue-builder-via-paralle.patch (text/plain), 7.82 KB, created by Kyle M Hall (khall) on 2021-08-10 13:26:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2021-08-10 13:26:18 UTC

Size: 7.82 KB

patch

obsolete

>From b211273a3fae441027b1ff8d6cdaae03efba71dc Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Mon, 9 Aug 2021 13:46:00 -0400
>Subject: [PATCH] Bug 28833: Speed up holds queue builder via parallel
> processing
>
>The holds queue builder can take a very long time to run on systems with many holds. For example, a partner with 124,784 unfilled ( not found ) holds, is taking about 64 minutes to run. If we run that same number of holds in 5 parallel chunks ( splitting the number of records as evenly as possible, but *not* taking into account the holds per bib ), it takes 21.5 minutes. If we use 10 loops, it takes less then 14 minutes.
>
>Test Plan:
>0) Install the Perl library Parallel::ForkManager
>1) Generate a huge number of holds ( a few thousand at the minimum )
>2) Run the holds queue builder, use the `time` utility to track how much
>   time it took to run
>3) Set HoldsQueueParallelLoopsCount to 10
>4) Repeat step 2, note the improvement in speed
>5) Experiment with other values for HoldsQueueParallelLoopsCount
>6) prove t/db_dependent/HoldsQueue.t
>---
> C4/HoldsQueue.pm                              | 49 ++++++++++++++-----
> .../data/mysql/atomicupdate/bug_28833.perl    |  9 ++++
> installer/data/mysql/mandatory/sysprefs.sql   |  1 +
> .../admin/preferences/circulation.pref        |  5 ++
> misc/cronjobs/holds/build_holds_queue.pl      |  4 +-
> 5 files changed, 56 insertions(+), 12 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug_28833.perl
>
>diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
>index 06b0b2a2ef..3e2d7a60f0 100644
>--- a/C4/HoldsQueue.pm
>+++ b/C4/HoldsQueue.pm
>@@ -30,8 +30,10 @@ use Koha::Items;
> use Koha::Patrons;
> use Koha::Libraries;
> 
>-use List::Util qw( shuffle );
> use List::MoreUtils qw( any );
>+use List::Util qw( shuffle );
>+use POSIX qw(ceil);
>+use Parallel::ForkManager;
> 
> our (@ISA, @EXPORT_OK);
> BEGIN {
>@@ -173,16 +175,13 @@ Top level function that turns reserves into tmp_holdsqueue and hold_fill_targets
> =cut
> 
> sub CreateQueue {
>-    my $dbh   = C4::Context->dbh;
>+    my $loops = shift || 10;
>+
>+    my $dbh = C4::Context->dbh;
> 
>     $dbh->do("DELETE FROM tmp_holdsqueue");  # clear the old table for new info
>     $dbh->do("DELETE FROM hold_fill_targets");
> 
>-    my $total_bibs            = 0;
>-    my $total_requests        = 0;
>-    my $total_available_items = 0;
>-    my $num_items_mapped      = 0;
>-
>     my $branches_to_use;
>     my $transport_cost_matrix;
>     my $use_transport_cost_matrix = C4::Context->preference("UseTransportCostMatrix");
>@@ -199,19 +198,47 @@ sub CreateQueue {
> 
>     my $bibs_with_pending_requests = GetBibsWithPendingHoldRequests();
> 
>+    # Split the list of bibs into groups to run in parallel
>+    if ( $loops > 1 ) {
>+        my $bibs_per_chunk = ceil( scalar @$bibs_with_pending_requests / $loops );
>+        my @chunks;
>+
>+        push( @chunks, [ splice @$bibs_with_pending_requests, 0, $bibs_per_chunk ] ) while @$bibs_with_pending_requests;
>+        push( @{$chunks[0]}, @$bibs_with_pending_requests ); # Add any remainders to the first parallel process
>+
>+        my $pm = Parallel::ForkManager->new($loops);
>+
>+        DATA_LOOP:
>+        foreach my $chunk (@chunks) {
>+            my $pid = $pm->start and next DATA_LOOP;
>+            _ProcessBiblios($chunk);
>+            $pm->finish;
>+        }
>+
>+        $pm->wait_all_children;
>+    } else {
>+        _ProcessBiblios($bibs_with_pending_requests, $branches_to_use, $transport_cost_matrix);
>+    }
>+}
>+
>+=head2 _ProcessBiblios
>+
>+=cut
>+
>+sub _ProcessBiblios {
>+    my $bibs_with_pending_requests = shift;
>+    my $branches_to_use = shift;
>+    my $transport_cost_matrix = shift;
>+
>     foreach my $biblionumber (@$bibs_with_pending_requests) {
>-        $total_bibs++;
>         my $hold_requests   = GetPendingHoldRequestsForBib($biblionumber);
>         my $available_items = GetItemsAvailableToFillHoldRequestsForBib($biblionumber, $branches_to_use);
>-        $total_requests        += scalar(@$hold_requests);
>-        $total_available_items += scalar(@$available_items);
> 
>         my $item_map = MapItemsToHoldRequests($hold_requests, $available_items, $branches_to_use, $transport_cost_matrix);
>         $item_map  or next;
>         my $item_map_size = scalar(keys %$item_map)
>           or next;
> 
>-        $num_items_mapped += $item_map_size;
>         CreatePicklistFromItemMap($item_map);
>         AddToHoldTargetMap($item_map);
>         if (($item_map_size < scalar(@$hold_requests  )) and
>diff --git a/installer/data/mysql/atomicupdate/bug_28833.perl b/installer/data/mysql/atomicupdate/bug_28833.perl
>new file mode 100644
>index 0000000000..1d0c1adcca
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_28833.perl
>@@ -0,0 +1,9 @@
>+$DBversion = 'XXX'; # will be replaced by the RM
>+if( CheckVersion( $DBversion ) ) {
>+    $dbh->do(q{
>+        INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES
>+        ('HoldsQueueParallelLoopsCount', '1', NULL, 'Number of parallel loops to use when running the holds queue builder', 'Integer');
>+    });
>+
>+    NewVersion( $DBversion, 28833, "Speed up holds queue builder via parallel processing");
>+}
>diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql
>index 6b66e42c14..0ad4dcb8cc 100644
>--- a/installer/data/mysql/mandatory/sysprefs.sql
>+++ b/installer/data/mysql/mandatory/sysprefs.sql
>@@ -241,6 +241,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('HoldsAutoFillPrintSlip','0',NULL,'If on, hold slip print dialog will be displayed automatically','YesNo'),
> ('HoldsLog','0',NULL,'If ON, log create/cancel/suspend/resume actions on holds.','YesNo'),
> ('HoldsNeedProcessingSIP', '0', NULL, 'Require staff to check-in before hold is set to waiting state', 'YesNo' ),
>+('HoldsQueueParallelLoopsCount', '1', NULL, 'Number of parallel loops to use when running the holds queue builder', 'Integer'),
> ('HoldsQueueSkipClosed', '0', NULL, 'If enabled, any libraries that are closed when the holds queue is built will be ignored for the purpose of filling holds.', 'YesNo'),
> ('HoldsSplitQueue','nothing','nothing|branch|itemtype|branch_itemtype','In the staff interface, split the holds view by the given criteria','Choice'),
> ('HoldsToPullStartDate','2',NULL,'Set the default start date for the Holds to pull list to this many days ago','Integer'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>index 3c10f1a69f..d3f26f09ac 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>@@ -74,6 +74,11 @@ Circulation:
>             - pref: HoldsToPullStartDate
>               class: integer
>             - day(s) ago. Note that the default end date is controlled by the system preference <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=ConfirmFutureHolds">ConfirmFutureHolds</a>.
>+        -
>+            - When building the holds queue, calculate hold matches using
>+            - pref: HoldsQueueParallelLoopsCount
>+              class: integer
>+            - parallel loop(s). The more loops used, the faster it will calculate and the more computing resources it will use.
>         -
>             - pref: AllowAllMessageDeletion
>               choices:
>diff --git a/misc/cronjobs/holds/build_holds_queue.pl b/misc/cronjobs/holds/build_holds_queue.pl
>index 6e1437dcd3..a3815511a3 100755
>--- a/misc/cronjobs/holds/build_holds_queue.pl
>+++ b/misc/cronjobs/holds/build_holds_queue.pl
>@@ -18,8 +18,10 @@ BEGIN {
> use Koha::Script -cron;
> use C4::HoldsQueue qw(CreateQueue);
> use C4::Log qw( cronlogaction );
>+use C4::Context;
> 
> cronlogaction();
> 
>-CreateQueue();
>+my $loops = C4::Context->preference('HoldsQueueParallelLoopsCount');
>+CreateQueue($loops);
> 
>-- 
>2.30.1 (Apple Git-130)

Actions: View | Diff | Splinter Review

Attachments on bug 28833: 123656 | 123657 | 123659 | 123671 | 123679 | 123688 | 123689 | 123783 | 123784 | 123785 | 123786 | 123996 | 126200 | 126202 | 126216 | 167349 | 173045 | 173046 | 173262