Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

At desk notification gets cancelled if loan is not indexed fast enough #3005

Closed
PascalRepond opened this issue Jul 13, 2022 · 0 comments · Fixed by #3023
Closed

At desk notification gets cancelled if loan is not indexed fast enough #3005

PascalRepond opened this issue Jul 13, 2022 · 0 comments · Fixed by #3023
Labels
bug Breaks something but is not blocking f: notifications p-High High priority (to be solved in the 2-3 next months)

Comments

@PascalRepond
Copy link
Contributor

PascalRepond commented Jul 13, 2022

Bug description:

Sometimes, the at_desk notification gets cancelled when it should be sent: item exists, loan exists, loan is AT_DESK.

Hypothesis: The notification processing takes places 1 second after the transaction. Depending on the server load at any given moment, the loan indexing may not be over when the notification is processed. The notification sees the old loan status and cancel the sending.

Expected behavior:

All at_desk notifications should be sent if configured as such.

Steps to Reproduce:

Difficult to reproduce as it might depend on the server load. It happens regularly in production.

Example

image
The availability notification is correctly sent 5 minutes later.

image
The loan is ITEM_AT_DESK but it was processed less than 1 second before the notification.

Context

Server:

Version:

v1.11.0

Anything else?

To ensure that the notification gets the correct loan state, we might force it to wait the loan's reindexing or delay the notification processing by 30 secs-1min.

It would also be nice to have a more robust notification log, that allows us to see which check did not pass so that a notification was not sent.

@PascalRepond PascalRepond added the bug Breaks something but is not blocking label Jul 13, 2022
@pronguen pronguen added f: notifications p-High High priority (to be solved in the 2-3 next months) labels Jul 13, 2022
lauren-d added a commit to lauren-d/rero-ils that referenced this issue Jul 27, 2022
* Changes string comparator.
* Closes rero#3005.

Co-Authored-by: Lauren-D <[email protected]>
rerowep pushed a commit that referenced this issue Jul 27, 2022
* Changes string comparator.
* Closes #3005.

Co-Authored-by: Lauren-D <[email protected]>
lauren-d added a commit to lauren-d/rero-ils that referenced this issue Jul 28, 2022
* Adds a delay before notification sent.
* Closes rero#3005.

Co-Authored-by: Lauren-D <[email protected]>
lauren-d added a commit to lauren-d/rero-ils that referenced this issue Jul 28, 2022
Use a delay to be sure the notification is sent AFTER the loan has
been indexed (avoid problem due to server load).

* Adds a delay before notification sent.
* Closes rero#3005.

Co-Authored-by: Lauren-D <[email protected]>
lauren-d added a commit that referenced this issue Jul 29, 2022
Use a delay to be sure the notification is sent AFTER the loan has
been indexed (avoid problem due to server load).

* Adds a delay before notification sent.
* Closes #3005.

Co-Authored-by: Lauren-D <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Breaks something but is not blocking f: notifications p-High High priority (to be solved in the 2-3 next months)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants