diff --git a/model/cluster_message.go b/model/cluster_message.go index 04d8673ceb..ffa3b5e59a 100644 --- 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" diff --git a/store/localcachelayer/channel_layer.go b/store/localcachelayer/channel_layer.go new file mode 100644 index 0000000000..4ced8f8c42 --- /dev/null +++ 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 +} diff --git a/store/localcachelayer/channel_layer_test.go b/store/localcachelayer/channel_layer_test.go new file mode 100644 index 0000000000..91396ffec3 --- /dev/null +++ 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) + }) +} diff --git a/store/localcachelayer/layer.go b/store/localcachelayer/layer.go index b799d5d163..036ba3276d 100644 --- 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) } diff --git a/store/localcachelayer/main_test.go b/store/localcachelayer/main_test.go index 6022826e81..bbff9843c0 100644 --- 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 } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index bf05278aaf..9bc32851ca 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_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 }