Hold Targeter V2 Repairs & Improvements

Bug #1677661 reported by Bill Erickson
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Undecided
Unassigned
2.12
Fix Released
Undecided
Unassigned

Bug Description

Evergreen 2.12

In preparing the new hold targeter for deployment (and based on other experiences in the field), I have prepared a set of changes to improve usability and stability of the new targeter.

Here's the list of the changes, comments welcome, branch en route:

* Make the batch targeter more resilient to a single-hold failure.

* Additional batch targeter info logging.

* Ensure all DateTime objects are created with a timezone, using "local" as needed, instead of defaulting to the "floating" time zone, since it's bad practice to compare floating dates with timezone-qualified dates (e.g. from the database).

* Set OSRF_LOG_CLIENT in hold_targeter_v2.pl for log tracing

* Removes the --target-all option

This option is confusingly named and is generally only used for testing purpose. The same behavior can be achieved by setting --reterget-interval "0s"

* Adds a 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.

* Replaces the --skip-viable option with a new --soft-retarget-interval option

My hope is this terminology "soft targeting" will be easier to understand/explain than "skip viable".

Additionally, the new option allows soft-targeting to occur based on the time since the last target attempt on the hold, instead of a binary on/off flag. This makes is possible to run a single hold targeter instance that treats holds one way or the other depending on how long it's been since the last retarget.

For example:

--retarget-interval 24h --soft-retarget-interval 12h

This would retarget all holds last targeted more then 24 hours ago as usual, but it would soft-retarget holds last targeted between 12 and 24 hours.

SOFT-TARGET NOTES:

1. Soft-targeting only updates prev_check_time if a full retarget is needed on the hold.

2. Soft-targeting updates the hold_copy_maps for all soft-targeted holds, regardless of whether a full retarget is required. With this, the copies used for opportunistic capture can be refreshed more frequently than the hold is retargeted.

3. The original --skip-viable behavior may be replicated by setting the same value for --soft-retarget-interval and --retarget-interval.

Revision history for this message
Bill Erickson (berick) wrote :
Revision history for this message
Bill Erickson (berick) wrote :

2 more commits pushed.

First commit removes the duplicate batch targeting code that lived in Utils::HoldTargeter, which existed before the open-ils.targeter service came to be. This includes moving the API docs into the open-ils.hold-targeter API documentation.

Second commit updates the Perl live tests to call the API instead of the now-deprecated function and to test the new --soft-retarget-interval option (removing --skip-viable).

tags: added: pullrequest
Changed in evergreen:
milestone: none → 2.12.1
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

Note that commits for bugs #1675899 and #1679279 are not included in the branch for this bug. I kept them separate since they are higher priority.

Revision history for this message
Blake GH (bmagic) wrote :

Does this address bug 1508208 ?

Changed in evergreen:
milestone: 2.12.1 → 2.12.2
Revision history for this message
Bill Erickson (berick) wrote :

Hi Blake, as discussed at the EG conf and noting here for reference, this will have no impact on bug #1508208.

Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

So far, everything is looking good. All live tests pass.

I want to make sure that I understand the soft retarget interval and it does what I expect before signing off.

Changed in evergreen:
milestone: 2.12.2 → none
status: New → Confirmed
Revision history for this message
Jason Stephenson (jstephenson) wrote :

It works for me! I like the soft retarget interval, I think that will be a useful feature.

Pushed to master and rel_2_12!

Thanks, Bill!

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Jason Stephenson (jstephenson) → nobody
Galen Charlton (gmc)
Changed in evergreen:
milestone: none → 2.12.2
milestone: 2.12.2 → 3.0-alpha
no longer affects: evergreen/3.0
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.