Circ Rules and Loan Policies
(UXPROD-789)
|
|
| Status: | Closed |
| Project: | UX Product |
| Components: | None |
| Affects versions: | None |
| Fix versions: | Poppy (R2 2023) | Parent: | Circ Rules and Loan Policies |
| Type: | New Feature | Priority: | P3 |
| Reporter: | Cheryl Malmborg | Assignee: | Tim Auger |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Release: | Poppy (R2 2023) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Epic Link: | Circ Rules and Loan Policies | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Front End Estimate: | Out of scope | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Back End Estimate: | XL < 15 days | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Development Team: | Volaris | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PO Rank: | 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Rank: Cornell (Full Sum 2021): | R4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Rank: Duke (Full Sum 2021): | R3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
Possible bug in circulation: https://github.com/folio-org/mod-circulation/blob/v22.1.3/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java#L125-L140 The selfcheck machine connects to FOLIO through SIP2 and checks out multiple items to the same patron at the same time (each checkout message does not wait for the processing result, and multiple checkout messages are sent at the same time). It is normal to valid the circulation rules when checkout item one by one in CheckOutByBarcodeResource.java. This is caused by a race condition: After refuseWhenItemLimitIsReached is called (line 125) many other operations take place before createLoan is called (line 140): https://github.com/folio-org/mod-circulation/blob/v22.1.3/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java#L125-L140 |
| Comments |
| Comment by Cheryl Malmborg [ 15/Nov/21 ] |
|
I've asked Tiewei Liu to add specifics about the self-check software they are using. Several libraries using self-check have not seen this problem. |
| Comment by Marc Johnson [ 15/Nov/21 ] |
|
What is the intention behind raising this bug? Is it intended that this be prioritised and worked on? |
| Comment by twliu [ 15/Nov/21 ] |
|
Marc, this is a go-live blocker for Shanghai Library. So it needs to be solved as soon as possible (by December). Cheryl, thank you for creating the ticket! Gang Zhou will add the specifics about the self-check software. |
| Comment by Cheryl Malmborg [ 15/Nov/21 ] |
|
Marc Johnson I am not sure how to characterize this issue. There seems to be an incompatibility between the self-checkout system Shanghai is using and folio processing. I know nothing about their self-check software. I think the first thing would be a clarification of how folio is keeping track of how many loans a patron has checked out. Are the patron's loans re-queried with each item checked out or does the module increment a session counter? |
| Comment by Gang Zhou [ 16/Nov/21 ] |
|
Slefcheck machines are from Shanghai RFID, Seaever, Bibliotheca |
| Comment by Marc Johnson [ 16/Nov/21 ] |
|
twliu Gang Zhou Cheryl Malmborg
I understand this is urgent and important. Khalilah Gambrell Stephanie Buck Hkaplanian Holly Mistlebauer Who needs to be involved in prioritising this, is it the POs / Capacity Planning / the PC?
I've done a little research into this area. It seems the item limit behaviour was implemented in
I believe the Vega team also did the work on sessions. A quick check of the code suggests to me:
Alexander Kurash How wrong is my understanding? |
| Comment by Julian Ladisch [ 22/Nov/21 ] |
|
The item limit check (refuseWhenItemLimitIsReached) is special because its outcome can change when other loans are created in parallel. The use case of a machine sending multiple check-out requests for a single patron in parallel is reasonable and should be supported for performance reasons. A race condition where parallel check-outs exceed the item limit can be completely avoided if PostgreSQL features are used:
|
| Comment by Stephanie Buck [ 19/Jan/22 ] |
|
Hi all. This has been moved from a bug to a feature. Raman Auramau and Alexander Kurash have been working on architecture for a solution. There will likely be more stories associated as we move forward. |
| Comment by Tim Auger [ 18/Sep/22 ] |
|
So, in a nutshell, for any item being checked out via SIP2, confirm the patron has not exceeded total number of items charged to their account according to circulation rules. It sounds like this check is not happening on each request? Gurleen Kaur1 Giorgi Ninua I think this can go to a Volaris developer to confirm existing behavior and, if the behavior is not correct, make corresponding changes to enforce the check for each checkout request. |
| Comment by Gurleen Kaur1 [ 19/Sep/22 ] |
|
Tim Auger , |
| Comment by Marc Johnson [ 21/Sep/22 ] |
The check is performed on every check out request. The concern that folks have is that when a SIP2 based self-checkout machine (or other client) makes that check out requests in parallel, then the checks could overlap with other check outs. As checks tend to be performed near the beginning of the process, the checks could falsely succeed because a check out that would lead to the item limit being reached had not completed before the check happens. Does that make sense? |
| Comment by Julian Ladisch [ 21/Sep/22 ] |
|
This is caused by a race condition: After refuseWhenItemLimitIsReached is called (line 125) many other operations take place before createLoan is called (line 140): https://github.com/folio-org/mod-circulation/blob/v22.1.3/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java#L125-L140 Assuming 9 items have been loaned and the item limit is 10. When running in sequence: item10 refuseWhenItemLimitIsReached <- this finds 9 loaned items and continues item10 other operations item10 createLoan item11 refuseWhenItemLimitIsReached <- this finds 10 loaned items and exits with item limit error item11 other operations item11 createLoan When running in parallel: item10 refuseWhenItemLimitIsReached <- this finds 9 loaned items and continues item11 refuseWhenItemLimitIsReached <- this finds 9 loaned items and continues item10 other operations item11 other operations item10 createLoan item11 createLoan item11 is incorrectly loaned because the item11 limit check runs between the item10 limit check and the item10 createLoan (race condition). |
| Comment by Tim Auger [ 21/Sep/22 ] |
|
Thanks, Marc Johnson Julian Ladisch for the additional info. This makes sense. And, given the speed of batch-like checkout via RFID enabled self-check systems and slow network speeds, could see this happen more frequently than anyone would like. Do either of you believe a refactor could resolve this or is it more than that? |
| Comment by Marc Johnson [ 21/Sep/22 ] |
I think that is better asked of the relevant development team. I believe the Vega team put together a proposal for resolving this. Personally, I don't think I would consider the likely solution to this to be a refactoring. I would expect it to involve significant changes. That this was originally a single issue for mod-circulation and is now a feature, suggests other folks also think there is considerably work involved. Roughly speaking, the system needs to be changed so that check outs for the same patron cannot occur concurrently. This can either happen at the whole process level or (as I think was proposed) that there would be some kind of locking mechanism around the item limit check so that couldn't be performed if any other check out was ongoing. |
| Comment by Tim Auger [ 21/Sep/22 ] |
|
Marc Johnson Cool. Based on the description and discussions I had with Vince, I thought it might be more than just restructuring. Thanks for the additional information. |
| Comment by Khalilah Gambrell [ 05/Oct/22 ] |
|
Gang Zhou and twliu - how has Shanghai addressed this issue?
cc: Tim Auger and Cheryl Malmborg |
| Comment by Tim Auger [ 24/Apr/23 ] |
|
Irina Pokhylets we should move this forward. |
| Comment by Tim Auger [ 02/May/23 ] |
|
Irina Pokhylets we can move forward with this. Just talked with Steph. Alex K. is who we can help with arch. Cheryl is the community PO |
| Comment by Irina Pokhylets [ 03/May/23 ] |
|
Based on the result of investigation
|
| Comment by Tim Auger [ 03/May/23 ] |
|
Irina Pokhylets thanks! Much appreciated. I changed the team for
|
| Comment by Irina Pokhylets [ 28/Sep/23 ] |
|
Closing the feature as all related tickets are closed. |