mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

MM-35396 Refactor GetThreadForUser store func to take membership as argument to prevent replica lag issues and reduce joins in query (#17754)

Просмотр исходного кода 
* Update store function GetThreadForUser to use master DB to fix replica lag

* Refactor GetThreadForUser store func to take membership as argument to prevent replica lag issues and reduce joins in query

* Add translation

* Fix test

* Updates per feedback

* Minor clean-up per feedback
Этот коммит содержится в:
Joram Wilander
2021-06-14 12:33:08 -04:00
коммит произвёл GitHub
родитель 24fb0033f4
Коммит d0778486ad
16 изменённых файлов: 119 добавлений и 46 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

4
api4/post_test.go
Просмотреть файл

@@ -2621,7 +2621,9 @@ func TestSetPostUnreadWithoutCollapsedThreads(t *testing.T) {
// MentionCountRoot should be zero so that supported clients don't show the channel as unread
require.Equal(t, channelUnread.MsgCountRoot, int64(0))
thread, err := th.App.GetThreadForUser(th.BasicUser.Id, th.BasicTeam.Id, rootPost1.Id, false)
threadMembership, err := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost1.Id)
require.Nil(t, err)
thread, err := th.App.GetThreadForUser(th.BasicTeam.Id, threadMembership, false)
require.Nil(t, err)
require.Equal(t, int64(2), thread.UnreadMentions)
require.Equal(t, int64(3), thread.UnreadReplies)

12
api4/user.go
Просмотреть файл

@@ -2871,15 +2871,21 @@ func getThreadForUser(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
extendedStr := r.URL.Query().Get("extended")
extended, _ := strconv.ParseBool(extendedStr)
threads, err := c.App.GetThreadForUser(c.Params.UserId, c.Params.TeamId, c.Params.ThreadId, extended)
threadMembership, err := c.App.GetThreadMembershipForUser(c.Params.UserId, c.Params.ThreadId)
if err != nil {
c.Err = err
return
}
w.Write([]byte(threads.ToJson()))
thread, err := c.App.GetThreadForUser(c.Params.TeamId, threadMembership, extended)
if err != nil {
c.Err = err
return
}
w.Write([]byte(thread.ToJson()))
}
func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) {

3
app/app_iface.go
Просмотреть файл

@@ -759,7 +759,8 @@ type AppIface interface {
GetTeamsForUser(userID string) ([]*model.Team, *model.AppError)
GetTeamsUnreadForUser(excludeTeamId string, userID string) ([]*model.TeamUnread, *model.AppError)
GetTermsOfService(id string) (*model.TermsOfService, *model.AppError)
GetThreadForUser(userID, teamID, threadId string, extended bool) (*model.ThreadResponse, *model.AppError)
GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, *model.AppError)
GetThreadMembershipForUser(userId, threadId string) (*model.ThreadMembership, *model.AppError)
GetThreadMembershipsForUser(userID, teamID string) ([]*model.ThreadMembership, error)
GetThreadsForUser(userID, teamID string, options model.GetUserThreadsOpts) (*model.Threads, *model.AppError)
GetUploadSession(uploadId string) (*model.UploadSession, *model.AppError)

6
app/channel.go
Просмотреть файл

@@ -2500,7 +2500,7 @@ func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string, collapse
if nErr != nil {
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, nErr.Error(), http.StatusInternalServerError)
}
thread, _ := a.Srv().Store.Thread().GetThreadForUser(userID, channel.TeamId, threadId, true)
thread, _ := a.Srv().Store.Thread().GetThreadForUser(channel.TeamId, threadMembership, true)
a.sanitizeProfiles(thread.Participants, false)
thread.Post.SanitizeProps()
@@ -2598,11 +2598,11 @@ func (a *App) markChannelAsUnreadFromPostCRTUnsupported(postID string, userID st
if err != nil {
return nil, err
}
_, nErr = a.Srv().Store.Thread().UpdateMembership(threadMembership)
threadMembership, nErr = a.Srv().Store.Thread().UpdateMembership(threadMembership)
if nErr != nil {
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, nErr.Error(), http.StatusInternalServerError)
}
thread, nErr := a.Srv().Store.Thread().GetThreadForUser(userID, channel.TeamId, threadId, true)
thread, nErr := a.Srv().Store.Thread().GetThreadForUser(channel.TeamId, threadMembership, true)
if nErr != nil {
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, nErr.Error(), http.StatusInternalServerError)
}

6
app/notification.go
Просмотреть файл

@@ -464,7 +464,11 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
}
if sendEvent {
message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_THREAD_UPDATED, team.Id, "", uid, nil)
userThread, err := a.Srv().Store.Thread().GetThreadForUser(uid, channel.TeamId, post.RootId, true)
threadMembership, err := a.Srv().Store.Thread().GetMembershipForUser(uid, post.RootId)
if err != nil {
return nil, errors.Wrapf(err, "cannot get thread membership %q for user %q", post.RootId, uid)
}
userThread, err := a.Srv().Store.Thread().GetThreadForUser(channel.TeamId, threadMembership, true)
if err != nil {
return nil, errors.Wrapf(err, "cannot get thread %q for user %q", post.RootId, uid)
}

26
app/opentracing/opentracing_layer.go
Просмотреть файл

@@ -9294,7 +9294,7 @@ func (a *OpenTracingAppLayer) GetTermsOfService(id string) (*model.TermsOfServic
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetThreadForUser(userID string, teamID string, threadId string, extended bool) (*model.ThreadResponse, *model.AppError) {
func (a *OpenTracingAppLayer) GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetThreadForUser")
@@ -9306,7 +9306,29 @@ func (a *OpenTracingAppLayer) GetThreadForUser(userID string, teamID string, thr
}()
defer span.Finish()
resultVar0, resultVar1 := a.app.GetThreadForUser(userID, teamID, threadId, extended)
resultVar0, resultVar1 := a.app.GetThreadForUser(teamID, threadMembership, extended)
if resultVar1 != nil {
span.LogFields(spanlog.Error(resultVar1))
ext.Error.Set(span, true)
}
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetThreadMembershipForUser(userId string, threadId string) (*model.ThreadMembership, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetThreadMembershipForUser")
a.ctx = newCtx
a.app.Srv().Store.SetContext(newCtx)
defer func() {
a.app.Srv().Store.SetContext(origCtx)
a.ctx = origCtx
}()
defer span.Finish()
resultVar0, resultVar1 := a.app.GetThreadMembershipForUser(userId, threadId)
if resultVar1 != nil {
span.LogFields(spanlog.Error(resultVar1))

12
app/post_test.go
Просмотреть файл

@@ -1952,21 +1952,27 @@ func TestFollowThreadSkipsParticipants(t *testing.T) {
_, err = th.App.CreatePost(th.Context, &model.Post{RootId: p1.Id, UserId: user.Id, ChannelId: channel.Id, Message: "Hola"}, channel, false, false)
require.Nil(t, err)
thread, err := th.App.GetThreadForUser(user.Id, th.BasicTeam.Id, p1.Id, false)
threadMembership, err := th.App.GetThreadMembershipForUser(user.Id, p1.Id)
require.Nil(t, err)
thread, err := th.App.GetThreadForUser(th.BasicTeam.Id, threadMembership, false)
require.Nil(t, err)
require.Len(t, thread.Participants, 1) // length should be 1, the original poster, since sysadmin was just mentioned but didn't post
_, err = th.App.CreatePost(th.Context, &model.Post{RootId: p1.Id, UserId: sysadmin.Id, ChannelId: channel.Id, Message: "sysadmin reply"}, channel, false, false)
require.Nil(t, err)
thread, err = th.App.GetThreadForUser(user.Id, th.BasicTeam.Id, p1.Id, false)
threadMembership, err = th.App.GetThreadMembershipForUser(user.Id, p1.Id)
require.Nil(t, err)
thread, err = th.App.GetThreadForUser(th.BasicTeam.Id, threadMembership, false)
require.Nil(t, err)
require.Len(t, thread.Participants, 2) // length should be 2, the original poster and sysadmin, since sysadmin participated now
// another user follows the thread
th.App.UpdateThreadFollowForUser(user2.Id, th.BasicTeam.Id, p1.Id, true)
thread, err = th.App.GetThreadForUser(user2.Id, th.BasicTeam.Id, p1.Id, false)
threadMembership, err = th.App.GetThreadMembershipForUser(user2.Id, p1.Id)
require.Nil(t, err)
thread, err = th.App.GetThreadForUser(th.BasicTeam.Id, threadMembership, false)
require.Nil(t, err)
require.Len(t, thread.Participants, 2) // length should be 2, since follow shouldn't update participant list, only user1 and sysadmin are participants
for _, p := range thread.Participants {

19
app/user.go
Просмотреть файл

@@ -2285,8 +2285,19 @@ func (a *App) GetThreadsForUser(userID, teamID string, options model.GetUserThre
return threads, nil
}
func (a *App) GetThreadForUser(userID, teamID, threadId string, extended bool) (*model.ThreadResponse, *model.AppError) {
thread, err := a.Srv().Store.Thread().GetThreadForUser(userID, teamID, threadId, extended)
func (a *App) GetThreadMembershipForUser(userId, threadId string) (*model.ThreadMembership, *model.AppError) {
threadMembership, err := a.Srv().Store.Thread().GetMembershipForUser(userId, threadId)
if err != nil {
return nil, model.NewAppError("GetThreadMembershipForUser", "app.user.get_thread_membership_for_user.app_error", nil, err.Error(), http.StatusInternalServerError)
}
if threadMembership == nil {
return nil, model.NewAppError("GetThreadMembershipForUser", "app.user.get_thread_membership_for_user.not_found", nil, "thread membership not found/followed", http.StatusNotFound)
}
return threadMembership, nil
}
func (a *App) GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, *model.AppError) {
thread, err := a.Srv().Store.Thread().GetThreadForUser(teamID, threadMembership, extended)
if err != nil {
return nil, model.NewAppError("GetThreadForUser", "app.user.get_threads_for_user.app_error", nil, err.Error(), http.StatusInternalServerError)
}
@@ -2349,14 +2360,16 @@ func (a *App) UpdateThreadReadForUser(userID, teamID, threadID string, timestamp
return nil, model.NewAppError("UpdateThreadsReadForUser", "app.user.update_threads_read_for_user.app_error", nil, nErr.Error(), http.StatusInternalServerError)
}
membership.LastViewed = timestamp
nErr = a.Srv().Store.Thread().MarkAsRead(userID, threadID, timestamp)
if nErr != nil {
return nil, model.NewAppError("UpdateThreadReadForUser", "app.user.update_thread_read_for_user.app_error", nil, nErr.Error(), http.StatusInternalServerError)
}
thread, err := a.GetThreadForUser(userID, teamID, threadID, false)
thread, err := a.GetThreadForUser(teamID, membership, false)
if err != nil {
return nil, err
}
message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_THREAD_READ_CHANGED, teamID, "", userID, nil)
message.Add("thread_id", threadID)
message.Add("timestamp", timestamp)

8
i18n/en.json
Просмотреть файл

@@ -6354,6 +6354,14 @@
"id": "app.user.get_recently_active_users.app_error",
"translation": "We encountered an error while finding the recently active users."
},
{
"id": "app.user.get_thread_membership_for_user.app_error",
"translation": "Unable to get user thread membership"
},
{
"id": "app.user.get_thread_membership_for_user.not_found",
"translation": "User thread membership doesn't exist"
},
{
"id": "app.user.get_threads_for_user.app_error",
"translation": "Unable to get user threads"

4
store/opentracinglayer/opentracinglayer.go
Просмотреть файл

@@ -8866,7 +8866,7 @@ func (s *OpenTracingLayerThreadStore) GetThreadFollowers(threadID string) ([]str
return result, err
}
func (s *OpenTracingLayerThreadStore) GetThreadForUser(userID string, teamID string, threadId string, extended bool) (*model.ThreadResponse, error) {
func (s *OpenTracingLayerThreadStore) GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ThreadStore.GetThreadForUser")
s.Root.Store.SetContext(newCtx)
@@ -8875,7 +8875,7 @@ func (s *OpenTracingLayerThreadStore) GetThreadForUser(userID string, teamID str
}()
defer span.Finish()
result, err := s.ThreadStore.GetThreadForUser(userID, teamID, threadId, extended)
result, err := s.ThreadStore.GetThreadForUser(teamID, threadMembership, extended)
if err != nil {
span.LogFields(spanlog.Error(err))
ext.Error.Set(span, true)

4
store/retrylayer/retrylayer.go
Просмотреть файл

@@ -9648,11 +9648,11 @@ func (s *RetryLayerThreadStore) GetThreadFollowers(threadID string) ([]string, e
}
func (s *RetryLayerThreadStore) GetThreadForUser(userID string, teamID string, threadId string, extended bool) (*model.ThreadResponse, error) {
func (s *RetryLayerThreadStore) GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, error) {
tries := 0
for {
result, err := s.ThreadStore.GetThreadForUser(userID, teamID, threadId, extended)
result, err := s.ThreadStore.GetThreadForUser(teamID, threadMembership, extended)
if err == nil {
return result, nil
}

35
store/sqlstore/thread_store.go
Просмотреть файл

@@ -321,7 +321,11 @@ func (s *SqlThreadStore) GetThreadFollowers(threadID string) ([]string, error) {
return users, nil
}
func (s *SqlThreadStore) GetThreadForUser(userId, teamId, threadId string, extended bool) (*model.ThreadResponse, error) {
func (s *SqlThreadStore) GetThreadForUser(teamId string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, error) {
if !threadMembership.Following {
return nil, nil // in case the thread is not followed anymore - return nil error to be interpreted as 404
}
type JoinedThread struct {
PostId string
Following bool
@@ -334,38 +338,45 @@ func (s *SqlThreadStore) GetThreadForUser(userId, teamId, threadId string, exten
model.Post
}
unreadRepliesQuery := "SELECT COUNT(Posts.Id) From Posts Where Posts.RootId=ThreadMemberships.PostId AND Posts.CreateAt >= ThreadMemberships.LastViewed AND Posts.DeleteAt=0"
unreadRepliesQuery, unreadRepliesArgs := sq.
Select("COUNT(Posts.Id)").
From("Posts").
Where(sq.And{
sq.Eq{"Posts.RootId": threadMembership.PostId},
sq.GtOrEq{"Posts.CreateAt": threadMembership.LastViewed},
sq.Eq{"Posts.DeleteAt": 0},
}).MustSql()
fetchConditions := sq.And{
sq.Or{sq.Eq{"Channels.TeamId": teamId}, sq.Eq{"Channels.TeamId": ""}},
sq.Eq{"ThreadMemberships.UserId": userId},
sq.Eq{"Threads.PostId": threadId},
sq.Eq{"Threads.PostId": threadMembership.PostId},
}
var thread JoinedThread
query, args, _ := s.getQueryBuilder().
Select("Threads.*, Posts.*, ThreadMemberships.LastViewed as LastViewedAt, ThreadMemberships.UnreadMentions as UnreadMentions, ThreadMemberships.Following").
query, threadArgs, _ := s.getQueryBuilder().
Select("Threads.*, Posts.*").
From("Threads").
Column(sq.Alias(sq.Expr(unreadRepliesQuery), "UnreadReplies")).
LeftJoin("Posts ON Posts.Id = Threads.PostId").
LeftJoin("Channels ON Posts.ChannelId = Channels.Id").
LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Threads.PostId").
Where(fetchConditions).ToSql()
err := s.GetReplica().SelectOne(&thread, query, args...)
args := append(unreadRepliesArgs, threadArgs...)
err := s.GetReplica().SelectOne(&thread, query, args...)
if err != nil {
return nil, err
}
if !thread.Following {
return nil, nil // in case the thread is not followed anymore - return nil error to be interpreted as 404
}
thread.LastViewedAt = threadMembership.LastViewed
thread.UnreadMentions = threadMembership.UnreadMentions
var users []*model.User
if extended {
var err error
users, err = s.User().GetProfileByIds(context.Background(), thread.Participants, &store.UserGetByIdsOpts{}, true)
if err != nil {
return nil, errors.Wrapf(err, "failed to get threads for user id=%s", userId)
return nil, errors.Wrapf(err, "failed to get thread for user id=%s", threadMembership.UserId)
}
} else {
for _, userId := range thread.Participants {

2
store/store.go
Просмотреть файл

@@ -283,7 +283,7 @@ type ThreadStore interface {
Update(thread *model.Thread) (*model.Thread, error)
Get(id string) (*model.Thread, error)
GetThreadsForUser(userId, teamID string, opts model.GetUserThreadsOpts) (*model.Threads, error)
GetThreadForUser(userID, teamID, threadId string, extended bool) (*model.ThreadResponse, error)
GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, error)
Delete(postID string) error
GetPosts(threadID string, since int64) ([]*model.Post, error)

14
store/storetest/mocks/ThreadStore.go
Просмотреть файл

@@ -180,13 +180,13 @@ func (_m *ThreadStore) GetThreadFollowers(threadID string) ([]string, error) {
return r0, r1
}
// GetThreadForUser provides a mock function with given fields: userID, teamID, threadId, extended
func (_m *ThreadStore) GetThreadForUser(userID string, teamID string, threadId string, extended bool) (*model.ThreadResponse, error) {
ret := _m.Called(userID, teamID, threadId, extended)
// GetThreadForUser provides a mock function with given fields: teamID, threadMembership, extended
func (_m *ThreadStore) GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, error) {
ret := _m.Called(teamID, threadMembership, extended)
var r0 *model.ThreadResponse
if rf, ok := ret.Get(0).(func(string, string, string, bool) *model.ThreadResponse); ok {
r0 = rf(userID, teamID, threadId, extended)
if rf, ok := ret.Get(0).(func(string, *model.ThreadMembership, bool) *model.ThreadResponse); ok {
r0 = rf(teamID, threadMembership, extended)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.ThreadResponse)
@@ -194,8 +194,8 @@ func (_m *ThreadStore) GetThreadForUser(userID string, teamID string, threadId s
}
var r1 error
if rf, ok := ret.Get(1).(func(string, string, string, bool) error); ok {
r1 = rf(userID, teamID, threadId, extended)
if rf, ok := ret.Get(1).(func(string, *model.ThreadMembership, bool) error); ok {
r1 = rf(teamID, threadMembership, extended)
} else {
r1 = ret.Error(1)
}

6
store/storetest/thread_store.go
Просмотреть файл

@@ -339,14 +339,14 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
newPosts := makeSomePosts()
m, err := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false, false)
require.NoError(t, err)
th, err := ss.Thread().GetThreadForUser(newPosts[0].UserId, "", newPosts[0].Id, false)
th, err := ss.Thread().GetThreadForUser("", m, false)
require.NoError(t, err)
require.Equal(t, int64(2), th.UnreadReplies)
m.LastViewed = newPosts[2].UpdateAt + 1
_, err = ss.Thread().UpdateMembership(m)
require.NoError(t, err)
th, err = ss.Thread().GetThreadForUser(newPosts[0].UserId, "", newPosts[0].Id, false)
th, err = ss.Thread().GetThreadForUser("", m, false)
require.NoError(t, err)
require.Equal(t, int64(0), th.UnreadReplies)
@@ -355,7 +355,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
_, err = ss.Post().Update(editedPost, newPosts[2])
require.NoError(t, err)
th, err = ss.Thread().GetThreadForUser(newPosts[0].UserId, "", newPosts[0].Id, false)
th, err = ss.Thread().GetThreadForUser("", m, false)
require.NoError(t, err)
require.Equal(t, int64(0), th.UnreadReplies)
})

4
store/timerlayer/timerlayer.go
Просмотреть файл

@@ -7990,10 +7990,10 @@ func (s *TimerLayerThreadStore) GetThreadFollowers(threadID string) ([]string, e
return result, err
}
func (s *TimerLayerThreadStore) GetThreadForUser(userID string, teamID string, threadId string, extended bool) (*model.ThreadResponse, error) {
func (s *TimerLayerThreadStore) GetThreadForUser(teamID string, threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, error) {
start := timemodule.Now()
result, err := s.ThreadStore.GetThreadForUser(userID, teamID, threadId, extended)
result, err := s.ThreadStore.GetThreadForUser(teamID, threadMembership, extended)
elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
if s.Root.Metrics != nil {