From fdec1ef1f5bfe3d3627917fb5b9698a8ded0c8a7 Mon Sep 17 00:00:00 2001 From: Allen Lai Date: Wed, 27 Nov 2019 22:14:40 +0800 Subject: [PATCH] MM-20100: Migrate channelGuestCountsCache cache from store/sqlstore/channel_store.go to the new store/localcachelayer (#13135) * init * migrate * add test * fix channel store * ... * missing ')' * change function interface * fix build * fix format * Update store/localcachelayer/layer.go Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com> * Update store/localcachelayer/channel_guest_layer_test.go Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com> * im so confused why my builds are failing * fix build * format * fix format * add license header * resolve some conflicts * add test * fix constant names * fix more constants * fix bugs * delete unused constants * Update store/localcachelayer/channel_layer.go Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com> * typo * another typo * get guest * change test * go fmt format * change test * remove getcountcache * delete method from interface * delete method * update test * rerun GoLang CI --- model/cluster_message.go | 1 + store/localcachelayer/channel_layer.go | 32 +++++++++++ store/localcachelayer/channel_layer_test.go | 62 +++++++++++++++++++++ store/localcachelayer/layer.go | 7 +++ store/localcachelayer/main_test.go | 3 + store/sqlstore/channel_store.go | 47 ---------------- store/store.go | 1 - store/timer_layer.go | 16 ------ 8 files changed, 105 insertions(+), 64 deletions(-) diff --git a/model/cluster_message.go b/model/cluster_message.go index 9dff611e03..65408a5754 100644 --- a/model/cluster_message.go +++ b/model/cluster_message.go @@ -19,6 +19,7 @@ const ( CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBERS = "inv_channel_members" CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_BY_NAME = "inv_channel_name" CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL = "inv_channel" + CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_GUEST_COUNT = "inv_channel_guest_count" CLUSTER_EVENT_INVALIDATE_CACHE_FOR_USER = "inv_user" CLUSTER_EVENT_INVALIDATE_CACHE_FOR_USER_TEAMS = "inv_user_teams" CLUSTER_EVENT_CLEAR_SESSION_CACHE_FOR_USER = "clear_session_user" diff --git a/store/localcachelayer/channel_layer.go b/store/localcachelayer/channel_layer.go index 4ced8f8c42..b7cbb3d94a 100644 --- a/store/localcachelayer/channel_layer.go +++ b/store/localcachelayer/channel_layer.go @@ -21,11 +21,21 @@ func (s *LocalCacheChannelStore) handleClusterInvalidateChannelMemberCounts(msg } } +func (s *LocalCacheChannelStore) handleClusterInvalidateChannelGuestCounts(msg *model.ClusterMessage) { + if msg.Data == CLEAR_CACHE_MESSAGE_DATA { + s.rootStore.channelGuestCountCache.Purge() + } else { + s.rootStore.channelGuestCountCache.Remove(msg.Data) + } +} + func (s LocalCacheChannelStore) ClearCaches() { s.rootStore.doClearCacheCluster(s.rootStore.channelMemberCountsCache) + s.rootStore.doClearCacheCluster(s.rootStore.channelGuestCountCache) s.ChannelStore.ClearCaches() if s.rootStore.metrics != nil { s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Purge") + s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Guest Count - Purge") } } @@ -36,6 +46,13 @@ func (s LocalCacheChannelStore) InvalidateMemberCount(channelId string) { } } +func (s LocalCacheChannelStore) InvalidateGuestCount(channelId string) { + s.rootStore.doInvalidateCacheCluster(s.rootStore.channelGuestCountCache, channelId) + if s.rootStore.metrics != nil { + s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Guests Count - Remove by channelId") + } +} + func (s LocalCacheChannelStore) GetMemberCount(channelId string, allowFromCache bool) (int64, *model.AppError) { if allowFromCache { if count := s.rootStore.doStandardReadCache(s.rootStore.channelMemberCountsCache, channelId); count != nil { @@ -51,6 +68,21 @@ func (s LocalCacheChannelStore) GetMemberCount(channelId string, allowFromCache return count, err } +func (s LocalCacheChannelStore) GetGuestCount(channelId string, allowFromCache bool) (int64, *model.AppError) { + if allowFromCache { + if count := s.rootStore.doStandardReadCache(s.rootStore.channelGuestCountCache, channelId); count != nil { + return count.(int64), nil + } + } + count, err := s.ChannelStore.GetGuestCount(channelId, allowFromCache) + + if allowFromCache && err == nil { + s.rootStore.doStandardAddToCache(s.rootStore.channelGuestCountCache, channelId, count) + } + + return count, err +} + func (s LocalCacheChannelStore) GetMemberCountFromCache(channelId string) int64 { if count := s.rootStore.doStandardReadCache(s.rootStore.channelMemberCountsCache, channelId); count != nil { return count.(int64) diff --git a/store/localcachelayer/channel_layer_test.go b/store/localcachelayer/channel_layer_test.go index 91396ffec3..ece3e8f8c7 100644 --- a/store/localcachelayer/channel_layer_test.go +++ b/store/localcachelayer/channel_layer_test.go @@ -86,3 +86,65 @@ func TestChannelStoreChannelMemberCountsCache(t *testing.T) { mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2) }) } + +func TestChannelStoreGuestCountCache(t *testing.T) { + countResult := int64(12) + + t.Run("first call not cached, second cached and returning same data", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + count, err := cachedStore.Channel().GetGuestCount("id", true) + require.Nil(t, err) + assert.Equal(t, count, countResult) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 1) + count, err = cachedStore.Channel().GetGuestCount("id", true) + require.Nil(t, err) + assert.Equal(t, count, countResult) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 1) + }) + + t.Run("first call not cached, second force no cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetGuestCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 1) + cachedStore.Channel().GetGuestCount("id", false) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 2) + }) + + t.Run("first call force no cached, second not cached, third cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetGuestCount("id", false) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 1) + cachedStore.Channel().GetGuestCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 2) + cachedStore.Channel().GetGuestCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 2) + }) + + t.Run("first call not cached, clear cache, second call not cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetGuestCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 1) + cachedStore.Channel().ClearCaches() + cachedStore.Channel().GetGuestCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 2) + }) + + t.Run("first call not cached, invalidate cache, second call not cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetGuestCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 1) + cachedStore.Channel().InvalidateGuestCount("id") + cachedStore.Channel().GetGuestCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetGuestCount", 2) + }) +} diff --git a/store/localcachelayer/layer.go b/store/localcachelayer/layer.go index 1b8ce26714..062b2dce81 100644 --- a/store/localcachelayer/layer.go +++ b/store/localcachelayer/layer.go @@ -20,6 +20,9 @@ const ( SCHEME_CACHE_SIZE = 20000 SCHEME_CACHE_SEC = 30 * 60 + CHANNEL_GUEST_COUNT_CACHE_SIZE = model.CHANNEL_CACHE_SIZE + CHANNEL_GUEST_COUNT_CACHE_SEC = 30 * 60 + WEBHOOK_CACHE_SIZE = 25000 WEBHOOK_CACHE_SEC = 15 * 60 @@ -53,6 +56,7 @@ type LocalCacheStore struct { emojiIdCacheByName *utils.Cache channel LocalCacheChannelStore channelMemberCountsCache *utils.Cache + channelGuestCountCache *utils.Cache webhook LocalCacheWebhookStore webhookCache *utils.Cache post LocalCachePostStore @@ -79,6 +83,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.emojiIdCacheByName = utils.NewLruWithParams(EMOJI_CACHE_SIZE, "EmojiByName", EMOJI_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_ID_BY_NAME) localCacheStore.emoji = LocalCacheEmojiStore{EmojiStore: baseStore.Emoji(), rootStore: &localCacheStore} localCacheStore.channelMemberCountsCache = utils.NewLruWithParams(CHANNEL_MEMBERS_COUNTS_CACHE_SIZE, "ChannelMemberCounts", CHANNEL_MEMBERS_COUNTS_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS) + localCacheStore.channelGuestCountCache = utils.NewLruWithParams(CHANNEL_GUEST_COUNT_CACHE_SIZE, "ChannelGuestsCount", CHANNEL_GUEST_COUNT_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_GUEST_COUNT) localCacheStore.channel = LocalCacheChannelStore{ChannelStore: baseStore.Channel(), rootStore: &localCacheStore} localCacheStore.postLastPostsCache = utils.NewLruWithParams(LAST_POSTS_CACHE_SIZE, "LastPost", LAST_POSTS_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_LAST_POSTS) localCacheStore.post = LocalCachePostStore{PostStore: baseStore.Post(), rootStore: &localCacheStore} @@ -93,6 +98,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_BY_ID, localCacheStore.emoji.handleClusterInvalidateEmojiById) cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_ID_BY_NAME, localCacheStore.emoji.handleClusterInvalidateEmojiIdByName) cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS, localCacheStore.channel.handleClusterInvalidateChannelMemberCounts) + cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_GUEST_COUNT, localCacheStore.channel.handleClusterInvalidateChannelGuestCounts) cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_LAST_POSTS, localCacheStore.post.handleClusterInvalidateLastPosts) cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_PROFILE_BY_IDS, localCacheStore.user.handleClusterInvalidateScheme) } @@ -185,6 +191,7 @@ func (s *LocalCacheStore) Invalidate() { s.doClearCacheCluster(s.emojiCacheById) s.doClearCacheCluster(s.emojiIdCacheByName) s.doClearCacheCluster(s.channelMemberCountsCache) + s.doClearCacheCluster(s.channelGuestCountCache) s.doClearCacheCluster(s.postLastPostsCache) s.doClearCacheCluster(s.userProfileByIdsCache) } diff --git a/store/localcachelayer/main_test.go b/store/localcachelayer/main_test.go index fbc32f65cf..6ea72854ce 100644 --- a/store/localcachelayer/main_test.go +++ b/store/localcachelayer/main_test.go @@ -59,10 +59,13 @@ func getMockStore() *mocks.Store { mockStore.On("Emoji").Return(&mockEmojiStore) mockCount := int64(10) + mockGuestCount := int64(12) mockChannelStore := mocks.ChannelStore{} mockChannelStore.On("ClearCaches").Return() mockChannelStore.On("GetMemberCount", "id", true).Return(mockCount, nil) mockChannelStore.On("GetMemberCount", "id", false).Return(mockCount, nil) + mockChannelStore.On("GetGuestCount", "id", true).Return(mockGuestCount, nil) + mockChannelStore.On("GetGuestCount", "id", false).Return(mockGuestCount, nil) mockStore.On("Channel").Return(&mockChannelStore) fakePosts := &model.PostList{} diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 4627117694..fe569410db 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -29,9 +29,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_GUESTS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE - CHANNEL_GUESTS_COUNTS_CACHE_SEC = 1800 // 30 mins - CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SEC = 1800 // 30 mins @@ -281,7 +278,6 @@ type publicChannel struct { } var channelPinnedPostCountsCache = utils.NewLru(CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SIZE) -var channelGuestCountsCache = utils.NewLru(CHANNEL_GUESTS_COUNTS_CACHE_SIZE) var allChannelMembersForUserCache = utils.NewLru(ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE) var allChannelMembersNotifyPropsForChannelCache = utils.NewLru(ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE) var channelCache = utils.NewLru(model.CHANNEL_CACHE_SIZE) @@ -289,7 +285,6 @@ var channelByNameCache = utils.NewLru(model.CHANNEL_CACHE_SIZE) func (s SqlChannelStore) ClearCaches() { channelPinnedPostCountsCache.Purge() - channelGuestCountsCache.Purge() allChannelMembersForUserCache.Purge() allChannelMembersNotifyPropsForChannelCache.Purge() channelCache.Purge() @@ -1665,46 +1660,9 @@ func (s SqlChannelStore) GetPinnedPostCount(channelId string, allowFromCache boo } func (s SqlChannelStore) InvalidateGuestCount(channelId string) { - channelGuestCountsCache.Remove(channelId) - if s.metrics != nil { - s.metrics.IncrementMemCacheInvalidationCounter("Channel Guest Counts - Remove by ChannelId") - } -} - -func (s SqlChannelStore) GetGuestCountFromCache(channelId string) int64 { - if cacheItem, ok := channelGuestCountsCache.Get(channelId); ok { - if s.metrics != nil { - s.metrics.IncrementMemCacheHitCounter("Channel Guest Counts") - } - return cacheItem.(int64) - } - - if s.metrics != nil { - s.metrics.IncrementMemCacheMissCounter("Channel Guest Counts") - } - - count, err := s.GetGuestCount(channelId, true) - if err != nil { - return 0 - } - - return count } func (s SqlChannelStore) GetGuestCount(channelId string, allowFromCache bool) (int64, *model.AppError) { - if allowFromCache { - if cacheItem, ok := channelGuestCountsCache.Get(channelId); ok { - if s.metrics != nil { - s.metrics.IncrementMemCacheHitCounter("Channel Guest Counts") - } - return cacheItem.(int64), nil - } - } - - if s.metrics != nil { - s.metrics.IncrementMemCacheMissCounter("Channel Guest Counts") - } - count, err := s.GetReplica().SelectInt(` SELECT count(*) @@ -1719,11 +1677,6 @@ func (s SqlChannelStore) GetGuestCount(channelId string, allowFromCache bool) (i if err != nil { return 0, model.NewAppError("SqlChannelStore.GetGuestCount", "store.sql_channel.get_member_count.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) } - - if allowFromCache { - channelGuestCountsCache.AddWithExpiresInSecs(channelId, count, CHANNEL_GUESTS_COUNTS_CACHE_SEC) - } - return count, nil } diff --git a/store/store.go b/store/store.go index bed48fb4a2..847edc0597 100644 --- a/store/store.go +++ b/store/store.go @@ -159,7 +159,6 @@ type ChannelStore interface { GetPinnedPostCountFromCache(channelId string) int64 GetPinnedPostCount(channelId string, allowFromCache bool) (int64, *model.AppError) InvalidateGuestCount(channelId string) - GetGuestCountFromCache(channelId string) int64 GetGuestCount(channelId string, allowFromCache bool) (int64, *model.AppError) GetPinnedPosts(channelId string) (*model.PostList, *model.AppError) RemoveMember(channelId string, userId string) *model.AppError diff --git a/store/timer_layer.go b/store/timer_layer.go index 4224db70ed..afa3bba47f 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -984,22 +984,6 @@ func (s *TimerLayerChannelStore) GetGuestCount(channelId string, allowFromCache return resultVar0, resultVar1 } -func (s *TimerLayerChannelStore) GetGuestCountFromCache(channelId string) int64 { - start := timemodule.Now() - - resultVar0 := s.ChannelStore.GetGuestCountFromCache(channelId) - - elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) - if s.Root.Metrics != nil { - success := "false" - if true { - success = "true" - } - s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.GetGuestCountFromCache", success, elapsed) - } - return resultVar0 -} - func (s *TimerLayerChannelStore) GetMember(channelId string, userId string) (*model.ChannelMember, *model.AppError) { start := timemodule.Now()