Skip to content

Commit

Permalink
Migrating team store cache to cache layer (mattermost#13075)
Browse files Browse the repository at this point in the history
Automatic Merge
  • Loading branch information
rvillablanca authored and mattermod committed Nov 29, 2019
1 parent e2a2a1a commit e034117
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 45 deletions.
1 change: 1 addition & 0 deletions model/cluster_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 13 additions & 0 deletions store/localcachelayer/layer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
)

Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -201,4 +213,5 @@ func (s *LocalCacheStore) Invalidate() {
s.doClearCacheCluster(s.channelGuestCountCache)
s.doClearCacheCluster(s.postLastPostsCache)
s.doClearCacheCluster(s.userProfileByIdsCache)
s.doClearCacheCluster(s.teamAllTeamIdsForUserCache)
}
6 changes: 6 additions & 0 deletions store/localcachelayer/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
79 changes: 79 additions & 0 deletions store/localcachelayer/team_layer.go
Original file line number Diff line number Diff line change
@@ -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
}
70 changes: 70 additions & 0 deletions store/localcachelayer/team_layer_test.go
Original file line number Diff line number Diff line change
@@ -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)
})

}
2 changes: 1 addition & 1 deletion store/sqlstore/supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 5 additions & 44 deletions store/sqlstore/team_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
ALL_TEAM_IDS_FOR_USER_CACHE_SIZE = model.SESSION_CACHE_SIZE
ALL_TEAM_IDS_FOR_USER_CACHE_SEC = 1800 // 30 mins
TEAM_MEMBER_EXISTS_ERROR = "store.sql_team.save_member.exists.app_error"
)

type SqlTeamStore struct {
SqlStore
metrics einterfaces.MetricsInterface
}

type teamMember struct {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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() {
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) InvalidateAllTeamIdsForUser(userId string) {}

func (s SqlTeamStore) ClearAllCustomRoleAssignments() *model.AppError {

Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down

0 comments on commit e034117

Please sign in to comment.