From 85cbda509c35a9d0b89c97fb07a1392695478538 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 17 May 2018 13:25:19 -0500 Subject: [PATCH] limit max size of cache, number of items and size of value #24 --- app/main.go | 2 +- app/rest/api/rest.go | 6 +-- app/rest/cache.go | 84 ++++++++++++++++++++++++++++++++++---- app/rest/cache_test.go | 91 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 168 insertions(+), 15 deletions(-) diff --git a/app/main.go b/app/main.go index e46b20a5..5c6e5bd7 100644 --- a/app/main.go +++ b/app/main.go @@ -92,7 +92,7 @@ func main() { }() exporter := migrator.Remark{DataStore: &dataService} - cache := rest.NewLoadingCache(4*time.Hour, 1*time.Minute, postFlushFn) + cache := rest.NewLoadingCache(rest.MaxValueSize(64*1024), rest.MaxKeys(1000), rest.PostFlushFn(postFlushFn)) activateBackup(&exporter) diff --git a/app/rest/api/rest.go b/app/rest/api/rest.go index f92b8bf8..60a95bd3 100644 --- a/app/rest/api/rest.go +++ b/app/rest/api/rest.go @@ -265,7 +265,7 @@ func (s *Rest) findCommentsCtrl(w http.ResponseWriter, r *http.Request) { } log.Printf("[DEBUG] get comments for %+v, sort %s, format %s", locator, sort, r.URL.Query().Get("format")) - data, err := s.Cache.Get(rest.URLKey(r), time.Hour, func() ([]byte, error) { + data, err := s.Cache.Get(rest.URLKey(r), 4*time.Hour, func() ([]byte, error) { comments, e := s.DataService.Find(locator, sort) if e != nil { return nil, e @@ -298,7 +298,7 @@ func (s *Rest) lastCommentsCtrl(w http.ResponseWriter, r *http.Request) { limit = 0 } - data, err := s.Cache.Get(rest.URLKey(r), time.Hour, func() ([]byte, error) { + data, err := s.Cache.Get(rest.URLKey(r), 4*time.Hour, func() ([]byte, error) { comments, e := s.DataService.Last(r.URL.Query().Get("site"), limit) if e != nil { return nil, e @@ -351,7 +351,7 @@ func (s *Rest) findUserCommentsCtrl(w http.ResponseWriter, r *http.Request) { log.Printf("[DEBUG] get comments for userID %s, %s", userID, siteID) - data, err := s.Cache.Get(rest.URLKey(r), time.Hour, func() ([]byte, error) { + data, err := s.Cache.Get(rest.URLKey(r), 4*time.Hour, func() ([]byte, error) { comments, count, e := s.DataService.User(siteID, userID, limit) if e != nil { return nil, e diff --git a/app/rest/cache.go b/app/rest/cache.go index 4c0c4e05..423d6e0a 100644 --- a/app/rest/cache.go +++ b/app/rest/cache.go @@ -1,6 +1,7 @@ package rest import ( + "log" "net/http" "strings" "time" @@ -14,15 +15,33 @@ type LoadingCache interface { Flush() } -// loadingCache implements LoadingCache interface on top of cache.Cache +// loadingCache implements LoadingCache interface on top of cache.Cache (go-cache) type loadingCache struct { - bytesCache *cache.Cache - postFlushFn func() + bytesCache *cache.Cache + postFlushFn func() + defaultExpiration time.Duration + cleanupInterval time.Duration + maxKeys int + maxValueSize int } // NewLoadingCache makes loadingCache implementation -func NewLoadingCache(defaultExpiration, cleanupInterval time.Duration, postFlushFn func()) LoadingCache { - return &loadingCache{bytesCache: cache.New(defaultExpiration, cleanupInterval), postFlushFn: postFlushFn} +func NewLoadingCache(options ...CacheOption) LoadingCache { + res := loadingCache{ + defaultExpiration: time.Hour, + cleanupInterval: 5 * time.Minute, + postFlushFn: func() {}, + maxKeys: 0, + maxValueSize: 0, + } + for _, opt := range options { + if err := opt(&res); err != nil { + log.Printf("[WARN] failed to set cache option, %v", err) + } + } + res.bytesCache = cache.New(res.defaultExpiration, res.cleanupInterval) + + return &res } // Get is loading cache method to get value by key or load via fn if not found @@ -34,7 +53,9 @@ func (lc *loadingCache) Get(key string, ttl time.Duration, fn func() ([]byte, er if data, err = fn(); err != nil { return data, err } - lc.bytesCache.Set(key, data, ttl) + if lc.allowed(data) { + lc.bytesCache.Set(key, data, ttl) + } return data, nil } @@ -46,8 +67,55 @@ func (lc *loadingCache) Flush() { } } -// URLKey gets url from request to use is as cache key -// admins will have separate keys in order tp prevent leak of admin-only data to regular users +func (lc *loadingCache) allowed(data []byte) bool { + if lc.maxValueSize > 0 && len(data) >= lc.maxValueSize { + return false + } + if lc.maxKeys > 0 && lc.bytesCache.ItemCount() >= lc.maxKeys { + return false + } + return true +} + +// CacheOption func type +type CacheOption func(lc *loadingCache) error + +// MaxValueSize functional option defines the largest value's size allowed to be cached +// By default it is 0, which means unlimited. +func MaxValueSize(max int) CacheOption { + return func(lc *loadingCache) error { + lc.maxValueSize = max + return nil + } +} + +// MaxKeys functional option defines how many keys to keep. +// By default it is 0, which means unlimited. +func MaxKeys(max int) CacheOption { + return func(lc *loadingCache) error { + lc.maxKeys = max + return nil + } +} + +// CleanupInterval functional option defines how often cleanup loop activated. +func CleanupInterval(interval time.Duration) CacheOption { + return func(lc *loadingCache) error { + lc.cleanupInterval = interval + return nil + } +} + +// PostFlushFn functional option defines how callback function called after each Flush. +func PostFlushFn(postFlushFn func()) CacheOption { + return func(lc *loadingCache) error { + lc.postFlushFn = postFlushFn + return nil + } +} + +// URLKey gets url from request to use it as cache key +// admins will have different keys in order to prevent leak of admin-only data to regular users func URLKey(r *http.Request) string { adminPrefix := "admin!!" key := strings.TrimPrefix(r.URL.String(), adminPrefix) // prevents attach with fake url to get admin view diff --git a/app/rest/cache_test.go b/app/rest/cache_test.go index 21b3321c..d10ebda4 100644 --- a/app/rest/cache_test.go +++ b/app/rest/cache_test.go @@ -1,6 +1,7 @@ package rest import ( + "fmt" "net/http" "sync/atomic" "testing" @@ -12,9 +13,7 @@ import ( func TestLoadingCache_Get(t *testing.T) { var postFnCall, coldCalls int32 - lc := NewLoadingCache(1*time.Minute, 200*time.Millisecond, func() { - atomic.AddInt32(&postFnCall, 1) - }) + lc := NewLoadingCache(CleanupInterval(200*time.Millisecond), PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) })) res, err := lc.Get("key", time.Minute, func() ([]byte, error) { atomic.AddInt32(&coldCalls, 1) @@ -39,6 +38,92 @@ func TestLoadingCache_Get(t *testing.T) { assert.Equal(t, int32(1), atomic.LoadInt32(&postFnCall)) } +func TestLoadingCache_MaxKeys(t *testing.T) { + var postFnCall, coldCalls int32 + lc := NewLoadingCache(CleanupInterval(200*time.Millisecond), PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) }), + MaxKeys(5), MaxValueSize(10)) + + // put 5 keys to cache + for i := 0; i < 5; i++ { + res, err := lc.Get(fmt.Sprintf("key-%d", i), 500*time.Millisecond, func() ([]byte, error) { + atomic.AddInt32(&coldCalls, 1) + return []byte(fmt.Sprintf("result-%d", i)), nil + }) + assert.Nil(t, err) + assert.Equal(t, fmt.Sprintf("result-%d", i), string(res)) + assert.Equal(t, int32(i+1), atomic.LoadInt32(&coldCalls)) + assert.Equal(t, int32(0), atomic.LoadInt32(&postFnCall)) + } + + // check if really cached + res, err := lc.Get("key-3", time.Minute, func() ([]byte, error) { + return []byte("result-blah"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-3", string(res), "should get cached") + + // try to cache after maxKeys reached + res, err = lc.Get("key-X", time.Minute, func() ([]byte, error) { + return []byte("result-X"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-X", string(res)) + + cc := atomic.LoadInt32(&coldCalls) + res, err = lc.Get("key-X", time.Minute, func() ([]byte, error) { + atomic.AddInt32(&coldCalls, 1) + return []byte("result-not-cached"), nil + }) + assert.Nil(t, err) + assert.Equal(t, cc+1, atomic.LoadInt32(&coldCalls)) + assert.Equal(t, "result-not-cached", string(res), "not cached") + + time.Sleep(time.Second) // let cleanup to remove + + // put to cache and make sure it cached + res, err = lc.Get("key-Z", time.Minute, func() ([]byte, error) { + return []byte("result-Z"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-Z", string(res)) + + res, err = lc.Get("key-Z", time.Minute, func() ([]byte, error) { + return []byte("result-Zzzz"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-Z", string(res), "got cached value") +} + +func TestLoadingCache_MaxSize(t *testing.T) { + lc := NewLoadingCache(CleanupInterval(200*time.Millisecond), MaxKeys(5), MaxValueSize(10)) + + // put good size value to cache and make sure it cached + res, err := lc.Get("key-Z", time.Minute, func() ([]byte, error) { + return []byte("result-Z"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-Z", string(res)) + + res, err = lc.Get("key-Z", time.Minute, func() ([]byte, error) { + return []byte("result-Zzzz"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-Z", string(res), "got cached value") + + // put too big value to cache and make sure it is not cached + res, err = lc.Get("key-Big", time.Minute, func() ([]byte, error) { + return []byte("1234567890"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "1234567890", string(res)) + + res, err = lc.Get("key-Big", time.Minute, func() ([]byte, error) { + return []byte("result-big"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-big", string(res), "got not cached value") + +} func TestLoadingCache_URLKey(t *testing.T) { r, err := http.NewRequest("GET", "http://blah/123", nil) assert.Nil(t, err)