From ce08f5cb895ed5409dc63badbbdbaa82c774893d Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Tue, 27 Jun 2023 10:47:19 -0400 Subject: [PATCH 1/2] Remove token pools from the cache upon deletion Signed-off-by: Andrew Richardson --- internal/assets/token_pool.go | 10 ++++++++++ internal/cache/cache.go | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/internal/assets/token_pool.go b/internal/assets/token_pool.go index f7bb1726a6..8939bb6219 100644 --- a/internal/assets/token_pool.go +++ b/internal/assets/token_pool.go @@ -211,6 +211,15 @@ func (am *assetManager) GetTokenPoolByNameOrID(ctx context.Context, poolNameOrID return pool, nil } +func (am *assetManager) removeTokenPoolFromCache(ctx context.Context, pool *core.TokenPool) { + cacheKeyName := fmt.Sprintf("ns=%s,poolnameorid=%s", am.namespace, pool.Name) + cacheKeyID := fmt.Sprintf("ns=%s,poolnameorid=%s", am.namespace, pool.ID) + cacheKeyLocator := fmt.Sprintf("ns=%s,connector=%s,poollocator=%s", am.namespace, pool.Connector, pool.Locator) + am.cache.Delete(cacheKeyName) + am.cache.Delete(cacheKeyID) + am.cache.Delete(cacheKeyLocator) +} + func (am *assetManager) GetTokenPoolByID(ctx context.Context, poolID *fftypes.UUID) (*core.TokenPool, error) { return am.database.GetTokenPoolByID(ctx, am.namespace, poolID) } @@ -240,6 +249,7 @@ func (am *assetManager) DeleteTokenPool(ctx context.Context, poolNameOrID string if err != nil { return err } + am.removeTokenPoolFromCache(ctx, pool) if err = am.database.DeleteTokenPool(ctx, am.namespace, pool.ID); err != nil { return err } diff --git a/internal/cache/cache.go b/internal/cache/cache.go index a32b8062d0..bb25dd72a1 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -113,6 +113,8 @@ type CInterface interface { GetInt(key string) int SetInt(key string, val int) + + Delete(key string) bool } type CCache struct { @@ -130,6 +132,7 @@ func (c *CCache) Set(key string, val interface{}) { } c.cache.Set(c.name+":"+key, val, c.cacheTTL) } + func (c *CCache) Get(key string) interface{} { if !c.enabled { return nil @@ -165,6 +168,10 @@ func (c *CCache) GetInt(key string) int { return 0 } +func (c *CCache) Delete(key string) bool { + return c.cache.Delete(key) +} + type cacheManager struct { ctx context.Context enabled bool From 7615c29f26a235d5d50829ebb0b62946654ab40c Mon Sep 17 00:00:00 2001 From: Chengxuan Xing Date: Tue, 27 Jun 2023 16:33:38 +0100 Subject: [PATCH 2/2] move to use the common lib Signed-off-by: Chengxuan Xing --- go.mod | 2 +- internal/cache/cache.go | 139 +++++------------------------------ internal/cache/cache_test.go | 5 +- mocks/cachemocks/manager.go | 10 +-- 4 files changed, 28 insertions(+), 128 deletions(-) diff --git a/go.mod b/go.mod index 9916f76083..97120ca081 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,6 @@ require ( github.com/hyperledger/firefly-common v1.2.17 github.com/hyperledger/firefly-signer v1.1.8 github.com/jarcoal/httpmock v1.2.0 - github.com/karlseguin/ccache v2.0.3+incompatible github.com/lib/pq v1.10.7 github.com/mattn/go-sqlite3 v1.14.16 github.com/prometheus/client_golang v1.14.0 @@ -49,6 +48,7 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/invopop/yaml v0.2.0 // indirect github.com/josharian/intern v1.0.0 // indirect + github.com/karlseguin/ccache v2.0.3+incompatible // indirect github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/magiconair/properties v1.8.7 // indirect diff --git a/internal/cache/cache.go b/internal/cache/cache.go index bb25dd72a1..ec676fddc9 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -19,12 +19,11 @@ package cache import ( "context" "strings" - "sync" "time" + "github.com/hyperledger/firefly-common/pkg/cache" "github.com/hyperledger/firefly-common/pkg/config" "github.com/hyperledger/firefly-common/pkg/i18n" - "github.com/karlseguin/ccache" "github.com/hyperledger/firefly/internal/coreconfig" "github.com/hyperledger/firefly/internal/coremsgs" @@ -55,7 +54,7 @@ func (cc *CConfig) UniqueName() (string, error) { if err != nil { return "", err } - return cc.namespace + "::" + category, nil + return category, nil } func (cc *CConfig) Category() (string, error) { @@ -101,97 +100,20 @@ func (cc *CConfig) TTL() time.Duration { type Manager interface { GetCache(cc *CConfig) (CInterface, error) ResetCachesForNamespace(ns string) - ListKeys() []string + ListCacheNames(namespace string) []string } -type CInterface interface { - Get(key string) interface{} - Set(key string, val interface{}) - - GetString(key string) string - SetString(key string, val string) - - GetInt(key string) int - SetInt(key string, val int) - - Delete(key string) bool -} - -type CCache struct { - enabled bool - ctx context.Context - namespace string - name string - cache *ccache.Cache - cacheTTL time.Duration -} - -func (c *CCache) Set(key string, val interface{}) { - if !c.enabled { - return - } - c.cache.Set(c.name+":"+key, val, c.cacheTTL) -} - -func (c *CCache) Get(key string) interface{} { - if !c.enabled { - return nil - } - if cached := c.cache.Get(c.name + ":" + key); cached != nil { - cached.Extend(c.cacheTTL) - return cached.Value() - } - return nil -} - -func (c *CCache) SetString(key string, val string) { - c.Set(key, val) -} - -func (c *CCache) GetString(key string) string { - val := c.Get(key) - if val != nil { - return c.Get(key).(string) - } - return "" -} - -func (c *CCache) SetInt(key string, val int) { - c.Set(key, val) -} - -func (c *CCache) GetInt(key string) int { - val := c.Get(key) - if val != nil { - return c.Get(key).(int) - } - return 0 -} - -func (c *CCache) Delete(key string) bool { - return c.cache.Delete(key) -} +type CInterface cache.CInterface type cacheManager struct { - ctx context.Context - enabled bool - m sync.Mutex - // maintain a list of named configured CCache, the name are unique configuration category id - // e.g. cache.batch - configuredCaches map[string]*CCache + ffcache cache.Manager } func (cm *cacheManager) ResetCachesForNamespace(ns string) { - cm.m.Lock() - defer cm.m.Unlock() - for k, c := range cm.configuredCaches { - if c.namespace == ns { - // Clear the cache to free the memory immediately - c.cache.Clear() - // Remove it from the map, so the next call will generate a new one - delete(cm.configuredCaches, k) - } - } + cm.ffcache.ResetCaches(ns) +} +func (cm *cacheManager) ListCacheNames(namespace string) []string { + return cm.ffcache.ListCacheNames(namespace) } func (cm *cacheManager) GetCache(cc *CConfig) (CInterface, error) { @@ -203,47 +125,24 @@ func (cm *cacheManager) GetCache(cc *CConfig) (CInterface, error) { if err != nil { return nil, err } - cm.m.Lock() - cache, exists := cm.configuredCaches[cacheName] - if !exists { - cache = &CCache{ - ctx: cc.ctx, - namespace: cc.namespace, - name: cacheName, - cache: ccache.New(ccache.Configure().MaxSize(maxSize)), - cacheTTL: cc.TTL(), - enabled: cm.enabled, - } - cm.configuredCaches[cacheName] = cache - } - cm.m.Unlock() - return cache, nil -} -func (cm *cacheManager) ListKeys() []string { - keys := make([]string, 0, len(cm.configuredCaches)) - for k := range cm.configuredCaches { - keys = append(keys, k) - } - return keys + return cm.ffcache.GetCache( + cc.ctx, + cc.namespace, + cacheName, + maxSize, + cc.TTL(), + cm.ffcache.IsEnabled(), + ) } - func NewCacheManager(ctx context.Context) Manager { cm := &cacheManager{ - ctx: ctx, - enabled: config.GetBool(coreconfig.CacheEnabled), - configuredCaches: map[string]*CCache{}, + ffcache: cache.NewCacheManager(ctx, config.GetBool(coreconfig.CacheEnabled)), } return cm } // should only be used for testing purpose func NewUmanagedCache(ctx context.Context, sizeLimit int64, ttl time.Duration) CInterface { - return &CCache{ - ctx: ctx, - name: "cache.unmanaged", - cache: ccache.New(ccache.Configure().MaxSize(sizeLimit)), - cacheTTL: ttl, - enabled: true, - } + return cache.NewUmanagedCache(ctx, sizeLimit, ttl) } diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index b3a8000a18..a5b129d907 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -48,11 +48,12 @@ func TestGetCacheReturnsSameCacheForSameConfig(t *testing.T) { cache1, _ := cacheManager.GetCache(NewCacheConfig(ctx, "cache.batch.limit", "cache.batch.ttl", "testnamespace")) assert.Equal(t, cache0, cache1) - assert.Equal(t, []string{"testnamespace::cache.batch"}, cacheManager.ListKeys()) + assert.Equal(t, []string{"testnamespace:cache.batch"}, cacheManager.ListCacheNames("testnamespace")) cache2, _ := cacheManager.GetCache(NewCacheConfig(ctx, "cache.batch.limit", "cache.batch.ttl", "")) assert.NotEqual(t, cache0, cache2) - assert.Equal(t, 2, len(cacheManager.ListKeys())) + assert.Equal(t, 1, len(cacheManager.ListCacheNames("testnamespace"))) + assert.Equal(t, 1, len(cacheManager.ListCacheNames("global"))) } func TestTwoSeparateCacheWorksIndependently(t *testing.T) { diff --git a/mocks/cachemocks/manager.go b/mocks/cachemocks/manager.go index bdb0f8f2c5..84592f0543 100644 --- a/mocks/cachemocks/manager.go +++ b/mocks/cachemocks/manager.go @@ -38,13 +38,13 @@ func (_m *Manager) GetCache(cc *cache.CConfig) (cache.CInterface, error) { return r0, r1 } -// ListKeys provides a mock function with given fields: -func (_m *Manager) ListKeys() []string { - ret := _m.Called() +// ListCacheNames provides a mock function with given fields: namespace +func (_m *Manager) ListCacheNames(namespace string) []string { + ret := _m.Called(namespace) var r0 []string - if rf, ok := ret.Get(0).(func() []string); ok { - r0 = rf() + if rf, ok := ret.Get(0).(func(string) []string); ok { + r0 = rf(namespace) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]string)