Add a covering index on tracks (album_id, length) #508
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
use subqueries instead of a left join
|
thanks! @cascooscuro can you confirm if the issue is solved when checking out this branch? with read timeouts removed |
|
Sometimes I still get a timeout, but only have seen it when connecting to jamstash the first time: For the rest of the operations like search, play I see it responds much quicker and have not timeouts. Maybe having the config option to modify the http timeout is good enough. Thanks both for your help. |
|
hmm maybe I'll just bump the timeout to 10 secs globally |
|
I'm assuming these calls come from here, and the timeout happens in I noticed that gonic reads entire Also, the join with albums for checking the directory is quite expensive, so if you have just one music folder, it's best to avoid sending the |
|
@lomereiter thanks very much for the work here 👌👌 going to test and ship this shortly also I think it makes sense to update the http logging middleware to print the response times for each request - making it easy for someone like @cascooscuro to identify endpoints which could be optimised for big libraries |
|
an update after the latest commit (1c7cb8f): |
|
In my opinion, Jamstash is badly designed for big libraries, and there's only so much we can do on gonic side: right at the start Jamstash is simultaneously doing multiple heavy API calls. Compare to Supersonic behavior, for example:
|
|
I had yet another look at it, and added an index for the |
lomereiter
force-pushed
the
tracks_album_id_idx
branch
from
0c48a4f
to
cea3882
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The covering index also includes length, so that SQLite can avoid reading the rows.
In my experiments with the DB provided by @cascooscuro (see #478), adding this index already helps to cut the query time from 2.8s to 1.0s. Replacing the join with subqueries cuts down the time further to 0.3s.
Query plan for the original query:
When using subqueries:
fixes #478