Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve cache context #23330

Merged
merged 12 commits into from
Mar 8, 2023
119 changes: 103 additions & 16 deletions modules/cache/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package cache
import (
"context"
"sync"
"time"

"code.gitea.io/gitea/modules/log"
)
Expand All @@ -14,65 +15,151 @@ import (
// 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 {
ctx context.Context
lunny marked this conversation as resolved.
Show resolved Hide resolved
data map[any]map[any]any
lock sync.RWMutex
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()
if cc.data[tp] == nil {
lunny marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
return cc.data[tp][key]
}

func (cc *cacheContext) Put(tp, key, value any) {
cc.lock.Lock()
defer cc.lock.Unlock()
if cc.data[tp] == nil {
cc.data[tp] = make(map[any]any)

if cc.discard {
return
}

d := cc.data[tp]
if d == nil {
d = make(map[any]any)
cc.data[tp] = d
}
cc.data[tp][key] = value
d[key] = value
}

func (cc *cacheContext) Delete(tp, key any) {
cc.lock.Lock()
defer cc.lock.Unlock()
if cc.data[tp] == nil {
return
}
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{}{}

/*
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).(*cacheContext); ok {
if !c.isDiscard() {
// reuse parent context
return ctx
}
lunny marked this conversation as resolved.
Show resolved Hide resolved
}
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 {
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)
}
log.Warn("cannot get cache context when getting data: %v", ctx)
return nil
}

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, may be misused for long-life tasks: %v", c)
return
}
c.Put(tp, key, value)
return
}
log.Warn("cannot get cache context when setting data: %v", ctx)
}

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, may be misused for long-life tasks: %v", c)
return
}
c.Delete(tp, key)
}
}
Expand Down
39 changes: 38 additions & 1 deletion modules/cache/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package cache
import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"
)
Expand All @@ -25,7 +26,7 @@ func TestWithCacheContext(t *testing.T) {
assert.EqualValues(t, 1, v.(int))

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")
assert.Nil(t, v)
Expand All @@ -38,4 +39,40 @@ func TestWithCacheContext(t *testing.T) {

v = GetContextData(ctx, field, "my_config1")
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
}
1 change: 0 additions & 1 deletion services/repository/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
defer finished()
ctx = cache.WithCacheContext(ctx)

repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, optsList[0].RepoUserName, optsList[0].RepoName)
if err != nil {
Expand Down