Long-frozen holds skew hold queue position calculation

[Bill Erickson]

Evergreen 3.0 / affects all versions

When a hold is frozen, its target state is locked into place until it is unfrozen and retargeted. Any copy maps that link to copies which are deleted or made otherwise unholdable over time will remain as long as the hold is frozen. This adds bulk to the hold copy map table and leads to bogus queue positions, since copy maps for all holds are included in the queue position calculation.

Note this does not affect hold capture.

I propose a --retarget-frozen option for the hold targeter so these holds may be forcibly retargeted to refresh their hold copy maps at the users's discretion.

[Jason Stephenson (jstephenson)]

Bill, I like the idea, but I have a question:

Is there any reason to automatically retarget frozen holds? That is, are you concerned about performance or have other reasons for adding an option to retarget frozen holds and not just do it as part of the ordinary retargeting process?

[Bill Erickson (berick)]

Jason, my only concern was requiring the hold targeter run longer, particularly since frozen holds don't *need* to be constantly retargeted. If the consensus is it's worth it to avoid adding a new targeter option, I have no objections to always retargeting frozen holds.

[Bill Erickson (berick)]

New targeter option and release notes pushed:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1744341-retarget-frozen-holds

[Michele Morgan (mmorgan)]

Here's another question related to this bug. Should suspended holds have copy maps at all?

I have a long frozen hold with 68 entries in the hold_copy_map.

I place a new hold on the same record for another patron, it gets 22 entries in the hold_copy_map. I then suspend the hold, the 22 entries in the hold_copy_map remain, but will get stale as the hold remains frozen and time passes.

I place a new hold on the same record for another patron and suspend it at the time of placement, the hold gets 0 entries in the hold_copy_map.

When I activate these three holds, they get retargeted, so each hold now has 22 entries in the hold_copy map.

Since a suspended hold will be retargeted when it's activated, regenerating the copy map, what's the advantage of having copy maps for frozen holds at all?

[Bill Erickson (berick)]

Michele, one reason we retain the copy maps for frozen holds is the maps are used to calculate hold queue position. When the maps don't exist, the holds are ignored when calculating queue positions for holds in the same queue.

Arguably it's a bug that holds suspended at placement time have no copy maps.

Related to this, I'm considering opening a new LP to change (or offer an alternative to) how we calculate the queue position. The hold copy map table is large with a lot of turn-over, so querying the table is not always fast.

As of EG 2.12, "reporter.hold_request_record" is a proper database table that's quick to query. This table can be used to calculate queue positions if you define the hold queue position as all holds (across all types) that ultimately target the same bib record. It changes the meaning a bit, but the exact values don't mean as much as how they change over time. We've been using it locally to great effect.

[Michele Morgan (mmorgan)]

See duplicate bug 1861719 for how this affects patrons viewing their suspended holds in the catalog.

Our hold targeter runs every 15 minutes, there's definitely no need to retarget suspended holds every 15 minutes along with the active ones. Having the option to retarget frozen holds on a different schedule than active holds would be a great improvement over never being able to retarget them at all.

Retargeting suspended holds once a day seems reasonable to me to update queue positions and potential copies for suspended holds, both of which are visible to patrons in their accounts.

Bill, it looks like your branch needs a rebase.

[Bill Erickson (berick)]

Rebased branch pushed:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1744341-retarget-frozen-holds-2

[Josh Stompro (u-launchpad-stompro-org)]

I tested this out with a copy of our production data, but I didn't have much luck confirming that it was fully working.

I have 3115 frozen holds with 5909 action.hold_copy_map entries. The action.hold_copy_map numbers didn't change after I used the --retarget-frozen option. That may be because there were no changes to make. I'll try and trigger some more changes.

But I did notice in the logs that one of the frozen holds made it into the retarget list, but was then rejected.

[2023-03-01 11:43:32] open-ils.hold-targeter [INFO:2438472:HoldTargeter.pm:404:167769258324384535] targeter: [hold 1431577] exiting => Hold is not eligible for targeting

Line 1261 seems to have another check to exclude a frozen hold.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=blob;f=Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm;h=58ccc0616598bece2df4ff1e9e1dc28f6fa951f3;hb=8dc431f3827eff75bf979f21a15e5ed3c9bc806e#l1261

This is the command I'm running
./hold_targeter.pl --verbose --parallel=5 --parallel-init-sleep=5 --soft-retarget-interval="12 hours" --retarget-interval="8 days" --retarget-frozen

[Josh Stompro (u-launchpad-stompro-org)]

I tried placing a new hold and using the suspend immediately option. I checked and there were no action.hold_copy_map entries for the new hold.

I ran the hold targeter with the --retarget-frozen option and I can see that the hold targeter is trying to process the hold,

[2023-03-01 12:52:49] open-ils.hold-targeter [INFO:2438457:HoldTargeter.pm:404:167769675124397182] targeter: [hold 1451044] processing...

[2023-03-01 12:52:49] open-ils.hold-targeter [INFO:2438457:HoldTargeter.pm:404:167769675124397182] targeter: [hold 1451044] exiting => Hold is not eligible for targeting

I'm not sure if just taking off the extra check for a frozen hold in "sub target {" in Utils/HoldTargeter.pm is the way. Or if there should be a separate mode just for handling frozen hold retargeting. I'm happy to test again in the future though.

Josh

[Bill Erickson (berick)]

Ugh, thanks, Josh. Here's a rebased branch with the additional changes to make the code do the thing.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1744341-retarget-frozen-holds-v3

[Josh Stompro (u-launchpad-stompro-org)]

I'm wondering if this will cause issues with frozen holds that have an expire_time set? I don't think the expire date mattered before this change for frozen holds, since they never made it to the "return unless $self->handle_expired_hold;" line.

Maybe frozen holds shouldn't have expire_time set? I'm seeing 19 frozen holds with it set and 3097 without it set in our production data. That makes me think that maybe some less used method to freeze holds isn't clearing the expire date. Like maybe the staff interface method of freezing a hold. That is probably a separate bug to deal with.

I'll continue testing.

[Josh Stompro (u-launchpad-stompro-org)]

The new version seems to be working like it should now.

Before running there were 5909 action.hold_copy_map entries associated with frozen holds, and after running with --retarget-frozen there are 10395 entries.

It did result in 5 out of the 19 frozen holds that had expiration dates set to be canceled. I tried suspending a few holds in the 3.10 angular and angularjs interfaces and in all cases the expire_time was set to null when suspending/freezing the hold. So the few holds in our production data must be from an old interface or some manual action. I asked on IRC and Jeff Davis said he also had a number of them in their 3.9.1 system, they were all several years old though, so nothing recent. So probably whatever bug caused them was from a while back.

I'll work on adding a DB upgrade script to fix any instances left over to prevent any frozen holds from being canceled unexpectedly.

Josh

[Josh Stompro (u-launchpad-stompro-org)]

Here is a branch with an added upgrade script.

user/stompro/lp1744341-retarget-frozen-holds-v3-withupgrade

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1744341-retarget-frozen-holds-v3-withupgrade

[Jeff Davis (jdavis-sitka)]

Here's a rebased version, in case anybody (not me) is in a position to test and sign off: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1744341-retarget-frozen-holds-v3-withupgrade-rebased / user/jeffdavis/lp1744341-retarget-frozen-holds-v3-withupgrade-rebased