From 83c113595ace467ad34f05e35fb2282fa8631a17 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Mon, 27 Feb 2017 17:43:23 -0500 Subject: Revert "Adding caching to get channel member (#5518)" This reverts commit ba028ed74b69bd1dd902344663fbf8ba4f1dfb87. --- 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, 13 insertions(+), 67 deletions(-) diff --git a/api/apitestlib.go b/api/apitestlib.go index 6c4952b3f..475469a36 100644 --- a/api/apitestlib.go +++ b/api/apitestlib.go @@ -194,15 +194,13 @@ 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, true); cmr.Err == nil { + if cmr := <-app.Srv.Store.Channel().GetMember(channel.Id, user.Id); 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) @@ -214,15 +212,13 @@ 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, true); cmr.Err == nil { + if cmr := <-app.Srv.Store.Channel().GetMember(channel.Id, user.Id); 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 8352b3b9d..533a2f0bb 100644 --- a/app/channel.go +++ b/app/channel.go @@ -180,7 +180,7 @@ func WaitForChannelMembership(channelId string, userId string) { time.Sleep(100 * time.Millisecond) - result := <-Srv.Store.Channel().GetMember(channelId, userId, true) + result := <-Srv.Store.Channel().GetMember(channelId, userId) // If the membership was found then return if result.Err == nil { @@ -219,7 +219,6 @@ func UpdateChannelMemberRoles(channelId string, userId string, newRoles string) return nil, result.Err } - InvalidateCacheForChannelMember(channelId, userId) InvalidateCacheForUser(userId) return member, nil } @@ -251,7 +250,6 @@ 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 @@ -340,7 +338,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, true) + cmchan := Srv.Store.Channel().GetMember(channel.Id, user.Id) if result := <-tmchan; result.Err != nil { return nil, result.Err @@ -595,7 +593,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, true); result.Err != nil { + if result := <-Srv.Store.Channel().GetMember(channelId, userId); result.Err != nil { return nil, result.Err } else { return result.Data.(*model.ChannelMember), nil @@ -644,7 +642,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, true) + memberChan := Srv.Store.Channel().GetMember(channel.Id, userId) if uresult := <-userChan; uresult.Err != nil { return uresult.Err @@ -797,7 +795,6 @@ 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 ce5e107ba..0c0a6a4da 100644 --- a/app/team.go +++ b/app/team.go @@ -424,7 +424,6 @@ 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 90dfacbdf..a50680806 100644 --- a/app/web_hub.go +++ b/app/web_hub.go @@ -173,18 +173,6 @@ 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 22610cd91..389f1f139 100644 --- a/einterfaces/cluster.go +++ b/einterfaces/cluster.go @@ -20,7 +20,6 @@ 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 068692074..ea32317a3 100644 --- a/store/sql_channel_store.go +++ b/store/sql_channel_store.go @@ -28,9 +28,6 @@ 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 @@ -46,7 +43,6 @@ 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() @@ -54,7 +50,6 @@ func ClearChannelCaches() { allChannelMembersNotifyPropsForChannelCache.Purge() channelCache.Purge() channelByNameCache.Purge() - channelMemberCache.Purge() } func NewSqlChannelStore(sqlStore *SqlStore) ChannelStore { @@ -781,36 +776,11 @@ func (s SqlChannelStore) GetMembers(channelId string, offset, limit int) StoreCh return storeChannel } -func (s SqlChannelStore) InvalidateMember(channelId string, userId string) { - channelMemberCache.Remove(channelId + userId) -} - -func (s SqlChannelStore) GetMember(channelId string, userId string, allowFromCache bool) StoreChannel { +func (s SqlChannelStore) GetMember(channelId string, userId string) 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 @@ -822,8 +792,6 @@ func (s SqlChannelStore) GetMember(channelId string, userId string, allowFromCac } } 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 a43b97706..df9c76905 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, false)).Data.(*model.ChannelMember) + member := (<-store.Channel().GetMember(o1.ChannelId, o1.UserId)).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, false); result.Err == nil { + if result := <-store.Channel().GetMember(model.NewId(), userId); result.Err == nil { t.Fatal("should've failed to get member for non-existant channel") } - if result := <-store.Channel().GetMember(c1.Id, model.NewId(), false); result.Err == nil { + if result := <-store.Channel().GetMember(c1.Id, model.NewId()); result.Err == nil { t.Fatal("should've failed to get member for non-existant user") } - if result := <-store.Channel().GetMember(c1.Id, userId, false); result.Err != nil { + if result := <-store.Channel().GetMember(c1.Id, userId); 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, false); result.Err != nil { + if result := <-store.Channel().GetMember(c2.Id, userId); 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 06564a8c2..4226f03a5 100644 --- a/store/store.go +++ b/store/store.go @@ -106,8 +106,7 @@ 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, allowFromCache bool) StoreChannel - InvalidateMember(channelId string, userId string) + GetMember(channelId string, userId string) StoreChannel GetAllChannelMembersForUser(userId string, allowFromCache bool) StoreChannel InvalidateAllChannelMembersForUser(userId string) IsUserInChannelUseCache(userId string, channelId string) bool -- cgit v1.2.3-1-g7c22