upstream/mattermost
1
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-05-28 04:35:04 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

MM-55476: Optimize PostStore.Get (#25448)

Browse source 
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 <build@mattermost.com>
This commit is contained in:
Agniva De Sarker 2023-12-14 14:09:29 +05:30 committed by GitHub
parent e644f4e4bb
commit 9ae451a121
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 41 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
server/channels/store/sqlstore/post_store.go
View file

@ -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"))

6
server/channels/store/storetest/post_store.go
View file

@ -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)