Bug 14736 - AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record with many items and many holds
Summary: AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record ...
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P3 normal (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-26 17:30 UTC by HB-NEKLS
Modified: 2016-12-14 12:14 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14736 - AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record with many items and many holds (2.66 KB, patch)
2016-08-24 11:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14736 - AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record with many items and many holds (2.72 KB, patch)
2016-10-14 08:13 UTC, Jesse Maseto
Details | Diff | Splinter Review
Bug 14736 - AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record with many items and many holds (2.78 KB, patch)
2016-11-18 10:28 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description HB-NEKLS 2015-08-26 17:30:10 UTC
This issue was originally reported in bug 14320. 

If the AllowRenewalIfOtherItemsAvailable sys pref is set to allow, and a borrower has an item checked out that has many items {30+} AND many holds {70+) on it, loading the checkouts table for this borrower takes FOREVER to load. The load time takes forever, because of the factoring that happens to determine if an item is truly available for renewal. 

At this time, the situation couldn't be improved in bug 14320, but I want this report out there, in case this becomes a bigger problem and/or someone comes up with a way to speed up the processing.
Comment 1 Kyle M Hall 2016-08-24 11:27:59 UTC
Created attachment 54813 [details] [review]
Bug 14736 - AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record with many items and many holds

If the AllowRenewalIfOtherItemsAvailable sys pref is set to allow, and a
borrower has an item checked out that has many items {30+} AND many
holds {70+) on it, loading the checkouts table for this borrower takes
FOREVER to load. The load time takes forever, because of the
factoring that happens to determine if an item is truly
available for renewal.

This patch swaps the use of GetMemberDetails for GetMember
and reorders the subroutine calls to check each items' renewability
from fastest to slowest.

In a test case, the results of pre patch were:
Start of loop: 2016-08-24T11:05:14
End of loop:   2016-08-24T11:05:29
Resulting in 15 seconds being spent in the loop

Post patch results were:
Start of loop: 2016-08-24T11:08:43
End of loop:   2016-08-24T11:08:48
Resulting in only 5 seconds being spent in the loop!

Test Plan:
1) Apply this patch
2) Note there are no changes in functionality for the renewals column of
   the patron checkouts table.

If you wish to go further and test the performance benefit:
1) Create a record with 50 items and 100 holds ( 50 waiting, 50 unfilled )
2) Check out one of the waiting holds to a patron
3) Time the amount of time it takes for the checkouts table to load
4) Apply this patch
5) Repeat step 3, you should see an improvement in load time
Comment 2 Jesse Maseto 2016-10-14 08:13:27 UTC
Created attachment 56463 [details] [review]
Bug 14736 - AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record with many items and many holds

If the AllowRenewalIfOtherItemsAvailable sys pref is set to allow, and a
borrower has an item checked out that has many items {30+} AND many
holds {70+) on it, loading the checkouts table for this borrower takes
FOREVER to load. The load time takes forever, because of the
factoring that happens to determine if an item is truly
available for renewal.

This patch swaps the use of GetMemberDetails for GetMember
and reorders the subroutine calls to check each items' renewability
from fastest to slowest.

In a test case, the results of pre patch were:
Start of loop: 2016-08-24T11:05:14
End of loop:   2016-08-24T11:05:29
Resulting in 15 seconds being spent in the loop

Post patch results were:
Start of loop: 2016-08-24T11:08:43
End of loop:   2016-08-24T11:08:48
Resulting in only 5 seconds being spent in the loop!

Test Plan:
1) Apply this patch
2) Note there are no changes in functionality for the renewals column of
   the patron checkouts table.

If you wish to go further and test the performance benefit:
1) Create a record with 50 items and 100 holds ( 50 waiting, 50 unfilled )
2) Check out one of the waiting holds to a patron
3) Time the amount of time it takes for the checkouts table to load
4) Apply this patch
5) Repeat step 3, you should see an improvement in load time

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Comment 3 Jonathan Druart 2016-11-18 10:28:17 UTC
Created attachment 57636 [details] [review]
Bug 14736 - AllowRenewalIfOtherItemsAvailable slows circulation down in case of a record with many items and many holds

If the AllowRenewalIfOtherItemsAvailable sys pref is set to allow, and a
borrower has an item checked out that has many items {30+} AND many
holds {70+) on it, loading the checkouts table for this borrower takes
FOREVER to load. The load time takes forever, because of the
factoring that happens to determine if an item is truly
available for renewal.

This patch swaps the use of GetMemberDetails for GetMember
and reorders the subroutine calls to check each items' renewability
from fastest to slowest.

In a test case, the results of pre patch were:
Start of loop: 2016-08-24T11:05:14
End of loop:   2016-08-24T11:05:29
Resulting in 15 seconds being spent in the loop

Post patch results were:
Start of loop: 2016-08-24T11:08:43
End of loop:   2016-08-24T11:08:48
Resulting in only 5 seconds being spent in the loop!

Test Plan:
1) Apply this patch
2) Note there are no changes in functionality for the renewals column of
   the patron checkouts table.

If you wish to go further and test the performance benefit:
1) Create a record with 50 items and 100 holds ( 50 waiting, 50 unfilled )
2) Check out one of the waiting holds to a patron
3) Time the amount of time it takes for the checkouts table to load
4) Apply this patch
5) Repeat step 3, you should see an improvement in load time

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 4 Jonathan Druart 2016-11-18 10:28:36 UTC
Great catch Kyle!
Comment 5 Kyle M Hall 2016-11-18 13:57:50 UTC
Pushed to master for 16.11!
Comment 6 Julian Maurice 2016-11-30 13:12:51 UTC
Pushed to 3.22.x, will be in 3.22.13
Comment 7 Mason James 2016-12-14 12:14:21 UTC
Pushed to 16.05.x, for 16.05.06 release