LibriVox / librivox-catalog

LibriVox catalog and reader workflow application
https://librivox.org
MIT License
36 stars 17 forks source link

Fix slow spinner using the main nav from the author page #186

Closed garethsime closed 4 months ago

garethsime commented 5 months ago

(Note: This was a problem for me on Firefox 121, I haven't tried other browsers.)

When I'm on an author page, and I click on one of the main nav items, I get a really long spinner. There are two problems that seem to play in together to give this result.

In the Javascript that handles menu item clicks in footer.php, we try to do a redirect to /search/<category>, but we don't actually stop execution there - the script keeps going. It sets up a spinner and tries to fetch new results etc.

This is the first problem - the results it tries to load are always from /author/get_results rather than from /search/get_results. The page seems to wait until that call has resolved before doing the redirect to /search/<category> and loading the actual search results. I've changed this handler to return immediately instead.

What exacerbated the problem for the author search is that the search would fetch /author/get_results with primary_key=0, so the backend would try to load every author and then every title for each of them, which was a lot. On my machine, it would eventually run out of memory and fail the request. You can get similar behavior by browsing to https://librivox.org/author/0 (but maybe do it locally).

The reader and group pages had the same problems, but their /get_results endpoints failed quickly if you passed in primary_key=0, so it wasn't as noticeable.

I've changed the controllers to explicitly check for empty/zero IDs before they try to run anything.

redrun45 commented 5 months ago

Just posting to say "someone has tried this out". Browsing to /author/0 on current master does indeed play havoc and cause a memory error on my local machine. I suppose our production environment powers through it, but... yikes!

This sounds like a straight technical improvement to me, so I'd love to have admins try it on the staging server.

notartom commented 4 months ago

I think this is one of those cases where we ask for forgiveness rather than permission.