From 487bb56a9b8f5c7a9efaabfc631f2f6c689ef74b Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Tue, 7 Feb 2017 12:36:37 -0800 Subject: Add caching for file infos (#5330) --- api/post_test.go | 4 +++- app/file.go | 2 +- app/notification.go | 4 ++-- app/post.go | 3 ++- store/sql_file_info_store.go | 43 ++++++++++++++++++++++++++++++++++++++- store/sql_file_info_store_test.go | 12 ++++++++--- store/store.go | 3 ++- 7 files changed, 61 insertions(+), 10 deletions(-) diff --git a/api/post_test.go b/api/post_test.go index d382786cc..c97da6f68 100644 --- a/api/post_test.go +++ b/api/post_test.go @@ -136,7 +136,7 @@ func TestCreatePost(t *testing.T) { } else if rpost9 := resp.Data.(*model.Post); len(rpost9.FileIds) != 3 { t.Fatal("post should have 3 files") } else { - infos := store.Must(app.Srv.Store.FileInfo().GetForPost(rpost9.Id)).([]*model.FileInfo) + infos := store.Must(app.Srv.Store.FileInfo().GetForPost(rpost9.Id, true)).([]*model.FileInfo) if len(infos) != 3 { t.Fatal("should've attached all 3 files to post") @@ -1184,6 +1184,7 @@ func TestGetMessageForNotification(t *testing.T) { post.FileIds = model.StringArray{testPng.Id, testJpg1.Id} store.Must(app.Srv.Store.FileInfo().AttachToPost(testJpg1.Id, post.Id)) + app.Srv.Store.FileInfo().InvalidateFileInfosForPostCache(post.Id) if message := app.GetMessageForNotification(post, translateFunc); message != "2 images sent: test1.png, test2.jpg" && message != "2 images sent: test2.jpg, test1.png" { t.Fatal("should've returned number of images:", message) } @@ -1196,6 +1197,7 @@ func TestGetMessageForNotification(t *testing.T) { } store.Must(app.Srv.Store.FileInfo().AttachToPost(testJpg2.Id, post.Id)) + app.Srv.Store.FileInfo().InvalidateFileInfosForPostCache(post.Id) post.FileIds = model.StringArray{testFile.Id, testJpg2.Id} if message := app.GetMessageForNotification(post, translateFunc); message != "2 files sent: test1.go, test3.jpg" && message != "2 files sent: test3.jpg, test1.go" { t.Fatal("should've returned number of mixed files:", message) diff --git a/app/file.go b/app/file.go index 4ddf7ac2d..095e4d032 100644 --- a/app/file.go +++ b/app/file.go @@ -318,7 +318,7 @@ func MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo { return []*model.FileInfo{} } else if newPost := result.Data.(*model.PostList).Posts[post.Id]; len(newPost.Filenames) != len(post.Filenames) { // Another thread has already created FileInfos for this post, so just return those - if result := <-Srv.Store.FileInfo().GetForPost(post.Id); result.Err != nil { + if result := <-Srv.Store.FileInfo().GetForPost(post.Id, false); result.Err != nil { l4g.Error(utils.T("api.file.migrate_filenames_to_file_infos.get_post_file_infos_again.app_error"), post.Id, result.Err) return []*model.FileInfo{} } else { diff --git a/app/notification.go b/app/notification.go index cc4e13ab1..1550c0393 100644 --- a/app/notification.go +++ b/app/notification.go @@ -26,7 +26,7 @@ import ( func SendNotifications(post *model.Post, team *model.Team, channel *model.Channel, sender *model.User) ([]string, *model.AppError) { pchan := Srv.Store.User().GetAllProfilesInChannel(channel.Id, true) - fchan := Srv.Store.FileInfo().GetForPost(post.Id) + fchan := Srv.Store.FileInfo().GetForPost(post.Id, true) var profileMap map[string]*model.User if result := <-pchan; result.Err != nil { @@ -414,7 +414,7 @@ func GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFun // extract the filenames from their paths and determine what type of files are attached var infos []*model.FileInfo - if result := <-Srv.Store.FileInfo().GetForPost(post.Id); result.Err != nil { + if result := <-Srv.Store.FileInfo().GetForPost(post.Id, true); result.Err != nil { l4g.Warn(utils.T("api.post.get_message_for_notification.get_files.error"), post.Id, result.Err) } else { infos = result.Data.([]*model.FileInfo) diff --git a/app/post.go b/app/post.go index 82fc733b4..5fddd3e78 100644 --- a/app/post.go +++ b/app/post.go @@ -457,7 +457,7 @@ func SearchPostsInTeam(terms string, userId string, teamId string, isOrSearch bo func GetFileInfosForPost(postId string) ([]*model.FileInfo, *model.AppError) { pchan := Srv.Store.Post().Get(postId) - fchan := Srv.Store.FileInfo().GetForPost(postId) + fchan := Srv.Store.FileInfo().GetForPost(postId, true) var infos []*model.FileInfo if result := <-fchan; result.Err != nil { @@ -476,6 +476,7 @@ func GetFileInfosForPost(postId string) ([]*model.FileInfo, *model.AppError) { } if len(post.Filenames) > 0 { + Srv.Store.FileInfo().InvalidateFileInfosForPostCache(postId) // The post has Filenames that need to be replaced with FileInfos infos = MigrateFilenamesToFileInfos(post) } diff --git a/store/sql_file_info_store.go b/store/sql_file_info_store.go index 762eac5d4..b1ad4b11a 100644 --- a/store/sql_file_info_store.go +++ b/store/sql_file_info_store.go @@ -3,13 +3,26 @@ package store import ( + "github.com/mattermost/platform/einterfaces" "github.com/mattermost/platform/model" + "github.com/mattermost/platform/utils" ) type SqlFileInfoStore struct { *SqlStore } +const ( + FILE_INFO_CACHE_SIZE = 25000 + FILE_INFO_CACHE_SEC = 900 // 15 minutes +) + +var fileInfoCache *utils.Cache = utils.NewLru(FILE_INFO_CACHE_SIZE) + +func ClearFileCaches() { + fileInfoCache.Purge() +} + func NewSqlFileInfoStore(sqlStore *SqlStore) FileInfoStore { s := &SqlFileInfoStore{sqlStore} @@ -119,12 +132,39 @@ func (fs SqlFileInfoStore) GetByPath(path string) StoreChannel { return storeChannel } -func (fs SqlFileInfoStore) GetForPost(postId string) StoreChannel { +func (s SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string) { + fileInfoCache.Remove(postId) +} + +func (fs SqlFileInfoStore) GetForPost(postId string, allowFromCache bool) StoreChannel { storeChannel := make(StoreChannel, 1) go func() { result := StoreResult{} + metrics := einterfaces.GetMetricsInterface() + + if allowFromCache { + if cacheItem, ok := fileInfoCache.Get(postId); ok { + if metrics != nil { + metrics.IncrementMemCacheHitCounter("File Info Cache") + } + + result.Data = cacheItem.([]*model.FileInfo) + storeChannel <- result + close(storeChannel) + return + } else { + if metrics != nil { + metrics.IncrementMemCacheMissCounter("File Info Cache") + } + } + } else { + if metrics != nil { + metrics.IncrementMemCacheMissCounter("File Info Cache") + } + } + var infos []*model.FileInfo if _, err := fs.GetReplica().Select(&infos, @@ -140,6 +180,7 @@ func (fs SqlFileInfoStore) GetForPost(postId string) StoreChannel { result.Err = model.NewLocAppError("SqlFileInfoStore.GetForPost", "store.sql_file_info.get_for_post.app_error", nil, "post_id="+postId+", "+err.Error()) } else { + fileInfoCache.AddWithExpiresInSecs(postId, infos, FILE_INFO_CACHE_SEC) result.Data = infos } diff --git a/store/sql_file_info_store_test.go b/store/sql_file_info_store_test.go index edb9dbd54..672e15ef2 100644 --- a/store/sql_file_info_store_test.go +++ b/store/sql_file_info_store_test.go @@ -114,7 +114,13 @@ func TestFileInfoGetForPost(t *testing.T) { infos[i] = Must(store.FileInfo().Save(info)).(*model.FileInfo) } - if result := <-store.FileInfo().GetForPost(postId); result.Err != nil { + if result := <-store.FileInfo().GetForPost(postId, false); result.Err != nil { + t.Fatal(result.Err) + } else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 { + t.Fatal("should've returned exactly 2 file infos") + } + + if result := <-store.FileInfo().GetForPost(postId, true); result.Err != nil { t.Fatal(result.Err) } else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 { t.Fatal("should've returned exactly 2 file infos") @@ -157,7 +163,7 @@ func TestFileInfoAttachToPost(t *testing.T) { info2 = Must(store.FileInfo().Get(info2.Id)).(*model.FileInfo) } - if result := <-store.FileInfo().GetForPost(postId); result.Err != nil { + if result := <-store.FileInfo().GetForPost(postId, false); result.Err != nil { t.Fatal(result.Err) } else if infos := result.Data.([]*model.FileInfo); len(infos) != 2 { t.Fatal("should've returned exactly 2 file infos") @@ -202,7 +208,7 @@ func TestFileInfoDeleteForPost(t *testing.T) { t.Fatal(result.Err) } - if infos := Must(store.FileInfo().GetForPost(postId)).([]*model.FileInfo); len(infos) != 0 { + if infos := Must(store.FileInfo().GetForPost(postId, false)).([]*model.FileInfo); len(infos) != 0 { t.Fatal("shouldn't have returned any file infos") } } diff --git a/store/store.go b/store/store.go index 96d9509b8..faf40c280 100644 --- a/store/store.go +++ b/store/store.go @@ -326,7 +326,8 @@ type FileInfoStore interface { Save(info *model.FileInfo) StoreChannel Get(id string) StoreChannel GetByPath(path string) StoreChannel - GetForPost(postId string) StoreChannel + GetForPost(postId string, allowFromCache bool) StoreChannel + InvalidateFileInfosForPostCache(postId string) AttachToPost(fileId string, postId string) StoreChannel DeleteForPost(postId string) StoreChannel } -- cgit v1.2.3-1-g7c22