From 4eade5928246d41b7cc4f32fa9d2a65614321e4e Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 17 Jan 2023 19:16:15 +0000 Subject: [PATCH 1/4] Add cache for common models. --- models/packages/descriptor.go | 87 +++++++++++++++++++++++++++++++---- 1 file changed, 79 insertions(+), 8 deletions(-) diff --git a/models/packages/descriptor.go b/models/packages/descriptor.go index 34f1cad87dc45..bad75792fbd4b 100644 --- a/models/packages/descriptor.go +++ b/models/packages/descriptor.go @@ -81,19 +81,23 @@ func (pd *PackageDescriptor) CalculateBlobSize() int64 { // GetPackageDescriptor gets the package description for a version func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDescriptor, error) { - p, err := GetPackageByID(ctx, pv.PackageID) + return getPackageDescriptor(ctx, pv, newQueryCache()) +} + +func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache) (*PackageDescriptor, error) { + p, err := c.QueryPackage(ctx, pv.PackageID) if err != nil { return nil, err } - o, err := user_model.GetUserByID(ctx, p.OwnerID) + o, err := c.QueryUser(ctx, p.OwnerID) if err != nil { return nil, err } - repository, err := repo_model.GetRepositoryByID(ctx, p.RepoID) - if err != nil && !repo_model.IsErrRepoNotExist(err) { + repository, err := c.QueryRepository(ctx, p.RepoID) + if err != nil { return nil, err } - creator, err := user_model.GetUserByID(ctx, pv.CreatorID) + creator, err := c.QueryUser(ctx, pv.CreatorID) if err != nil { return nil, err } @@ -119,7 +123,7 @@ func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDesc pfds := make([]*PackageFileDescriptor, 0, len(pfs)) for _, pf := range pfs { - pfd, err := GetPackageFileDescriptor(ctx, pf) + pfd, err := getPackageFileDescriptor(ctx, pf, c) if err != nil { return nil, err } @@ -177,7 +181,11 @@ func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDesc // GetPackageFileDescriptor gets a package file descriptor for a package file func GetPackageFileDescriptor(ctx context.Context, pf *PackageFile) (*PackageFileDescriptor, error) { - pb, err := GetBlobByID(ctx, pf.BlobID) + return getPackageFileDescriptor(ctx, pf, newQueryCache()) +} + +func getPackageFileDescriptor(ctx context.Context, pf *PackageFile, c *cache) (*PackageFileDescriptor, error) { + pb, err := c.QueryBlob(ctx, pf.BlobID) if err != nil { return nil, err } @@ -194,9 +202,13 @@ func GetPackageFileDescriptor(ctx context.Context, pf *PackageFile) (*PackageFil // GetPackageDescriptors gets the package descriptions for the versions func GetPackageDescriptors(ctx context.Context, pvs []*PackageVersion) ([]*PackageDescriptor, error) { + return getPackageDescriptors(ctx, pvs, newQueryCache()) +} + +func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache) ([]*PackageDescriptor, error) { pds := make([]*PackageDescriptor, 0, len(pvs)) for _, pv := range pvs { - pd, err := GetPackageDescriptor(ctx, pv) + pd, err := getPackageDescriptor(ctx, pv, c) if err != nil { return nil, err } @@ -204,3 +216,62 @@ func GetPackageDescriptors(ctx context.Context, pvs []*PackageVersion) ([]*Packa } return pds, nil } + +type cache struct { + Packages map[int64]*Package + Users map[int64]*user_model.User + Repositories map[int64]*repo_model.Repository + Blobs map[int64]*PackageBlob +} + +func newQueryCache() *cache { + return &cache{ + Packages: make(map[int64]*Package), + Users: make(map[int64]*user_model.User), + Repositories: map[int64]*repo_model.Repository{0: nil}, // 0 is an expected value + Blobs: make(map[int64]*PackageBlob), + } +} + +func (c *cache) QueryPackage(ctx context.Context, id int64) (*Package, error) { + if p, found := c.Packages[id]; found { + return p, nil + } + + p, err := GetPackageByID(ctx, id) + c.Packages[id] = p + return p, err +} + +func (c *cache) QueryUser(ctx context.Context, id int64) (*user_model.User, error) { + if u, found := c.Users[id]; found { + return u, nil + } + + u, err := user_model.GetUserByID(ctx, id) + c.Users[id] = u + return u, err +} + +func (c *cache) QueryRepository(ctx context.Context, id int64) (*repo_model.Repository, error) { + if r, found := c.Repositories[id]; found { + return r, nil + } + + r, err := repo_model.GetRepositoryByID(ctx, id) + if err != nil && !repo_model.IsErrRepoNotExist(err) { + err = nil + } + c.Repositories[id] = r + return r, err +} + +func (c *cache) QueryBlob(ctx context.Context, id int64) (*PackageBlob, error) { + if b, found := c.Blobs[id]; found { + return b, nil + } + + b, err := GetBlobByID(ctx, id) + c.Blobs[id] = b + return b, err +} From d35d45d2ea31f45d5c97390fab01f8148e6b774d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 13 Apr 2025 12:55:44 +0800 Subject: [PATCH 2/4] fix merge --- models/packages/descriptor.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/models/packages/descriptor.go b/models/packages/descriptor.go index dcdf0d3be54a0..7ada1e5be70b1 100644 --- a/models/packages/descriptor.go +++ b/models/packages/descriptor.go @@ -114,12 +114,9 @@ func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache) (*P if err != nil { return nil, err } - repository, err := c.QueryRepository(ctx, p.RepoID) - if err != nil { - return nil, err var repository *repo_model.Repository if p.RepoID > 0 { - repository, err = repo_model.GetRepositoryByID(ctx, p.RepoID) + repository, err = c.QueryRepository(ctx, p.RepoID) if err != nil && !repo_model.IsErrRepoNotExist(err) { return nil, err } From 045cf8bc6eb46e0781228e88b203989318f5f9c3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 13 Apr 2025 13:44:21 +0800 Subject: [PATCH 3/4] fix --- models/packages/descriptor.go | 82 +++----------------- modules/cache/context.go | 138 +++++++--------------------------- modules/cache/context_test.go | 50 ++++++------ modules/cache/ephemeral.go | 108 ++++++++++++++++++++++++++ 4 files changed, 171 insertions(+), 207 deletions(-) create mode 100644 modules/cache/ephemeral.go diff --git a/models/packages/descriptor.go b/models/packages/descriptor.go index 7ada1e5be70b1..1ea181c72320b 100644 --- a/models/packages/descriptor.go +++ b/models/packages/descriptor.go @@ -11,6 +11,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/packages/alpine" "code.gitea.io/gitea/modules/packages/arch" @@ -102,26 +103,26 @@ func (pd *PackageDescriptor) CalculateBlobSize() int64 { // GetPackageDescriptor gets the package description for a version func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDescriptor, error) { - return getPackageDescriptor(ctx, pv, newQueryCache()) + return getPackageDescriptor(ctx, pv, cache.NewEphemeralCache()) } -func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache) (*PackageDescriptor, error) { - p, err := c.QueryPackage(ctx, pv.PackageID) +func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache.EphemeralCache) (*PackageDescriptor, error) { + p, err := cache.GetWithEphemeralCache(ctx, c, "package", pv.PackageID, GetPackageByID) if err != nil { return nil, err } - o, err := c.QueryUser(ctx, p.OwnerID) + o, err := cache.GetWithEphemeralCache(ctx, c, "user", p.OwnerID, user_model.GetUserByID) if err != nil { return nil, err } var repository *repo_model.Repository if p.RepoID > 0 { - repository, err = c.QueryRepository(ctx, p.RepoID) + repository, err = cache.GetWithEphemeralCache(ctx, c, "repo", p.RepoID, repo_model.GetRepositoryByID) if err != nil && !repo_model.IsErrRepoNotExist(err) { return nil, err } } - creator, err := c.QueryUser(ctx, pv.CreatorID) + creator, err := cache.GetWithEphemeralCache(ctx, c, "user", pv.CreatorID, user_model.GetUserByID) if err != nil { if errors.Is(err, util.ErrNotExist) { creator = user_model.NewGhostUser() @@ -229,11 +230,11 @@ func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache) (*P // GetPackageFileDescriptor gets a package file descriptor for a package file func GetPackageFileDescriptor(ctx context.Context, pf *PackageFile) (*PackageFileDescriptor, error) { - return getPackageFileDescriptor(ctx, pf, newQueryCache()) + return getPackageFileDescriptor(ctx, pf, cache.NewEphemeralCache()) } -func getPackageFileDescriptor(ctx context.Context, pf *PackageFile, c *cache) (*PackageFileDescriptor, error) { - pb, err := c.QueryBlob(ctx, pf.BlobID) +func getPackageFileDescriptor(ctx context.Context, pf *PackageFile, c *cache.EphemeralCache) (*PackageFileDescriptor, error) { + pb, err := cache.GetWithEphemeralCache(ctx, c, "package_file_blob", pf.BlobID, GetBlobByID) if err != nil { return nil, err } @@ -263,10 +264,10 @@ func GetPackageFileDescriptors(ctx context.Context, pfs []*PackageFile) ([]*Pack // GetPackageDescriptors gets the package descriptions for the versions func GetPackageDescriptors(ctx context.Context, pvs []*PackageVersion) ([]*PackageDescriptor, error) { - return getPackageDescriptors(ctx, pvs, newQueryCache()) + return getPackageDescriptors(ctx, pvs, cache.NewEphemeralCache()) } -func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache) ([]*PackageDescriptor, error) { +func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache.EphemeralCache) ([]*PackageDescriptor, error) { pds := make([]*PackageDescriptor, 0, len(pvs)) for _, pv := range pvs { pd, err := getPackageDescriptor(ctx, pv, c) @@ -277,62 +278,3 @@ func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache) } return pds, nil } - -type cache struct { - Packages map[int64]*Package - Users map[int64]*user_model.User - Repositories map[int64]*repo_model.Repository - Blobs map[int64]*PackageBlob -} - -func newQueryCache() *cache { - return &cache{ - Packages: make(map[int64]*Package), - Users: make(map[int64]*user_model.User), - Repositories: map[int64]*repo_model.Repository{0: nil}, // 0 is an expected value - Blobs: make(map[int64]*PackageBlob), - } -} - -func (c *cache) QueryPackage(ctx context.Context, id int64) (*Package, error) { - if p, found := c.Packages[id]; found { - return p, nil - } - - p, err := GetPackageByID(ctx, id) - c.Packages[id] = p - return p, err -} - -func (c *cache) QueryUser(ctx context.Context, id int64) (*user_model.User, error) { - if u, found := c.Users[id]; found { - return u, nil - } - - u, err := user_model.GetUserByID(ctx, id) - c.Users[id] = u - return u, err -} - -func (c *cache) QueryRepository(ctx context.Context, id int64) (*repo_model.Repository, error) { - if r, found := c.Repositories[id]; found { - return r, nil - } - - r, err := repo_model.GetRepositoryByID(ctx, id) - if err != nil && !repo_model.IsErrRepoNotExist(err) { - err = nil - } - c.Repositories[id] = r - return r, err -} - -func (c *cache) QueryBlob(ctx context.Context, id int64) (*PackageBlob, error) { - if b, found := c.Blobs[id]; found { - return b, nil - } - - b, err := GetBlobByID(ctx, id) - c.Blobs[id] = b - return b, err -} diff --git a/modules/cache/context.go b/modules/cache/context.go index 85eb9e6790e24..5b65ec9bb0290 100644 --- a/modules/cache/context.go +++ b/modules/cache/context.go @@ -5,75 +5,17 @@ package cache import ( "context" - "sync" "time" - - "code.gitea.io/gitea/modules/log" ) -// cacheContext is a context that can be used to cache data in a request level context -// This is useful for caching data that is expensive to calculate and is likely to be -// used multiple times in a request. -type cacheContext struct { - data map[any]map[any]any - lock sync.RWMutex - created time.Time - discard bool -} - -func (cc *cacheContext) Get(tp, key any) any { - cc.lock.RLock() - defer cc.lock.RUnlock() - return cc.data[tp][key] -} - -func (cc *cacheContext) Put(tp, key, value any) { - cc.lock.Lock() - defer cc.lock.Unlock() - - if cc.discard { - return - } - - d := cc.data[tp] - if d == nil { - d = make(map[any]any) - cc.data[tp] = d - } - d[key] = value -} - -func (cc *cacheContext) Delete(tp, key any) { - cc.lock.Lock() - defer cc.lock.Unlock() - delete(cc.data[tp], key) -} - -func (cc *cacheContext) Discard() { - cc.lock.Lock() - defer cc.lock.Unlock() - cc.data = nil - cc.discard = true -} - -func (cc *cacheContext) isDiscard() bool { - cc.lock.RLock() - defer cc.lock.RUnlock() - return cc.discard -} - -// cacheContextLifetime is the max lifetime of cacheContext. -// Since cacheContext is used to cache data in a request level context, 5 minutes is enough. -// If a cacheContext is used more than 5 minutes, it's probably misuse. -const cacheContextLifetime = 5 * time.Minute - -var timeNow = time.Now +type cacheContextKeyType struct{} -func (cc *cacheContext) Expired() bool { - return timeNow().Sub(cc.created) > cacheContextLifetime -} +var cacheContextKey = cacheContextKeyType{} -var cacheContextKey = struct{}{} +// contextCacheLifetime is the max lifetime of context cache. +// Since context cache is used to cache data in a request level context, 5 minutes is enough. +// If a context cache is used more than 5 minutes, it's probably abused. +const contextCacheLifetime = 5 * time.Minute /* Since there are both WithCacheContext and WithNoCacheContext, @@ -103,78 +45,52 @@ So: */ func WithCacheContext(ctx context.Context) context.Context { - if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { + if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { if !c.isDiscard() { - // reuse parent context - return ctx + return ctx // reuse parent context } } - // FIXME: review the use of this nolint directive - return context.WithValue(ctx, cacheContextKey, &cacheContext{ //nolint:staticcheck - data: make(map[any]map[any]any), - created: timeNow(), - }) + return context.WithValue(ctx, cacheContextKey, NewEphemeralCache(contextCacheLifetime)) } -func WithNoCacheContext(ctx context.Context) context.Context { - if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { +func withNoCacheContext(ctx context.Context) context.Context { + if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { // The caller want to run long-life tasks, but the parent context is a cache context. // So we should disable and clean the cache data, or it will be kept in memory for a long time. - c.Discard() + c.discard() return ctx } - return ctx } -func GetContextData(ctx context.Context, tp, key any) any { - if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { - if c.Expired() { - // The warning means that the cache context is misused for long-life task, - // it can be resolved with WithNoCacheContext(ctx). - log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c) - return nil - } +func getContextData(ctx context.Context, tp, key any) (any, bool) { + if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { return c.Get(tp, key) } - return nil + return nil, false } -func SetContextData(ctx context.Context, tp, key, value any) { - if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { - if c.Expired() { - // The warning means that the cache context is misused for long-life task, - // it can be resolved with WithNoCacheContext(ctx). - log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c) - return - } +func setContextData(ctx context.Context, tp, key, value any) { + if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { c.Put(tp, key, value) - return } } -func RemoveContextData(ctx context.Context, tp, key any) { - if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { - if c.Expired() { - // The warning means that the cache context is misused for long-life task, - // it can be resolved with WithNoCacheContext(ctx). - log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c) - return - } +func removeContextData(ctx context.Context, tp, key any) { + if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { c.Delete(tp, key) } } // GetWithContextCache returns the cache value of the given key in the given context. +// FIXME: in most cases, the "context cache" should not be used, because it has uncontrollable behaviors +// For example, these calls: +// * GetWithContextCache(TargetID) -> OtherCodeCreateModel(TargetID) -> GetWithContextCache(TargetID) +// Will cause the second call is not able to get the correct created target. +// UNLESS it is certain that the target won't be changed during the request, DO NOT use it. func GetWithContextCache[T, K any](ctx context.Context, groupKey string, targetKey K, f func(context.Context, K) (T, error)) (T, error) { - v := GetContextData(ctx, groupKey, targetKey) - if vv, ok := v.(T); ok { - return vv, nil - } - t, err := f(ctx, targetKey) - if err != nil { - return t, err + if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { + return GetWithEphemeralCache(ctx, c, groupKey, targetKey, f) } - SetContextData(ctx, groupKey, targetKey, t) - return t, nil + return f(ctx, targetKey) } diff --git a/modules/cache/context_test.go b/modules/cache/context_test.go index 23dd789dbc28b..d8c082e3527f5 100644 --- a/modules/cache/context_test.go +++ b/modules/cache/context_test.go @@ -8,27 +8,29 @@ import ( "testing" "time" + "code.gitea.io/gitea/modules/test" + "github.com/stretchr/testify/assert" ) func TestWithCacheContext(t *testing.T) { ctx := WithCacheContext(t.Context()) - v := GetContextData(ctx, "empty_field", "my_config1") + v, _ := getContextData(ctx, "empty_field", "my_config1") assert.Nil(t, v) const field = "system_setting" - v = GetContextData(ctx, field, "my_config1") + v, _ = getContextData(ctx, field, "my_config1") assert.Nil(t, v) - SetContextData(ctx, field, "my_config1", 1) - v = GetContextData(ctx, field, "my_config1") + setContextData(ctx, field, "my_config1", 1) + v, _ = getContextData(ctx, field, "my_config1") assert.NotNil(t, v) assert.Equal(t, 1, v.(int)) - RemoveContextData(ctx, field, "my_config1") - RemoveContextData(ctx, field, "my_config2") // remove a non-exist key + removeContextData(ctx, field, "my_config1") + removeContextData(ctx, field, "my_config2") // remove a non-exist key - v = GetContextData(ctx, field, "my_config1") + v, _ = getContextData(ctx, field, "my_config1") assert.Nil(t, v) vInt, err := GetWithContextCache(ctx, field, "my_config1", func(context.Context, string) (int, error) { @@ -37,17 +39,13 @@ func TestWithCacheContext(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, vInt) - v = GetContextData(ctx, field, "my_config1") + v, _ = getContextData(ctx, field, "my_config1") assert.EqualValues(t, 1, v) - now := timeNow - defer func() { - timeNow = now - }() - timeNow = func() time.Time { - return now().Add(5 * time.Minute) - } - v = GetContextData(ctx, field, "my_config1") + defer test.MockVariableValue(&timeNow, func() time.Time { + return time.Now().Add(5 * time.Minute) + })() + v, _ = getContextData(ctx, field, "my_config1") assert.Nil(t, v) } @@ -56,23 +54,23 @@ func TestWithNoCacheContext(t *testing.T) { const field = "system_setting" - v := GetContextData(ctx, field, "my_config1") + v, _ := getContextData(ctx, field, "my_config1") assert.Nil(t, v) - SetContextData(ctx, field, "my_config1", 1) - v = GetContextData(ctx, field, "my_config1") + setContextData(ctx, field, "my_config1", 1) + v, _ = getContextData(ctx, field, "my_config1") assert.Nil(t, v) // still no cache ctx = WithCacheContext(ctx) - v = GetContextData(ctx, field, "my_config1") + v, _ = getContextData(ctx, field, "my_config1") assert.Nil(t, v) - SetContextData(ctx, field, "my_config1", 1) - v = GetContextData(ctx, field, "my_config1") + setContextData(ctx, field, "my_config1", 1) + v, _ = getContextData(ctx, field, "my_config1") assert.NotNil(t, v) - ctx = WithNoCacheContext(ctx) - v = GetContextData(ctx, field, "my_config1") + ctx = withNoCacheContext(ctx) + v, _ = getContextData(ctx, field, "my_config1") assert.Nil(t, v) - SetContextData(ctx, field, "my_config1", 1) - v = GetContextData(ctx, field, "my_config1") + setContextData(ctx, field, "my_config1", 1) + v, _ = getContextData(ctx, field, "my_config1") assert.Nil(t, v) // still no cache } diff --git a/modules/cache/ephemeral.go b/modules/cache/ephemeral.go new file mode 100644 index 0000000000000..c99ac2d51f684 --- /dev/null +++ b/modules/cache/ephemeral.go @@ -0,0 +1,108 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package cache + +import ( + "context" + "sync" + "time" + + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" +) + +// EphemeralCache is a cache that can be used to store data in a request level context +// This is useful for caching data that is expensive to calculate and is likely to be +// used multiple times in a request. +type EphemeralCache struct { + data map[any]map[any]any + lock sync.RWMutex + created time.Time + checkLifeTime time.Duration + discarded bool +} + +var timeNow = time.Now + +func NewEphemeralCache(checkLifeTime ...time.Duration) *EphemeralCache { + return &EphemeralCache{ + data: make(map[any]map[any]any), + created: timeNow(), + checkLifeTime: util.OptionalArg(checkLifeTime, 0), + } +} + +func (cc *EphemeralCache) checkExceededLifeTime(tp, key any) bool { + if cc.checkLifeTime > 0 && timeNow().Sub(cc.created) > cc.checkLifeTime { + log.Warn("EphemeralCache is expired, is highly likely to be abused for long-life tasks: %v, %v", tp, key) + return true + } + return false +} + +func (cc *EphemeralCache) Get(tp, key any) (any, bool) { + if cc.checkExceededLifeTime(tp, key) { + return nil, false + } + cc.lock.RLock() + defer cc.lock.RUnlock() + ret, ok := cc.data[tp][key] + return ret, ok +} + +func (cc *EphemeralCache) Put(tp, key, value any) { + if cc.checkExceededLifeTime(tp, key) { + return + } + + cc.lock.Lock() + defer cc.lock.Unlock() + + if cc.discarded { + return + } + + d := cc.data[tp] + if d == nil { + d = make(map[any]any) + cc.data[tp] = d + } + d[key] = value +} + +func (cc *EphemeralCache) Delete(tp, key any) { + if cc.checkExceededLifeTime(tp, key) { + return + } + + cc.lock.Lock() + defer cc.lock.Unlock() + delete(cc.data[tp], key) +} + +func (cc *EphemeralCache) discard() { + cc.lock.Lock() + defer cc.lock.Unlock() + cc.data = nil + cc.discarded = true +} + +func (cc *EphemeralCache) isDiscard() bool { + cc.lock.RLock() + defer cc.lock.RUnlock() + return cc.discarded +} + +func GetWithEphemeralCache[T, K any](ctx context.Context, c *EphemeralCache, groupKey string, targetKey K, f func(context.Context, K) (T, error)) (T, error) { + v, has := c.Get(groupKey, targetKey) + if vv, ok := v.(T); has && ok { + return vv, nil + } + t, err := f(ctx, targetKey) + if err != nil { + return t, err + } + c.Put(groupKey, targetKey, t) + return t, nil +} From 7533b333d82caf0544f406d2508817b8a4411ae2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 13 Apr 2025 14:02:26 +0800 Subject: [PATCH 4/4] refactor cache --- modules/cache/context.go | 67 ++++------------------------------- modules/cache/context_test.go | 46 ++++++------------------ modules/cache/ephemeral.go | 18 ---------- 3 files changed, 17 insertions(+), 114 deletions(-) diff --git a/modules/cache/context.go b/modules/cache/context.go index 5b65ec9bb0290..23f7c23a522de 100644 --- a/modules/cache/context.go +++ b/modules/cache/context.go @@ -17,79 +17,26 @@ var cacheContextKey = cacheContextKeyType{} // If a context cache is used more than 5 minutes, it's probably abused. const contextCacheLifetime = 5 * time.Minute -/* -Since there are both WithCacheContext and WithNoCacheContext, -it may be confusing when there is nesting. - -Some cases to explain the design: - -When: -- A, B or C means a cache context. -- A', B' or C' means a discard cache context. -- ctx means context.Backgrand(). -- A(ctx) means a cache context with ctx as the parent context. -- B(A(ctx)) means a cache context with A(ctx) as the parent context. -- With is alias of WithCacheContext. -- WithNo is alias of WithNoCacheContext. - -So: -- With(ctx) -> A(ctx) -- With(With(ctx)) -> A(ctx), not B(A(ctx)), always reuse parent cache context if possible. -- With(With(With(ctx))) -> A(ctx), not C(B(A(ctx))), ditto. -- WithNo(ctx) -> ctx, not A'(ctx), don't create new cache context if we don't have to. -- WithNo(With(ctx)) -> A'(ctx) -- WithNo(WithNo(With(ctx))) -> A'(ctx), not B'(A'(ctx)), don't create new cache context if we don't have to. -- With(WithNo(With(ctx))) -> B(A'(ctx)), not A(ctx), never reuse a discard cache context. -- WithNo(With(WithNo(With(ctx)))) -> B'(A'(ctx)) -- With(WithNo(With(WithNo(With(ctx))))) -> C(B'(A'(ctx))), so there's always only one not-discard cache context. -*/ - func WithCacheContext(ctx context.Context) context.Context { - if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { - if !c.isDiscard() { - return ctx // reuse parent context - } - } - return context.WithValue(ctx, cacheContextKey, NewEphemeralCache(contextCacheLifetime)) -} - -func withNoCacheContext(ctx context.Context) context.Context { - if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { - // The caller want to run long-life tasks, but the parent context is a cache context. - // So we should disable and clean the cache data, or it will be kept in memory for a long time. - c.discard() + if c := GetContextCache(ctx); c != nil { return ctx } - return ctx -} - -func getContextData(ctx context.Context, tp, key any) (any, bool) { - if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { - return c.Get(tp, key) - } - return nil, false -} - -func setContextData(ctx context.Context, tp, key, value any) { - if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { - c.Put(tp, key, value) - } + return context.WithValue(ctx, cacheContextKey, NewEphemeralCache(contextCacheLifetime)) } -func removeContextData(ctx context.Context, tp, key any) { - if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { - c.Delete(tp, key) - } +func GetContextCache(ctx context.Context) *EphemeralCache { + c, _ := ctx.Value(cacheContextKey).(*EphemeralCache) + return c } // GetWithContextCache returns the cache value of the given key in the given context. -// FIXME: in most cases, the "context cache" should not be used, because it has uncontrollable behaviors +// FIXME: in some cases, the "context cache" should not be used, because it has uncontrollable behaviors // For example, these calls: // * GetWithContextCache(TargetID) -> OtherCodeCreateModel(TargetID) -> GetWithContextCache(TargetID) // Will cause the second call is not able to get the correct created target. // UNLESS it is certain that the target won't be changed during the request, DO NOT use it. func GetWithContextCache[T, K any](ctx context.Context, groupKey string, targetKey K, f func(context.Context, K) (T, error)) (T, error) { - if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok { + if c := GetContextCache(ctx); c != nil { return GetWithEphemeralCache(ctx, c, groupKey, targetKey, f) } return f(ctx, targetKey) diff --git a/modules/cache/context_test.go b/modules/cache/context_test.go index d8c082e3527f5..8371c2b9080e0 100644 --- a/modules/cache/context_test.go +++ b/modules/cache/context_test.go @@ -15,22 +15,22 @@ import ( func TestWithCacheContext(t *testing.T) { ctx := WithCacheContext(t.Context()) - - v, _ := getContextData(ctx, "empty_field", "my_config1") + c := GetContextCache(ctx) + v, _ := c.Get("empty_field", "my_config1") assert.Nil(t, v) const field = "system_setting" - v, _ = getContextData(ctx, field, "my_config1") + v, _ = c.Get(field, "my_config1") assert.Nil(t, v) - setContextData(ctx, field, "my_config1", 1) - v, _ = getContextData(ctx, field, "my_config1") + c.Put(field, "my_config1", 1) + v, _ = c.Get(field, "my_config1") assert.NotNil(t, v) assert.Equal(t, 1, v.(int)) - removeContextData(ctx, field, "my_config1") - removeContextData(ctx, field, "my_config2") // remove a non-exist key + c.Delete(field, "my_config1") + c.Delete(field, "my_config2") // remove a non-exist key - v, _ = getContextData(ctx, field, "my_config1") + v, _ = c.Get(field, "my_config1") assert.Nil(t, v) vInt, err := GetWithContextCache(ctx, field, "my_config1", func(context.Context, string) (int, error) { @@ -39,38 +39,12 @@ func TestWithCacheContext(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, vInt) - v, _ = getContextData(ctx, field, "my_config1") + v, _ = c.Get(field, "my_config1") assert.EqualValues(t, 1, v) defer test.MockVariableValue(&timeNow, func() time.Time { return time.Now().Add(5 * time.Minute) })() - v, _ = getContextData(ctx, field, "my_config1") - assert.Nil(t, v) -} - -func TestWithNoCacheContext(t *testing.T) { - ctx := t.Context() - - const field = "system_setting" - - v, _ := getContextData(ctx, field, "my_config1") - assert.Nil(t, v) - setContextData(ctx, field, "my_config1", 1) - v, _ = getContextData(ctx, field, "my_config1") - assert.Nil(t, v) // still no cache - - ctx = WithCacheContext(ctx) - v, _ = getContextData(ctx, field, "my_config1") - assert.Nil(t, v) - setContextData(ctx, field, "my_config1", 1) - v, _ = getContextData(ctx, field, "my_config1") - assert.NotNil(t, v) - - ctx = withNoCacheContext(ctx) - v, _ = getContextData(ctx, field, "my_config1") + v, _ = c.Get(field, "my_config1") assert.Nil(t, v) - setContextData(ctx, field, "my_config1", 1) - v, _ = getContextData(ctx, field, "my_config1") - assert.Nil(t, v) // still no cache } diff --git a/modules/cache/ephemeral.go b/modules/cache/ephemeral.go index c99ac2d51f684..6996010ac4b45 100644 --- a/modules/cache/ephemeral.go +++ b/modules/cache/ephemeral.go @@ -20,7 +20,6 @@ type EphemeralCache struct { lock sync.RWMutex created time.Time checkLifeTime time.Duration - discarded bool } var timeNow = time.Now @@ -59,10 +58,6 @@ func (cc *EphemeralCache) Put(tp, key, value any) { cc.lock.Lock() defer cc.lock.Unlock() - if cc.discarded { - return - } - d := cc.data[tp] if d == nil { d = make(map[any]any) @@ -81,19 +76,6 @@ func (cc *EphemeralCache) Delete(tp, key any) { delete(cc.data[tp], key) } -func (cc *EphemeralCache) discard() { - cc.lock.Lock() - defer cc.lock.Unlock() - cc.data = nil - cc.discarded = true -} - -func (cc *EphemeralCache) isDiscard() bool { - cc.lock.RLock() - defer cc.lock.RUnlock() - return cc.discarded -} - func GetWithEphemeralCache[T, K any](ctx context.Context, c *EphemeralCache, groupKey string, targetKey K, f func(context.Context, K) (T, error)) (T, error) { v, has := c.Get(groupKey, targetKey) if vv, ok := v.(T); has && ok {