diff --git a/modules/cache/context.go b/modules/cache/context.go index 5b65ec9bb0..23f7c23a52 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 - } + if c := GetContextCache(ctx); c != nil { + return ctx } 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() - 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) - } -} - -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 d8c082e352..8371c2b908 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") + v, _ = c.Get(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") - 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 c99ac2d51f..6996010ac4 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 {