diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 22d3d2a2c7..e48dbfdcae 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -98,7 +98,7 @@ func setupTestHelper(dbStore store.Store, searchEngine *searchengine.Broker, ent } if includeCache { // Adds the cache layer to the test store - s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider2) + s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) } th := &TestHelper{ diff --git a/api4/openGraph.go b/api4/openGraph.go index a822318f2a..818d87f7d8 100644 --- a/api4/openGraph.go +++ b/api4/openGraph.go @@ -8,12 +8,12 @@ import ( "time" "github.com/mattermost/mattermost-server/v5/model" - "github.com/mattermost/mattermost-server/v5/services/cache2" + "github.com/mattermost/mattermost-server/v5/services/cache" ) const OPEN_GRAPH_METADATA_CACHE_SIZE = 10000 -var openGraphDataCache = cache2.NewLRU(&cache2.LRUOptions{ +var openGraphDataCache = cache.NewLRU(&cache.LRUOptions{ Size: OPEN_GRAPH_METADATA_CACHE_SIZE, }) diff --git a/api4/system.go b/api4/system.go index bed235090a..f17667ae79 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/cache2" + "github.com/mattermost/mattermost-server/v5/services/cache" "github.com/mattermost/mattermost-server/v5/services/filesstore" ) @@ -26,7 +26,7 @@ const ( MAX_SERVER_BUSY_SECONDS = 86400 ) -var redirectLocationDataCache = cache2.NewLRU(&cache2.LRUOptions{ +var redirectLocationDataCache = cache.NewLRU(&cache.LRUOptions{ Size: REDIRECT_LOCATION_CACHE_SIZE, }) diff --git a/app/helper_test.go b/app/helper_test.go index 4b77a7f38c..273631e677 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -76,7 +76,7 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo if includeCacheLayer { // Adds the cache layer to the test store - s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider2) + s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) } th := &TestHelper{ diff --git a/app/post.go b/app/post.go index fa05e69fd5..7c1f5b9b46 100644 --- a/app/post.go +++ b/app/post.go @@ -15,7 +15,7 @@ import ( "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin" - "github.com/mattermost/mattermost-server/v5/services/cache2" + "github.com/mattermost/mattermost-server/v5/services/cache" "github.com/mattermost/mattermost-server/v5/store" "github.com/mattermost/mattermost-server/v5/utils" ) @@ -121,7 +121,7 @@ func (a *App) deduplicateCreatePost(post *model.Post) (foundPost *model.Post, er // it hasn't previously been seen. var postId string nErr := a.Srv().seenPendingPostIdsCache.Get(post.PendingPostId, &postId) - if nErr == cache2.ErrKeyNotFound { + if nErr == cache.ErrKeyNotFound { a.Srv().seenPendingPostIdsCache.SetWithExpiry(post.PendingPostId, unknownPostId, PENDING_POST_IDS_CACHE_TTL) return nil, nil } diff --git a/app/post_metadata.go b/app/post_metadata.go index e9481a9a7a..069cef01d6 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -17,7 +17,7 @@ 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/cache2" + "github.com/mattermost/mattermost-server/v5/services/cache" "github.com/mattermost/mattermost-server/v5/utils/imgutils" "github.com/mattermost/mattermost-server/v5/utils/markdown" ) @@ -31,7 +31,7 @@ const LINK_CACHE_SIZE = 10000 const LINK_CACHE_DURATION = 1 * time.Hour const MaxMetadataImageSize = MaxOpenGraphResponseSize -var linkCache = cache2.NewLRU(&cache2.LRUOptions{ +var linkCache = cache.NewLRU(&cache.LRUOptions{ Size: LINK_CACHE_SIZE, }) diff --git a/app/server.go b/app/server.go index 03b3ba9e3c..465988fca8 100644 --- a/app/server.go +++ b/app/server.go @@ -36,8 +36,6 @@ import ( "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin" "github.com/mattermost/mattermost-server/v5/services/cache" - "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" "github.com/mattermost/mattermost-server/v5/services/httpservice" "github.com/mattermost/mattermost-server/v5/services/imageproxy" @@ -110,9 +108,9 @@ type Server struct { newStore func() store.Store htmlTemplateWatcher *utils.HTMLTemplateWatcher - sessionCache cache2.Cache - seenPendingPostIdsCache cache2.Cache - statusCache cache2.Cache + sessionCache cache.Cache + seenPendingPostIdsCache cache.Cache + statusCache cache.Cache configListenerId string licenseListenerId string logListenerId string @@ -163,8 +161,6 @@ type Server struct { CacheProvider cache.Provider - CacheProvider2 cache2.Provider - tracer *tracing.Tracer timestampLastDiagnosticSent time.Time } @@ -247,22 +243,18 @@ func NewServer(options ...Option) (*Server, error) { // at the moment we only have this implementation // in the future the cache provider will be built based on the loaded config - s.CacheProvider = new(lru.CacheProvider) - - s.CacheProvider.Connect() - - s.CacheProvider2 = cache2.NewProvider() - if err := s.CacheProvider2.Connect(); err != nil { + s.CacheProvider = cache.NewProvider() + if err := s.CacheProvider.Connect(); err != nil { return nil, errors.Wrapf(err, "Unable to connect to cache provider") } - s.sessionCache = s.CacheProvider2.NewCache(&cache2.CacheOptions{ + s.sessionCache = s.CacheProvider.NewCache(&cache.CacheOptions{ Size: model.SESSION_CACHE_SIZE, }) - s.seenPendingPostIdsCache = s.CacheProvider2.NewCache(&cache2.CacheOptions{ + s.seenPendingPostIdsCache = s.CacheProvider.NewCache(&cache.CacheOptions{ Size: PENDING_POST_IDS_CACHE_SIZE, }) - s.statusCache = s.CacheProvider2.NewCache(&cache2.CacheOptions{ + s.statusCache = s.CacheProvider.NewCache(&cache.CacheOptions{ Size: model.STATUS_CACHE_SIZE, }) @@ -303,7 +295,7 @@ func NewServer(options ...Option) (*Server, error) { s.sqlStore, s.Metrics, s.Cluster, - s.CacheProvider2, + s.CacheProvider, ), s.SearchEngine, s.Config(), @@ -702,11 +694,7 @@ func (s *Server) Shutdown() error { } if s.CacheProvider != nil { - s.CacheProvider.Close() - } - - if s.CacheProvider2 != nil { - if err = s.CacheProvider2.Close(); err != nil { + if err = s.CacheProvider.Close(); err != nil { mlog.Error("Unable to cleanly shutdown cache", mlog.Err(err)) } } diff --git a/migrations/helper_test.go b/migrations/helper_test.go index b9cde39607..89c6463206 100644 --- a/migrations/helper_test.go +++ b/migrations/helper_test.go @@ -47,7 +47,7 @@ func setupTestHelper(enterprise bool) *TestHelper { panic(err) } // Adds the cache layer to the test store - s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider2) + s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) th := &TestHelper{ App: app.New(app.ServerConnector(s)), diff --git a/services/cache/cache.go b/services/cache/cache.go index da3581f6bd..59da07e525 100644 --- a/services/cache/cache.go +++ b/services/cache/cache.go @@ -4,57 +4,46 @@ package cache import ( + "errors" "time" ) -// Cache is a representation of any cache store that has keys and values +// ErrKeyNotFound is the error when the given key is not found +var ErrKeyNotFound = errors.New("key not found") + +// Cache is a representation of a cache store that aims to replace cache.Cache type Cache interface { // Purge is used to completely clear the cache. - Purge() + Purge() error - // Add adds the given key and value to the store without an expiry. - Add(key string, value interface{}) + // Set adds the given key and value to the store without an expiry. If the key already exists, + // it will overwrite the previous value. + Set(key string, value interface{}) error - // AddWithDefaultExpires adds the given key and value to the store with the default expiry. - AddWithDefaultExpires(key string, value interface{}) + // SetWithDefaultExpiry adds the given key and value to the store with the default expiry. If + // the key already exists, it will overwrite the previoous value + SetWithDefaultExpiry(key string, value interface{}) error - // AddWithExpiresInSecs adds the given key and value to the cache with the given expiry. - AddWithExpiresInSecs(key string, value interface{}, expireAtSecs int64) + // SetWithExpiry adds the given key and value to the cache with the given expiry. If the key + // already exists, it will overwrite the previoous value + SetWithExpiry(key string, value interface{}, ttl time.Duration) error - // Get returns the value stored in the cache for a key, or nil if no value is present. The ok result indicates whether value was found in the cache. - Get(key string) (value interface{}, ok bool) + // Get the content stored in the cache for the given key, and decode it into the value interface. + // Return ErrKeyNotFound if the key is missing from the cache + Get(key string, value interface{}) error - // GetOrAdd returns the existing value for the key if present. Otherwise, it stores and returns the given value. The loaded result is true if the value was loaded, false if stored. - // This API intentionally deviates from the Add-only variants above for simplicity. We should simplify the entire API in the future. - GetOrAdd(key string, value interface{}, ttl time.Duration) (actual interface{}, loaded bool) - - // Remove deletes the value for a key. - Remove(key string) + // Remove deletes the value for a given key. + Remove(key string) error // Keys returns a slice of the keys in the cache. - Keys() []string + Keys() ([]string, error) // Len returns the number of items in the cache. - Len() int - - // Name identifies this cache instance among others in the system. - Name() string + Len() (int, error) // GetInvalidateClusterEvent returns the cluster event configured when this cache was created. GetInvalidateClusterEvent() string -} - -// Provider defines how to create new caches -type Provider interface { - // Connect opens a new connection to the cache using specific provider parameters. - Connect() - - // NewCache creates a new cache with given size. - NewCache(size int) Cache - - // NewCacheWithParams creates a new cache with the given parameters. - NewCacheWithParams(size int, name string, defaultExpiry int64, invalidateClusterEvent string) Cache - - // Close releases any resources used by the cache provider. - Close() + + // Name returns the name of the cache + Name() string } diff --git a/services/cache2/lru.go b/services/cache/lru.go similarity index 99% rename from services/cache2/lru.go rename to services/cache/lru.go index b059149e17..b28cfa27ba 100644 --- a/services/cache2/lru.go +++ b/services/cache/lru.go @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -package cache2 +package cache import ( "container/list" diff --git a/services/cache/lru/lru.go b/services/cache/lru/lru.go deleted file mode 100644 index a527c05636..0000000000 --- a/services/cache/lru/lru.go +++ /dev/null @@ -1,232 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -// This files was copied/modified from https://github.com/hashicorp/golang-lru -// which was (see below) - -// This package provides a simple LRU cache. It is based on the -// LRU implementation in groupcache: -// https://github.com/golang/groupcache/tree/master/lru - -package lru - -import ( - "container/list" - "sync" - "time" - - "github.com/mattermost/mattermost-server/v5/services/cache" -) - -// Cache is a thread-safe fixed size LRU cache. -type Cache struct { - size int - evictList *list.List - items map[string]*list.Element - lock sync.RWMutex - name string - defaultExpiry int64 - invalidateClusterEvent string - currentGeneration int64 - len int -} - -// CacheProvider is an implementation of cache.Provider to create a new Lru Cache -type CacheProvider struct{} - -// NewCache creates a new lru.Cache with given size. -func (c *CacheProvider) NewCache(size int) cache.Cache { - return New(size) -} - -// NewCacheWithParams creates a new lru.Cache with the given parameters. -func (c *CacheProvider) NewCacheWithParams(size int, name string, defaultExpiry int64, invalidateClusterEvent string) cache.Cache { - return NewWithParams(size, name, defaultExpiry, invalidateClusterEvent) -} - -// Connect opens a new connection to the cache using specific provider parameters. -func (c *CacheProvider) Connect() { - -} - -// Close releases any resources used by the cache provider. -func (c *CacheProvider) Close() { - -} - -// entry is used to hold a value in the evictList. -type entry struct { - key string - value interface{} - expires time.Time - generation int64 -} - -// New creates an LRU of the given size. -func New(size int) *Cache { - return &Cache{ - size: size, - evictList: list.New(), - items: make(map[string]*list.Element, size), - } -} - -// NewWithParams creates an LRU with the given parameters. -func NewWithParams(size int, name string, defaultExpiry int64, invalidateClusterEvent string) *Cache { - lru := New(size) - lru.name = name - lru.defaultExpiry = defaultExpiry - lru.invalidateClusterEvent = invalidateClusterEvent - return lru -} - -// Purge is used to completely clear the cache. -func (c *Cache) Purge() { - c.lock.Lock() - defer c.lock.Unlock() - - c.len = 0 - c.currentGeneration++ -} - -// Add adds the given key and value to the store without an expiry. -func (c *Cache) Add(key string, value interface{}) { - c.AddWithExpiresInSecs(key, value, 0) -} - -// AddWithDefaultExpires adds the given key and value to the store with the default expiry. -func (c *Cache) AddWithDefaultExpires(key string, value interface{}) { - c.AddWithExpiresInSecs(key, value, c.defaultExpiry) -} - -// AddWithExpiresInSecs adds the given key and value to the cache with the given expiry. -func (c *Cache) AddWithExpiresInSecs(key string, value interface{}, expireAtSecs int64) { - c.lock.Lock() - defer c.lock.Unlock() - - c.add(key, value, time.Duration(expireAtSecs)*time.Second) -} - -func (c *Cache) add(key string, value interface{}, ttl time.Duration) { - var expires time.Time - if ttl > 0 { - expires = time.Now().Add(ttl) - } - - // Check for existing item, ignoring expiry since we'd update anyway. - if ent, ok := c.items[key]; ok { - c.evictList.MoveToFront(ent) - e := ent.Value.(*entry) - e.value = value - e.expires = expires - if e.generation != c.currentGeneration { - e.generation = c.currentGeneration - c.len++ - } - return - } - - // Add new item - ent := &entry{key, value, expires, c.currentGeneration} - entry := c.evictList.PushFront(ent) - c.items[key] = entry - c.len++ - - if c.evictList.Len() > c.size { - c.removeElement(c.evictList.Back()) - } -} - -// Get returns the value stored in the cache for a key, or nil if no value is present. The ok result indicates whether value was found in the cache. -func (c *Cache) Get(key string) (value interface{}, ok bool) { - c.lock.Lock() - defer c.lock.Unlock() - - return c.getValue(key) -} - -func (c *Cache) getValue(key string) (value interface{}, ok bool) { - if ent, ok := c.items[key]; ok { - e := ent.Value.(*entry) - - if e.generation != c.currentGeneration || (!e.expires.IsZero() && time.Now().After(e.expires)) { - c.removeElement(ent) - return nil, false - } - - c.evictList.MoveToFront(ent) - return ent.Value.(*entry).value, true - } - - return nil, false -} - -// GetOrAdd returns the existing value for the key if present. Otherwise, it stores and returns the given value. The loaded result is true if the value was loaded, false if stored. -// This API intentionally deviates from the Add-only variants above for simplicity. We should simplify the entire API in the future. -func (c *Cache) GetOrAdd(key string, value interface{}, ttl time.Duration) (actual interface{}, loaded bool) { - c.lock.Lock() - defer c.lock.Unlock() - - // Check for existing item - if actualValue, ok := c.getValue(key); ok { - return actualValue, true - } - - c.add(key, value, ttl) - - return value, false -} - -// Remove deletes the value for a key. -func (c *Cache) Remove(key string) { - c.lock.Lock() - defer c.lock.Unlock() - - if ent, ok := c.items[key]; ok { - c.removeElement(ent) - } -} - -// Keys returns a slice of the keys in the cache, from oldest to newest. -func (c *Cache) Keys() []string { - c.lock.RLock() - defer c.lock.RUnlock() - - keys := make([]string, c.len) - i := 0 - for ent := c.evictList.Back(); ent != nil; ent = ent.Prev() { - e := ent.Value.(*entry) - if e.generation == c.currentGeneration { - keys[i] = e.key - i++ - } - } - - return keys -} - -// Len returns the number of items in the cache. -func (c *Cache) Len() int { - c.lock.RLock() - defer c.lock.RUnlock() - return c.len -} - -// Name identifies this cache instance among others in the system. -func (c *Cache) Name() string { - return c.name -} - -// GetInvalidateClusterEvent returns the cluster event configured when this cache was created. -func (c *Cache) GetInvalidateClusterEvent() string { - return c.invalidateClusterEvent -} - -func (c *Cache) removeElement(e *list.Element) { - c.evictList.Remove(e) - kv := e.Value.(*entry) - if kv.generation == c.currentGeneration { - c.len-- - } - delete(c.items, kv.key) -} diff --git a/services/cache/lru/lru_test.go b/services/cache/lru/lru_test.go deleted file mode 100644 index 9d7a9b1491..0000000000 --- a/services/cache/lru/lru_test.go +++ /dev/null @@ -1,125 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -// This files was copied/modified from https://github.com/hashicorp/golang-lru -// which was (see below) - -// This package provides a simple LRU cache. It is based on the -// LRU implementation in groupcache: -// https://github.com/golang/groupcache/tree/master/lru - -package lru - -import ( - "fmt" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestLRU(t *testing.T) { - l := New(128) - - for i := 0; i < 256; i++ { - l.Add(fmt.Sprintf("%d", i), i) - } - require.Equalf(t, l.Len(), 128, "bad len: %v", l.Len()) - - for i, k := range l.Keys() { - v, ok := l.Get(k) - require.True(t, ok, "bad key: %v", k) - require.Equalf(t, fmt.Sprintf("%d", v), k, "bad key: %v", k) - require.Equalf(t, i+128, v, "bad key: %v", k) - } - for i := 0; i < 128; i++ { - _, ok := l.Get(fmt.Sprintf("%d", i)) - require.False(t, ok, "should be evicted") - } - for i := 128; i < 256; i++ { - _, ok := l.Get(fmt.Sprintf("%d", i)) - require.True(t, ok, "should not be evicted") - } - for i := 128; i < 192; i++ { - l.Remove(fmt.Sprintf("%d", i)) - _, ok := l.Get(fmt.Sprintf("%d", i)) - require.False(t, ok, "should be deleted") - } - - l.Get("192") // expect 192 to be last key in l.Keys() - - for i, k := range l.Keys() { - require.Falsef(t, i < 63 && k != fmt.Sprintf("%d", i+193), "out of order key: %v", k) - require.Falsef(t, i == 63 && k != "192", "out of order key: %v", k) - } - - l.Purge() - require.Equalf(t, l.Len(), 0, "bad len: %v", l.Len()) - _, ok := l.Get("200") - require.False(t, ok, "should contain nothing") -} - -func TestLRUExpire(t *testing.T) { - l := New(128) - - l.AddWithExpiresInSecs("1", 1, 1) - l.AddWithExpiresInSecs("2", 2, 1) - l.AddWithExpiresInSecs("3", 3, 0) - - time.Sleep(time.Millisecond * 2100) - - r1, ok := l.Get("1") - require.False(t, ok, r1) - - _, ok2 := l.Get("3") - require.True(t, ok2, "should exist") -} - -func TestLRUGetOrAdd(t *testing.T) { - l := New(128) - - // First GetOrAdd should save - value, loaded := l.GetOrAdd("1", 1, 0) - assert.Equal(t, 1, value) - assert.False(t, loaded) - - // Second GetOrAdd should load original value, ignoring new value - value, loaded = l.GetOrAdd("1", 10, 0) - assert.Equal(t, 1, value) - assert.True(t, loaded) - - // Third GetOrAdd should still load original value - value, loaded = l.GetOrAdd("1", 1, 0) - assert.Equal(t, 1, value) - assert.True(t, loaded) - - // First GetOrAdd on a new key should save - value, loaded = l.GetOrAdd("2", 2, 0) - assert.Equal(t, 2, value) - assert.False(t, loaded) - - l.Remove("1") - - // GetOrAdd after a remove should save - value, loaded = l.GetOrAdd("1", 10, 0) - assert.Equal(t, 10, value) - assert.False(t, loaded) - - // GetOrAdd after another key was removed should load original value for key - value, loaded = l.GetOrAdd("2", 2, 0) - assert.Equal(t, 2, value) - assert.True(t, loaded) - - // GetOrAdd should expire - value, loaded = l.GetOrAdd("3", 3, 500*time.Millisecond) - assert.Equal(t, 3, value) - assert.False(t, loaded) - value, loaded = l.GetOrAdd("3", 4, 500*time.Millisecond) - assert.Equal(t, 3, value) - assert.True(t, loaded) - time.Sleep(1 * time.Second) - value, loaded = l.GetOrAdd("3", 5, 500*time.Millisecond) - assert.Equal(t, 5, value) - assert.False(t, loaded) -} diff --git a/services/cache2/lru_test.go b/services/cache/lru_test.go similarity index 92% rename from services/cache2/lru_test.go rename to services/cache/lru_test.go index bf6d7e8e41..a11448e096 100644 --- a/services/cache2/lru_test.go +++ b/services/cache/lru_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -package cache2 +package cache import ( "fmt" @@ -9,8 +9,6 @@ import ( "time" "github.com/mattermost/mattermost-server/v5/model" - "github.com/mattermost/mattermost-server/v5/services/cache/lru" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -212,14 +210,6 @@ func TestLRUMarshalUnMarshal(t *testing.T) { func BenchmarkLRU(b *testing.B) { value1 := "simplestring" - b.Run("simple=old", func(b *testing.B) { - for i := 0; i < b.N; i++ { - l := lru.New(1) - l.Add("test", value1) - _, ok := l.Get("test") - require.True(b, ok) - } - }) b.Run("simple=new", func(b *testing.B) { for i := 0; i < b.N; i++ { @@ -270,14 +260,7 @@ func BenchmarkLRU(b *testing.B) { "key9": "value value value value value value value value value value9", }, } - b.Run("complex=old", func(b *testing.B) { - for i := 0; i < b.N; i++ { - l := lru.New(1) - l.Add("test", value2) - _, ok := l.Get("test") - require.True(b, ok) - } - }) + b.Run("complex=new", func(b *testing.B) { for i := 0; i < b.N; i++ { l2 := NewLRU(&LRUOptions{ @@ -361,14 +344,6 @@ func BenchmarkLRU(b *testing.B) { TermsOfServiceCreateAt: 111111, } - b.Run("User=old", func(b *testing.B) { - for i := 0; i < b.N; i++ { - l := lru.New(1) - l.Add("test", user) - _, ok := l.Get("test") - require.True(b, ok) - } - }) b.Run("User=new", func(b *testing.B) { for i := 0; i < b.N; i++ { l2 := NewLRU(&LRUOptions{ @@ -449,14 +424,6 @@ func BenchmarkLRU(b *testing.B) { }, } - b.Run("Post=old", func(b *testing.B) { - for i := 0; i < b.N; i++ { - l := lru.New(1) - l.Add("test", post) - _, ok := l.Get("test") - require.True(b, ok) - } - }) b.Run("Post=new", func(b *testing.B) { for i := 0; i < b.N; i++ { l2 := NewLRU(&LRUOptions{ @@ -480,14 +447,7 @@ func BenchmarkLRU(b *testing.B) { LastActivityAt: 111111, ActiveChannel: "ActiveChannel", } - b.Run("Status=old", func(b *testing.B) { - for i := 0; i < b.N; i++ { - l := lru.New(1) - l.Add("test", status) - _, ok := l.Get("test") - require.True(b, ok) - } - }) + b.Run("Status=new", func(b *testing.B) { for i := 0; i < b.N; i++ { l2 := NewLRU(&LRUOptions{ diff --git a/services/cache2/mocks/Cache.go b/services/cache/mocks/Cache.go similarity index 100% rename from services/cache2/mocks/Cache.go rename to services/cache/mocks/Cache.go diff --git a/services/cache2/mocks/Provider.go b/services/cache/mocks/Provider.go similarity index 71% rename from services/cache2/mocks/Provider.go rename to services/cache/mocks/Provider.go index 940f9ddc81..60408ad599 100644 --- a/services/cache2/mocks/Provider.go +++ b/services/cache/mocks/Provider.go @@ -2,8 +2,10 @@ package mocks -import cache2 "github.com/mattermost/mattermost-server/v5/services/cache2" -import mock "github.com/stretchr/testify/mock" +import ( + cache "github.com/mattermost/mattermost-server/v5/services/cache" + mock "github.com/stretchr/testify/mock" +) // Provider is an autogenerated mock type for the Provider type type Provider struct { @@ -39,15 +41,15 @@ func (_m *Provider) Connect() error { } // NewCache provides a mock function with given fields: opts -func (_m *Provider) NewCache(opts *cache2.CacheOptions) cache2.Cache { +func (_m *Provider) NewCache(opts *cache.CacheOptions) cache.Cache { ret := _m.Called(opts) - var r0 cache2.Cache - if rf, ok := ret.Get(0).(func(*cache2.CacheOptions) cache2.Cache); ok { + var r0 cache.Cache + if rf, ok := ret.Get(0).(func(*cache.CacheOptions) cache.Cache); ok { r0 = rf(opts) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(cache2.Cache) + r0 = ret.Get(0).(cache.Cache) } } diff --git a/services/cache2/provider.go b/services/cache/provider.go similarity index 98% rename from services/cache2/provider.go rename to services/cache/provider.go index 23409c73cd..cb63530248 100644 --- a/services/cache2/provider.go +++ b/services/cache/provider.go @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -package cache2 +package cache import "time" diff --git a/services/cache2/provider_test.go b/services/cache/provider_test.go similarity index 98% rename from services/cache2/provider_test.go rename to services/cache/provider_test.go index 5a22d3af00..bc85573c5f 100644 --- a/services/cache2/provider_test.go +++ b/services/cache/provider_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -package cache2 +package cache import ( "testing" diff --git a/services/cache2/cache.go b/services/cache2/cache.go deleted file mode 100644 index 8c88f0a56e..0000000000 --- a/services/cache2/cache.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -package cache2 - -import ( - "errors" - "time" -) - -// ErrKeyNotFound is the error when the given key is not found -var ErrKeyNotFound = errors.New("key not found") - -// Cache (under package cache2) is a representation of a cache store that aims to replace cache.Cache -type Cache interface { - // Purge is used to completely clear the cache. - Purge() error - - // Set adds the given key and value to the store without an expiry. If the key already exists, - // it will overwrite the previous value. - Set(key string, value interface{}) error - - // SetWithDefaultExpiry adds the given key and value to the store with the default expiry. If - // the key already exists, it will overwrite the previoous value - SetWithDefaultExpiry(key string, value interface{}) error - - // SetWithExpiry adds the given key and value to the cache with the given expiry. If the key - // already exists, it will overwrite the previoous value - SetWithExpiry(key string, value interface{}, ttl time.Duration) error - - // Get the content stored in the cache for the given key, and decode it into the value interface. - // Return ErrKeyNotFound if the key is missing from the cache - Get(key string, value interface{}) error - - // Remove deletes the value for a given key. - Remove(key string) error - - // Keys returns a slice of the keys in the cache. - Keys() ([]string, error) - - // Len returns the number of items in the cache. - Len() (int, error) - - // GetInvalidateClusterEvent returns the cluster event configured when this cache was created. - GetInvalidateClusterEvent() string - - // Name returns the name of the cache - Name() string -} diff --git a/store/localcachelayer/layer.go b/store/localcachelayer/layer.go index feb2a10ddd..08a9ec7fc0 100644 --- a/store/localcachelayer/layer.go +++ b/store/localcachelayer/layer.go @@ -8,7 +8,7 @@ import ( "github.com/mattermost/mattermost-server/v5/einterfaces" "github.com/mattermost/mattermost-server/v5/model" - "github.com/mattermost/mattermost-server/v5/services/cache2" + "github.com/mattermost/mattermost-server/v5/services/cache" "github.com/mattermost/mattermost-server/v5/store" ) @@ -68,47 +68,47 @@ type LocalCacheStore struct { cluster einterfaces.ClusterInterface reaction LocalCacheReactionStore - reactionCache cache2.Cache + reactionCache cache.Cache fileInfo LocalCacheFileInfoStore - fileInfoCache cache2.Cache + fileInfoCache cache.Cache role LocalCacheRoleStore - roleCache cache2.Cache - rolePermissionsCache cache2.Cache + roleCache cache.Cache + rolePermissionsCache cache.Cache scheme LocalCacheSchemeStore - schemeCache cache2.Cache + schemeCache cache.Cache emoji LocalCacheEmojiStore - emojiCacheById cache2.Cache - emojiIdCacheByName cache2.Cache + emojiCacheById cache.Cache + emojiIdCacheByName cache.Cache channel LocalCacheChannelStore - channelMemberCountsCache cache2.Cache - channelGuestCountCache cache2.Cache - channelPinnedPostCountsCache cache2.Cache - channelByIdCache cache2.Cache + channelMemberCountsCache cache.Cache + channelGuestCountCache cache.Cache + channelPinnedPostCountsCache cache.Cache + channelByIdCache cache.Cache webhook LocalCacheWebhookStore - webhookCache cache2.Cache + webhookCache cache.Cache post LocalCachePostStore - postLastPostsCache cache2.Cache - lastPostTimeCache cache2.Cache + postLastPostsCache cache.Cache + lastPostTimeCache cache.Cache user LocalCacheUserStore - userProfileByIdsCache cache2.Cache - profilesInChannelCache cache2.Cache + userProfileByIdsCache cache.Cache + profilesInChannelCache cache.Cache team LocalCacheTeamStore - teamAllTeamIdsForUserCache cache2.Cache + teamAllTeamIdsForUserCache cache.Cache termsOfService LocalCacheTermsOfServiceStore - termsOfServiceCache cache2.Cache + termsOfServiceCache cache.Cache } -func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface, cacheProvider cache2.Provider) LocalCacheStore { +func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface, cacheProvider cache.Provider) LocalCacheStore { localCacheStore := LocalCacheStore{ Store: baseStore, @@ -116,7 +116,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf metrics: metrics, } // Reactions - localCacheStore.reactionCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.reactionCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: REACTION_CACHE_SIZE, Name: "Reaction", DefaultExpiry: REACTION_CACHE_SEC * time.Second, @@ -125,13 +125,13 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.reaction = LocalCacheReactionStore{ReactionStore: baseStore.Reaction(), rootStore: &localCacheStore} // Roles - localCacheStore.roleCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.roleCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: ROLE_CACHE_SIZE, Name: "Role", DefaultExpiry: ROLE_CACHE_SEC * time.Second, InvalidateClusterEvent: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES, }) - localCacheStore.rolePermissionsCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.rolePermissionsCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: ROLE_CACHE_SIZE, Name: "RolePermission", DefaultExpiry: ROLE_CACHE_SEC * time.Second, @@ -140,7 +140,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.role = LocalCacheRoleStore{RoleStore: baseStore.Role(), rootStore: &localCacheStore} // Schemes - localCacheStore.schemeCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.schemeCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: SCHEME_CACHE_SIZE, Name: "Scheme", DefaultExpiry: SCHEME_CACHE_SEC * time.Second, @@ -149,7 +149,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.scheme = LocalCacheSchemeStore{SchemeStore: baseStore.Scheme(), rootStore: &localCacheStore} // FileInfo - localCacheStore.fileInfoCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.fileInfoCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: FILE_INFO_CACHE_SIZE, Name: "FileInfo", DefaultExpiry: FILE_INFO_CACHE_SEC * time.Second, @@ -158,7 +158,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.fileInfo = LocalCacheFileInfoStore{FileInfoStore: baseStore.FileInfo(), rootStore: &localCacheStore} // Webhooks - localCacheStore.webhookCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.webhookCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: WEBHOOK_CACHE_SIZE, Name: "Webhook", DefaultExpiry: WEBHOOK_CACHE_SEC * time.Second, @@ -167,13 +167,13 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.webhook = LocalCacheWebhookStore{WebhookStore: baseStore.Webhook(), rootStore: &localCacheStore} // Emojis - localCacheStore.emojiCacheById = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.emojiCacheById = cacheProvider.NewCache(&cache.CacheOptions{ Size: EMOJI_CACHE_SIZE, Name: "EmojiById", DefaultExpiry: EMOJI_CACHE_SEC * time.Second, InvalidateClusterEvent: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_BY_ID, }) - localCacheStore.emojiIdCacheByName = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.emojiIdCacheByName = cacheProvider.NewCache(&cache.CacheOptions{ Size: EMOJI_CACHE_SIZE, Name: "EmojiByName", DefaultExpiry: EMOJI_CACHE_SEC * time.Second, @@ -182,25 +182,25 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.emoji = LocalCacheEmojiStore{EmojiStore: baseStore.Emoji(), rootStore: &localCacheStore} // Channels - localCacheStore.channelPinnedPostCountsCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.channelPinnedPostCountsCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SIZE, Name: "ChannelPinnedPostsCounts", DefaultExpiry: CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SEC * time.Second, InvalidateClusterEvent: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_PINNEDPOSTS_COUNTS, }) - localCacheStore.channelMemberCountsCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.channelMemberCountsCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: CHANNEL_MEMBERS_COUNTS_CACHE_SIZE, Name: "ChannelMemberCounts", DefaultExpiry: CHANNEL_MEMBERS_COUNTS_CACHE_SEC * time.Second, InvalidateClusterEvent: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS, }) - localCacheStore.channelGuestCountCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.channelGuestCountCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: CHANNEL_GUEST_COUNT_CACHE_SIZE, Name: "ChannelGuestsCount", DefaultExpiry: CHANNEL_GUEST_COUNT_CACHE_SEC * time.Second, InvalidateClusterEvent: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_GUEST_COUNT, }) - localCacheStore.channelByIdCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.channelByIdCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: model.CHANNEL_CACHE_SIZE, Name: "channelById", DefaultExpiry: CHANNEL_CACHE_SEC * time.Second, @@ -209,13 +209,13 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.channel = LocalCacheChannelStore{ChannelStore: baseStore.Channel(), rootStore: &localCacheStore} // Posts - localCacheStore.postLastPostsCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.postLastPostsCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: LAST_POSTS_CACHE_SIZE, Name: "LastPost", DefaultExpiry: LAST_POSTS_CACHE_SEC * time.Second, InvalidateClusterEvent: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_LAST_POSTS, }) - localCacheStore.lastPostTimeCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.lastPostTimeCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: LAST_POST_TIME_CACHE_SIZE, Name: "LastPostTime", DefaultExpiry: LAST_POST_TIME_CACHE_SEC * time.Second, @@ -224,7 +224,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.post = LocalCachePostStore{PostStore: baseStore.Post(), rootStore: &localCacheStore} // TOS - localCacheStore.termsOfServiceCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.termsOfServiceCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: TERMS_OF_SERVICE_CACHE_SIZE, Name: "TermsOfService", DefaultExpiry: TERMS_OF_SERVICE_CACHE_SEC * time.Second, @@ -233,13 +233,13 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.termsOfService = LocalCacheTermsOfServiceStore{TermsOfServiceStore: baseStore.TermsOfService(), rootStore: &localCacheStore} // Users - localCacheStore.userProfileByIdsCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.userProfileByIdsCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: USER_PROFILE_BY_ID_CACHE_SIZE, Name: "UserProfileByIds", DefaultExpiry: USER_PROFILE_BY_ID_SEC * time.Second, InvalidateClusterEvent: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_PROFILE_BY_IDS, }) - localCacheStore.profilesInChannelCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.profilesInChannelCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: PROFILES_IN_CHANNEL_CACHE_SIZE, Name: "ProfilesInChannel", DefaultExpiry: PROFILES_IN_CHANNEL_CACHE_SEC * time.Second, @@ -248,7 +248,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.user = LocalCacheUserStore{UserStore: baseStore.User(), rootStore: &localCacheStore} // Teams - localCacheStore.teamAllTeamIdsForUserCache = cacheProvider.NewCache(&cache2.CacheOptions{ + localCacheStore.teamAllTeamIdsForUserCache = cacheProvider.NewCache(&cache.CacheOptions{ Size: TEAM_CACHE_SIZE, Name: "Team", DefaultExpiry: TEAM_CACHE_SEC * time.Second, @@ -328,7 +328,7 @@ func (s LocalCacheStore) DropAllTables() { s.Store.DropAllTables() } -func (s *LocalCacheStore) doInvalidateCacheCluster(cache cache2.Cache, key string) { +func (s *LocalCacheStore) doInvalidateCacheCluster(cache cache.Cache, key string) { cache.Remove(key) if s.cluster != nil { msg := &model.ClusterMessage{ @@ -340,11 +340,11 @@ func (s *LocalCacheStore) doInvalidateCacheCluster(cache cache2.Cache, key strin } } -func (s *LocalCacheStore) doStandardAddToCache(cache cache2.Cache, key string, value interface{}) { +func (s *LocalCacheStore) doStandardAddToCache(cache cache.Cache, key string, value interface{}) { cache.SetWithDefaultExpiry(key, value) } -func (s *LocalCacheStore) doStandardReadCache(cache cache2.Cache, key string, value interface{}) error { +func (s *LocalCacheStore) doStandardReadCache(cache cache.Cache, key string, value interface{}) error { if err := cache.Get(key, value); err == nil { if s.metrics != nil { s.metrics.IncrementMemCacheHitCounter(cache.Name()) @@ -358,7 +358,7 @@ func (s *LocalCacheStore) doStandardReadCache(cache cache2.Cache, key string, va } } -func (s *LocalCacheStore) doClearCacheCluster(cache cache2.Cache) { +func (s *LocalCacheStore) doClearCacheCluster(cache cache.Cache) { cache.Purge() if s.cluster != nil { msg := &model.ClusterMessage{ diff --git a/store/localcachelayer/main_test.go b/store/localcachelayer/main_test.go index 272c65a521..fafd171529 100644 --- a/store/localcachelayer/main_test.go +++ b/store/localcachelayer/main_test.go @@ -7,10 +7,10 @@ import ( "fmt" "testing" - "github.com/mattermost/mattermost-server/v5/services/cache2" + "github.com/mattermost/mattermost-server/v5/services/cache" "github.com/mattermost/mattermost-server/v5/model" - cachemocks "github.com/mattermost/mattermost-server/v5/services/cache2/mocks" + cachemocks "github.com/mattermost/mattermost-server/v5/services/cache/mocks" "github.com/mattermost/mattermost-server/v5/store" "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" "github.com/mattermost/mattermost-server/v5/testlib" @@ -19,10 +19,10 @@ import ( var mainHelper *testlib.MainHelper -func getMockCacheProvider() cache2.Provider { +func getMockCacheProvider() cache.Provider { mockCacheProvider := cachemocks.Provider{} mockCacheProvider.On("NewCache", mock.Anything). - Return(cache2.NewLRU(&cache2.LRUOptions{Size: 128})) + Return(cache.NewLRU(&cache.LRUOptions{Size: 128})) return &mockCacheProvider } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index fef6789f07..c546f0f5dd 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -16,7 +16,7 @@ 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/cache2" + "github.com/mattermost/mattermost-server/v5/services/cache" "github.com/mattermost/mattermost-server/v5/store" sq "github.com/Masterminds/squirrel" @@ -335,13 +335,13 @@ type publicChannel struct { Purpose string `json:"purpose"` } -var allChannelMembersForUserCache = cache2.NewLRU(&cache2.LRUOptions{ +var allChannelMembersForUserCache = cache.NewLRU(&cache.LRUOptions{ Size: ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE, }) -var allChannelMembersNotifyPropsForChannelCache = cache2.NewLRU(&cache2.LRUOptions{ +var allChannelMembersNotifyPropsForChannelCache = cache.NewLRU(&cache.LRUOptions{ Size: ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE, }) -var channelByNameCache = cache2.NewLRU(&cache2.LRUOptions{ +var channelByNameCache = cache.NewLRU(&cache.LRUOptions{ Size: model.CHANNEL_CACHE_SIZE, }) diff --git a/web/web_test.go b/web/web_test.go index a0725efa46..5c888bb5c7 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -82,7 +82,7 @@ func setupTestHelper(t testing.TB, store store.Store, includeCacheLayer bool) *T } if includeCacheLayer { // Adds the cache layer to the test store - s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider2) + s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) } prevListenAddress := *s.Config().ServiceSettings.ListenAddress