From 0185aa4ada55b712c20fe2da6de65fe55f409a22 Mon Sep 17 00:00:00 2001 From: Umputun Date: Mon, 4 Jun 2018 13:41:46 -0500 Subject: [PATCH] add control of total cached memory size --- app/main.go | 21 ++++++++------ app/rest/cache/cache.go | 24 ++++++++++++++-- app/rest/cache/cache_test.go | 56 ++++++++++++++++++++++++++++-------- app/rest/cache/options.go | 9 ++++++ 4 files changed, 87 insertions(+), 23 deletions(-) diff --git a/app/main.go b/app/main.go index bc887eb3..05e97fdc 100644 --- a/app/main.go +++ b/app/main.go @@ -40,13 +40,15 @@ type Opts struct { AvatarStore string `long:"avatars" env:"AVATAR_STORE" default:"./var/avatars" description:"avatars location"` ImageProxy bool `long:"img-proxy" env:"IMG_PROXY" description:"enable image proxy"` - MaxCommentSize int `long:"max-comment" env:"MAX_COMMENT_SIZE" default:"2048" description:"max comment size"` - MaxCachedItems int `long:"max-cache-items" env:"MAX_CACHE_ITEMS" default:"1000" description:"max cached items"` - MaxCachedValue int `long:"max-cache-value" env:"MAX_CACHE_VALUE" default:"65536" description:"max size of cached value"` - SecretKey string `long:"secret" env:"SECRET" required:"true" description:"secret key"` - LowScore int `long:"low-score" env:"LOW_SCORE" default:"-5" description:"low score threshold"` - CriticalScore int `long:"critical-score" env:"CRITICAL_SCORE" default:"-10" description:"critical score threshold"` - ReadOnlyAge int `long:"read-age" env:"READONLY_AGE" default:"0" description:"read-only age of comments"` + MaxCommentSize int `long:"max-comment" env:"MAX_COMMENT_SIZE" default:"2048" description:"max comment size"` + MaxCachedItems int `long:"max-cache-items" env:"MAX_CACHE_ITEMS" default:"1000" description:"max cached items"` + MaxCachedValue int `long:"max-cache-value" env:"MAX_CACHE_VALUE" default:"65536" description:"max size of cached value"` + MaxCacheSize int `long:"max-cache-size" env:"MAX_CACHE_SIZE" default:"50000000" description:"max size of total cache"` + + SecretKey string `long:"secret" env:"SECRET" required:"true" description:"secret key"` + LowScore int `long:"low-score" env:"LOW_SCORE" default:"-5" description:"low score threshold"` + CriticalScore int `long:"critical-score" env:"CRITICAL_SCORE" default:"-10" description:"critical score threshold"` + ReadOnlyAge int `long:"read-age" env:"READONLY_AGE" default:"0" description:"read-only age of comments"` GoogleCID string `long:"google-cid" env:"REMARK_GOOGLE_CID" description:"Google OAuth client ID"` GoogleCSEC string `long:"google-csec" env:"REMARK_GOOGLE_CSEC" description:"Google OAuth client secret"` @@ -113,8 +115,11 @@ func New(opts Opts) (*Application, error) { MaxCommentSize: opts.MaxCommentSize, } - loadingCache := cache.NewLoadingCache(cache.MaxValSize(opts.MaxCachedValue), cache.MaxKeys(opts.MaxCachedItems), + loadingCache, err := cache.NewLoadingCache(cache.MaxValSize(opts.MaxCachedValue), cache.MaxKeys(opts.MaxCachedItems), cache.PostFlushFn(postFlushFn(opts.Sites, opts.Port))) + if err != nil { + return nil, err + } jwtService := auth.NewJWT(opts.SecretKey, strings.HasPrefix(opts.RemarkURL, "https://"), 7*24*time.Hour) diff --git a/app/rest/cache/cache.go b/app/rest/cache/cache.go index 82cf2412..7e02947e 100644 --- a/app/rest/cache/cache.go +++ b/app/rest/cache/cache.go @@ -3,6 +3,7 @@ package cache import ( "log" "strings" + "sync/atomic" "github.com/hashicorp/golang-lru" "github.com/pkg/errors" @@ -38,10 +39,12 @@ type loadingCache struct { postFlushFn func() maxKeys int maxValueSize int + maxCacheSize int64 + currentSize int64 } // NewLoadingCache makes loadingCache implementation -func NewLoadingCache(options ...Option) LoadingCache { +func NewLoadingCache(options ...Option) (LoadingCache, error) { res := loadingCache{ postFlushFn: func() {}, maxKeys: 1000, @@ -53,11 +56,19 @@ func NewLoadingCache(options ...Option) LoadingCache { } } + onEvicted := func(key interface{}, value interface{}) { + size := len(value.([]byte)) + atomic.AddInt64(&res.currentSize, -1*int64(size)) + } + + var err error // OnEvicted called automatically for expired and manually deleted - res.bytesCache, _ = lru.New(res.maxKeys) + if res.bytesCache, err = lru.NewWithEvict(res.maxKeys, onEvicted); err != nil { + return nil, errors.Wrap(err, "failed to make cache") + } log.Printf("[DEBUG] create lru cache, maxKeys=%d, maxValueSize=%d", res.maxKeys, res.maxValueSize) - return &res + return &res, nil } // Get is loading cache method to get value by key or load via fn if not found @@ -71,6 +82,13 @@ func (lc *loadingCache) Get(key string, fn func() ([]byte, error)) (data []byte, } if lc.allowed(data) { lc.bytesCache.Add(key, data) + atomic.AddInt64(&lc.currentSize, int64(len(data))) + + if lc.maxCacheSize > 0 && atomic.LoadInt64(&lc.currentSize) > lc.maxCacheSize { + for atomic.LoadInt64(&lc.currentSize) > lc.maxCacheSize { + lc.bytesCache.RemoveOldest() + } + } } return data, nil } diff --git a/app/rest/cache/cache_test.go b/app/rest/cache/cache_test.go index 58cd599c..0b22dead 100644 --- a/app/rest/cache/cache_test.go +++ b/app/rest/cache/cache_test.go @@ -18,8 +18,8 @@ import ( func TestLoadingCache_Get(t *testing.T) { var postFnCall, coldCalls int32 - lc := NewLoadingCache(PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) })) - + lc, err := NewLoadingCache(PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) })) + require.Nil(t, err) res, err := lc.Get("key", func() ([]byte, error) { atomic.AddInt32(&coldCalls, 1) return []byte("result"), nil @@ -50,16 +50,17 @@ func TestLoadingCache_Get(t *testing.T) { func TestLoadingCache_MaxKeys(t *testing.T) { var postFnCall, coldCalls int32 - lc := NewLoadingCache(PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) }), + lc, err := NewLoadingCache(PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) }), MaxKeys(5), MaxValSize(10)) + require.Nil(t, err) // put 5 keys to cache for i := 0; i < 5; i++ { - res, err := lc.Get(fmt.Sprintf("key-%d", i), func() ([]byte, error) { + res, e := lc.Get(fmt.Sprintf("key-%d", i), func() ([]byte, error) { atomic.AddInt32(&coldCalls, 1) return []byte(fmt.Sprintf("result-%d", i)), nil }) - assert.Nil(t, err) + assert.Nil(t, e) 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)) @@ -96,9 +97,9 @@ func TestLoadingCache_MaxKeys(t *testing.T) { assert.Equal(t, 5, lc.(*loadingCache).bytesCache.Len()) } -func TestLoadingCache_MaxSize(t *testing.T) { - lc := NewLoadingCache(MaxKeys(5), MaxValSize(10)) - +func TestLoadingCache_MaxValueSize(t *testing.T) { + lc, err := NewLoadingCache(MaxKeys(5), MaxValSize(10)) + require.Nil(t, err) // put good size value to cache and make sure it cached res, err := lc.Get("key-Z", func() ([]byte, error) { return []byte("result-Z"), nil @@ -124,7 +125,34 @@ func TestLoadingCache_MaxSize(t *testing.T) { }) assert.Nil(t, err) assert.Equal(t, "result-big", string(res), "got not cached value") +} +func TestLoadingCache_MaxCacheSize(t *testing.T) { + lc, err := NewLoadingCache(MaxKeys(50), MaxCacheSize(20)) + require.Nil(t, err) + + // put good size value to cache and make sure it cached + res, err := lc.Get("key-Z", func() ([]byte, error) { + return []byte("result-Z"), nil + }) + assert.Nil(t, err) + assert.Equal(t, "result-Z", string(res)) + assert.Equal(t, int64(8), lc.(*loadingCache).currentSize) + + _, err = lc.Get("key-Z2", func() ([]byte, error) { + return []byte("result-Z"), nil + }) + assert.Nil(t, err) + assert.Equal(t, int64(16), lc.(*loadingCache).currentSize) + + // this will cause removal + _, err = lc.Get("key-Z3", func() ([]byte, error) { + return []byte("result-Z"), nil + }) + assert.Nil(t, err) + assert.Equal(t, int64(16), lc.(*loadingCache).currentSize) + + assert.Equal(t, 2, lc.(*loadingCache).bytesCache.Len()) } func TestLoadingCache_URLKey(t *testing.T) { @@ -146,7 +174,8 @@ func TestLoadingCache_URLKey(t *testing.T) { func TestLoadingCache_Parallel(t *testing.T) { var coldCalls int32 - lc := NewLoadingCache() + lc, err := NewLoadingCache() + require.Nil(t, err) res, err := lc.Get("key", func() ([]byte, error) { return []byte("value"), nil @@ -173,7 +202,8 @@ func TestLoadingCache_Parallel(t *testing.T) { } func TestLoadingCache_Scopes(t *testing.T) { - lc := NewLoadingCache() + lc, err := NewLoadingCache() + require.Nil(t, err) res, err := lc.Get(Key("key", "s1", "s2"), func() ([]byte, error) { return []byte("value"), nil @@ -204,7 +234,8 @@ func TestLoadingCache_Scopes(t *testing.T) { } func TestLoadingCache_Flush(t *testing.T) { - lc := NewLoadingCache() + lc, err := NewLoadingCache() + require.Nil(t, err) addToCache := func(key string, scopes ...string) { res, err := lc.Get(key, func() ([]byte, error) { @@ -249,7 +280,8 @@ func TestLoadingCache_Flush(t *testing.T) { } func TestLoadingCache_FlushFailed(t *testing.T) { - lc := NewLoadingCache() + lc, err := NewLoadingCache() + require.Nil(t, err) val, err := lc.Get("invalid-composite", func() ([]byte, error) { return []byte("value"), nil }) diff --git a/app/rest/cache/options.go b/app/rest/cache/options.go index f10dd15c..64986e72 100644 --- a/app/rest/cache/options.go +++ b/app/rest/cache/options.go @@ -28,6 +28,15 @@ func MaxKeys(max int) Option { } } +// MaxCacheSize functional option defines the total size of cached data. +// By default it is 0, which means unlimited. +func MaxCacheSize(max int64) Option { + return func(lc *loadingCache) error { + lc.maxCacheSize = max + return nil + } +} + // PostFlushFn functional option defines how callback function called after each Flush. func PostFlushFn(postFlushFn func()) Option { return func(lc *loadingCache) error {