Improve cache context (#23330)

Related to: #22294 #23186 #23054

Replace: #23218

Some discussion is in the comments of #23218.

Highlights:
- Add Expiration for cache context. If a cache context has been used for
more than 10s, the cache data will be ignored, and warning logs will be
printed.
- Add `discard` field to `cacheContext`, a `cacheContext` with `discard`
true will drop all cached data and won't store any new one.
- Introduce `WithNoCacheContext`, if one wants to run long-life tasks,
but the parent context is a cache context,
`WithNoCacheContext(perentCtx)` will discard the cache data, so it will
be safe to keep the context for a long time.
- It will be fine to treat an original context as a cache context, like
`GetContextData(context.Backgraud())`, no warning logs will be printed.

Some cases about nesting:

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` means `WithCacheContext`
- `WithNo` means `WithNoCacheContext`

So:
- `With(ctx)` -> *A(ctx)*
- `With(With(ctx))` -> *A(ctx)*, not *B(A(ctx))*
- `With(With(With(ctx)))` -> *A(ctx)*, not *C(B(A(ctx)))*
- `WithNo(ctx)` -> *ctx*, not *~A~(ctx)*
- `WithNo(With(ctx))` -> *~A~(ctx)*
- `WithNo(WithNo(With(ctx)))` -> *~A~(ctx)*, not *~B~(~A~(ctx))*
- `With(WithNo(With(ctx)))` -> *B(~A~(ctx))*
- `WithNo(With(WithNo(With(ctx))))` -> *~B~(~A~(ctx))*
- `With(WithNo(With(WithNo(With(ctx)))))` -> *C(~B~(~A~(ctx)))*
This commit is contained in:
Jason Song 2023-03-09 01:57:05 +08:00 committed by GitHub
parent d949d8e074
commit 1960ad5c90
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 141 additions and 18 deletions

View File

@ -6,6 +6,7 @@ package cache
import ( import (
"context" "context"
"sync" "sync"
"time"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
) )
@ -14,65 +15,151 @@ import (
// This is useful for caching data that is expensive to calculate and is likely to be // This is useful for caching data that is expensive to calculate and is likely to be
// used multiple times in a request. // used multiple times in a request.
type cacheContext struct { type cacheContext struct {
ctx context.Context
data map[any]map[any]any data map[any]map[any]any
lock sync.RWMutex lock sync.RWMutex
created time.Time
discard bool
} }
func (cc *cacheContext) Get(tp, key any) any { func (cc *cacheContext) Get(tp, key any) any {
cc.lock.RLock() cc.lock.RLock()
defer cc.lock.RUnlock() defer cc.lock.RUnlock()
if cc.data[tp] == nil {
return nil
}
return cc.data[tp][key] return cc.data[tp][key]
} }
func (cc *cacheContext) Put(tp, key, value any) { func (cc *cacheContext) Put(tp, key, value any) {
cc.lock.Lock() cc.lock.Lock()
defer cc.lock.Unlock() defer cc.lock.Unlock()
if cc.data[tp] == nil {
cc.data[tp] = make(map[any]any) if cc.discard {
return
} }
cc.data[tp][key] = value
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) { func (cc *cacheContext) Delete(tp, key any) {
cc.lock.Lock() cc.lock.Lock()
defer cc.lock.Unlock() defer cc.lock.Unlock()
if cc.data[tp] == nil {
return
}
delete(cc.data[tp], key) 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, 10s is enough.
// If a cacheContext is used more than 10s, it's probably misuse.
const cacheContextLifetime = 10 * time.Second
var timeNow = time.Now
func (cc *cacheContext) Expired() bool {
return timeNow().Sub(cc.created) > cacheContextLifetime
}
var cacheContextKey = struct{}{} var cacheContextKey = struct{}{}
/*
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 { func WithCacheContext(ctx context.Context) context.Context {
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
if !c.isDiscard() {
// reuse parent context
return ctx
}
}
return context.WithValue(ctx, cacheContextKey, &cacheContext{ return context.WithValue(ctx, cacheContextKey, &cacheContext{
ctx: ctx,
data: make(map[any]map[any]any), data: make(map[any]map[any]any),
created: timeNow(),
}) })
} }
func WithNoCacheContext(ctx context.Context) context.Context {
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); 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()
return ctx
}
return ctx
}
func GetContextData(ctx context.Context, tp, key any) any { func GetContextData(ctx context.Context, tp, key any) any {
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { 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, may be misused for long-life tasks: %v", c)
return nil
}
return c.Get(tp, key) return c.Get(tp, key)
} }
log.Warn("cannot get cache context when getting data: %v", ctx)
return nil return nil
} }
func SetContextData(ctx context.Context, tp, key, value any) { func SetContextData(ctx context.Context, tp, key, value any) {
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { 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, may be misused for long-life tasks: %v", c)
return
}
c.Put(tp, key, value) c.Put(tp, key, value)
return return
} }
log.Warn("cannot get cache context when setting data: %v", ctx)
} }
func RemoveContextData(ctx context.Context, tp, key any) { func RemoveContextData(ctx context.Context, tp, key any) {
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok { 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, may be misused for long-life tasks: %v", c)
return
}
c.Delete(tp, key) c.Delete(tp, key)
} }
} }

View File

@ -6,6 +6,7 @@ package cache
import ( import (
"context" "context"
"testing" "testing"
"time"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -25,7 +26,7 @@ func TestWithCacheContext(t *testing.T) {
assert.EqualValues(t, 1, v.(int)) assert.EqualValues(t, 1, v.(int))
RemoveContextData(ctx, field, "my_config1") RemoveContextData(ctx, field, "my_config1")
RemoveContextData(ctx, field, "my_config2") // remove an non-exist key 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) assert.Nil(t, v)
@ -38,4 +39,40 @@ func TestWithCacheContext(t *testing.T) {
v = GetContextData(ctx, field, "my_config1") v = GetContextData(ctx, field, "my_config1")
assert.EqualValues(t, 1, v) assert.EqualValues(t, 1, v)
now := timeNow
defer func() {
timeNow = now
}()
timeNow = func() time.Time {
return now().Add(10 * time.Second)
}
v = GetContextData(ctx, field, "my_config1")
assert.Nil(t, v)
}
func TestWithNoCacheContext(t *testing.T) {
ctx := context.Background()
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")
assert.Nil(t, v)
SetContextData(ctx, field, "my_config1", 1)
v = GetContextData(ctx, field, "my_config1")
assert.Nil(t, v) // still no cache
} }

View File

@ -80,7 +80,6 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("PushUpdates: %s/%s", optsList[0].RepoUserName, optsList[0].RepoName)) ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("PushUpdates: %s/%s", optsList[0].RepoUserName, optsList[0].RepoName))
defer finished() defer finished()
ctx = cache.WithCacheContext(ctx)
repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, optsList[0].RepoUserName, optsList[0].RepoName) repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, optsList[0].RepoUserName, optsList[0].RepoName)
if err != nil { if err != nil {