From 6a5dd550c8a4366fea81b9a44a70e09b0d4a0629 Mon Sep 17 00:00:00 2001 From: Siyuan Liu Date: Tue, 2 Jun 2020 09:01:30 -0400 Subject: [PATCH] migrate direct usage of lru to cache2 (#14508) Automatic Merge --- api4/openGraph.go | 16 ++++++---- api4/system.go | 17 +++++----- app/post_metadata.go | 37 +++++++++++----------- store/sqlstore/channel_store.go | 56 +++++++++++++++++++-------------- 4 files changed, 71 insertions(+), 55 deletions(-) diff --git a/api4/openGraph.go b/api4/openGraph.go index 4879e10fa6..a822318f2a 100644 --- a/api4/openGraph.go +++ b/api4/openGraph.go @@ -5,14 +5,17 @@ package api4 import ( "net/http" + "time" "github.com/mattermost/mattermost-server/v5/model" - "github.com/mattermost/mattermost-server/v5/services/cache/lru" + "github.com/mattermost/mattermost-server/v5/services/cache2" ) const OPEN_GRAPH_METADATA_CACHE_SIZE = 10000 -var openGraphDataCache = lru.New(OPEN_GRAPH_METADATA_CACHE_SIZE) +var openGraphDataCache = cache2.NewLRU(&cache2.LRUOptions{ + Size: OPEN_GRAPH_METADATA_CACHE_SIZE, +}) func (api *API) InitOpenGraph() { api.BaseRoutes.OpenGraph.Handle("", api.ApiSessionRequired(getOpenGraphMetadata)).Methods("POST") @@ -43,15 +46,16 @@ func getOpenGraphMetadata(c *Context, w http.ResponseWriter, r *http.Request) { return } - ogJSONGeneric, ok := openGraphDataCache.Get(url) - if ok { - w.Write(ogJSONGeneric.([]byte)) + var ogJSONGeneric []byte + err := openGraphDataCache.Get(url, &ogJSONGeneric) + if err == nil { + w.Write(ogJSONGeneric) return } og := c.App.GetOpenGraphMetadata(url) ogJSON, err := og.ToJSON() - openGraphDataCache.AddWithExpiresInSecs(url, ogJSON, 3600) // Cache would expire after 1 hour + openGraphDataCache.SetWithExpiry(url, ogJSON, 1*time.Hour) if err != nil { w.Write([]byte(`{"url": ""}`)) return diff --git a/api4/system.go b/api4/system.go index 680ae85a47..1ed4ff731e 100644 --- a/api4/system.go +++ b/api4/system.go @@ -16,7 +16,7 @@ import ( "github.com/mattermost/mattermost-server/v5/audit" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" - "github.com/mattermost/mattermost-server/v5/services/cache/lru" + "github.com/mattermost/mattermost-server/v5/services/cache2" "github.com/mattermost/mattermost-server/v5/services/filesstore" ) @@ -26,7 +26,9 @@ const ( MAX_SERVER_BUSY_SECONDS = 86400 ) -var redirectLocationDataCache = lru.New(REDIRECT_LOCATION_CACHE_SIZE) +var redirectLocationDataCache = cache2.NewLRU(&cache2.LRUOptions{ + Size: REDIRECT_LOCATION_CACHE_SIZE, +}) func (api *API) InitSystem() { api.BaseRoutes.System.Handle("/ping", api.ApiHandler(getSystemPing)).Methods("GET") @@ -405,8 +407,9 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) { return } - if location, ok := redirectLocationDataCache.Get(url); ok { - m["location"] = location.(string) + var location string + if err := redirectLocationDataCache.Get(url, &location); err == nil { + m["location"] = location w.Write([]byte(model.MapToJson(m))) return } @@ -419,7 +422,7 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) { res, err := client.Head(url) if err != nil { // Cache failures to prevent retries. - redirectLocationDataCache.AddWithExpiresInSecs(url, "", 3600) // Expires after 1 hour + redirectLocationDataCache.SetWithExpiry(url, "", 1*time.Hour) // Always return a success status and a JSON string to limit information returned to client. w.Write([]byte(model.MapToJson(m))) return @@ -429,8 +432,8 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) { res.Body.Close() }() - location := res.Header.Get("Location") - redirectLocationDataCache.AddWithExpiresInSecs(url, location, 3600) // Expires after 1 hour + location = res.Header.Get("Location") + redirectLocationDataCache.SetWithExpiry(url, location, 1*time.Hour) m["location"] = location w.Write([]byte(model.MapToJson(m))) diff --git a/app/post_metadata.go b/app/post_metadata.go index c2efe89542..b23051074b 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -17,16 +17,23 @@ import ( "github.com/dyatlov/go-opengraph/opengraph" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" - "github.com/mattermost/mattermost-server/v5/services/cache/lru" + "github.com/mattermost/mattermost-server/v5/services/cache2" "github.com/mattermost/mattermost-server/v5/utils/imgutils" "github.com/mattermost/mattermost-server/v5/utils/markdown" ) +type linkMetadataCache struct { + OpenGraph *opengraph.OpenGraph + PostImage *model.PostImage +} + const LINK_CACHE_SIZE = 10000 -const LINK_CACHE_DURATION = 3600 +const LINK_CACHE_DURATION = 1 * time.Hour const MaxMetadataImageSize = MaxOpenGraphResponseSize -var linkCache = lru.New(LINK_CACHE_SIZE) +var linkCache = cache2.NewLRU(&cache2.LRUOptions{ + Size: LINK_CACHE_SIZE, +}) func (a *App) InitPostMetadata() { // Dump any cached links if the proxy settings have changed so image URLs can be updated @@ -439,19 +446,13 @@ func resolveMetadataURL(requestURL string, siteURL string) string { } func getLinkMetadataFromCache(requestURL string, timestamp int64) (*opengraph.OpenGraph, *model.PostImage, bool) { - cached, ok := linkCache.Get(strconv.FormatInt(model.GenerateLinkMetadataHash(requestURL, timestamp), 16)) - if !ok { + var cached linkMetadataCache + err := linkCache.Get(strconv.FormatInt(model.GenerateLinkMetadataHash(requestURL, timestamp), 16), &cached) + if err != nil { return nil, nil, false } - switch v := cached.(type) { - case *opengraph.OpenGraph: - return v, nil, true - case *model.PostImage: - return nil, v, true - default: - return nil, nil, true - } + return cached.OpenGraph, cached.PostImage, true } func (a *App) getLinkMetadataFromDatabase(requestURL string, timestamp int64) (*opengraph.OpenGraph, *model.PostImage, bool) { @@ -495,14 +496,12 @@ func (a *App) saveLinkMetadataToDatabase(requestURL string, timestamp int64, og } func cacheLinkMetadata(requestURL string, timestamp int64, og *opengraph.OpenGraph, image *model.PostImage) { - var val interface{} - if og != nil { - val = og - } else if image != nil { - val = image + metadata := linkMetadataCache{ + OpenGraph: og, + PostImage: image, } - linkCache.AddWithExpiresInSecs(strconv.FormatInt(model.GenerateLinkMetadataHash(requestURL, timestamp), 16), val, LINK_CACHE_DURATION) + linkCache.SetWithExpiry(strconv.FormatInt(model.GenerateLinkMetadataHash(requestURL, timestamp), 16), metadata, LINK_CACHE_DURATION) } func (a *App) parseLinkMetadata(requestURL string, body io.Reader, contentType string) (*opengraph.OpenGraph, *model.PostImage, error) { diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 65518b235e..a1c7bc8eca 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -10,6 +10,7 @@ import ( "sort" "strconv" "strings" + "time" "github.com/mattermost/gorp" "github.com/pkg/errors" @@ -18,18 +19,18 @@ import ( "github.com/mattermost/mattermost-server/v5/einterfaces" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" - "github.com/mattermost/mattermost-server/v5/services/cache/lru" + "github.com/mattermost/mattermost-server/v5/services/cache2" "github.com/mattermost/mattermost-server/v5/store" ) const ( - ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE = model.SESSION_CACHE_SIZE - ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SEC = 900 // 15 mins + ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE = model.SESSION_CACHE_SIZE + ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_DURATION = 15 * time.Minute // 15 mins - 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 + ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE = model.SESSION_CACHE_SIZE + ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_DURATION = 30 * time.Minute // 30 mins - CHANNEL_CACHE_SEC = 900 // 15 mins + CHANNEL_CACHE_DURATION = 15 * time.Minute // 15 mins ) type SqlChannelStore struct { @@ -334,9 +335,15 @@ type publicChannel struct { Purpose string `json:"purpose"` } -var allChannelMembersForUserCache = lru.New(ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE) -var allChannelMembersNotifyPropsForChannelCache = lru.New(ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE) -var channelByNameCache = lru.New(model.CHANNEL_CACHE_SIZE) +var allChannelMembersForUserCache = cache2.NewLRU(&cache2.LRUOptions{ + Size: ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE, +}) +var allChannelMembersNotifyPropsForChannelCache = cache2.NewLRU(&cache2.LRUOptions{ + Size: ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE, +}) +var channelByNameCache = cache2.NewLRU(&cache2.LRUOptions{ + Size: model.CHANNEL_CACHE_SIZE, +}) func (s SqlChannelStore) ClearCaches() { allChannelMembersForUserCache.Purge() @@ -1157,8 +1164,9 @@ func (s SqlChannelStore) GetByNames(teamId string, names []string, allowFromCach continue } visited[name] = struct{}{} - if cacheItem, ok := channelByNameCache.Get(teamId + name); ok { - channels = append(channels, cacheItem.(*model.Channel)) + var cacheItem *model.Channel + if err := channelByNameCache.Get(teamId+name, &cacheItem); err == nil { + channels = append(channels, cacheItem) } else { misses = append(misses, name) } @@ -1188,7 +1196,7 @@ func (s SqlChannelStore) GetByNames(teamId string, names []string, allowFromCach return nil, model.NewAppError("SqlChannelStore.GetByName", "store.sql_channel.get_by_name.existing.app_error", nil, "teamId="+teamId+", "+err.Error(), http.StatusInternalServerError) } for _, channel := range dbChannels { - channelByNameCache.AddWithExpiresInSecs(teamId+channel.Name, channel, CHANNEL_CACHE_SEC) + channelByNameCache.SetWithExpiry(teamId+channel.Name, channel, CHANNEL_CACHE_DURATION) channels = append(channels, channel) } // Not all channels are in cache. Increment aggregate miss counter. @@ -1219,11 +1227,12 @@ func (s SqlChannelStore) getByName(teamId string, name string, includeDeleted bo channel := model.Channel{} if allowFromCache { - if cacheItem, ok := channelByNameCache.Get(teamId + name); ok { + var cacheItem *model.Channel + if err := channelByNameCache.Get(teamId+name, &cacheItem); err == nil { if s.metrics != nil { s.metrics.IncrementMemCacheHitCounter("Channel By Name") } - return cacheItem.(*model.Channel), nil + return cacheItem, nil } if s.metrics != nil { s.metrics.IncrementMemCacheMissCounter("Channel By Name") @@ -1237,7 +1246,7 @@ func (s SqlChannelStore) getByName(teamId string, name string, includeDeleted bo return nil, model.NewAppError("SqlChannelStore.GetByName", "store.sql_channel.get_by_name.existing.app_error", nil, "teamId="+teamId+", "+"name="+name+", "+err.Error(), http.StatusInternalServerError) } - channelByNameCache.AddWithExpiresInSecs(teamId+name, &channel, CHANNEL_CACHE_SEC) + channelByNameCache.SetWithExpiry(teamId+name, &channel, CHANNEL_CACHE_DURATION) return &channel, nil } @@ -1593,11 +1602,11 @@ func (s SqlChannelStore) InvalidateAllChannelMembersForUser(userId string) { } func (s SqlChannelStore) IsUserInChannelUseCache(userId string, channelId string) bool { - if cacheItem, ok := allChannelMembersForUserCache.Get(userId); ok { + var ids map[string]string + if err := allChannelMembersForUserCache.Get(userId, &ids); err == nil { if s.metrics != nil { s.metrics.IncrementMemCacheHitCounter("All Channel Members for User") } - ids := cacheItem.(map[string]string) if _, ok := ids[channelId]; ok { return true } @@ -1660,11 +1669,11 @@ func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCac cache_key += "_deleted" } if allowFromCache { - if cacheItem, ok := allChannelMembersForUserCache.Get(cache_key); ok { + var ids map[string]string + if err := allChannelMembersForUserCache.Get(cache_key, &ids); err == nil { if s.metrics != nil { s.metrics.IncrementMemCacheHitCounter("All Channel Members for User") } - ids := cacheItem.(map[string]string) return ids, nil } } @@ -1710,7 +1719,7 @@ func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCac ids := data.ToMapStringString() if allowFromCache { - allChannelMembersForUserCache.AddWithExpiresInSecs(cache_key, ids, ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SEC) + allChannelMembersForUserCache.SetWithExpiry(cache_key, ids, ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_DURATION) } return ids, nil } @@ -1729,11 +1738,12 @@ type allChannelMemberNotifyProps struct { func (s SqlChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId string, allowFromCache bool) (map[string]model.StringMap, *model.AppError) { if allowFromCache { - if cacheItem, ok := allChannelMembersNotifyPropsForChannelCache.Get(channelId); ok { + var cacheItem map[string]model.StringMap + if err := allChannelMembersNotifyPropsForChannelCache.Get(channelId, &cacheItem); err == nil { if s.metrics != nil { s.metrics.IncrementMemCacheHitCounter("All Channel Members Notify Props for Channel") } - return cacheItem.(map[string]model.StringMap), nil + return cacheItem, nil } } @@ -1756,7 +1766,7 @@ func (s SqlChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId str props[data[i].UserId] = data[i].NotifyProps } - allChannelMembersNotifyPropsForChannelCache.AddWithExpiresInSecs(channelId, props, ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SEC) + allChannelMembersNotifyPropsForChannelCache.SetWithExpiry(channelId, props, ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_DURATION) return props, nil }