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

Improve performance by using cached location details #1733

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jcoyne
Copy link
Contributor

@jcoyne jcoyne commented Aug 16, 2023

This improved the performance of https://requests-folio-dev.stanford.edu/pages/new?item_id=L284&origin=LANE-MED&origin_location=LANE-SAL3 to 30s from a baseline of 35-42s (lots of variation).

Fixes #1676

@jcoyne jcoyne force-pushed the performance-location2 branch 4 times, most recently from 2a58638 to 5343a2b Compare August 17, 2023 03:39
This slows down the query especially in cases where there are a lot of items.
@jcoyne jcoyne force-pushed the performance-location2 branch from 5343a2b to 66c8b74 Compare August 17, 2023 14:13
@cbeer
Copy link
Member

cbeer commented Aug 17, 2023

Thanks for providing the timing information. It's unfortunate that this doesn't get us into the land of good performance. It's certainly not clear to me that shaving off a couple seconds of an awful number is worth the trade-off in complexity (and potential error introduced from using cached data).

Using cached data in Searchworks seems like a necessary evil, and the stakes are pretty low if we're wrong -- we either fail to draw a button or draw a button that shouldn't be there (and, as Searchworks doesn't particularly care about the values of the paging/scanning configuration, the kind of errors feel minimized).

To me, requests feels different: if we apply the wrong rules, material could get shipped to the "wrong" places. Caching also makes it harder for our colleagues maintaining the location details to verify their changes. Maybe there's alternatives to this particular caching approach, or making a 2nd query to graphql to get the location details... but it doesn't seem like this path is all that fruitful?

I'm also surprised not retrieving the location details saves much time at all -- it should just be extra (easily compressed) data down the wire. Maybe it's worth exploring our caching options in graphql to try to improve retrieval of repeated locations

@jcoyne
Copy link
Contributor Author

jcoyne commented Aug 17, 2023

@cbeer I'd like to see this on top of #1694, because I think the two together might be significant per my research in sul-dlss/folio-graphql#107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate slow performance of page requests with lots of items
2 participants