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
2025-02-25 18:55:24 -06:00 · 2019-11-27 22:14:40 +08:00
parent 31ac88ef69
commit fdec1ef1f5
8 changed files with 105 additions and 64 deletions
--- 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"
--- 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)
--- 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)
+	})
+}
--- 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)
 }
--- 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{}
--- 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
 }

--- 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
--- 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()