From 487ffeae6d4fbfa9c71bbbecdd6dfacfe1b9f117 Mon Sep 17 00:00:00 2001 From: Bill Erickson Date: Fri, 17 Jun 2016 11:37:29 -0400 Subject: [PATCH] hold targeter reify Signed-off-by: Bill Erickson --- .../src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm | 128 +++++++++++++-------- Open-ILS/src/support-scripts/hold_targeter.pl | 112 ++++-------------- .../support-scripts/test-scripts/hold_targeter.pl | 26 ----- 3 files changed, 100 insertions(+), 166 deletions(-) delete mode 100755 Open-ILS/src/support-scripts/test-scripts/hold_targeter.pl diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm index ab9f8e3ae6..cb373548f9 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm @@ -1,4 +1,18 @@ package OpenILS::Utils::HoldTargeter; +# --------------------------------------------------------------- +# Copyright (C) 2016 King County Library System +# Author: Bill Erickson +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# --------------------------------------------------------------- use strict; use warnings; use DateTime; @@ -24,22 +38,30 @@ sub new { } # Target and retarget holds. -# By default, targets all holds that need targeting. +# By default, targets all holds that need targeting, meaning those that +# have either never been targeted or those whose prev_check_time exceeds +# the retarget interval. # # Optional parameters: # # hold => -# -- ID of a specific hold to target. +# -- ID of a specific hold to target. +# +# return_count => 1 +# -- If set, avoid collecting result objects for every hold processed +# and simply return the number of holds processed. This is useful +# for batch processing to avoid storing result data in memory for +# potentially hundreds of thousands of holds. # # retarget_interval => -# -- If set, this overrides the value found in the +# -- If set, this overrides the value found in the # 'circ.holds.retarget_interval' global flag. # # newest_first => 1 # -- If set, holds will be targeted in reverse order of create_time. # This is useful for targeting / re-targeting newer holds first. # -# target_all => 1 +# target_all => 1 # -- Forces targeting / re-targeting of all active holds. # This is primarily usefulf or testing. USE WITH CAUTION. # @@ -47,26 +69,30 @@ sub new { sub target { my ($self, %args) = @_; - foreach (qw/hold retarget_interval newest_first target_all/) { + foreach (qw/hold retarget_interval newest_first target_all return_count/) { $self->{$_} = $args{$_} if exists $args{$_}; } $self->init; + my $count = 0; my @responses; + for my $hold_id ($self->find_holds_to_target) { my $single = OpenILS::Utils::HoldTargeter::Single->new(parent => $self); $single->target($hold_id); - push(@responses, $single->result); + push(@responses, $single->result) unless $self->{return_count}; + $count++; } - return \@responses; + + return $self->{return_count} ? $count : \@responses; } sub find_holds_to_target { my $self = shift; return ($self->{hold}) if $self->{hold}; - + my $query = { select => {ahr => ['id']}, from => 'ahr', @@ -85,7 +111,7 @@ sub find_holds_to_target { if (!$self->{target_all}) { # Unless we're retargeting all holds, limit to holds that have no - # prev_check_time or those whose prev_check_time occurred + # prev_check_time or those whose prev_check_time occurred # before the retarget interval. my $date = DateTime->now->subtract( @@ -98,7 +124,7 @@ sub find_holds_to_target { } # Newest-first sorting cares only about hold create_time. - $query->{order_by} = + $query->{order_by} = [{class => 'ahr', field => 'request_time', direction => 'DESC'}] if $self->{newest_first}; @@ -134,18 +160,18 @@ sub init { name => 'circ.holds.retarget_interval', enabled => 't' })->[0]; - + # If no flag is present, default to a 24-hour retarget interval. $interval = $interval ? $interval->value : '24h'; } # Convert the interval to seconds for current and future use. $self->{retarget_interval} = interval_to_seconds($interval); - + # An org unit is considered closed for retargeting purposes # if it's closed both now and at the next re-target date. - my $next_check_time = + my $next_check_time = DateTime->now->add(seconds => $self->{retarget_interval}) ->strftime('%F %T%z'); @@ -159,7 +185,7 @@ sub init { } } - my $closed = + my $closed = $self->editor->search_actor_org_unit_closed_date($closed_orgs_query); # Map of org id to 1. Any org in the map is closed. @@ -173,7 +199,7 @@ sub get_ou_setting { $c->{$org_id} = {} unless $c->{$org_id}; - $c->{$org_id}->{$setting} = + $c->{$org_id}->{$setting} = $U->ou_ancestor_setting_value($org_id, $setting, $self->{editor}) unless exists $c->{$org_id}->{$setting}; @@ -272,7 +298,7 @@ sub copies { return $self->{copies}; } -# Final set of potential copies, including those that may not be +# Final set of potential copies, including those that may not be # currently targetable, that may be eligible for recall processing. sub recall_copies { my ($self, $recall_copies) = @_; @@ -363,7 +389,7 @@ sub get_hold_copies { my $query = { select => { - acp => ['id', 'status', 'circ_lib'], + acp => ['id', 'status', 'circ_lib'], ahr => ['current_copy'] }, from => { @@ -428,7 +454,7 @@ sub get_hold_copies { unless ($hold_type eq 'C' || $hold_type eq 'I' || $hold_type eq 'P') { # For volume and higher level holds, avoid targeting copies that # act as instances of monograph parts. - # TODO: Should this include 'R' holds? The original hold + # TODO: Should this include 'R' holds? The original hold # targeter does not include them either. $query->{from}->{acp}->{acpm} = { type => 'left', @@ -505,7 +531,7 @@ sub get_hold_copies { $self->{eligible_copy_count}." potential copies"); # Let the caller know we encountered the copy they were interested in. - $self->{found_copy} = 1 if $self->{find_copy} + $self->{found_copy} = 1 if $self->{find_copy} && grep {$_->{id} eq $self->{find_copy}} @$copies; $self->copies($copies); @@ -519,8 +545,8 @@ sub update_copy_maps { my $e = $self->editor; my $resp = $e->json_query({from => [ - 'action.hold_request_regen_copy_maps', - $self->hold_id, + 'action.hold_request_regen_copy_maps', + $self->hold_id, '{' . join(',', map {$_->{id}} @{$self->copies}) . '}' ]}); @@ -532,8 +558,8 @@ sub update_copy_maps { } # Returns a map of proximity values to arrays of copy hashes. -# The copy hash arrays are weighted consistent with the org unit hold -# target weight, meaning that a given copy may appear more than once +# The copy hash arrays are weighted consistent with the org unit hold +# target weight, meaning that a given copy may appear more than once # in its proximity list. sub compile_weighted_proximity_map { my $self = shift; @@ -546,7 +572,7 @@ sub compile_weighted_proximity_map { where => {hold => $self->hold_id} }); - my %copy_prox_map = + my %copy_prox_map = map {$_->{target_copy} => $_->{proximity}} @$hold_copy_maps; my %prox_map; @@ -555,7 +581,7 @@ sub compile_weighted_proximity_map { $prox_map{$prox} ||= []; my $weight = $self->parent->get_ou_setting( - $copy_hash->{circ_lib}, + $copy_hash->{circ_lib}, 'circ.holds.org_unit_target_weight') || 1; # Each copy is added to the list once per target weight. @@ -600,7 +626,7 @@ sub filter_closed_date_copies { return 1; } -# Limit the set of potential copies to those that are +# Limit the set of potential copies to those that are # in a targetable status. # Returns true if filtering completes without error, false otherwise. sub filter_copies_by_status { @@ -656,8 +682,8 @@ sub inspect_previous_target { return 1; } -# Returns true if we have at least one potential copy remaining, thus -# targeting should continue. Otherwise, the hold is updated to reflect +# Returns true if we have at least one potential copy remaining, thus +# targeting should continue. Otherwise, the hold is updated to reflect # that there is no target and returns false to stop targeting. sub handle_no_copies { my ($self, %args) = @_; @@ -668,14 +694,14 @@ sub handle_no_copies { return 1 if @{$self->copies} || $self->{valid_previous_copy}; } - # At this point, all copies have been inspected and none - # have yielded a targetable item. + # At this point, all copies have been inspected and none + # have yielded a targetable item. if ($args{process_recalls}) { # See if we have any copies/circs to recall. return unless $self->process_recalls; } - + my $hold = $self->hold; $hold->clear_current_copy; $hold->prev_check_time('now'); @@ -692,7 +718,7 @@ sub handle_no_copies { # F or R hold is encountered. Returns undef otherwise. sub attempt_force_recall_target { my $self = shift; - return $self->copies->[0] if + return $self->copies->[0] if $self->hold->hold_type eq 'R' || $self->hold->hold_type eq 'F'; return undef; } @@ -750,7 +776,7 @@ sub trim_copies_by_target_loop { my @circ_lib_list = keys %circ_lib_map; # Which target loop iteration are we currently on? - my $current_loop = $e->json_query({ + my $current_loop = $e->json_query({ distinct => 1, select => {aufhmxl => ['max']}, from => 'aufhmxl', @@ -759,7 +785,7 @@ sub trim_copies_by_target_loop { $current_loop = $current_loop ? $current_loop->{max} : 1; - # List of org units we've already tried targeting + # List of org units we've already tried targeting # within the current target loop. my $exclude_libs = $e->json_query({ distinct => 1, @@ -794,7 +820,7 @@ sub trim_copies_by_target_loop { my @new_copies; for my $copy (@copies) { - push(@new_copies, $copy) + push(@new_copies, $copy) if grep {$copy->{circ_lib} eq $_} @keep_libs; } @@ -844,7 +870,7 @@ sub attempt_prev_copy_retarget { "previously targeted copy [".$prev_copy->{id}."]" ); return $prev_copy; } - + return undef; } @@ -857,7 +883,7 @@ sub find_nearest_copy { my %seen; # Pick a copy at random from each tier of the proximity map, - # starting at the lowest proximity and working up, until a + # starting at the lowest proximity and working up, until a # copy is found that is suitable for targeting. for my $prox (sort {$a <=> $b} keys %prox_map) { my @copies = @{$prox_map{$prox}}; @@ -899,7 +925,7 @@ sub copy_is_permitted { return 1 if $resp->[0]->{success}; - # Copy is confirmed non-viable. + # Copy is confirmed non-viable. # Remove it from our potentials list. $self->copies([ grep {$_->{id} ne $copy->{id}} @{$self->copies} @@ -950,7 +976,7 @@ sub log_unfulfilled_hold { $unful->circ_lib($circ_lib); $unful->current_copy($prev_id); - $e->create_action_unfulfilled_hold_list($unful) or + $e->create_action_unfulfilled_hold_list($unful) or return $self->exit_targeter("Error creating unfulfilled_hold_list", 1); return 1; @@ -962,17 +988,17 @@ sub process_recalls { my $pu_lib = $self->hold->pickup_lib; - my $threshold = + my $threshold = $self->parent->get_ou_setting($pu_lib, 'circ.holds.recall_threshold') or return 1; - my $interval = + my $interval = $self->parent->get_ou_setting($pu_lib, 'circ.holds.recall_return_interval') or return 1; # Give me the ID of every checked out copy living at the hold # pickup library. - my @copy_ids = map {$_->{id}} + my @copy_ids = map {$_->{id}} grep {$_->{circ_lib} eq $pu_lib} @{$self->recall_copies}; return 1 unless @copy_ids; @@ -1002,7 +1028,7 @@ sub process_recalls { seconds => interval_to_seconds($interval))->iso8601(); if (DateTime->compare( - DateTime::Format::ISO8601->parse_datetime($threshold_date), + DateTime::Format::ISO8601->parse_datetime($threshold_date), DateTime::Format::ISO8601->parse_datetime($return_date)) == 1) { $return_date = $threshold_date; } @@ -1012,7 +1038,7 @@ sub process_recalls { renewal_remaining => 0, ); - my $fine_rules = + my $fine_rules = $self->parent->get_ou_setting($pu_lib, 'circ.holds.recall_fine_rules'); # If the OU hasn't defined new fine rules for recalls, keep them @@ -1028,13 +1054,13 @@ sub process_recalls { # Copy updated fields into circ object. $circ->$_($update_fields{$_}) for keys %update_fields; - $e->update_action_circulation($circ) + $e->update_action_circulation($circ) or return $self->exit_targeter( "Error updating circulation object in process_recalls", 1); # Create trigger event for notifying current user my $ses = OpenSRF::AppSession->create('open-ils.trigger'); - $ses->request('open-ils.trigger.event.autocreate', + $ses->request('open-ils.trigger.event.autocreate', 'circ.recall.target', $circ, $circ->circ_lib); return 1; @@ -1055,9 +1081,9 @@ sub target { or return $self->exit_targeter("No hold found", 1); return $self->exit_targeter("Hold is not eligible for targeting") - if $hold->capture_time || - $hold->cancel_time || - $hold->fulfillment_time || + if $hold->capture_time || + $hold->cancel_time || + $hold->fulfillment_time || $U->is_true($hold->frozen); $self->hold($hold); @@ -1066,11 +1092,11 @@ sub target { return unless $self->get_hold_copies; return unless $self->update_copy_maps; - # Confirm that we have something to work on. If we have no + # Confirm that we have something to work on. If we have no # copies at this point, there's also nothing to recall. return unless $self->handle_no_copies; - # Trim the set of working copies down to those that are + # Trim the set of working copies down to those that are # currently targetable. return unless $self->filter_copies_by_status; return unless $self->filter_copies_in_use; diff --git a/Open-ILS/src/support-scripts/hold_targeter.pl b/Open-ILS/src/support-scripts/hold_targeter.pl index 2ca196e1ae..376d691685 100755 --- a/Open-ILS/src/support-scripts/hold_targeter.pl +++ b/Open-ILS/src/support-scripts/hold_targeter.pl @@ -1,99 +1,33 @@ #!/usr/bin/perl -# --------------------------------------------------------------------- -# Usage: -# hold_targeter.pl -# --------------------------------------------------------------------- - +#---------------------------------------------------------------- +# Batch hold targeter +#---------------------------------------------------------------- use strict; use warnings; -use OpenSRF::Utils::JSON; use OpenSRF::System; -use OpenSRF::Utils::SettingsClient; -use OpenSRF::MultiSession; -use OpenSRF::EX qw(:try); - -my $config = shift || die "bootstrap config required\n"; -my $lockfile = shift || "/tmp/hold_targeter-LOCK"; - -if (-e $lockfile) { - die "I seem to be running already. If not remove $lockfile, try again\n"; -} - -open(F, ">$lockfile"); -print F $$; -close F; - -my $settings; -my $parallel; - -try { - OpenSRF::System->bootstrap_client( config_file => $config ); - $settings = OpenSRF::Utils::SettingsClient->new; - $parallel = $settings->config_value( hold_targeter => 'parallel' ) || 1; -} otherwise { - my $e = shift; - warn "$e\n"; - unlink $lockfile; - exit 1; -}; - -if ($parallel == 1) { +use OpenILS::Utils::Fieldmapper; +use OpenILS::Utils::HoldTargeter; +#---------------------------------------------------------------- +# Usage: +# ./hold_targeter.pl /openils/conf/opensrf_core.xml +#---------------------------------------------------------------- - try { - my $r = OpenSRF::AppSession - ->create( 'open-ils.storage' ) - ->request( 'open-ils.storage.action.hold_request.copy_targeter' => '24h' ); +my $osrf_config = shift || '/openils/conf/opensrf_core.xml'; - while (!$r->complete) { - my $start = time; - $r->recv(timeout => 3600); - last if (time() - $start) >= 3600; - }; - } otherwise { - my $e = shift; - warn "Failure in single-session targeter:\n$e\n"; - }; +OpenSRF::System->bootstrap_client(config_file => $osrf_config); +Fieldmapper->import( + IDL => OpenSRF::Utils::SettingsClient->new->config_value("IDL")); +OpenILS::Utils::CStoreEditor::init(); -} else { +my $targeter = OpenILS::Utils::HoldTargeter->new; - try { - my $multi_targeter = OpenSRF::MultiSession->new( - app => 'open-ils.storage', - cap => $parallel, - api_level => 1, - session_hash_function => sub { - my $ses = shift; - my $req = shift; - return $_[-1]; # last parameter is the ID of the metarecord associated with the - # request's target; using this as the hash function value ensures - # that parallel targeters won't try to simultaneously handle two - # hold requests that have overlapping pools of copies that could - # fill those requests - } - ); - - my $storage = OpenSRF::AppSession->create("open-ils.storage"); - - my $r = $storage->request('open-ils.storage.action.hold_request.targetable_holds.id_list', '24h'); - while ( my $h = $r->recv ) { - if ($r->failed) { - print $r->failed->stringify . "\n"; - last; - } - if (my $hold = $h->content) { - $multi_targeter->request( 'open-ils.storage.action.hold_request.copy_targeter', '', $hold->[0], $hold->[1]); - } - } - - $storage->disconnect(); - - $multi_targeter->session_wait(1); - $multi_targeter->disconnect; - } otherwise { - my $e = shift; - warn "Failure in multi-session targeter:\n$e\n"; - } -} +my $start = time; +my $count = $targeter->target( + # Return only the number processed, + # instead of a result blob for each hold. + return_count => 1 +); -unlink $lockfile; +my $minutes = sprintf('%0.2f', (time - $start) / 60); +print "Processed $count holds in $minutes minutes.\n"; diff --git a/Open-ILS/src/support-scripts/test-scripts/hold_targeter.pl b/Open-ILS/src/support-scripts/test-scripts/hold_targeter.pl deleted file mode 100755 index 2d249a1bd4..0000000000 --- a/Open-ILS/src/support-scripts/test-scripts/hold_targeter.pl +++ /dev/null @@ -1,26 +0,0 @@ -#!/usr/bin/perl -#---------------------------------------------------------------- -# Simple hold targeter test script -#---------------------------------------------------------------- - -require '../oils_header.pl'; -use strict; use warnings; -use OpenSRF::AppSession; -use OpenILS::Utils::Fieldmapper; -use OpenILS::Utils::HoldTargeter; -use Data::Dumper; - -my $config = shift; # path to opensrf_core.xml -osrf_connect($config); # connect to jabber - -my $targeter = OpenILS::Utils::HoldTargeter->new; - -my $responses = $targeter->target( - #hold => 80, - #retarget_interval => '12h', - #newest_first => 1, - #target_all => 1 -); - -print Dumper($responses); -print "Processed " . scalar(@$responses) . " holds\n"; -- 2.11.0