[GH-13106] Migrate channelMemberCountsCache cache from store/sqlstore/channel_store.go to the new store/localcachelayer (#13128)

* Migrate channelMemberCountsCache cache from store/sqlstore/channel_store.go to the new store/localcachelayer * Move cache functions to layer functions over the caches * Directly remove and purge caches in handleClusterInvalidate to avoid loops and prepare details for layer with multiple caches
2019-11-19 09:55:01 +01:00
--- a/model/cluster_message.go
+++ b/model/cluster_message.go
@@ -24,6 +24,7 @@ const (
 	CLUSTER_EVENT_CLEAR_SESSION_CACHE_FOR_USER                      = "clear_session_user"
 	CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES                        = "inv_roles"
 	CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES                      = "inv_schemes"
+	CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS        = "inv_channel_member_counts"
 	CLUSTER_EVENT_CLEAR_SESSION_CACHE_FOR_ALL_USERS                 = "inv_all_user_sessions"
 	CLUSTER_EVENT_INSTALL_PLUGIN                                    = "install_plugin"
 	CLUSTER_EVENT_REMOVE_PLUGIN                                     = "remove_plugin"
--- a/store/localcachelayer/channel_layer.go
+++ b/store/localcachelayer/channel_layer.go
@@ -0,0 +1,65 @@
+// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package localcachelayer
+
+import (
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/store"
+)
+
+type LocalCacheChannelStore struct {
+	store.ChannelStore
+	rootStore *LocalCacheStore
+}
+
+func (s *LocalCacheChannelStore) handleClusterInvalidateChannelMemberCounts(msg *model.ClusterMessage) {
+	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
+		s.rootStore.channelMemberCountsCache.Purge()
+	} else {
+		s.rootStore.channelMemberCountsCache.Remove(msg.Data)
+	}
+}
+
+func (s LocalCacheChannelStore) ClearCaches() {
+	s.rootStore.doClearCacheCluster(s.rootStore.channelMemberCountsCache)
+	s.ChannelStore.ClearCaches()
+	if s.rootStore.metrics != nil {
+		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Purge")
+	}
+}
+
+func (s LocalCacheChannelStore) InvalidateMemberCount(channelId string) {
+	s.rootStore.doInvalidateCacheCluster(s.rootStore.channelMemberCountsCache, channelId)
+	if s.rootStore.metrics != nil {
+		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - 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 {
+			return count.(int64), nil
+		}
+	}
+	count, err := s.ChannelStore.GetMemberCount(channelId, allowFromCache)
+
+	if allowFromCache && err == nil {
+		s.rootStore.doStandardAddToCache(s.rootStore.channelMemberCountsCache, 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)
+	}
+
+	count, err := s.GetMemberCount(channelId, true)
+	if err != nil {
+		return 0
+	}
+
+	return count
+}
--- a/store/localcachelayer/channel_layer_test.go
+++ b/store/localcachelayer/channel_layer_test.go
@@ -0,0 +1,88 @@
+package localcachelayer
+
+import (
+	"testing"
+
+	"github.com/mattermost/mattermost-server/store/storetest"
+	"github.com/mattermost/mattermost-server/store/storetest/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestChannelStore(t *testing.T) {
+	StoreTest(t, storetest.TestReactionStore)
+}
+
+func TestChannelStoreChannelMemberCountsCache(t *testing.T) {
+	countResult := int64(10)
+
+	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().GetMemberCount("id", true)
+		require.Nil(t, err)
+		assert.Equal(t, count, countResult)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+		count, err = cachedStore.Channel().GetMemberCount("id", true)
+		require.Nil(t, err)
+		assert.Equal(t, count, countResult)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+	})
+
+	t.Run("first call not cached, second force no cached", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Channel().GetMemberCount("id", true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+		cachedStore.Channel().GetMemberCount("id", false)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 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().GetMemberCount("id", false)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+		cachedStore.Channel().GetMemberCount("id", true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2)
+		cachedStore.Channel().GetMemberCount("id", true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2)
+	})
+
+	t.Run("first call with GetMemberCountFromCache not cached, second cached and returning same data", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		count := cachedStore.Channel().GetMemberCountFromCache("id")
+		assert.Equal(t, count, countResult)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+		count = cachedStore.Channel().GetMemberCountFromCache("id")
+		assert.Equal(t, count, countResult)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+	})
+
+	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().GetMemberCount("id", true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+		cachedStore.Channel().ClearCaches()
+		cachedStore.Channel().GetMemberCount("id", true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 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().GetMemberCount("id", true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1)
+		cachedStore.Channel().InvalidateMemberCount("id")
+		cachedStore.Channel().GetMemberCount("id", true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2)
+	})
+}
--- a/store/localcachelayer/layer.go
+++ b/store/localcachelayer/layer.go
@@ -20,19 +20,24 @@ const (
 	SCHEME_CACHE_SIZE = 20000
 	SCHEME_CACHE_SEC  = 30 * 60

+	CHANNEL_MEMBERS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE
+	CHANNEL_MEMBERS_COUNTS_CACHE_SEC  = 30 * 60
+
 	CLEAR_CACHE_MESSAGE_DATA = ""
 )

 type LocalCacheStore struct {
 	store.Store
-	metrics       einterfaces.MetricsInterface
-	cluster       einterfaces.ClusterInterface
-	reaction      LocalCacheReactionStore
-	reactionCache *utils.Cache
-	role          LocalCacheRoleStore
-	roleCache     *utils.Cache
-	scheme        LocalCacheSchemeStore
-	schemeCache   *utils.Cache
+	metrics                  einterfaces.MetricsInterface
+	cluster                  einterfaces.ClusterInterface
+	reaction                 LocalCacheReactionStore
+	reactionCache            *utils.Cache
+	role                     LocalCacheRoleStore
+	roleCache                *utils.Cache
+	scheme                   LocalCacheSchemeStore
+	schemeCache              *utils.Cache
+	channel                  LocalCacheChannelStore
+	channelMemberCountsCache *utils.Cache
 }

 func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface) LocalCacheStore {
@@ -47,11 +52,14 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
 	localCacheStore.role = LocalCacheRoleStore{RoleStore: baseStore.Role(), rootStore: &localCacheStore}
 	localCacheStore.schemeCache = utils.NewLruWithParams(SCHEME_CACHE_SIZE, "Scheme", SCHEME_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES)
 	localCacheStore.scheme = LocalCacheSchemeStore{SchemeStore: baseStore.Scheme(), 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.channel = LocalCacheChannelStore{ChannelStore: baseStore.Channel(), rootStore: &localCacheStore}

 	if cluster != nil {
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS, localCacheStore.reaction.handleClusterInvalidateReaction)
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES, localCacheStore.role.handleClusterInvalidateRole)
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES, localCacheStore.scheme.handleClusterInvalidateScheme)
+		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS, localCacheStore.channel.handleClusterInvalidateChannelMemberCounts)
 	}
 	return localCacheStore
 }
@@ -68,6 +76,10 @@ func (s LocalCacheStore) Scheme() store.SchemeStore {
 	return s.scheme
 }

+func (s LocalCacheStore) Channel() store.ChannelStore {
+	return s.channel
+}
+
 func (s LocalCacheStore) DropAllTables() {
 	s.Invalidate()
 	s.Store.DropAllTables()
@@ -118,4 +130,5 @@ func (s *LocalCacheStore) doClearCacheCluster(cache *utils.Cache) {

 func (s *LocalCacheStore) Invalidate() {
 	s.doClearCacheCluster(s.reactionCache)
+	s.doClearCacheCluster(s.channelMemberCountsCache)
 }
--- a/store/localcachelayer/main_test.go
+++ b/store/localcachelayer/main_test.go
@@ -41,6 +41,13 @@ func getMockStore() *mocks.Store {
 	mockSchemesStore.On("PermanentDeleteAll").Return(nil)
 	mockStore.On("Scheme").Return(&mockSchemesStore)

+	mockCount := int64(10)
+	mockChannelStore := mocks.ChannelStore{}
+	mockChannelStore.On("ClearCaches").Return()
+	mockChannelStore.On("GetMemberCount", "id", true).Return(mockCount, nil)
+	mockChannelStore.On("GetMemberCount", "id", false).Return(mockCount, nil)
+	mockStore.On("Channel").Return(&mockChannelStore)
+
 	return &mockStore
 }

--- 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_MEMBERS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE
-	CHANNEL_MEMBERS_COUNTS_CACHE_SEC  = 1800 // 30 mins
-
 	CHANNEL_GUESTS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE
 	CHANNEL_GUESTS_COUNTS_CACHE_SEC  = 1800 // 30 mins

@@ -283,7 +280,6 @@ type publicChannel struct {
 	Purpose     string `json:"purpose"`
 }

-var channelMemberCountsCache = utils.NewLru(CHANNEL_MEMBERS_COUNTS_CACHE_SIZE)
 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)
@@ -292,7 +288,6 @@ var channelCache = utils.NewLru(model.CHANNEL_CACHE_SIZE)
 var channelByNameCache = utils.NewLru(model.CHANNEL_CACHE_SIZE)

 func (s SqlChannelStore) ClearCaches() {
-	channelMemberCountsCache.Purge()
 	channelPinnedPostCountsCache.Purge()
 	channelGuestCountsCache.Purge()
 	allChannelMembersForUserCache.Purge()
@@ -301,7 +296,6 @@ func (s SqlChannelStore) ClearCaches() {
 	channelByNameCache.Purge()

 	if s.metrics != nil {
-		s.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Purge")
 		s.metrics.IncrementMemCacheInvalidationCounter("Channel Pinned Post Counts - Purge")
 		s.metrics.IncrementMemCacheInvalidationCounter("All Channel Members for User - Purge")
 		s.metrics.IncrementMemCacheInvalidationCounter("All Channel Members Notify Props for Channel - Purge")
@@ -1585,46 +1579,14 @@ func (s SqlChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId str
 }

 func (s SqlChannelStore) InvalidateMemberCount(channelId string) {
-	channelMemberCountsCache.Remove(channelId)
-	if s.metrics != nil {
-		s.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Remove by ChannelId")
-	}
 }

 func (s SqlChannelStore) GetMemberCountFromCache(channelId string) int64 {
-	if cacheItem, ok := channelMemberCountsCache.Get(channelId); ok {
-		if s.metrics != nil {
-			s.metrics.IncrementMemCacheHitCounter("Channel Member Counts")
-		}
-		return cacheItem.(int64)
-	}
-
-	if s.metrics != nil {
-		s.metrics.IncrementMemCacheMissCounter("Channel Member Counts")
-	}
-
-	count, err := s.GetMemberCount(channelId, true)
-	if err != nil {
-		return 0
-	}
-
+	count, _ := s.GetMemberCount(channelId, true)
 	return count
 }

 func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) (int64, *model.AppError) {
-	if allowFromCache {
-		if cacheItem, ok := channelMemberCountsCache.Get(channelId); ok {
-			if s.metrics != nil {
-				s.metrics.IncrementMemCacheHitCounter("Channel Member Counts")
-			}
-			return cacheItem.(int64), nil
-		}
-	}
-
-	if s.metrics != nil {
-		s.metrics.IncrementMemCacheMissCounter("Channel Member Counts")
-	}
-
 	count, err := s.GetReplica().SelectInt(`
 		SELECT
 			count(*)
@@ -1639,10 +1601,6 @@ func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) (
 		return 0, model.NewAppError("SqlChannelStore.GetMemberCount", "store.sql_channel.get_member_count.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError)
 	}

-	if allowFromCache {
-		channelMemberCountsCache.AddWithExpiresInSecs(channelId, count, CHANNEL_MEMBERS_COUNTS_CACHE_SEC)
-	}
-
 	return count, nil
 }