Denormalize community_id into post_aggregates for a 1000x speed-up when loading posts #3653

sunaurus · 2023-07-18T08:58:14Z

Credit to @phiresky for this idea, originally posted in comments of #2994

This PR adds community_id to post_aggregates (& a new index on post_aggregates) to enable joining community directly to post_aggregates when querying posts.

On lemm.ee, this optimization speeds up the query for front page of subscribed posts ~1000x, from several seconds to to just milliseconds. You can check a before/after of query plans here: https://gist.github.com/sunaurus/856e03165bb0c0010505afeebde45230

dessalines

Take a look at these join filters:

This hints that the same process could be done with post.creator_id -> post_aggregates.creator_id .

I also think this makes the case that either:

post_aggregates should be the "main" table that the others are joined to, and primarily filter from.
Or that everywhere a post.XXX column is in the filters or joins, it should be replaced with post_aggregates.XXX

I also don't know what new indexes should be added, but lets make sure they're actually getting used.

migrations/2023-07-18-082614_post_aggregates_community_id/up.sql

crates/db_views/src/post_view.rs

sunaurus · 2023-07-18T14:46:38Z

At this point, the difference is very small, but indeed it is a bit faster to treat post_aggregates as the main table completely. Will push additional commits soon.

phiresky · 2023-07-18T14:48:05Z

post_aggregates should be the "main" table that the others are joined to, and primarily filter from.

Is there a specific reason the post_aggregates and post table are spilt? In general I think it would make sense to have everything in the same table. (same for comments /person...). 1:1 relations aren't that great in postgresql in general imo, tables with a huge amount of columns work pretty well. It would increase contention a bit because every change in votes would lock the post row for a bit (just writes, not reads) but I don't think that justifies the split. Or is there other reasons for the spilt?

sunaurus · 2023-07-18T15:06:40Z

New commits contain the following changes

Remove the unused index
Added creator_id to post_aggregates
Made post_aggregates the "main" table for PostQuery

sunaurus · 2023-07-18T15:25:53Z

Made post_aggregates the main table for PostView as well now (this was already very fast before, but indeed it is even faster now with these changes)

crates/db_views/src/post_view.rs

migrations/2023-07-18-082614_post_aggregates_community_id/up.sql

dessalines · 2023-07-19T14:58:52Z

migrations/2023-07-18-082614_post_aggregates_community_id/up.sql

+
+ALTER TABLE post_aggregates
+    ALTER COLUMN community_id SET NOT NULL,
+    ALTER COLUMN creator_id SET NOT NULL;


After this one, much like the post table has indexes on community_id and creator_id, it would probably be a good idea to add these two indexes to post_aggregates now too:

IE:

create index idx_post_aggregates_community on post_aggregates (community_id); create index idx_post_aggregates_creator on post_aggregates (creator_id);

Sure, added these indexes as well

@dessalines I just discovered that adding these two indexes messes up the query plan again 😃

Edit: hmm, I just got some 3-second query plans with the indexes. Then I dropped the indexes - back to a few ms. ~~Then re-added, and it's still at a few ms. So I can't reproduce the problem at the moment~~, but I will investigate a bit more..

Ok, the query plans are definitely consistently worse with those indexes.
With indexes: https://explain.depesz.com/s/W7cF
Without indexes: https://explain.depesz.com/s/4qds

I reverted these indexes for now, let me know if you disagree with that

wait, you didn't add any index to get the speedup? the "right" index should be post_aggregates(community_id, featured_local desc, hot_rank desc)

it should use that one when the user has only subscribed to a few communities, and the idx_featured_local_hot_rank when subscribed to many communities

Discussed a bit on Matrix already, but commenting here as well for visibility:

the "right" index should be post_aggregates(community_id, featured_local desc, hot_rank desc)

This index also messes up the query plan. It indeed makes the queries super fast for users with few subscriptions, but it makes them slow again for others.

Without that index
User with 86 subscriptions: 3ms query execution
User with 0 subscriptions: 700 ms query execution
(I didn't measure exact time, but it's also very fast for logged out users without the index)

With that index
User with 86 subscrptions: 2-3 second query execution
User with 0 subscriptions: 1ms query execution

phiresky on Matrix said:

weird. do the query plans have larger misestimates?
if so try setting default statistics target to 1000 and run analyze again. if not then I don't know why PG is being dumb

I will see tomorrow if playing around with statistics will help. I know there's also CREATE STATISTICS in postgres, but I've never used it so would need to investigate it a bit.

The limit and sort is the main culprit, but also its weird to me that postgres sometimes chooses a slower index.

We can tweak those indexes later, I'll try to tag the issues as DB

dessalines

Looks great, thx!

I'll let @Nutomic or @phiresky look over before merging.

…en loading posts

…en loading posts (#3653) * Denormalize community_id into post_aggregates for a 1000x speed-up when loading posts * Remove unused index * Add creator_id to post_aggregates * Use post_aggregates as main table for PostQuery * Make post_aggregates the main table for PostView * Reformat SQL

sunaurus requested review from Nutomic, dessalines and phiresky as code owners July 18, 2023 08:58

sunaurus mentioned this pull request Jul 18, 2023

[Bug]: Lemmy-ui loads extremely slowly in Safari LemmyNet/lemmy-ui#1410

Open

4 tasks

dessalines reviewed Jul 18, 2023

View reviewed changes

migrations/2023-07-18-082614_post_aggregates_community_id/up.sql Outdated Show resolved Hide resolved

dessalines reviewed Jul 18, 2023

View reviewed changes

crates/db_views/src/post_view.rs Show resolved Hide resolved

crates/db_views/src/post_view.rs Show resolved Hide resolved

phiresky reviewed Jul 18, 2023

View reviewed changes

crates/db_views/src/post_view.rs Outdated Show resolved Hide resolved

sunaurus requested review from dessalines and phiresky July 18, 2023 15:26

sunaurus force-pushed the post_aggregates_community_id branch 2 times, most recently from e71461e to 565df53 Compare July 19, 2023 13:34

dessalines reviewed Jul 19, 2023

View reviewed changes

sunaurus requested a review from dessalines July 19, 2023 15:05

sunaurus force-pushed the post_aggregates_community_id branch from ed91fc0 to f58d660 Compare July 19, 2023 15:07

dessalines approved these changes Jul 19, 2023

View reviewed changes

sunaurus added 6 commits July 19, 2023 21:13

Denormalize community_id into post_aggregates for a 1000x speed-up wh…

bb4c7c6

…en loading posts

Remove unused index

fb696be

Add creator_id to post_aggregates

b20ab9f

Use post_aggregates as main table for PostQuery

7866547

Make post_aggregates the main table for PostView

00091c1

Reformat SQL

021d3a6

sunaurus force-pushed the post_aggregates_community_id branch from f58d660 to 021d3a6 Compare July 19, 2023 18:14

Nutomic approved these changes Jul 20, 2023

View reviewed changes

dessalines approved these changes Jul 20, 2023

View reviewed changes

dessalines merged commit b511c2e into LemmyNet:main Jul 20, 2023
1 check passed

sunaurus deleted the post_aggregates_community_id branch July 20, 2023 21:04

This comment was marked as abuse.

Sign in to view

This was referenced Aug 11, 2023

Slow SQL queries #2877

Closed

PostgreSQL EXPLAIN shows faster performance listing posts for community #3839

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Denormalize community_id into post_aggregates for a 1000x speed-up when loading posts #3653

Denormalize community_id into post_aggregates for a 1000x speed-up when loading posts #3653

sunaurus commented Jul 18, 2023

dessalines left a comment

sunaurus commented Jul 18, 2023 •

edited

phiresky commented Jul 18, 2023 •

edited

sunaurus commented Jul 18, 2023

sunaurus commented Jul 18, 2023 •

edited

dessalines Jul 19, 2023

sunaurus Jul 19, 2023

sunaurus Jul 19, 2023 •

edited

sunaurus Jul 19, 2023

sunaurus Jul 19, 2023 •

edited

phiresky Jul 19, 2023 •

edited

sunaurus Jul 19, 2023 •

edited

dessalines Jul 20, 2023

dessalines left a comment

This comment was marked as abuse.

Denormalize community_id into post_aggregates for a 1000x speed-up when loading posts #3653

Denormalize community_id into post_aggregates for a 1000x speed-up when loading posts #3653

Conversation

sunaurus commented Jul 18, 2023

dessalines left a comment

Choose a reason for hiding this comment

sunaurus commented Jul 18, 2023 • edited

phiresky commented Jul 18, 2023 • edited

sunaurus commented Jul 18, 2023

sunaurus commented Jul 18, 2023 • edited

dessalines Jul 19, 2023

Choose a reason for hiding this comment

sunaurus Jul 19, 2023

Choose a reason for hiding this comment

sunaurus Jul 19, 2023 • edited

Choose a reason for hiding this comment

sunaurus Jul 19, 2023

Choose a reason for hiding this comment

sunaurus Jul 19, 2023 • edited

Choose a reason for hiding this comment

phiresky Jul 19, 2023 • edited

Choose a reason for hiding this comment

sunaurus Jul 19, 2023 • edited

Choose a reason for hiding this comment

dessalines Jul 20, 2023

Choose a reason for hiding this comment

dessalines left a comment

Choose a reason for hiding this comment

This comment was marked as abuse.

sunaurus commented Jul 18, 2023 •

edited

phiresky commented Jul 18, 2023 •

edited

sunaurus commented Jul 18, 2023 •

edited

sunaurus Jul 19, 2023 •

edited

sunaurus Jul 19, 2023 •

edited

phiresky Jul 19, 2023 •

edited

sunaurus Jul 19, 2023 •

edited