improve performance of community followers inbox query #3482
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
|
This is probably THE most important thing to add in 0.18.1. My remote DB is hitting close to 1Gbs of data transfers. |
phiresky
force-pushed
the
literally-fix-lemmy
branch
from
179715d
to
bd98d2d
Compare
This comment was marked as abuse.
This comment was marked as abuse.
Nutomic
reviewed
Jul 5, 2023
| @@ -11,19 +17,37 @@ use lemmy_db_schema::{ | |||
|
|
|||
| type CommunityFollowerViewTuple = (Community, Person); | |||
|
|
|||
| sql_function!(fn coalesce(x: diesel::sql_types::Nullable<diesel::sql_types::Text>, y: diesel::sql_types::Text) -> diesel::sql_types::Text); | |||
There was a problem hiding this comment.
Would be great if this could be made generic to work for multiple types, or better yet built into diesel.
|
Great find! I think the apub crate should generally not use any views, so there might be more to optimize. Edit: Federation tests recently keep failing in CI, but I confirmed that its passing locally for this PR. |
|
Just a few charts after deploying this + #3483:
|
Nutomic
approved these changes
Jul 5, 2023
dessalines
approved these changes
Jul 5, 2023
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.
i found a huge perf issue. get_follower_inboxes fetches all followers of a community (including thousands of copies of the whole damn community object, person object, and community_follower object) for every single action (e.g. like). it transfers that list to rust (like 20000 elements with huge objects each, including private keys and whatever). rust then finds all the federated instances it has to send the like to from that list (like 100 strings).
so basically the query uselessly transfers huge amounts of data just to get a list of instance_count urls.
on lemmy.world we looked at pg_stats_statements (thanks jelloeater) and that community_follower query absolutely dominates the stats, eating up 8 days of query time per day. Since yesterday, it has fetched over 4 billion rows.
that query is also timing out a lot due to the statement_timeout=10s we set.
This PR improves on the situation by doing the filtering on the database side. It's not optimal since the result can't be perfectly indexed, but it seems the main issue is the huge amount of data transfer between rust and PG. after some quick testing: the new query takes around 100ms and returns only tiny amounts of data. it should basically fix a huge amount of lemmys current perf problems.
i also replaced the second query in followers which was doing a similar amount of unnecessary work (but much more seldom).
i did not extensively test this for correctness.