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

OwnsAlbum takes insanely long in large album trees #388

Closed
bobobo1618 opened this issue Jun 1, 2021 · 1 comment
Closed

OwnsAlbum takes insanely long in large album trees #388

bobobo1618 opened this issue Jun 1, 2021 · 1 comment
Labels
API Related to the backend api server written in Go bug Something isn't working
Projects

Comments

@bobobo1618
Copy link

Describe the bug
When a user has many albums, OwnsAlbum calls GetChildrenFromAlbums with a list of all the albums a user owns, which gets the recursive list of all albums the user owns, and checks whether the album in question is contained in the set of all albums the user owns.

This can take a long time in a tree with many albums. In my case, I have >10k "albums", all of which must be returned in order to determine whether the user owns the album in question.

Since this check is run often, a site with many albums slows to a halt. In particular, even thumbnails take >3s to load.

To Reproduce
Steps to reproduce the behavior:

  1. Have many albums
  2. Run the server
  3. Observe /app/graphql/models/album.go:53 SLOW SQL >= 200ms log messages and >3s load times for most requests.

Expected behavior
Requests shouldn't be slow.

Rather than recursively getting all the children of the user (10k results), the code should recursively get all the parents of the album being checked (4 results).

e.g. turn this query on its head:

WITH recursive super_albums AS (
        SELECT * FROM albums AS leaf WHERE id = 4875
        UNION ALL
        SELECT parent.* from albums AS parent JOIN super_albums ON parent.id = super_albums.parent_album_id
)

SELECT * FROM super_albums;

I've tested this approach in code and it works, bringing thumbnail request latency down to ~300ms, from 3s.

Unfortunately, I'm not yet sure if I can send a PR, as my employer's open-source program forbids contribution to AGPL-licensed projects. I'm looking to see if I can work something out with them.

@bobobo1618 bobobo1618 added the bug Something isn't working label Jun 1, 2021
@viktorstrate
Copy link
Member

Hey, thank you so much for writing this issue. I could not think of a better way of doing this that didn't introduce redundancy in the database when I wrote it.

This is a super simple solution that would massiveley improve probably the biggest bottleneck performance wise.
And I will definitley remember that approach for general problems in the future as well.

Unfortunately, I'm not yet sure if I can send a PR, as my employer's open-source program forbids contribution to AGPL-licensed projects. I'm looking to see if I can work something out with them.

I really hope you can figure something out that would allow you to contribute code.

@viktorstrate viktorstrate added the API Related to the backend api server written in Go label Jun 2, 2021
@viktorstrate viktorstrate added this to To do in Roadmap via automation Jun 25, 2021
Roadmap automation moved this from To do to Done Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Related to the backend api server written in Go bug Something isn't working
Projects
Development

No branches or pull requests

2 participants