From e101e1c02039c5ceea5f207af51ed819539c654c Mon Sep 17 00:00:00 2001 From: Taufiq Rahman Date: Fri, 14 Jun 2019 19:22:47 +0600 Subject: [PATCH] Migrate "Post.GetPostsSince" to Sync by default #10976 (#11129) * Migrate "Post.GetPostsSince" to Sync by default #10976 * Update GetPostsSince to Sync by default #10976 * Update GetPostsSince to Sync by default #10976 * Update GetPostsSince to Sync by default #10976 --- app/post.go | 6 +- store/sqlstore/post_store.go | 96 ++++++++++++++---------------- store/store.go | 2 +- store/storetest/mocks/PostStore.go | 19 ++++-- store/storetest/post_store.go | 8 +-- 5 files changed, 64 insertions(+), 67 deletions(-) diff --git a/app/post.go b/app/post.go index f727a1a524..b4d65491c3 100644 --- a/app/post.go +++ b/app/post.go @@ -615,11 +615,7 @@ func (a *App) GetPostsEtag(channelId string) string { } func (a *App) GetPostsSince(channelId string, time int64) (*model.PostList, *model.AppError) { - result := <-a.Srv.Store.Post().GetPostsSince(channelId, time, true) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.PostList), nil + return a.Srv.Store.Post().GetPostsSince(channelId, time, true) } func (a *App) GetSinglePost(postId string) (*model.Post, *model.AppError) { diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index c7bf1dbf21..a206f9e5bc 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -501,40 +501,34 @@ func (s *SqlPostStore) GetPosts(channelId string, offset int, limit int, allowFr return list, err } -func (s *SqlPostStore) GetPostsSince(channelId string, time int64, allowFromCache bool) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if allowFromCache { - // If the last post in the channel's time is less than or equal to the time we are getting posts since, - // we can safely return no posts. - if cacheItem, ok := s.lastPostTimeCache.Get(channelId); ok && cacheItem.(int64) <= time { - if s.metrics != nil { - s.metrics.IncrementMemCacheHitCounter("Last Post Time") - } - list := model.NewPostList() - result.Data = list - return - } else { - if s.metrics != nil { - s.metrics.IncrementMemCacheMissCounter("Last Post Time") - } - } - } else { +func (s *SqlPostStore) GetPostsSince(channelId string, time int64, allowFromCache bool) (*model.PostList, *model.AppError) { + if allowFromCache { + // If the last post in the channel's time is less than or equal to the time we are getting posts since, + // we can safely return no posts. + if cacheItem, ok := s.lastPostTimeCache.Get(channelId); ok && cacheItem.(int64) <= time { if s.metrics != nil { - s.metrics.IncrementMemCacheMissCounter("Last Post Time") + s.metrics.IncrementMemCacheHitCounter("Last Post Time") } + list := model.NewPostList() + return list, nil } + } - var posts []*model.Post - _, err := s.GetReplica().Select(&posts, - `(SELECT - * - FROM - Posts - WHERE - (UpdateAt > :Time - AND ChannelId = :ChannelId) + if s.metrics != nil { + s.metrics.IncrementMemCacheMissCounter("Last Post Time") + } + + var posts []*model.Post + _, err := s.GetReplica().Select(&posts, + `(SELECT + * + FROM + Posts + WHERE + (UpdateAt > :Time + AND ChannelId = :ChannelId) LIMIT 1000) - UNION + UNION (SELECT * FROM @@ -548,34 +542,32 @@ func (s *SqlPostStore) GetPostsSince(channelId string, time int64, allowFromCach Posts WHERE UpdateAt > :Time - AND ChannelId = :ChannelId - LIMIT 1000) temp_tab)) - ORDER BY CreateAt DESC`, - map[string]interface{}{"ChannelId": channelId, "Time": time}) + AND ChannelId = :ChannelId + LIMIT 1000) temp_tab)) + ORDER BY CreateAt DESC`, + map[string]interface{}{"ChannelId": channelId, "Time": time}) - if err != nil { - result.Err = model.NewAppError("SqlPostStore.GetPostsSince", "store.sql_post.get_posts_since.app_error", nil, "channelId="+channelId+err.Error(), http.StatusInternalServerError) - } else { + if err != nil { + return nil, model.NewAppError("SqlPostStore.GetPostsSince", "store.sql_post.get_posts_since.app_error", nil, "channelId="+channelId+err.Error(), http.StatusInternalServerError) + } - list := model.NewPostList() + list := model.NewPostList() - var latestUpdate int64 = 0 + var latestUpdate int64 = 0 - for _, p := range posts { - list.AddPost(p) - if p.UpdateAt > time { - list.AddOrder(p.Id) - } - if latestUpdate < p.UpdateAt { - latestUpdate = p.UpdateAt - } - } - - s.lastPostTimeCache.AddWithExpiresInSecs(channelId, latestUpdate, LAST_POST_TIME_CACHE_SEC) - - result.Data = list + for _, p := range posts { + list.AddPost(p) + if p.UpdateAt > time { + list.AddOrder(p.Id) } - }) + if latestUpdate < p.UpdateAt { + latestUpdate = p.UpdateAt + } + } + + s.lastPostTimeCache.AddWithExpiresInSecs(channelId, latestUpdate, LAST_POST_TIME_CACHE_SEC) + + return list, nil } func (s *SqlPostStore) GetPostsBefore(channelId string, postId string, limit int, offset int) (*model.PostList, *model.AppError) { diff --git a/store/store.go b/store/store.go index efb0edea53..8b7caaaf04 100644 --- a/store/store.go +++ b/store/store.go @@ -223,7 +223,7 @@ type PostStore interface { GetFlaggedPostsForChannel(userId, channelId string, offset int, limit int) (*model.PostList, *model.AppError) GetPostsBefore(channelId string, postId string, numPosts int, offset int) (*model.PostList, *model.AppError) GetPostsAfter(channelId string, postId string, numPosts int, offset int) (*model.PostList, *model.AppError) - GetPostsSince(channelId string, time int64, allowFromCache bool) StoreChannel + GetPostsSince(channelId string, time int64, allowFromCache bool) (*model.PostList, *model.AppError) GetEtag(channelId string, allowFromCache bool) string Search(teamId string, userId string, params *model.SearchParams) StoreChannel AnalyticsUserCountsWithPostsByDay(teamId string) (model.AnalyticsRows, *model.AppError) diff --git a/store/storetest/mocks/PostStore.go b/store/storetest/mocks/PostStore.go index 4368982240..6d77dbcaf5 100644 --- a/store/storetest/mocks/PostStore.go +++ b/store/storetest/mocks/PostStore.go @@ -454,19 +454,28 @@ func (_m *PostStore) GetPostsCreatedAt(channelId string, time int64) ([]*model.P } // GetPostsSince provides a mock function with given fields: channelId, time, allowFromCache -func (_m *PostStore) GetPostsSince(channelId string, time int64, allowFromCache bool) store.StoreChannel { +func (_m *PostStore) GetPostsSince(channelId string, time int64, allowFromCache bool) (*model.PostList, *model.AppError) { ret := _m.Called(channelId, time, allowFromCache) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, int64, bool) store.StoreChannel); ok { + var r0 *model.PostList + if rf, ok := ret.Get(0).(func(string, int64, bool) *model.PostList); ok { r0 = rf(channelId, time, allowFromCache) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.PostList) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, int64, bool) *model.AppError); ok { + r1 = rf(channelId, time, allowFromCache) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetRepliesForExport provides a mock function with given fields: parentId diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 8041a6106b..de1ba034e7 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -994,7 +994,7 @@ func testPostStoreGetPostsSince(t *testing.T, ss store.Store) { o5, err = ss.Post().Save(o5) require.Nil(t, err) - r1 := (<-ss.Post().GetPostsSince(o1.ChannelId, o1.CreateAt, false)).Data.(*model.PostList) + r1, _ := ss.Post().GetPostsSince(o1.ChannelId, o1.CreateAt, false) if r1.Order[0] != o5.Id { t.Fatal("invalid order") @@ -1020,7 +1020,7 @@ func testPostStoreGetPostsSince(t *testing.T, ss store.Store) { t.Fatal("Missing parent") } - r2 := (<-ss.Post().GetPostsSince(o1.ChannelId, o5.UpdateAt, true)).Data.(*model.PostList) + r2, _ := ss.Post().GetPostsSince(o1.ChannelId, o5.UpdateAt, true) if len(r2.Order) != 0 { t.Fatal("wrong size ", len(r2.Posts)) @@ -1134,7 +1134,7 @@ func testPostStoreSearch(t *testing.T, ss store.Store) { tt := []struct { name string searchParams *model.SearchParams - extectedResultsCount int + expectedResultsCount int expectedMessageResultIds []string }{ { @@ -1231,7 +1231,7 @@ func testPostStoreSearch(t *testing.T, ss store.Store) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { result := (<-ss.Post().Search(teamId, userId, tc.searchParams)).Data.(*model.PostList) - require.Len(t, result.Order, tc.extectedResultsCount) + require.Len(t, result.Order, tc.expectedResultsCount) for _, expectedMessageResultId := range tc.expectedMessageResultIds { assert.Contains(t, result.Order, expectedMessageResultId) }