From ba028ed74b69bd1dd902344663fbf8ba4f1dfb87 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Fri, 24 Feb 2017 09:15:36 -0500 Subject: Adding caching to get channel member (#5518) --- api/apitestlib.go | 8 ++++++-- app/channel.go | 11 +++++++---- app/team.go | 1 + app/web_hub.go | 12 ++++++++++++ einterfaces/cluster.go | 1 + store/sql_channel_store.go | 34 +++++++++++++++++++++++++++++++++- store/sql_channel_store_test.go | 10 +++++----- store/store.go | 3 ++- 8 files changed, 67 insertions(+), 13 deletions(-) diff --git a/api/apitestlib.go b/api/apitestlib.go index 475469a36..6c4952b3f 100644 --- a/api/apitestlib.go +++ b/api/apitestlib.go @@ -194,13 +194,15 @@ func UpdateUserToNonTeamAdmin(user *model.User, team *model.Team) { func MakeUserChannelAdmin(user *model.User, channel *model.Channel) { utils.DisableDebugLogForTest() - if cmr := <-app.Srv.Store.Channel().GetMember(channel.Id, user.Id); cmr.Err == nil { + if cmr := <-app.Srv.Store.Channel().GetMember(channel.Id, user.Id, true); cmr.Err == nil { cm := cmr.Data.(*model.ChannelMember) cm.Roles = "channel_admin channel_user" if sr := <-app.Srv.Store.Channel().UpdateMember(cm); sr.Err != nil { utils.EnableDebugLogForTest() panic(sr.Err) } + + app.InvalidateCacheForChannelMember(cm.ChannelId, cm.UserId) } else { utils.EnableDebugLogForTest() panic(cmr.Err) @@ -212,13 +214,15 @@ func MakeUserChannelAdmin(user *model.User, channel *model.Channel) { func MakeUserChannelUser(user *model.User, channel *model.Channel) { utils.DisableDebugLogForTest() - if cmr := <-app.Srv.Store.Channel().GetMember(channel.Id, user.Id); cmr.Err == nil { + if cmr := <-app.Srv.Store.Channel().GetMember(channel.Id, user.Id, true); cmr.Err == nil { cm := cmr.Data.(*model.ChannelMember) cm.Roles = "channel_user" if sr := <-app.Srv.Store.Channel().UpdateMember(cm); sr.Err != nil { utils.EnableDebugLogForTest() panic(sr.Err) } + + app.InvalidateCacheForChannelMember(cm.ChannelId, cm.UserId) } else { utils.EnableDebugLogForTest() panic(cmr.Err) diff --git a/app/channel.go b/app/channel.go index fb4198c37..cff9564b5 100644 --- a/app/channel.go +++ b/app/channel.go @@ -175,7 +175,7 @@ func WaitForChannelMembership(channelId string, userId string) { time.Sleep(100 * time.Millisecond) - result := <-Srv.Store.Channel().GetMember(channelId, userId) + result := <-Srv.Store.Channel().GetMember(channelId, userId, true) // If the membership was found then return if result.Err == nil { @@ -214,6 +214,7 @@ func UpdateChannelMemberRoles(channelId string, userId string, newRoles string) return nil, result.Err } + InvalidateCacheForChannelMember(channelId, userId) InvalidateCacheForUser(userId) return member, nil } @@ -245,6 +246,7 @@ func UpdateChannelMemberNotifyProps(data map[string]string, channelId string, us if result := <-Srv.Store.Channel().UpdateMember(member); result.Err != nil { return nil, result.Err } else { + InvalidateCacheForChannelMember(channelId, userId) InvalidateCacheForUser(userId) InvalidateCacheForChannelMembersNotifyProps(channelId) return member, nil @@ -333,7 +335,7 @@ func addUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM } tmchan := Srv.Store.Team().GetMember(channel.TeamId, user.Id) - cmchan := Srv.Store.Channel().GetMember(channel.Id, user.Id) + cmchan := Srv.Store.Channel().GetMember(channel.Id, user.Id, true) if result := <-tmchan; result.Err != nil { return nil, result.Err @@ -588,7 +590,7 @@ func GetChannelsUserNotIn(teamId string, userId string, offset int, limit int) ( } func GetChannelMember(channelId string, userId string) (*model.ChannelMember, *model.AppError) { - if result := <-Srv.Store.Channel().GetMember(channelId, userId); result.Err != nil { + if result := <-Srv.Store.Channel().GetMember(channelId, userId, true); result.Err != nil { return nil, result.Err } else { return result.Data.(*model.ChannelMember), nil @@ -637,7 +639,7 @@ func GetChannelCounts(teamId string, userId string) (*model.ChannelCounts, *mode func JoinChannel(channel *model.Channel, userId string) *model.AppError { userChan := Srv.Store.User().Get(userId) - memberChan := Srv.Store.Channel().GetMember(channel.Id, userId) + memberChan := Srv.Store.Channel().GetMember(channel.Id, userId, true) if uresult := <-userChan; uresult.Err != nil { return uresult.Err @@ -790,6 +792,7 @@ func removeUserFromChannel(userIdToRemove string, removerUserId string, channel } InvalidateCacheForUser(userIdToRemove) + InvalidateCacheForChannelMember(channel.Id, userIdToRemove) InvalidateCacheForChannelMembers(channel.Id) message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_REMOVED, "", channel.Id, "", nil) diff --git a/app/team.go b/app/team.go index 0c0a6a4da..ce5e107ba 100644 --- a/app/team.go +++ b/app/team.go @@ -424,6 +424,7 @@ func LeaveTeam(team *model.Team, user *model.User) *model.AppError { for _, channel := range *channelList { if channel.Type != model.CHANNEL_DIRECT { InvalidateCacheForChannelMembers(channel.Id) + InvalidateCacheForChannelMember(channel.Id, user.Id) if result := <-Srv.Store.Channel().RemoveMember(channel.Id, user.Id); result.Err != nil { return result.Err } diff --git a/app/web_hub.go b/app/web_hub.go index a214ffb5e..13852e8eb 100644 --- a/app/web_hub.go +++ b/app/web_hub.go @@ -170,6 +170,18 @@ func InvalidateCacheForUser(userId string) { } } +func InvalidateCacheForChannelMember(channelId string, userId string) { + InvalidateCacheForChannelMemberSkipClusterSend(channelId, userId) + + if einterfaces.GetClusterInterface() != nil { + einterfaces.GetClusterInterface().InvalidateCacheForChannelMember(channelId, userId) + } +} + +func InvalidateCacheForChannelMemberSkipClusterSend(channelId string, userId string) { + Srv.Store.Channel().InvalidateMember(channelId, userId) +} + func InvalidateCacheForUserSkipClusterSend(userId string) { Srv.Store.Channel().InvalidateAllChannelMembersForUser(userId) Srv.Store.User().InvalidateProfilesInChannelCacheByUser(userId) diff --git a/einterfaces/cluster.go b/einterfaces/cluster.go index 389f1f139..22610cd91 100644 --- a/einterfaces/cluster.go +++ b/einterfaces/cluster.go @@ -20,6 +20,7 @@ type ClusterInterface interface { InvalidateCacheForChannelMembersNotifyProps(channelId string) InvalidateCacheForChannelPosts(channelId string) InvalidateCacheForWebhook(webhookId string) + InvalidateCacheForChannelMember(channelId string, userId string) Publish(event *model.WebSocketEvent) UpdateStatus(status *model.Status) GetLogs() ([]string, *model.AppError) diff --git a/store/sql_channel_store.go b/store/sql_channel_store.go index aac5bfed3..e7501ae69 100644 --- a/store/sql_channel_store.go +++ b/store/sql_channel_store.go @@ -28,6 +28,9 @@ const ( ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE = model.SESSION_CACHE_SIZE ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SEC = 1800 // 30 mins + CHANNEL_MEMBER_CACHE_SIZE = model.SESSION_CACHE_SIZE + CHANNEL_MEMBER_CACHE_SEC = 900 // 15 mins + CHANNEL_MEMBERS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE CHANNEL_MEMBERS_COUNTS_CACHE_SEC = 1800 // 30 mins @@ -43,6 +46,7 @@ var allChannelMembersForUserCache = utils.NewLru(ALL_CHANNEL_MEMBERS_FOR_USER_CA var allChannelMembersNotifyPropsForChannelCache = utils.NewLru(ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE) var channelCache = utils.NewLru(model.CHANNEL_CACHE_SIZE) var channelByNameCache = utils.NewLru(model.CHANNEL_CACHE_SIZE) +var channelMemberCache = utils.NewLru(CHANNEL_MEMBER_CACHE_SIZE) func ClearChannelCaches() { channelMemberCountsCache.Purge() @@ -50,6 +54,7 @@ func ClearChannelCaches() { allChannelMembersNotifyPropsForChannelCache.Purge() channelCache.Purge() channelByNameCache.Purge() + channelMemberCache.Purge() } func NewSqlChannelStore(sqlStore *SqlStore) ChannelStore { @@ -776,11 +781,36 @@ func (s SqlChannelStore) GetMembers(channelId string, offset, limit int) StoreCh return storeChannel } -func (s SqlChannelStore) GetMember(channelId string, userId string) StoreChannel { +func (s SqlChannelStore) InvalidateMember(channelId string, userId string) { + channelMemberCache.Remove(channelId + userId) +} + +func (s SqlChannelStore) GetMember(channelId string, userId string, allowFromCache bool) StoreChannel { storeChannel := make(StoreChannel, 1) go func() { result := StoreResult{} + metrics := einterfaces.GetMetricsInterface() + + if allowFromCache { + if cacheItem, ok := channelMemberCache.Get(channelId + userId); ok { + if metrics != nil { + metrics.IncrementMemCacheHitCounter("Channel Member") + } + result.Data = cacheItem.(*model.ChannelMember) + storeChannel <- result + close(storeChannel) + return + } else { + if metrics != nil { + metrics.IncrementMemCacheMissCounter("Channel Member") + } + } + } else { + if metrics != nil { + metrics.IncrementMemCacheMissCounter("Channel Member") + } + } var member model.ChannelMember @@ -792,6 +822,8 @@ func (s SqlChannelStore) GetMember(channelId string, userId string) StoreChannel } } else { result.Data = &member + + channelMemberCache.AddWithExpiresInSecs(channelId+userId, &member, CHANNEL_MEMBER_CACHE_SEC) } storeChannel <- result diff --git a/store/sql_channel_store_test.go b/store/sql_channel_store_test.go index 002f9cb66..3075498f7 100644 --- a/store/sql_channel_store_test.go +++ b/store/sql_channel_store_test.go @@ -497,7 +497,7 @@ func TestChannelMemberStore(t *testing.T) { t.Fatal("Member update time incorrect on delete") } - member := (<-store.Channel().GetMember(o1.ChannelId, o1.UserId)).Data.(*model.ChannelMember) + member := (<-store.Channel().GetMember(o1.ChannelId, o1.UserId, false)).Data.(*model.ChannelMember) if member.ChannelId != o1.ChannelId { t.Fatal("should have go member") } @@ -968,15 +968,15 @@ func TestGetMember(t *testing.T) { } Must(store.Channel().SaveMember(m2)) - if result := <-store.Channel().GetMember(model.NewId(), userId); result.Err == nil { + if result := <-store.Channel().GetMember(model.NewId(), userId, false); result.Err == nil { t.Fatal("should've failed to get member for non-existant channel") } - if result := <-store.Channel().GetMember(c1.Id, model.NewId()); result.Err == nil { + if result := <-store.Channel().GetMember(c1.Id, model.NewId(), false); result.Err == nil { t.Fatal("should've failed to get member for non-existant user") } - if result := <-store.Channel().GetMember(c1.Id, userId); result.Err != nil { + if result := <-store.Channel().GetMember(c1.Id, userId, false); result.Err != nil { t.Fatal("shouldn't have errored when getting member", result.Err) } else if member := result.Data.(*model.ChannelMember); member.ChannelId != c1.Id { t.Fatal("should've gotten member of channel 1") @@ -984,7 +984,7 @@ func TestGetMember(t *testing.T) { t.Fatal("should've gotten member for user") } - if result := <-store.Channel().GetMember(c2.Id, userId); result.Err != nil { + if result := <-store.Channel().GetMember(c2.Id, userId, false); result.Err != nil { t.Fatal("shouldn't have errored when getting member", result.Err) } else if member := result.Data.(*model.ChannelMember); member.ChannelId != c2.Id { t.Fatal("should've gotten member of channel 2") diff --git a/store/store.go b/store/store.go index 3da331209..752b6cc28 100644 --- a/store/store.go +++ b/store/store.go @@ -106,7 +106,8 @@ type ChannelStore interface { SaveMember(member *model.ChannelMember) StoreChannel UpdateMember(member *model.ChannelMember) StoreChannel GetMembers(channelId string, offset, limit int) StoreChannel - GetMember(channelId string, userId string) StoreChannel + GetMember(channelId string, userId string, allowFromCache bool) StoreChannel + InvalidateMember(channelId string, userId string) GetAllChannelMembersForUser(userId string, allowFromCache bool) StoreChannel InvalidateAllChannelMembersForUser(userId string) IsUserInChannelUseCache(userId string, channelId string) bool -- cgit v1.2.3-1-g7c22