Optimize search queries for saved entities #2007
Conversation
|
we missed the followed user bug because a lack of unit tests. i could add some more. |
isaacsolo
force-pushed
the
is-improve-search-saved
branch
from
109cb32
to
2e59c4e
Compare
There was a problem hiding this comment.
This a fantastic change!! Just a few minor comments.
I did have one question regarding having saved_tracks as a subset of the track results - I think we discussed this over slack, but it seems like this could result in a scenario where we have a weak match in a saved track, which in today's live implementation would be returned separately in the saved tracks field, but in this proposed change wouldn't score highly enough to make it into our results at all.
I suspect we don't care, but this is a small product change so it may be worth coming up with an example or two to see if this issue manifests, and if so, running it by Forrest over slack to see that we're fine with the change. WDYT @dmanjunath?
| search_str, | ||
| limit, | ||
| offset, | ||
| True, |
There was a problem hiding this comment.
Since we never set personalized to True anymore (afaict), do we still need this argument present in all of the query functions or could we lose it?
There was a problem hiding this comment.
Yeah I'll remove this.
| @@ -401,33 +358,34 @@ def submit_and_add(search_type): | |||
|
|
|||
| if searchKind in [SearchKind.all, SearchKind.tracks]: | |||
| submit_and_add("tracks") | |||
There was a problem hiding this comment.
Not that it really matters, but we can lower the max_workers in in the threadpool now, since we only ever perform 4 requests in parallel.
There was a problem hiding this comment.
Ooh that's why it was set at 8. Yup I'll change it.
isaacsolo
force-pushed
the
is-improve-search-saved
branch
from
f70dd4e
to
975db1a
Compare
isaacsolo
force-pushed
the
is-improve-search-saved
branch
2 times, most recently
from
4deb491
to
6d6f801
Compare
isaacsolo
force-pushed
the
is-improve-search-saved
branch
2 times, most recently
from
f450fc9
to
94e7361
Compare
isaacsolo
force-pushed
the
is-improve-search-saved
branch
from
94e7361
to
d38ca94
Compare
isaacsolo
force-pushed
the
is-improve-search-saved
branch
from
603f3e9
to
da0d467
Compare
| ).fetchall() | ||
|
|
||
| # track_ids is list of tuples - simplify to 1-D list | ||
| track_ids = [i[0] for i in track_data] | ||
| saved_tracks = set([i[0] for i in track_data if i[3]]) # if track has user ID, the current user saved that track |
There was a problem hiding this comment.
we should pull out these indices into named variables so it's clear what they are
Yup it's already on sandbox 2. I'm keeping that updated for testing. |
Description
This change reduces the db calls needed for searches when a user is logged in: for both full searches and autocomplete. Previously, each saved entity such as saved tracks, playlists, albums, or followed users would require an additional db call which could be avoided to optimize performance by around 1.5-2x.
It also fixes a bug with the search results for followed users where followed users was equivalent to users (nothing was filtered).
With this change, saved_tracks is a subset of tracks. A saved track might exist in the saved_tracks result but not in the all tracks result. We're essentially filtering the top X tracks for any tracks that user has saved. The same applies for other entities: playlists, followed users, albums.
In order to avoid saved tracks from being buried under the top X tracks limit, this change adds a similarity score boost for any entity the current user has saved.
Tests
Added a bunch of new tests for autocomplete, internal/external search, different entities.
Ran against a prod snapshot. Compared search results to prod. Ran locust load testing and came up with 1.5x improvement in latency. Tested latency of autocomplete and full searches. Notes updated in Optimize search queries doc.
How will this change be monitored?