[open-ils-commits] [GIT] Evergreen ILS branch master updated. 238dba987ce2fb111682d94c02e11f1635fb60e1

Evergreen Git git at git.evergreen-ils.org
Thu May 18 15:07:02 EDT 2017


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Evergreen ILS".

The branch, master has been updated
       via  238dba987ce2fb111682d94c02e11f1635fb60e1 (commit)
       via  d1c2d9c45d15d882d42bb48a7411717a8b7b347e (commit)
       via  54dc1aac3802103df9113fd18c2ed2bffaa8a1ed (commit)
       via  3073bcb696e5ffe144dd526fc4f54d904a7e0783 (commit)
      from  04ba0cb4b666ecca4fa53c08ea42ed7a9e2b7771 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 238dba987ce2fb111682d94c02e11f1635fb60e1
Author: Bill Erickson <berickxx at gmail.com>
Date:   Fri Mar 31 10:21:45 2017 -0400

    LP#1677661 Hold targeter live test use API, etc.
    
    Teach the hold targeter live tests to call the open-ils.hold.targeter
    API instead of calling the now-defunct Utils::HoldTargeter batch
    targeter function.  Apart from the loss of this function, calling the
    API allows the live tests to exercise more of the targeting code.
    
    Teach the live tests to test --soft-retarget-interval instead of the
    deprecated --skip-viable option.
    
    Signed-off-by: Bill Erickson <berickxx at gmail.com>
    Signed-off-by: Jason Stephenson <jason at sigio.com>

diff --git a/Open-ILS/src/perlmods/live_t/20-hold-targeter.t b/Open-ILS/src/perlmods/live_t/20-hold-targeter.t
index 3dead77..d04b384 100644
--- a/Open-ILS/src/perlmods/live_t/20-hold-targeter.t
+++ b/Open-ILS/src/perlmods/live_t/20-hold-targeter.t
@@ -7,20 +7,28 @@ diag("General hold targeter tests");
 
 use OpenILS::Const qw/:const/;
 use OpenILS::Utils::TestUtils;
-use OpenILS::Utils::HoldTargeter;
+use OpenILS::Application::AppUtils;
 use OpenILS::Utils::CStoreEditor qw/:funcs/;
 
 my $script = OpenILS::Utils::TestUtils->new();
-my $targeter = OpenILS::Utils::HoldTargeter->new;
+my $U = 'OpenILS::Application::AppUtils';
 my $e = new_editor();
 
 $script->bootstrap;
 $e->init;
 
+sub target {
+    return $U->simplereq(
+        'open-ils.hold-targeter', 
+        'open-ils.hold-targeter.target', 
+        @_
+    );
+}
+
 # == Targeting Concerto hold 67.  Title hold.
 
 my $hold_id = 67;
-my $result = $targeter->target(hold => $hold_id)->[0];
+my $result = target({hold => $hold_id});
 
 ok($result->{success}, "Targeting hold $hold_id returned success");
 
@@ -45,7 +53,7 @@ is(scalar(grep {$_->target_copy->call_number->record != 70} @$maps), 0,
 # Retarget to confirm a new copy is selected and that the previously
 # targeted item has a new entry in action.unfulfilled_hold_list.
 
-$result = $targeter->target(hold => $hold_id)->[0];
+$result = target({hold => $hold_id});
 
 isnt($result->{target}, $current_copy->id, 
     'Second targeter run on hold 67 selected targeted a different copy');
@@ -57,15 +65,15 @@ isnt($unfulfilled, undef, 'Previous copy has unfulfilled hold entry');
 
 my $prev_target = $result->{target};
 
-$result = $targeter->target(hold => $hold_id, skip_viable => 1)->[0];
+$result = target({hold => $hold_id, soft_retarget_interval => '0s'});
 
 is($result->{target}, $prev_target, 
-    "Hold $hold_id target remains the same with --skip-viable");
+    "Hold $hold_id target remains the same with soft_retarget_interval");
 
 $maps = $e->search_action_hold_copy_map({hold => $hold_id});
 
 is(scalar(@$maps), 29, 
-    "Hold $hold_id retains 29 mapped potential copies with --skip-viable");
+    "Hold $hold_id retains 29 mapped potential copies with soft_retarget_interval");
 
 
 # == Metarecord hold tests
@@ -74,7 +82,7 @@ is(scalar(@$maps), 29,
 # holdable_format '{"0":[{"_attr":"mr_hold_format","_val":"score"}]}'.
 
 $hold_id = 263;
-$result = $targeter->target(hold => $hold_id)->[0];
+$result = target({hold => $hold_id});
 
 ok($result->{success}, "Targeting hold $hold_id returned success");
 
@@ -115,7 +123,7 @@ $e->xact_commit;
 
 # This time no copies should be targeted, since no records match
 # the holdable_formats criteria.
-$result = $targeter->target(hold => $hold_id)->[0];
+$result = target({hold => $hold_id});
 
 isnt($result->{success}, 1, 
     'Unable to target MR hold without copies matching holdable_format');
@@ -133,7 +141,7 @@ $hold->clear_holdable_formats;
 $e->update_action_hold_request($hold) or die $e->die_event;
 $e->xact_commit;
 
-$result = $targeter->target(hold => $hold_id)->[0];
+$result = target({hold => $hold_id});
 
 $current_copy = $e->retrieve_asset_copy([
     $result->{target},

commit d1c2d9c45d15d882d42bb48a7411717a8b7b347e
Author: Bill Erickson <berickxx at gmail.com>
Date:   Fri Mar 31 10:21:25 2017 -0400

    LP#1677661 Targeter V2 remove unused batch API
    
    Remove the unusued batch target() function from Utils::HoldTargeter to
    avoid code duplication.  The same (but more resilient) batch targeting
    construct exists in the open-ils.hold-targeter API.
    
    Move API docs from Utils::HoldTargeter to the open-ils.hold-targeter API
    docs for added visbility / findability.
    
    Signed-off-by: Bill Erickson <berickxx at gmail.com>
    Signed-off-by: Jason Stephenson <jason at sigio.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm
index 22fd3e9..afca2fc 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm
@@ -18,29 +18,54 @@ __PACKAGE__->register_method(
         desc     => q/Batch or single hold targeter./,
         params   => [
             {   name => 'args',
-                desc => 'Hash of targeter options',
-                type => 'hash'
+                type => 'hash',
+                desc => q/
+API Options:
+
+return_count - Return number of holds processed so far instead 
+  of hold targeter result summary objects.
+
+return_throttle - Only reply each time this many holds have been 
+  targeted.  This prevents dumping a fast stream of responses
+  at the client if the client doesn't need them.
+
+Targeter Options:
+
+hold => <id> OR [<id>, <id>, ...]
+ (Re)target one or more specific holds.  Specified as a single hold ID
+ or an array ref of hold IDs.
+
+retarget_interval => <interval string>
+  Override the 'circ.holds.retarget_interval' global_flag value.
+
+soft_retarget_interval => <interval string>
+  Apply soft retarget logic to holds whose prev_check_time sits
+  between the retarget_interval and the soft_retarget_interval.
+
+next_check_interval => <interval string>
+  Use this interval to determine when the targeter will run next
+  instead of relying on the retarget_interval.  This value is used
+  to determine if an org unit will be closed during the next iteration
+  of the targeter.  Applying a specific interval is useful when
+  the retarget_interval is shorter than the time between targeter runs.
+
+newest_first => 1
+  Target holds in reverse order of create_time. 
+
+parallel_count => n
+  Number of parallel targeters running.  This acts as the indication
+  that other targeter instances are running.
+
+parallel_slot => n [starts at 1]
+  Sets the parallel targeter instance slot.  Used to determine
+  which holds to process to avoid conflicts with other running instances.
+/
             }
         ],
-        return => {
-            desc => q/
-                TODO
-            /
-        }
+        return => {desc => 'See API Options for return types'}
     }
 );
 
-# args:
-#
-#   return_count - Return number of holds processed so far instead 
-#       of hold targeter result summary objects.
-#
-#   return_throttle - Only reply each time this many holds have been 
-#       targeted.  This prevents dumping a fast stream of responses
-#       at the client if the client doesn't need them.
-#
-#   See OpenILS::Utils::HoldTargeter::target() docs.
-
 sub hold_targeter {
     my ($self, $client, $args) = @_;
 
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
index b8553d3..9aab446 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
@@ -26,7 +26,7 @@ use OpenILS::Utils::CStoreEditor qw/:funcs/;
 our $U = "OpenILS::Application::AppUtils";
 our $dt_parser = DateTime::Format::ISO8601->new;
 
-# See target() for runtime arguments.
+# See open-ils.hold-targeter API docs for runtime arguments.
 sub new {
     my ($class, %args) = @_;
     my $self = {
@@ -37,71 +37,7 @@ sub new {
     return bless($self, $class);
 }
 
-# Target and retarget holds.
-# 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.
-#
-# Returns an array of targeter response objects, one entry per hold
-# targeted.  See also return_count.
-#
-# Optional parameters:
-#
-# hold => <id> / [<id>, <id>, ...]
-#  (Re)target one or more specific holds.  Specified as a single hold ID
-#  or an array ref of hold IDs.
-#
-# return_count => 1
-#   Return the total number of holds processed instead of a result
-#   object for every targeted hold.  Ideal for large batch targeting.
-#
-# retarget_interval => <interval string>
-#   Override the 'circ.holds.retarget_interval' global_flag value.
-#
-# soft_retarget_interval => <interval string>
-#   Apply soft retarget logic to holds whose prev_check_time sits
-#   between the retarget_interval and the soft_retarget_interval.
-#
-# next_check_interval => <interval string>
-#   Use this interval to determine when the targeter will run next
-#   instead of relying on the retarget_interval.  This value is used
-#   to determine if an org unit will be closed during the next iteration
-#   of the targeter.  Applying a specific interval is useful when
-#   the retarget_interval is shorter than the time between targeter runs.
-#
-# newest_first => 1
-#   Target holds in reverse order of create_time. 
-#
-# parallel_count => n
-#   Number of parallel targeters running.  This acts as the indication
-#   that other targeter instances are running.
-#
-# parallel_slot => n [starts at 1]
-#   Sets the parallel targeter instance position/slot.  Used to determine
-#   which holds to process to avoid conflicts with other running instances.
-#
-sub target {
-    my ($self, %args) = @_;
-
-    $self->{$_} = $args{$_} for keys %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) unless $self->{return_count};
-        $count++;
-    }
-
-    return $self->{return_count} ? $count : \@responses;
-}
-
+# Returns a list of hold ID's
 sub find_holds_to_target {
     my $self = shift;
 

commit 54dc1aac3802103df9113fd18c2ed2bffaa8a1ed
Author: Bill Erickson <berickxx at gmail.com>
Date:   Thu Mar 30 11:47:20 2017 -0400

    LP#1677661 Targeter V2 extras release notes
    
    Signed-off-by: Bill Erickson <berickxx at gmail.com>
    Signed-off-by: Jason Stephenson <jason at sigio.com>

diff --git a/docs/RELEASE_NOTES_NEXT/Administration/hold-targeter-v2-improvements.adoc b/docs/RELEASE_NOTES_NEXT/Administration/hold-targeter-v2-improvements.adoc
new file mode 100644
index 0000000..06a357a
--- /dev/null
+++ b/docs/RELEASE_NOTES_NEXT/Administration/hold-targeter-v2-improvements.adoc
@@ -0,0 +1,36 @@
+Hold Targeter V2 Repairs and Improvements
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+* Make the batch targeter more resilient to a single-hold failure.
+* Additional batch targeter info logging.
+* Set OSRF_LOG_CLIENT in hold_targeter_v2.pl for log tracing
+* Removes the confusingly named --target-all option
+ ** The same behavior can be achieved by using --retarget-interval "0s"
+* Removes --skip-viable (see --soft-retarget-interval below)
+
+New --next-check-interval Option
+++++++++++++++++++++++++++++++++
+Specify how long after the current run time the targeter will retarget
+the currently affected holds. Applying a specific interval is useful
+when the retarget-interval is shorter than the time between targeter
+runs.
+
+For example, if the targeter is run nightly at midnight with a
+--retarget-interval 36h, you would set --next-check-interval to 48hr,
+since the holds won't be processed again until 48 hours later. This
+ensures that the org unit closed date checks are looking at the correct
+date. 
+
+This setting overrides the default behavior of calculating the next 
+retarget time from the retarget-interval.
+
+New --soft-retarget-interval Option
++++++++++++++++++++++++++++++++++++
+This is a replacement for (and rebranding of) the --skip-viable option. 
+The new option allows for time-based soft-targeting instead simple binary 
+on/off soft-targeting.
+
+How soft-targeting works:
+* Update hold copy maps for all affected holds
+* Holds with viable targets (on the pull list) are otherwise left alone.
+* Holds without viable targets are retargeted in the usual manner. 
+

commit 3073bcb696e5ffe144dd526fc4f54d904a7e0783
Author: Bill Erickson <berickxx at gmail.com>
Date:   Wed Mar 22 11:47:21 2017 -0400

    LP#1677661 Hold Targeter V2 Repairs & Improvements
    
    * Make the batch targeter more resilient to a single-hold failure.
    
    * Additional batch targeter info logging.
    
    * Set OSRF_LOG_CLIENT in hold_targeter_v2.pl for log tracing
    
    * Removes the confusingly name --target-all option
    
    * Adds a new --next-check-interval option for specifying when the
      targeter will next affect the currently processed holds, which may be
      different that now + retarget-interval in cases where the targeter is
      not constantly running.
    
    * Replaces the --skip-viable option with a new --soft-retarget-interval
      option, allowing for time-based soft-targeting.
    
    * Soft-targeting now updates hold_copy_maps for all affected holds, not
      just those requiring a full retarget.
    
    Signed-off-by: Bill Erickson <berickxx at gmail.com>
    Signed-off-by: Jason Stephenson <jason at sigio.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm
index 037f230..22fd3e9 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/HoldTargeter.pm
@@ -4,6 +4,7 @@ use warnings;
 use OpenILS::Application;
 use base qw/OpenILS::Application/;
 use OpenILS::Utils::HoldTargeter;
+use OpenSRF::Utils::Logger qw(:logger);
 
 __PACKAGE__->register_method(
     method    => 'hold_targeter',
@@ -50,15 +51,26 @@ sub hold_targeter {
     my $throttle = $args->{return_throttle} || 1;
     my $count = 0;
 
-    for my $hold_id ($targeter->find_holds_to_target) {
+    my @hold_ids = $targeter->find_holds_to_target;
+    my $total = scalar(@hold_ids);
+
+    $logger->info("targeter processing $total holds");
+
+    for my $hold_id (@hold_ids) {
         $count++;
 
-        my $single = OpenILS::Utils::HoldTargeter::Single->new(
-            parent => $targeter,
-            skip_viable => $args->{skip_viable}
-        );
+        my $single = 
+            OpenILS::Utils::HoldTargeter::Single->new(parent => $targeter);
 
-        $single->target($hold_id);
+        # Don't let an explosion on a single hold stop processing
+        eval { $single->target($hold_id) };
+
+        if ($@) {
+            my $msg = "Targeter failed processing hold: $hold_id : $@";
+            $single->error(1);
+            $logger->error($msg);
+            $single->message($msg) unless $single->message;
+        }
 
         if (($count % $throttle) == 0) { 
             # Time to reply to the caller.  Return either the number
@@ -66,6 +78,8 @@ sub hold_targeter {
 
             my $res = $args->{return_count} ? $count : $single->result;
             $client->respond($res);
+
+            $logger->info("targeted $count of $total holds");
         }
     }
 
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
index eaed9b4..b8553d3 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
@@ -58,19 +58,19 @@ sub new {
 # retarget_interval => <interval string>
 #   Override the 'circ.holds.retarget_interval' global_flag value.
 #
-# newest_first => 1
-#   Target holds in reverse order of create_time. 
+# soft_retarget_interval => <interval string>
+#   Apply soft retarget logic to holds whose prev_check_time sits
+#   between the retarget_interval and the soft_retarget_interval.
 #
-# skip_viable => 1
-#   Avoid retargeting holds whose current_copy is still viable and
-#   permitted.  This is useful for repairing holds whose targeted copy
-#   has become non-viable for a given hold because its status changed or
-#   policies affecting the hold/copy no longer allow it to be targeted.
-#   This setting can be used in conjunction with any other settings.
+# next_check_interval => <interval string>
+#   Use this interval to determine when the targeter will run next
+#   instead of relying on the retarget_interval.  This value is used
+#   to determine if an org unit will be closed during the next iteration
+#   of the targeter.  Applying a specific interval is useful when
+#   the retarget_interval is shorter than the time between targeter runs.
 #
-# target_all => 1
-#   USE WITH CAUTION.  Forces (re)targeting of all active holds.  This
-#   is primarily useful or testing.
+# newest_first => 1
+#   Target holds in reverse order of create_time. 
 #
 # parallel_count => n
 #   Number of parallel targeters running.  This acts as the indication
@@ -91,10 +91,9 @@ sub target {
     my @responses;
 
     for my $hold_id ($self->find_holds_to_target) {
-        my $single = OpenILS::Utils::HoldTargeter::Single->new(
-            parent => $self,
-            skip_viable => $args{skip_viable}
-        );
+        my $single = 
+            OpenILS::Utils::HoldTargeter::Single->new(parent => $self);
+
         $single->target($hold_id);
         push(@responses, $single->result) unless $self->{return_count};
         $count++;
@@ -128,19 +127,14 @@ 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
-        # before the retarget interval.
-
-        my $date = DateTime->now->subtract(
-            seconds => $self->{retarget_interval});
-
-        $query->{where}->{'-or'} = [
-            {prev_check_time => undef},
-            {prev_check_time => {'<=' => $date->strftime('%F %T%z')}}
-        ];
-    }
+    # Target holds that have no prev_check_time or those whose re-target
+    # time has come.  If a soft_retarget_time is specified, that acts as
+    # the boundary.  Otherwise, the retarget_time is used.
+    my $start_time = $self->{soft_retarget_time} || $self->{retarget_time};
+    $query->{where}->{'-or'} = [
+        {prev_check_time => undef},
+        {prev_check_time => {'<=' => $start_time->strftime('%F %T%z')}}
+    ];
 
     # parallel < 1 means no parallel
     my $parallel = ($self->{parallel_count} || 0) > 1 ? 
@@ -206,55 +200,76 @@ sub init {
     my $self = shift;
     my $e = $self->editor;
 
-    my $closed_orgs_query = {
-        close_start => {'<=', 'now'},
-        close_end => {'>=', 'now'}
-    };
+    # See if the caller provided an interval
+    my $interval = $self->{retarget_interval};
 
-    if (!$self->{target_all}) {
+    if (!$interval) { 
+        # See if we have a global flag value for the interval
 
-        # See if the caller provided an interval
-        my $interval = $self->{retarget_interval};
+        $interval = $e->search_config_global_flag({
+            name => 'circ.holds.retarget_interval',
+            enabled => 't'
+        })->[0];
 
-        if (!$interval) {
-            # See if we have a global flag value for the interval
+        # If no flag is present, default to a 24-hour retarget interval.
+        $interval = $interval ? $interval->value : '24h';
+    }
 
-            $interval = $e->search_config_global_flag({
-                name => 'circ.holds.retarget_interval',
-                enabled => 't'
-            })->[0];
+    my $retarget_seconds = interval_to_seconds($interval);
 
-            # If no flag is present, default to a 24-hour retarget interval.
-            $interval = $interval ? $interval->value : '24h';
-        }
+    $self->{retarget_time} = DateTime->now(time_zone => 'local')
+        ->subtract(seconds => $retarget_seconds);
 
-        # Convert the interval to seconds for current and future use.
-        $self->{retarget_interval} = interval_to_seconds($interval);
+    $logger->info("Using retarget time: ".
+        $self->{retarget_time}->strftime('%F %T%z'));
 
-        # An org unit is considered closed for retargeting purposes
-        # if it's closed both now and at the next re-target date.
+    if ($self->{soft_retarget_interval}) {
 
-        my $next_check_time =
-            DateTime->now->add(seconds => $self->{retarget_interval})
-            ->strftime('%F %T%z');
+        my $secs = OpenSRF::Utils->interval_to_seconds(
+            $self->{soft_retarget_interval});
 
-        $closed_orgs_query = {
-            '-and' => [
-                $closed_orgs_query, {
-                    close_start => {'<=', $next_check_time},
-                    close_end => {'>=', $next_check_time}
-                }
-            ]
-        }
+        $self->{soft_retarget_time} = 
+            DateTime->now(time_zone => 'local')->subtract(seconds => $secs);
+
+        $logger->info("Using soft retarget time: " .
+            $self->{soft_retarget_time}->strftime('%F %T%z'));
     }
 
-    my $closed =
-        $self->editor->search_actor_org_unit_closed_date($closed_orgs_query);
+    # Holds targeted in the current targeter instance not be retargeted
+    # until the next check date.  If a next_check_interval is provided
+    # it overrides the retarget_interval.
+    my $next_check_secs = 
+        $self->{next_check_interval} ?
+        OpenSRF::Utils->interval_to_seconds($self->{next_check_interval}) :
+        $retarget_seconds;
+
+    my $next_check_date = 
+        DateTime->now(time_zone => 'local')->add(seconds => $next_check_secs);
+
+    my $next_check_time = $next_check_date->strftime('%F %T%z');
+
+    $logger->info("Next check time: $next_check_time");
+
+    # An org unit is considered closed for retargeting purposes
+    # if it's closed both now and at the next re-target date.
+    my $closed = $self->editor->search_actor_org_unit_closed_date({
+        '-and' => [{   
+            close_start => {'<=', 'now'},
+            close_end => {'>=', 'now'}
+        }, {
+            close_start => {'<=', $next_check_time},
+            close_end => {'>=', $next_check_time}
+        }]
+    });
+
+    my @closed_orgs = map {$_->org_unit} @$closed;
+    $logger->info("closed org unit IDs: @closed_orgs");
 
     # Map of org id to 1. Any org in the map is closed.
-    $self->{closed_orgs} = {map {$_->org_unit => 1} @$closed};
+    $self->{closed_orgs} = {map {$_ => 1} @closed_orgs};
 }
 
+
 # Org unit setting fetch+cache
 # $e is the OpenILS::Utils::HoldTargeter::Single editor.  Use it if
 # provided to avoid timeouts on the in-transaction child editor.
@@ -453,7 +468,8 @@ sub handle_expired_hold {
 
     my $ex_time =
         $dt_parser->parse_datetime(cleanse_ISO8601($hold->expire_time));
-    return 1 unless DateTime->compare($ex_time, DateTime->now) < 0;
+    return 1 unless 
+        DateTime->compare($ex_time, DateTime->now(time_zone => 'local')) < 0;
 
     # Hold is expired --
 
@@ -821,24 +837,40 @@ sub inspect_previous_target {
     # exit if previous target is no longer valid.
     return 1 unless $prev;
 
-    if ($self->{skip_viable}) {
-        # In skip_viable mode, leave the hold as-is if the existing
-        # current_copy is still permitted.
-        # Note: viability checking is done this late in the process
-        # (specifically after other potential copies have been fetched)
-        # because we first need to confirm the current_copy is a valid
-        # potential copy (e.g. it's holdable, non-deleted, etc.), which
-        # copy_is_permitted, which only checks hold matrix policies,
-        # does not check.
+    my $soft_retarget = 0;
+
+    if ($self->parent->{soft_retarget_time}) {
+        # A hold is soft-retarget-able if its prev_check_time is
+        # later then the retarget_time, i.e. it sits between the
+        # soft_retarget_time and the retarget_time.
 
-        return $self->exit_targeter("Skipping with viable target = $prev_id")
-            if $self->copy_is_permitted($prev);
+        my $pct = $dt_parser->parse_datetime(
+            cleanse_ISO8601($hold->prev_check_time));
 
-        # Previous copy is now confirmed non-viable.
+        $soft_retarget =
+            DateTime->compare($pct, $self->parent->{retarget_time}) > 0;
+    }
+
+    if ($soft_retarget) {
+
+        # In soft-retarget mode, if the existing copy is still a valid
+        # target for the hold, exit early.
+        if ($self->copy_is_permitted($prev)) {
+
+            # Commit to persist the updated action.hold_copy_map's
+            $self->editor->commit;
+
+            return $self->exit_targeter(
+                "Exiting early on soft-retarget with viable copy = $prev_id");
+
+        } else {
+            $self->log_hold("soft retarget failed because copy $prev_id is ".
+                "no longer targetable for this hold.  Retargeting...");
+        }
 
     } else {
 
-        # Previous copy may be targetable.  Keep it around for later
+        # Previous copy /may/ be targetable.  Keep it around for later
         # in case we need to confirm its viability and re-use it.
         $self->{valid_previous_copy} = $prev;
     }
diff --git a/Open-ILS/src/support-scripts/hold_targeter_v2.pl b/Open-ILS/src/support-scripts/hold_targeter_v2.pl
index 06d1c3a..93eaf4d 100755
--- a/Open-ILS/src/support-scripts/hold_targeter_v2.pl
+++ b/Open-ILS/src/support-scripts/hold_targeter_v2.pl
@@ -6,6 +6,7 @@ use OpenSRF::System;
 use OpenSRF::AppSession;
 use OpenSRF::Utils::SettingsClient;
 use OpenILS::Utils::Fieldmapper;
+$ENV{OSRF_LOG_CLIENT} = 1;
 #----------------------------------------------------------------
 # Batch hold (re)targeter
 #
@@ -18,25 +19,25 @@ my $osrf_config = '/openils/conf/opensrf_core.xml';
 my $lockfile = '/tmp/hold_targeter-LOCK';
 my $parallel = 0;
 my $verbose = 0;
-my $target_all;
-my $skip_viable;
 my $retarget_interval;
+my $soft_retarget_interval;
+my $next_check_interval;
 my $recv_timeout = 3600;
 my $parallel_init_sleep = 0;
 
 # how often the server sends a summary reply per backend.
-my $return_throttle = 50;
+my $return_throttle = 500;
 
 GetOptions(
-    'osrf-config=s'     => \$osrf_config,
-    'lockfile=s'        => \$lockfile,
-    'parallel=i'        => \$parallel,
-    'verbose'           => \$verbose,
-    'target-all'        => \$target_all,
-    'skip-viable'       => \$skip_viable,
-    'retarget-interval=s'   => \$retarget_interval,
+    'help'                  => \$help,
+    'osrf-config=s'         => \$osrf_config,
+    'lockfile=s'            => \$lockfile,
+    'parallel=i'            => \$parallel,
+    'verbose'               => \$verbose,
     'parallel-init-sleep=i' => \$parallel_init_sleep,
-    'help'              => \$help
+    'retarget-interval=s'   => \$retarget_interval,
+    'next-check-interval=s'    => \$next_check_interval,
+    'soft-retarget-interval=s' => \$soft_retarget_interval,
 ) || die "\nSee --help for more\n";
 
 sub help {
@@ -58,7 +59,6 @@ General Options
     --lockfile [/tmp/hold_targeter-LOCK]
         Full path to lock file
 
-
     --verbose
         Print process counts
 
@@ -76,15 +76,31 @@ Targeting Options
 
         Defaults to no sleep.
 
-    --target-all
-        Target all active holds, regardless of when they were last targeted.
-
-    --skip-viable
-        Avoid modifying holds that currently target viable copies.  In
-        other words, only (re)target holds in a non-viable state.
+    --soft-retarget-interval
+        Holds whose previous check time sits between the
+        --soft-retarget-interval and the --retarget-interval are 
+        treated like this:
+        
+        1. The list of potential copies is updated for all matching holds.
+        2. Holds that have a viable target are otherwise left untouched,
+           including their prev_check_time.
+        3. Holds with no viable target are fully retargeted.
+
+    --next-check-interval
+        Specify how long after the current run time the targeter will
+        retarget the currently affected holds.  Applying a specific
+        interval is useful when the retarget_interval is shorter than
+        the time between targeter runs.
+
+        This value is used to determine if an org unit will be closed
+        during the next iteration of the targeter.  It overrides the
+        default behavior of calculating the next retarget time from the
+        retarget-interval.
 
     --retarget-interval
-        Override the 'circ.holds.retarget_interval' global_flag value. 
+        Retarget holds whose previous check time occured before the
+        requested interval.
+        Overrides the 'circ.holds.retarget_interval' global_flag value. 
 
 HELP
 
@@ -130,9 +146,9 @@ sub run_batches {
                 return_throttle => $return_throttle,
                 parallel_count  => $parallel,
                 parallel_slot   => $slot,
-                skip_viable     => $skip_viable,
-                target_all      => $target_all,
-                retarget_interval => $retarget_interval
+                retarget_interval      => $retarget_interval,
+                next_check_interval    => $next_check_interval,
+                soft_retarget_interval => $soft_retarget_interval
             }
         );
 
@@ -151,6 +167,8 @@ sub run_batches {
         for my $req (@reqs) {
             # Pull all responses off the receive queues.
             while (my $resp = $req->recv(0)) {
+                die $req->failed . "\n" if $req->failed;
+
                 $verbose && print sprintf(
                     "Targeter [%d] processed %d holds\n",
                     $req->{_parallel_slot},

-----------------------------------------------------------------------

Summary of changes:
 .../lib/OpenILS/Application/HoldTargeter.pm        |   87 ++++++--
 .../src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm |  224 +++++++++-----------
 Open-ILS/src/perlmods/live_t/20-hold-targeter.t    |   28 ++-
 Open-ILS/src/support-scripts/hold_targeter_v2.pl   |   62 ++++--
 .../hold-targeter-v2-improvements.adoc             |   36 +++
 5 files changed, 253 insertions(+), 184 deletions(-)
 create mode 100644 docs/RELEASE_NOTES_NEXT/Administration/hold-targeter-v2-improvements.adoc


hooks/post-receive
-- 
Evergreen ILS


More information about the open-ils-commits mailing list