From 9ae451a121ad2e7203f5c9cca423fc2c532e8ea3 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 14 Dec 2023 14:09:29 +0530 Subject: [PATCH] MM-55476: Optimize PostStore.Get (#25448) While loading a thread, we were unnecessarily re-counting the replies for all posts in a thread even if they would be the same number. While this is needed in other queries where the posts can be from different threads or they can be random ids, but to load a single post thread, there is no need to recompute it again and again. Therefore, we use a CTE to precompute the replycount and then just plug in the value in the subsequent query. This gives an improvement in the query plan as well: OLD: ``` explain (analyze, buffers) SELECT p.id, p.rootid, p.createat, (SELECT count(*) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyC ount FROM Posts p WHERE (p.Id = 'h3cer597jb8abbcbitpghpomua' OR p.RootId = 'h3cer597jb8abbcbitpghpomua') AND p.DeleteAt = 0; QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------------------------------------ Bitmap Heap Scan on posts p (cost=45.39..1042149.61 rows=2748 width=49) (actual time=3.156..7906.215 rows=5353 loops=1) Recheck Cond: (((id)::text = 'h3cer597jb8abbcbitpghpomua'::text) OR (((rootid)::text = 'h3cer597jb8abbcbitpghpomua'::text) AND (deleteat = 0))) Filter: (deleteat = 0) Heap Blocks: exact=5308 Buffers: shared hit=610244 -> BitmapOr (cost=45.39..45.39 rows=2748 width=0) (actual time=0.918..0.920 rows=0 loops=1) Buffers: shared hit=47 -> Bitmap Index Scan on posts_pkey (cost=0.00..1.68 rows=1 width=0) (actual time=0.028..0.028 rows=1 loops=1) Index Cond: ((id)::text = 'h3cer597jb8abbcbitpghpomua'::text) Buffers: shared hit=4 -> Bitmap Index Scan on idx_posts_root_id_delete_at (cost=0.00..42.34 rows=2747 width=0) (actual time=0.889..0.890 rows=5352 loops=1) Index Cond: (((rootid)::text = 'h3cer597jb8abbcbitpghpomua'::text) AND (deleteat = 0)) Buffers: shared hit=43 SubPlan 1 -> Aggregate (cost=378.10..378.11 rows=1 width=8) (actual time=1.474..1.474 rows=1 loops=5353) Buffers: shared hit=604889 -> Index Only Scan using idx_posts_root_id_delete_at on posts (cost=0.57..343.85 rows=13699 width=0) (actual time=0.016..1.039 rows=5352 loops=5353) Index Cond: ((rootid = (CASE WHEN ((p.rootid)::text = ''::text) THEN p.id ELSE p.rootid END)::text) AND (deleteat = 0)) Heap Fetches: 0 Buffers: shared hit=604889 Planning Time: 0.194 ms Execution Time: 7906.846 ms ``` NEW: ``` explain analyze with replycount as (select count(*) as num from posts where rootid='h3cer597jb8abbcbitpghpomua' and deleteat=0) select id, rootid, createat, replycount.num from posts, replycount where id='h3cer597jb8abbcbitpghpomua' or rootid='h3cer597jb8abbcbitpghpomua' and deleteat=0; ------------------------------------------------------------------------------------------------------------------------------------------------------------------- Nested Loop (cost=123.16..3215.48 rows=2748 width=49) (actual time=2.960..9.775 rows=5353 loops=1) -> Aggregate (cost=77.78..77.79 rows=1 width=8) (actual time=1.455..1.456 rows=1 loops=1) -> Index Only Scan using idx_posts_root_id_delete_at on posts posts_1 (cost=0.57..70.91 rows=2747 width=0) (actual time=0.056..1.145 rows=5352 loops=1) Index Cond: ((rootid = 'h3cer597jb8abbcbitpghpomua'::text) AND (deleteat = 0)) Heap Fetches: 0 -> Bitmap Heap Scan on posts (cost=45.39..3110.20 rows=2748 width=41) (actual time=1.501..7.747 rows=5353 loops=1) Recheck Cond: (((id)::text = 'h3cer597jb8abbcbitpghpomua'::text) OR (((rootid)::text = 'h3cer597jb8abbcbitpghpomua'::text) AND (deleteat = 0))) Heap Blocks: exact=5308 -> BitmapOr (cost=45.39..45.39 rows=2748 width=0) (actual time=0.797..0.798 rows=0 loops=1) -> Bitmap Index Scan on posts_pkey (cost=0.00..1.68 rows=1 width=0) (actual time=0.014..0.014 rows=1 loops=1) Index Cond: ((id)::text = 'h3cer597jb8abbcbitpghpomua'::text) -> Bitmap Index Scan on idx_posts_root_id_delete_at (cost=0.00..42.34 rows=2747 width=0) (actual time=0.782..0.782 rows=5352 loops=1) Index Cond: (((rootid)::text = 'h3cer597jb8abbcbitpghpomua'::text) AND (deleteat = 0)) Planning Time: 0.220 ms Execution Time: 10.052 ms (15 rows) ``` Observe the `loops=5353` in the first query, and `loops=1` in the next. https://mattermost.atlassian.net/browse/MM-55476 ```release-note Optimize createPost performance ``` Co-authored-by: Mattermost Build --- server/channels/store/sqlstore/post_store.go | 45 ++++++++++++++----- server/channels/store/storetest/post_store.go | 6 +++ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/server/channels/store/sqlstore/post_store.go b/server/channels/store/sqlstore/post_store.go index 3e702b4b0a2..1582c1cc75b 100644 --- a/server/channels/store/sqlstore/post_store.go +++ b/server/channels/store/sqlstore/post_store.go @@ -691,7 +691,6 @@ func (s *SqlPostStore) Get(ctx context.Context, id string, opts model.GetPostsOp if id == "" { return nil, store.NewErrInvalidInput("Post", "id", id) } - var post model.Post postFetchQuery := "SELECT p.*, (SELECT count(*) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE p.Id = ? AND p.DeleteAt = 0" err := s.DBXFromContext(ctx).Get(&post, postFetchQuery, id) @@ -715,14 +714,40 @@ func (s *SqlPostStore) Get(ctx context.Context, id string, opts model.GetPostsOp return nil, errors.Wrapf(err, "invalid rootId with value=%s", rootId) } - query := s.getQueryBuilder(). - Select("p.*, (SELECT count(*) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount"). - From("Posts p"). - Where(sq.Or{ - sq.Eq{"p.Id": rootId}, - sq.Eq{"p.RootId": rootId}, - }). - Where(sq.Eq{"p.DeleteAt": 0}) + var query sq.SelectBuilder + if s.DriverName() == model.DatabaseDriverMysql { + query = s.getQueryBuilder(). + Select("p.*, (SELECT count(*) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount"). + From("Posts p"). + Where(sq.And{ + sq.Or{ + sq.Eq{"p.Id": rootId}, + sq.Eq{"p.RootId": rootId}, + }, + sq.Eq{"p.DeleteAt": 0}, + }) + } else { + query = s.getQueryBuilder(). + Select("p.*, replycount.num as ReplyCount"). + PrefixExpr(s.getQueryBuilder(). + Select(). + Prefix("WITH replycount as ("). + Columns("count(*) as num"). + From("posts"). + Where(sq.And{ + sq.Eq{"RootId": rootId}, + sq.Eq{"DeleteAt": 0}, + }).Suffix(")"), + ). + From("Posts p, replycount"). + Where(sq.And{ + sq.Or{ + sq.Eq{"p.Id": rootId}, + sq.Eq{"p.RootId": rootId}, + }, + sq.Eq{"p.DeleteAt": 0}, + }) + } var sort string if opts.Direction != "" { @@ -815,7 +840,7 @@ func (s *SqlPostStore) GetSingle(id string, inclDeleted bool) (*model.Post, erro Where(sq.Eq{"p.Id": id}) replyCountSubQuery := s.getQueryBuilder(). - Select("COUNT(Posts.Id)"). + Select("COUNT(*)"). From("Posts"). Where(sq.Expr("Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0")) diff --git a/server/channels/store/storetest/post_store.go b/server/channels/store/storetest/post_store.go index 0dde75add5c..646dc0bbbdc 100644 --- a/server/channels/store/storetest/post_store.go +++ b/server/channels/store/storetest/post_store.go @@ -821,6 +821,8 @@ func testPostStoreGetForThread(t *testing.T, rctx request.CTX, ss store.Store) { } r1, err = ss.Post().Get(context.Background(), o1.Id, opts, o1.UserId, map[string]bool{}) require.NoError(t, err) + require.Equal(t, r1.Posts[r1.Order[0]].ReplyCount, int64(4)) + require.Equal(t, r1.Posts[r1.Order[1]].ReplyCount, int64(4)) require.Len(t, r1.Order, 2) // including the root post require.Len(t, r1.Posts, 2) assert.LessOrEqual(t, r1.Posts[r1.Order[1]].CreateAt, m1.CreateAt) @@ -852,6 +854,10 @@ func testPostStoreGetForThread(t *testing.T, rctx request.CTX, ss store.Store) { } r1, err = ss.Post().Get(context.Background(), o1.Id, opts, o1.UserId, map[string]bool{}) require.NoError(t, err) + require.Equal(t, r1.Posts[r1.Order[0]].ReplyCount, int64(4)) + require.Equal(t, r1.Posts[r1.Order[1]].ReplyCount, int64(4)) + require.Equal(t, r1.Posts[r1.Order[2]].ReplyCount, int64(4)) + require.Equal(t, r1.Posts[r1.Order[3]].ReplyCount, int64(4)) require.Len(t, r1.Order, 4) // including the root post require.Len(t, r1.Posts, 4) assert.GreaterOrEqual(t, r1.Posts[r1.Order[len(r1.Order)-1]].CreateAt, lastPostCreateAt)