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

Optimize asset list endpoint #1904

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Optimize asset list endpoint #1904

merged 2 commits into from
Mar 25, 2024

Conversation

jjnesbitt
Copy link
Member

Closes #1891

The existing asset list endpoint is inefficient with how it queries the database, resulting in unnecessarily long response times from that endpoint. This is improved in the following ways:

  • Paginate and fetch IDs of desired assets first, before doing left joins
  • Remove duplicate queries as a result of class inheritance
  • Remove extra query in raise_if_unauthorized method

The results of these improvements are shown below. Note that these are the times for the SQL queries themselves, not the overall page load, as that can be inflated by the amount of data returned, and isn't useful for this discussion.

Operation Original (ms) Improved (ms) Speedup
No filters 300 30 10x
Ordering only 295 92 3.2x
Path filter only 235 75 3.1x
Ordering + path filter 240 140 1.7x

A couple things to note going forward:

  • The count query (the query that returns the total number of hits from which you're returning a single page) is usually slower than the query of interest itself, which makes sense. We may want to rethink the structure of our API respones if further optimization is desired, as if we were to remove this count, we would see a pretty widespread 2x speedup.
  • The structure of the asset list endpoint is getting pretty involved at this point, as is the rest of the asset endpoints. We may want to consider a refactor of that file in the future.

* Paginate and fetch IDs of desired assets first, before doing left joins
* Remove duplicate queries as a result of class inheritance
* Remove extra query in raise_if_unauthorized method
This test was fetching an unordered list and comparing it to an ordered list, resulting in inconsistent failures.
@jjnesbitt jjnesbitt added patch Increment the patch version when merged release Create a release when this pr is merged labels Mar 25, 2024
@jjnesbitt jjnesbitt merged commit 3a34cc0 into master Mar 25, 2024
11 checks passed
@jjnesbitt jjnesbitt deleted the improve-asset-list-query branch March 25, 2024 17:49
@dandibot
Copy link
Member

🚀 PR was released in v0.3.81 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slow response times from asset list endpoint
3 participants