Migrating team store cache to cache layer (#13075)

Automatic Merge

model/cluster_message.go

@@ -32,6 +32,7 @@ const (
 	CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_PINNEDPOSTS_COUNTS   = "inv_channel_pinnedposts_counts"
 	CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS        = "inv_channel_member_counts"
 	CLUSTER_EVENT_INVALIDATE_CACHE_FOR_LAST_POSTS                   = "inv_last_posts"
+	CLUSTER_EVENT_INVALIDATE_CACHE_FOR_TEAMS                        = "inv_teams"
 	CLUSTER_EVENT_CLEAR_SESSION_CACHE_FOR_ALL_USERS                 = "inv_all_user_sessions"
 	CLUSTER_EVENT_INSTALL_PLUGIN                                    = "install_plugin"
 	CLUSTER_EVENT_REMOVE_PLUGIN                                     = "remove_plugin"

store/localcachelayer/layer.go

@@ -41,6 +41,9 @@ const (
 	USER_PROFILE_BY_ID_CACHE_SIZE = 20000
 	USER_PROFILE_BY_ID_SEC        = 30 * 60
 
+	TEAM_CACHE_SIZE = 20000
+	TEAM_CACHE_SEC  = 30 * 60
+
 	CLEAR_CACHE_MESSAGE_DATA = ""
 )
 
@@ -67,6 +70,8 @@ type LocalCacheStore struct {
 	postLastPostsCache           *utils.Cache
 	user                         LocalCacheUserStore
 	userProfileByIdsCache        *utils.Cache
+	team                         LocalCacheTeamStore
+	teamAllTeamIdsForUserCache   *utils.Cache
 }
 
 func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface) LocalCacheStore {
@@ -94,6 +99,8 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
 	localCacheStore.post = LocalCachePostStore{PostStore: baseStore.Post(), rootStore: &localCacheStore}
 	localCacheStore.userProfileByIdsCache = utils.NewLruWithParams(USER_PROFILE_BY_ID_CACHE_SIZE, "UserProfileByIds", USER_PROFILE_BY_ID_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_PROFILE_BY_IDS)
 	localCacheStore.user = LocalCacheUserStore{UserStore: baseStore.User(), rootStore: &localCacheStore}
+	localCacheStore.teamAllTeamIdsForUserCache = utils.NewLruWithParams(TEAM_CACHE_SIZE, "Team", TEAM_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_TEAMS)
+	localCacheStore.team = LocalCacheTeamStore{TeamStore: baseStore.Team(), rootStore: &localCacheStore}
 
 	if cluster != nil {
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS, localCacheStore.reaction.handleClusterInvalidateReaction)
@@ -107,6 +114,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
 		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)
+		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_TEAMS, localCacheStore.team.handleClusterInvalidateTeam)
 	}
 	return localCacheStore
 }
@@ -143,6 +151,10 @@ func (s LocalCacheStore) User() store.UserStore {
 	return s.user
 }
 
+func (s LocalCacheStore) Team() store.TeamStore {
+	return s.team
+}
+
 func (s LocalCacheStore) DropAllTables() {
 	s.Invalidate()
 	s.Store.DropAllTables()
@@ -201,4 +213,5 @@ func (s *LocalCacheStore) Invalidate() {
 	s.doClearCacheCluster(s.channelGuestCountCache)
 	s.doClearCacheCluster(s.postLastPostsCache)
 	s.doClearCacheCluster(s.userProfileByIdsCache)
+	s.doClearCacheCluster(s.teamAllTeamIdsForUserCache)
 }

store/localcachelayer/main_test.go

@@ -85,6 +85,12 @@ func getMockStore() *mocks.Store {
 	mockUserStore.On("GetProfileByIds", []string{"123"}, &store.UserGetByIdsOpts{}, false).Return(fakeUser, nil)
 	mockStore.On("User").Return(&mockUserStore)
 
+	fakeUserTeamIds := []string{"1", "2", "3"}
+	mockTeamStore := mocks.TeamStore{}
+	mockTeamStore.On("GetUserTeamIds", "123", true).Return(fakeUserTeamIds, nil)
+	mockTeamStore.On("GetUserTeamIds", "123", false).Return(fakeUserTeamIds, nil)
+	mockStore.On("Team").Return(&mockTeamStore)
+
 	return &mockStore
 }
 

store/localcachelayer/team_layer.go

@@ -0,0 +1,79 @@
+// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package localcachelayer
+
+import (
+	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store"
+)
+
+type LocalCacheTeamStore struct {
+	store.TeamStore
+	rootStore *LocalCacheStore
+}
+
+func (s *LocalCacheTeamStore) handleClusterInvalidateTeam(msg *model.ClusterMessage) {
+	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
+		s.rootStore.teamAllTeamIdsForUserCache.Purge()
+	} else {
+		s.rootStore.teamAllTeamIdsForUserCache.Remove(msg.Data)
+	}
+}
+
+func (s LocalCacheTeamStore) ClearCaches() {
+	s.rootStore.teamAllTeamIdsForUserCache.Purge()
+	if s.rootStore.metrics != nil {
+		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("All Team Ids for User - Purge")
+	}
+}
+
+func (s LocalCacheTeamStore) InvalidateAllTeamIdsForUser(userId string) {
+	s.rootStore.doInvalidateCacheCluster(s.rootStore.teamAllTeamIdsForUserCache, userId)
+	if s.rootStore.metrics != nil {
+		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("All Team Ids for User - Remove by UserId")
+	}
+}
+
+func (s LocalCacheTeamStore) GetUserTeamIds(userID string, allowFromCache bool) ([]string, *model.AppError) {
+	if !allowFromCache {
+		return s.TeamStore.GetUserTeamIds(userID, allowFromCache)
+	}
+
+	if userTeamIds := s.rootStore.doStandardReadCache(s.rootStore.teamAllTeamIdsForUserCache, userID); userTeamIds != nil {
+		return userTeamIds.([]string), nil
+	}
+
+	userTeamIds, err := s.TeamStore.GetUserTeamIds(userID, allowFromCache)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(userTeamIds) > 0 {
+		s.rootStore.doStandardAddToCache(s.rootStore.teamAllTeamIdsForUserCache, userID, userTeamIds)
+	}
+
+	return userTeamIds, nil
+}
+
+func (s LocalCacheTeamStore) Update(team *model.Team) (*model.Team, *model.AppError) {
+	var oldTeam *model.Team
+	var err *model.AppError
+	if team.DeleteAt != 0 {
+		oldTeam, err = s.TeamStore.Get(team.Id)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	tm, err := s.TeamStore.Update(team)
+	if err != nil {
+		return nil, err
+	}
+
+	if oldTeam != nil && oldTeam.DeleteAt == 0 {
+		s.rootStore.doClearCacheCluster(s.rootStore.teamAllTeamIdsForUserCache)
+	}
+
+	return tm, err
+}

store/localcachelayer/team_layer_test.go

@@ -0,0 +1,70 @@
+// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package localcachelayer
+
+import (
+	"testing"
+
+	"github.com/mattermost/mattermost-server/v5/store/storetest"
+	"github.com/mattermost/mattermost-server/v5/store/storetest/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestTeamStore(t *testing.T) {
+	StoreTest(t, storetest.TestTeamStore)
+}
+
+func TestTeamStoreCache(t *testing.T) {
+	fakeUserId := "123"
+	fakeUserTeamIds := []string{"1", "2", "3"}
+
+	t.Run("first call not cached, second cached and returning same data", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		gotUserTeamIds, err := cachedStore.Team().GetUserTeamIds(fakeUserId, true)
+		require.Nil(t, err)
+		assert.Equal(t, fakeUserTeamIds, gotUserTeamIds)
+		mockStore.Team().(*mocks.TeamStore).AssertNumberOfCalls(t, "GetUserTeamIds", 1)
+
+		gotUserTeamIds, err = cachedStore.Team().GetUserTeamIds(fakeUserId, true)
+		require.Nil(t, err)
+		assert.Equal(t, fakeUserTeamIds, gotUserTeamIds)
+		mockStore.Team().(*mocks.TeamStore).AssertNumberOfCalls(t, "GetUserTeamIds", 1)
+	})
+
+	t.Run("first call not cached, second force no cached", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		gotUserTeamIds, err := cachedStore.Team().GetUserTeamIds(fakeUserId, true)
+		require.Nil(t, err)
+		assert.Equal(t, fakeUserTeamIds, gotUserTeamIds)
+		mockStore.Team().(*mocks.TeamStore).AssertNumberOfCalls(t, "GetUserTeamIds", 1)
+
+		gotUserTeamIds, err = cachedStore.Team().GetUserTeamIds(fakeUserId, false)
+		require.Nil(t, err)
+		assert.Equal(t, fakeUserTeamIds, gotUserTeamIds)
+		mockStore.Team().(*mocks.TeamStore).AssertNumberOfCalls(t, "GetUserTeamIds", 2)
+	})
+
+	t.Run("first call not cached, invalidate, and then not cached again", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		gotUserTeamIds, err := cachedStore.Team().GetUserTeamIds(fakeUserId, true)
+		require.Nil(t, err)
+		assert.Equal(t, fakeUserTeamIds, gotUserTeamIds)
+		mockStore.Team().(*mocks.TeamStore).AssertNumberOfCalls(t, "GetUserTeamIds", 1)
+
+		cachedStore.Team().InvalidateAllTeamIdsForUser(fakeUserId)
+
+		gotUserTeamIds, err = cachedStore.Team().GetUserTeamIds(fakeUserId, true)
+		require.Nil(t, err)
+		assert.Equal(t, fakeUserTeamIds, gotUserTeamIds)
+		mockStore.Team().(*mocks.TeamStore).AssertNumberOfCalls(t, "GetUserTeamIds", 2)
+	})
+
+}

store/sqlstore/supplier.go

@@ -123,7 +123,7 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter
 
 	supplier.initConnection()
 
-	supplier.stores.team = NewSqlTeamStore(supplier, metrics)
+	supplier.stores.team = NewSqlTeamStore(supplier)
 	supplier.stores.channel = NewSqlChannelStore(supplier, metrics)
 	supplier.stores.post = NewSqlPostStore(supplier, metrics)
 	supplier.stores.user = NewSqlUserStore(supplier, metrics)

store/sqlstore/team_store.go

@@ -11,21 +11,16 @@ import (
 
 	sq "github.com/Masterminds/squirrel"
 	"github.com/mattermost/gorp"
-	"github.com/mattermost/mattermost-server/v5/einterfaces"
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/store"
-	"github.com/mattermost/mattermost-server/v5/utils"
 )
 
 const (
-	TEAM_MEMBER_EXISTS_ERROR         = "store.sql_team.save_member.exists.app_error"
+	TEAM_MEMBER_EXISTS_ERROR = "store.sql_team.save_member.exists.app_error"
-	ALL_TEAM_IDS_FOR_USER_CACHE_SIZE = model.SESSION_CACHE_SIZE
-	ALL_TEAM_IDS_FOR_USER_CACHE_SEC  = 1800 // 30 mins
 )
 
 type SqlTeamStore struct {
 	SqlStore
-	metrics einterfaces.MetricsInterface
 }
 
 type teamMember struct {
@@ -155,10 +150,9 @@ func (db teamMemberWithSchemeRolesList) ToModel() []*model.TeamMember {
 	return tms
 }
 
-func NewSqlTeamStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface) store.TeamStore {
+func NewSqlTeamStore(sqlStore SqlStore) store.TeamStore {
 	s := &SqlTeamStore{
 		sqlStore,
-		metrics,
 	}
 
 	for _, db := range sqlStore.GetAllConns() {
@@ -245,11 +239,6 @@ func (s SqlTeamStore) Update(team *model.Team) (*model.Team, *model.AppError) {
 		return nil, model.NewAppError("SqlTeamStore.Update", "store.sql_team.update.app_error", nil, "id="+team.Id, http.StatusInternalServerError)
 	}
 
-	if oldTeam.DeleteAt == 0 && team.DeleteAt != 0 {
-		// Invalidate this cache after any team deletion
-		allTeamIdsForUserCache.Purge()
-	}
-
 	return team, nil
 }
 
@@ -933,21 +922,9 @@ func (s SqlTeamStore) ResetAllTeamSchemes() *model.AppError {
 	return nil
 }
 
-var allTeamIdsForUserCache = utils.NewLru(ALL_TEAM_IDS_FOR_USER_CACHE_SIZE)
+func (s SqlTeamStore) ClearCaches() {}
 
-func (s SqlTeamStore) ClearCaches() {
+func (s SqlTeamStore) InvalidateAllTeamIdsForUser(userId string) {}
-	allTeamIdsForUserCache.Purge()
-	if s.metrics != nil {
-		s.metrics.IncrementMemCacheInvalidationCounter("All Team Ids for User - Purge")
-	}
-}
-
-func (s SqlTeamStore) InvalidateAllTeamIdsForUser(userId string) {
-	allTeamIdsForUserCache.Remove(userId)
-	if s.metrics != nil {
-		s.metrics.IncrementMemCacheInvalidationCounter("All Team Ids for User - Remove by UserId")
-	}
-}
 
 func (s SqlTeamStore) ClearAllCustomRoleAssignments() *model.AppError {
 
@@ -1035,21 +1012,8 @@ func (s SqlTeamStore) GetAllForExportAfter(limit int, afterId string) ([]*model.
 	return data, nil
 }
 
-// GetUserTeamIds get the team ids to which the user belongs to
+// GetUserTeamIds get the team ids to which the user belongs to. allowFromCache parameter does not have any effect in this Store
 func (s SqlTeamStore) GetUserTeamIds(userID string, allowFromCache bool) ([]string, *model.AppError) {
-	if allowFromCache {
-		if cacheItem, ok := allTeamIdsForUserCache.Get(userID); ok {
-			if s.metrics != nil {
-				s.metrics.IncrementMemCacheHitCounter("All Team Ids for User")
-			}
-			return cacheItem.([]string), nil
-		}
-	}
-
-	if s.metrics != nil {
-		s.metrics.IncrementMemCacheMissCounter("All Team Ids for User")
-	}
-
 	var teamIds []string
 	_, err := s.GetReplica().Select(&teamIds,
 		`SELECT
@@ -1067,9 +1031,6 @@ func (s SqlTeamStore) GetUserTeamIds(userID string, allowFromCache bool) ([]stri
 		return []string{}, model.NewAppError("SqlTeamStore.GetUserTeamIds", "store.sql_team.get_user_team_ids.app_error", nil, "userID="+userID+" "+err.Error(), http.StatusInternalServerError)
 	}
 
-	if allowFromCache {
-		allTeamIdsForUserCache.AddWithExpiresInSecs(userID, teamIds, ALL_TEAM_IDS_FOR_USER_CACHE_SEC)
-	}
 	return teamIds, nil
 }