IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

Migrating file info store cache to the cache layer (#13045)

Browse Source 
* Migrating file info store cache to the cache layer

* Remove unrelated changes

* Updating timer layer store

* Fixing usage of invalidate cache

* Removed repeated tests and unnecesiary require

* Using doInvalidateCacheCluster instead of removing the cache directly

* Addressing PR comments

* Fixing imports

* Fixing tests

* Fixing license header
This commit is contained in:
Jesús Espino
2020-03-09 18:34:25 +01:00
committed by GitHub
parent b70d5df5c2
commit cd382412fd
11 changed files with 167 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/post.go
View File

@@ -1141,7 +1141,8 @@ func (a *App) GetFileInfosForPostWithMigration(postId string) ([]*model.FileInfo
post := result.Data.(*model.Post)
if len(post.Filenames) > 0 {
a.Srv().Store.FileInfo().InvalidateFileInfosForPostCache(postId)
a.Srv().Store.FileInfo().InvalidateFileInfosForPostCache(postId, false)
a.Srv().Store.FileInfo().InvalidateFileInfosForPostCache(postId, true)
// The post has Filenames that need to be replaced with FileInfos
infos = a.MigrateFilenamesToFileInfos(post)
}

1
model/cluster_message.go
View File

@@ -27,6 +27,7 @@ const (
CLUSTER_EVENT_INVALIDATE_CACHE_FOR_PROFILE_BY_IDS = "inv_profile_ids"
CLUSTER_EVENT_INVALIDATE_CACHE_FOR_PROFILE_IN_CHANNEL = "inv_profile_in_channel"
CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES = "inv_schemes"
CLUSTER_EVENT_INVALIDATE_CACHE_FOR_FILE_INFOS = "inv_file_infos"
CLUSTER_EVENT_INVALIDATE_CACHE_FOR_WEBHOOKS = "inv_webhooks"
CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_BY_ID = "inv_emojis_by_id"
CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_ID_BY_NAME = "inv_emojis_id_by_name"

66
store/localcachelayer/file_info_layer.go Normal file
View File

@@ -0,0 +1,66 @@
// Copyright (c) 2015-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 LocalCacheFileInfoStore struct {
store.FileInfoStore
rootStore *LocalCacheStore
}
func (s *LocalCacheFileInfoStore) handleClusterInvalidateFileInfo(msg *model.ClusterMessage) {
if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
s.rootStore.fileInfoCache.Purge()
return
}
s.rootStore.fileInfoCache.Remove(msg.Data)
}
func (s LocalCacheFileInfoStore) GetForPost(postId string, readFromMaster, includeDeleted, allowFromCache bool) ([]*model.FileInfo, *model.AppError) {
if !allowFromCache {
return s.FileInfoStore.GetForPost(postId, readFromMaster, includeDeleted, allowFromCache)
}
cacheKey := postId
if includeDeleted {
cacheKey += "_deleted"
}
if fileInfo := s.rootStore.doStandardReadCache(s.rootStore.fileInfoCache, cacheKey); fileInfo != nil {
return fileInfo.([]*model.FileInfo), nil
}
fileInfos, err := s.FileInfoStore.GetForPost(postId, readFromMaster, includeDeleted, allowFromCache)
if err != nil {
return nil, err
}
if len(fileInfos) > 0 {
s.rootStore.doStandardAddToCache(s.rootStore.fileInfoCache, cacheKey, fileInfos)
}
return fileInfos, nil
}
func (s LocalCacheFileInfoStore) ClearCaches() {
s.rootStore.fileInfoCache.Purge()
if s.rootStore.metrics != nil {
s.rootStore.metrics.IncrementMemCacheInvalidationCounter("File Info Cache - Purge")
}
}
func (s LocalCacheFileInfoStore) InvalidateFileInfosForPostCache(postId string, deleted bool) {
cacheKey := postId
if deleted {
cacheKey += "_deleted"
}
s.rootStore.doInvalidateCacheCluster(s.rootStore.fileInfoCache, cacheKey)
if s.rootStore.metrics != nil {
s.rootStore.metrics.IncrementMemCacheInvalidationCounter("File Info Cache - Remove by PostId")
}
}

59
store/localcachelayer/file_info_layer_test.go Normal file
View File

@@ -0,0 +1,59 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package localcachelayer
import (
"testing"
"github.com/mattermost/mattermost-server/v5/model"
"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 TestFileInfoStore(t *testing.T) {
StoreTest(t, storetest.TestFileInfoStore)
}
func TestFileInfoStoreCache(t *testing.T) {
fakeFileInfo := model.FileInfo{PostId: "123"}
t.Run("first call not cached, second cached and returning same data", func(t *testing.T) {
mockStore := getMockStore()
mockCacheProvider := getMockCacheProvider()
cachedStore := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
fileInfos, err := cachedStore.FileInfo().GetForPost("123", true, true, true)
require.Nil(t, err)
assert.Equal(t, fileInfos, []*model.FileInfo{&fakeFileInfo})
mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 1)
assert.Equal(t, fileInfos, []*model.FileInfo{&fakeFileInfo})
cachedStore.FileInfo().GetForPost("123", true, true, true)
mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 1)
})
t.Run("first call not cached, second force no cached", func(t *testing.T) {
mockStore := getMockStore()
mockCacheProvider := getMockCacheProvider()
cachedStore := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
cachedStore.FileInfo().GetForPost("123", true, true, true)
mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 1)
cachedStore.FileInfo().GetForPost("123", true, true, false)
mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 2)
})
t.Run("first call not cached, invalidate, and then not cached again", func(t *testing.T) {
mockStore := getMockStore()
mockCacheProvider := getMockCacheProvider()
cachedStore := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
cachedStore.FileInfo().GetForPost("123", true, true, true)
mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 1)
cachedStore.FileInfo().InvalidateFileInfosForPostCache("123", true)
cachedStore.FileInfo().GetForPost("123", true, true, true)
mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 2)
})
}

18
store/localcachelayer/layer.go
View File

@@ -20,6 +20,9 @@ const (
SCHEME_CACHE_SIZE = 20000
SCHEME_CACHE_SEC = 30 * 60
FILE_INFO_CACHE_SIZE = 25000
FILE_INFO_CACHE_SEC = 30 * 60
CHANNEL_GUEST_COUNT_CACHE_SIZE = model.CHANNEL_CACHE_SIZE
CHANNEL_GUEST_COUNT_CACHE_SEC = 30 * 60
@@ -65,6 +68,9 @@ type LocalCacheStore struct {
reaction LocalCacheReactionStore
reactionCache cache.Cache
fileInfo LocalCacheFileInfoStore
fileInfoCache cache.Cache
role LocalCacheRoleStore
roleCache cache.Cache
@@ -118,6 +124,10 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
localCacheStore.schemeCache = cacheProvider.NewCacheWithParams(SCHEME_CACHE_SIZE, "Scheme", SCHEME_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES)
localCacheStore.scheme = LocalCacheSchemeStore{SchemeStore: baseStore.Scheme(), rootStore: &localCacheStore}
// FileInfo
localCacheStore.fileInfoCache = cacheProvider.NewCacheWithParams(FILE_INFO_CACHE_SIZE, "FileInfo", FILE_INFO_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_FILE_INFOS)
localCacheStore.fileInfo = LocalCacheFileInfoStore{FileInfoStore: baseStore.FileInfo(), rootStore: &localCacheStore}
// Webhooks
localCacheStore.webhookCache = cacheProvider.NewCacheWithParams(WEBHOOK_CACHE_SIZE, "Webhook", WEBHOOK_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_WEBHOOKS)
localCacheStore.webhook = LocalCacheWebhookStore{WebhookStore: baseStore.Webhook(), rootStore: &localCacheStore}
@@ -156,6 +166,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
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_FILE_INFOS, localCacheStore.fileInfo.handleClusterInvalidateFileInfo)
cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_LAST_POST_TIME, localCacheStore.post.handleClusterInvalidateLastPostTime)
cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_WEBHOOKS, localCacheStore.webhook.handleClusterInvalidateWebhook)
cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_BY_ID, localCacheStore.emoji.handleClusterInvalidateEmojiById)
@@ -185,6 +196,10 @@ func (s LocalCacheStore) Scheme() store.SchemeStore {
return s.scheme
}
func (s LocalCacheStore) FileInfo() store.FileInfoStore {
return s.fileInfo
}
func (s LocalCacheStore) Webhook() store.WebhookStore {
return s.webhook
}
@@ -263,6 +278,9 @@ func (s *LocalCacheStore) doClearCacheCluster(cache cache.Cache) {
func (s *LocalCacheStore) Invalidate() {
s.doClearCacheCluster(s.reactionCache)
s.doClearCacheCluster(s.schemeCache)
s.doClearCacheCluster(s.roleCache)
s.doClearCacheCluster(s.fileInfoCache)
s.doClearCacheCluster(s.webhookCache)
s.doClearCacheCluster(s.emojiCacheById)
s.doClearCacheCluster(s.emojiIdCacheByName)

12
store/localcachelayer/main_test.go
View File

@@ -116,6 +116,12 @@ func getMockCacheProvider() *mocks.CacheProvider {
mock.AnythingOfType("int64"),
mock.AnythingOfType("string")).Return(lru.New(128))
mockCacheProvider.On("NewCacheWithParams",
mock.AnythingOfType("int"),
"FileInfo",
mock.AnythingOfType("int64"),
mock.AnythingOfType("string")).Return(lru.New(128))
return &mockCacheProvider
}
@@ -147,6 +153,12 @@ func getMockStore() *mocks.Store {
mockSchemesStore.On("PermanentDeleteAll").Return(nil)
mockStore.On("Scheme").Return(&mockSchemesStore)
fakeFileInfo := model.FileInfo{PostId: "123"}
mockFileInfoStore := mocks.FileInfoStore{}
mockFileInfoStore.On("GetForPost", "123", true, true, false).Return([]*model.FileInfo{&fakeFileInfo}, nil)
mockFileInfoStore.On("GetForPost", "123", true, true, true).Return([]*model.FileInfo{&fakeFileInfo}, nil)
mockStore.On("FileInfo").Return(&mockFileInfoStore)
fakeWebhook := model.IncomingWebhook{Id: "123"}
mockWebhookStore := mocks.WebhookStore{}
mockWebhookStore.On("GetIncoming", "123", true).Return(&fakeWebhook, nil)

4
store/opentracing_layer.go
View File

@@ -2694,7 +2694,7 @@ func (s *OpenTracingLayerFileInfoStore) GetWithOptions(page int, perPage int, op
return resultVar0, resultVar1
}
func (s *OpenTracingLayerFileInfoStore) InvalidateFileInfosForPostCache(postId string) {
func (s *OpenTracingLayerFileInfoStore) InvalidateFileInfosForPostCache(postId string, deleted bool) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.InvalidateFileInfosForPostCache")
s.Root.Store.SetContext(newCtx)
@@ -2703,7 +2703,7 @@ func (s *OpenTracingLayerFileInfoStore) InvalidateFileInfosForPostCache(postId s
}()
defer span.Finish()
s.FileInfoStore.InvalidateFileInfosForPostCache(postId)
s.FileInfoStore.InvalidateFileInfosForPostCache(postId, deleted)
}

44
store/sqlstore/file_info_store.go
View File

@@ -12,8 +12,6 @@ import (
"github.com/mattermost/mattermost-server/v5/einterfaces"
"github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/services/cache"
"github.com/mattermost/mattermost-server/v5/services/cache/lru"
"github.com/mattermost/mattermost-server/v5/store"
)
@@ -22,18 +20,7 @@ type SqlFileInfoStore struct {
metrics einterfaces.MetricsInterface
}
const (
FILE_INFO_CACHE_SIZE = 25000
FILE_INFO_CACHE_SEC = 1800 // 30 minutes
)
var fileInfoCache cache.Cache = lru.New(FILE_INFO_CACHE_SIZE)
func (fs SqlFileInfoStore) ClearCaches() {
fileInfoCache.Purge()
if fs.metrics != nil {
fs.metrics.IncrementMemCacheInvalidationCounter("File Info Cache - Purge")
}
}
func newSqlFileInfoStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface) store.FileInfoStore {
@@ -182,35 +169,10 @@ func (fs SqlFileInfoStore) GetByPath(path string) (*model.FileInfo, *model.AppEr
return info, nil
}
func (fs SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string) {
fileInfoCache.Remove(postId)
if fs.metrics != nil {
fs.metrics.IncrementMemCacheInvalidationCounter("File Info Cache - Remove by PostId")
}
func (fs SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string, deleted bool) {
}
func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster, includeDeleted, allowFromCache bool) ([]*model.FileInfo, *model.AppError) {
cacheKey := postId
if includeDeleted {
cacheKey += "_deleted"
}
if allowFromCache {
if cacheItem, ok := fileInfoCache.Get(cacheKey); ok {
if fs.metrics != nil {
fs.metrics.IncrementMemCacheHitCounter("File Info Cache")
}
return cacheItem.([]*model.FileInfo), nil
}
if fs.metrics != nil {
fs.metrics.IncrementMemCacheMissCounter("File Info Cache")
}
} else {
if fs.metrics != nil {
fs.metrics.IncrementMemCacheMissCounter("File Info Cache")
}
}
var infos []*model.FileInfo
dbmap := fs.GetReplica()
@@ -238,10 +200,6 @@ func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster, includeDele
return nil, model.NewAppError("SqlFileInfoStore.GetForPost",
"store.sql_file_info.get_for_post.app_error", nil, "post_id="+postId+", "+err.Error(), http.StatusInternalServerError)
}
if len(infos) > 0 {
fileInfoCache.AddWithExpiresInSecs(cacheKey, infos, FILE_INFO_CACHE_SEC)
}
return infos, nil
}

2
store/store.go
View File

@@ -500,7 +500,7 @@ type FileInfoStore interface {
GetForPost(postId string, readFromMaster, includeDeleted, allowFromCache bool) ([]*model.FileInfo, *model.AppError)
GetForUser(userId string) ([]*model.FileInfo, *model.AppError)
GetWithOptions(page, perPage int, opt *model.GetFileInfosOptions) ([]*model.FileInfo, *model.AppError)
InvalidateFileInfosForPostCache(postId string)
InvalidateFileInfosForPostCache(postId string, deleted bool)
AttachToPost(fileId string, postId string, creatorId string) *model.AppError
DeleteForPost(postId string) (string, *model.AppError)
PermanentDelete(fileId string) *model.AppError

6
store/storetest/mocks/FileInfoStore.go
View File

@@ -183,9 +183,9 @@ func (_m *FileInfoStore) GetWithOptions(page int, perPage int, opt *model.GetFil
return r0, r1
}
// InvalidateFileInfosForPostCache provides a mock function with given fields: postId
func (_m *FileInfoStore) InvalidateFileInfosForPostCache(postId string) {
_m.Called(postId)
// InvalidateFileInfosForPostCache provides a mock function with given fields: postId, deleted
func (_m *FileInfoStore) InvalidateFileInfosForPostCache(postId string, deleted bool) {
_m.Called(postId, deleted)
}
// PermanentDelete provides a mock function with given fields: fileId

4
store/timer_layer.go
View File

@@ -2479,10 +2479,10 @@ func (s *TimerLayerFileInfoStore) GetWithOptions(page int, perPage int, opt *mod
return resultVar0, resultVar1
}
func (s *TimerLayerFileInfoStore) InvalidateFileInfosForPostCache(postId string) {
func (s *TimerLayerFileInfoStore) InvalidateFileInfosForPostCache(postId string, deleted bool) {
start := timemodule.Now()
s.FileInfoStore.InvalidateFileInfosForPostCache(postId)
s.FileInfoStore.InvalidateFileInfosForPostCache(postId, deleted)
elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
if s.Root.Metrics != nil {