Migrates Channel.IncrementMentionCount to sync by default (#11237)

* Migrates Channel.IncrementMentionCount to sync by default

* Calls to Channel.IncrementMentionCount are concurrent now

app/notification.go

@@ -65,7 +65,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	hereNotification := false
 	channelNotification := false
 	allNotification := false
-	updateMentionChans := []store.StoreChannel{}
+	updateMentionChans := []chan *model.AppError{}
 
 	if channel.Type == model.CHANNEL_DIRECT {
 		var otherUserId string
@@ -136,8 +136,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 		}
 
 		if len(m.OtherPotentialMentions) > 0 && !post.IsSystemMessage() {
-			if result := <-a.Srv.Store.User().GetProfilesByUsernames(m.OtherPotentialMentions, &model.ViewUsersRestrictions{Teams: []string{team.Id}}); result.Err == nil {
+			if profilesResult := <-a.Srv.Store.User().GetProfilesByUsernames(m.OtherPotentialMentions, &model.ViewUsersRestrictions{Teams: []string{team.Id}}); profilesResult.Err == nil {
-				channelMentions := model.UserSlice(result.Data.([]*model.User)).FilterByActive(true)
+				channelMentions := model.UserSlice(profilesResult.Data.([]*model.User)).FilterByActive(true)
 
 				var outOfChannelMentions model.UserSlice
 				var outOfGroupsMentions model.UserSlice
@@ -176,7 +176,12 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	mentionedUsersList := make([]string, 0, len(mentionedUserIds))
 	for id := range mentionedUserIds {
 		mentionedUsersList = append(mentionedUsersList, id)
-		updateMentionChans = append(updateMentionChans, a.Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id))
+		umc := make(chan *model.AppError, 1)
+		go func() {
+			umc <- a.Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id)
+			close(umc)
+		}()
+		updateMentionChans = append(updateMentionChans, umc)
 	}
 
 	notification := &postNotification{
@@ -275,8 +280,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	// Make sure all mention updates are complete to prevent race
 	// Probably better to batch these DB updates in the future
 	// MUST be completed before push notifications send
-	for _, uchan := range updateMentionChans {
+	for _, umc := range updateMentionChans {
-		if result := <-uchan; result.Err != nil {
+		if err := <-umc; err != nil {
 			mlog.Warn(fmt.Sprintf("Failed to update mention count, post_id=%v channel_id=%v err=%v", post.Id, post.ChannelId, result.Err), mlog.String("post_id", post.Id))
 		}
 	}

store/sqlstore/channel_store.go

@@ -1783,22 +1783,22 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string)
 	})
 }
 
-func (s SqlChannelStore) IncrementMentionCount(channelId string, userId string) store.StoreChannel {
+func (s SqlChannelStore) IncrementMentionCount(channelId string, userId string) *model.AppError {
-	return store.Do(func(result *store.StoreResult) {
+	_, err := s.GetMaster().Exec(
-		_, err := s.GetMaster().Exec(
+		`UPDATE
-			`UPDATE
+			ChannelMembers
-				ChannelMembers
+		SET
-			SET
+			MentionCount = MentionCount + 1,
-				MentionCount = MentionCount + 1,
+			LastUpdateAt = :LastUpdateAt
-				LastUpdateAt = :LastUpdateAt
+		WHERE
-			WHERE
+			UserId = :UserId
-				UserId = :UserId
+				AND ChannelId = :ChannelId`,
-					AND ChannelId = :ChannelId`,
+		map[string]interface{}{"ChannelId": channelId, "UserId": userId, "LastUpdateAt": model.GetMillis()})
-			map[string]interface{}{"ChannelId": channelId, "UserId": userId, "LastUpdateAt": model.GetMillis()})
+	if err != nil {
-		if err != nil {
+		return model.NewAppError("SqlChannelStore.IncrementMentionCount", "store.sql_channel.increment_mention_count.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
-			result.Err = model.NewAppError("SqlChannelStore.IncrementMentionCount", "store.sql_channel.increment_mention_count.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
+	}
-		}
+
-	})
+	return nil
 }
 
 func (s SqlChannelStore) GetAll(teamId string) ([]*model.Channel, *model.AppError) {

store/store.go

@@ -176,7 +176,7 @@ type ChannelStore interface {
 	PermanentDeleteMembersByUser(userId string) StoreChannel
 	PermanentDeleteMembersByChannel(channelId string) *model.AppError
 	UpdateLastViewedAt(channelIds []string, userId string) StoreChannel
-	IncrementMentionCount(channelId string, userId string) StoreChannel
+	IncrementMentionCount(channelId string, userId string) *model.AppError
 	AnalyticsTypeCount(teamId string, channelType string) (int64, *model.AppError)
 	GetMembersForUser(teamId string, userId string) StoreChannel
 	GetMembersForUserWithPagination(teamId, userId string, page, perPage int) StoreChannel

store/storetest/channel_store.go

@@ -1722,22 +1722,22 @@ func testChannelStoreIncrementMentionCount(t *testing.T, ss store.Store) {
 	m1.NotifyProps = model.GetDefaultChannelNotifyProps()
 	store.Must(ss.Channel().SaveMember(&m1))
 
-	err = (<-ss.Channel().IncrementMentionCount(m1.ChannelId, m1.UserId)).Err
+	err = ss.Channel().IncrementMentionCount(m1.ChannelId, m1.UserId)
 	if err != nil {
 		t.Fatal("failed to update")
 	}
 
-	err = (<-ss.Channel().IncrementMentionCount(m1.ChannelId, "missing id")).Err
+	err = ss.Channel().IncrementMentionCount(m1.ChannelId, "missing id")
 	if err != nil {
 		t.Fatal("failed to update")
 	}
 
-	err = (<-ss.Channel().IncrementMentionCount("missing id", m1.UserId)).Err
+	err = ss.Channel().IncrementMentionCount("missing id", m1.UserId)
 	if err != nil {
 		t.Fatal("failed to update")
 	}
 
-	err = (<-ss.Channel().IncrementMentionCount("missing id", "missing id")).Err
+	err = ss.Channel().IncrementMentionCount("missing id", "missing id")
 	if err != nil {
 		t.Fatal("failed to update")
 	}

store/storetest/mocks/ChannelStore.go

@@ -980,15 +980,15 @@ func (_m *ChannelStore) GetTeamChannels(teamId string) (*model.ChannelList, *mod
 }
 
 // IncrementMentionCount provides a mock function with given fields: channelId, userId
-func (_m *ChannelStore) IncrementMentionCount(channelId string, userId string) store.StoreChannel {
+func (_m *ChannelStore) IncrementMentionCount(channelId string, userId string) *model.AppError {
 	ret := _m.Called(channelId, userId)
 
-	var r0 store.StoreChannel
+	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok {
 		r0 = rf(channelId, userId)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.AppError)
 		}
 	}
 

store/storetest/user_store.go

@@ -1851,7 +1851,8 @@ func testUserUnreadCount(t *testing.T, ss store.Store) {
 	// Post one message with mention to open channel
 	_, err := ss.Post().Save(&p1)
 	require.Nil(t, err)
-	store.Must(ss.Channel().IncrementMentionCount(c1.Id, u2.Id))
+	err = ss.Channel().IncrementMentionCount(c1.Id, u2.Id)
+	require.Nil(t, err)
 
 	// Post 2 messages without mention to direct channel
 	p2 := model.Post{}
@@ -1861,7 +1862,8 @@ func testUserUnreadCount(t *testing.T, ss store.Store) {
 
 	_, err = ss.Post().Save(&p2)
 	require.Nil(t, err)
-	store.Must(ss.Channel().IncrementMentionCount(c2.Id, u2.Id))
+	err = ss.Channel().IncrementMentionCount(c2.Id, u2.Id)
+	require.Nil(t, err)
 
 	p3 := model.Post{}
 	p3.ChannelId = c2.Id
@@ -1870,7 +1872,8 @@ func testUserUnreadCount(t *testing.T, ss store.Store) {
 	_, err = ss.Post().Save(&p3)
 	require.Nil(t, err)
 
-	store.Must(ss.Channel().IncrementMentionCount(c2.Id, u2.Id))
+	err = ss.Channel().IncrementMentionCount(c2.Id, u2.Id)
+	require.Nil(t, err)
 
 	badge := (<-ss.User().GetUnreadCount(u2.Id)).Data.(int64)
 	if badge != 3 {