Bug 28966 - Holds queue viewer too slow to load for large numbers of holds
Summary: Holds queue viewer too slow to load for large numbers of holds
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 29015
Blocks: 28974
  Show dependency treegraph
 
Reported: 2021-09-07 16:54 UTC by Kyle M Hall
Modified: 2022-02-13 13:39 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28966: Prefetch patron data for holds queue viewer (6.57 KB, patch)
2021-09-07 16:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28966: Prefetch patron data for holds queue viewer (6.60 KB, patch)
2021-09-07 17:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28966: Prefetch patron data for holds queue viewer (6.60 KB, patch)
2021-09-07 17:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28966: Prefetch patron data for holds queue viewer (3.73 KB, patch)
2021-09-07 18:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28966: Prefetch patron data for holds queue viewer (4.37 KB, patch)
2021-09-08 11:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28966: Prefetch patron data for holds queue viewer (4.37 KB, patch)
2021-09-08 12:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28966: Prefetch patron data for holds queue viewer (4.41 KB, patch)
2021-09-14 07:53 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 28966: Prefetch patron data for holds queue viewer (4.33 KB, patch)
2021-09-27 19:04 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2021-09-07 16:54:50 UTC
We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.
Comment 1 Kyle M Hall 2021-09-07 16:58:01 UTC
Created attachment 124626 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same
Comment 2 Kyle M Hall 2021-09-07 17:24:54 UTC
Created attachment 124627 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same
Comment 3 Kyle M Hall 2021-09-07 17:47:14 UTC
Created attachment 124628 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same
Comment 4 Kyle M Hall 2021-09-07 18:08:24 UTC
Created attachment 124629 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same
Comment 5 Kyle M Hall 2021-09-08 11:06:07 UTC
Created attachment 124639 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same
Comment 6 Kyle M Hall 2021-09-08 12:17:36 UTC
Created attachment 124652 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same
Comment 7 Emmi Takkinen 2021-09-14 07:53:48 UTC
Created attachment 124854 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same

Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 8 Joonas Kylmälä 2021-09-19 11:27:26 UTC
hmm, this disables the hide_patron_infos_if_needed variable which is probably not wanted, though the feature is quite broken anyways but this makes it more broken.
Comment 9 Kyle M Hall 2021-09-27 19:04:31 UTC
Created attachment 125377 [details] [review]
Bug 28966: Prefetch patron data for holds queue viewer

We have a partner processing thousands of holds per day, per branch. At this number of holds, the script loads very slow if it manages to load at all. The primary slowdown is the individual fetch of the patron for each hold to be displayed. If we prefetch those patrons, the entire script should load much faster.

Test Plan:
1) Create as many holds as you can ( up to a few thousand if you can )
2) Run the holds queue viewer, note the load time
3) Apply this patch
4) Restart all the things!
5) Reload the holds queue viewer, load time should be improved
6) Note the HTML generated looks the same

Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 10 Joonas Kylmälä 2021-10-03 08:39:35 UTC
(In reply to Joonas Kylmälä from comment #8)
> hmm, this disables the hide_patron_infos_if_needed variable which is
> probably not wanted, though the feature is quite broken anyways but this
> makes it more broken.

Just to make it clear and try to get this moving here, I'm holding off from signing-off on this until we have some clarity to this.
Comment 11 Jonathan Druart 2021-12-02 09:30:18 UTC
This is going backward IMO, why not move to DBIC/Koha::Object instead?
Comment 12 Fridolin Somers 2022-01-31 23:54:13 UTC
(In reply to Jonathan Druart from comment #11)
> This is going backward IMO, why not move to DBIC/Koha::Object instead?

Indeed we should be go back to direct SQL even if faster.
Se should use DBIC for all tables, with prefetch.

But we could start with adding pagination Bug 28974 while working on rewriting to use DBIC.
Comment 13 Jonathan Druart 2022-02-01 08:35:34 UTC
(In reply to Fridolin Somers from comment #12)
> (In reply to Jonathan Druart from comment #11)
> > This is going backward IMO, why not move to DBIC/Koha::Object instead?
> 
> Indeed we should be go back to direct SQL even if faster.

Is there a "not" missing here?
Comment 14 Fridolin Somers 2022-02-02 06:03:54 UTC
Oupsy :

> Indeed we should be go back to direct SQL even if faster.
Indeed we should not go back to direct SQL even if faster.
Comment 15 Katrin Fischer 2022-02-13 13:39:56 UTC
I gather from the discussion that this should be changed to use DBIC/Koha::Object instead of SQL. Moving to Failed QA for now to trigger action.