From 88f1acbdfc526fa69a8f48f634f5de0307b48508 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Mon, 22 Jan 2024 12:33:59 +0300 Subject: [PATCH] [#165] Cancel context in outdated list session Signed-off-by: Denis Kirillov --- api/cache/listsession.go | 10 ++++++- api/handler/handlers_test.go | 16 +++++------ api/handler/object_list_test.go | 48 +++++++++++++++++++++++++++++++++ api/layer/cache.go | 2 +- api/layer/layer.go | 4 +-- api/layer/versioning_test.go | 2 +- cmd/s3-gw/app.go | 2 +- 7 files changed, 69 insertions(+), 15 deletions(-) diff --git a/api/cache/listsession.go b/api/cache/listsession.go index 0cf051f9..7858c109 100644 --- a/api/cache/listsession.go +++ b/api/cache/listsession.go @@ -55,7 +55,9 @@ func NewListSessionCache(config *Config) *ListSessionCache { zap.String("expected", fmt.Sprintf("%T", session))) } - // todo session.Cancel() + if !session.Acquired.Load() { + session.Cancel() + } }).Build() return &ListSessionCache{cache: gc, logger: config.Logger} } @@ -79,6 +81,12 @@ func (l *ListSessionCache) GetListSession(key ListSessionKey) *data.ListSession // PutListSession puts a list of object versions to cache. func (l *ListSessionCache) PutListSession(key ListSessionKey, session *data.ListSession) error { + s := l.GetListSession(key) + if s != nil && s != session { + if !s.Acquired.Load() { + s.Cancel() + } + } return l.cache.Set(key, session) } diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index 450ffc7b..44e23979 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -46,6 +46,7 @@ type handlerContext struct { layerFeatures *layer.FeatureSettingsMock treeMock *tree.ServiceClientMemory + cache *layer.Cache } func (hc *handlerContext) Handler() *handler { @@ -126,14 +127,14 @@ func (c *configMock) ResolveNamespaceAlias(ns string) string { } func prepareHandlerContext(t *testing.T) *handlerContext { - return prepareHandlerContextBase(t, false) + return prepareHandlerContextBase(t, layer.DefaultCachesConfigs(zap.NewExample())) } func prepareHandlerContextWithMinCache(t *testing.T) *handlerContext { - return prepareHandlerContextBase(t, true) + return prepareHandlerContextBase(t, getMinCacheConfig(zap.NewExample())) } -func prepareHandlerContextBase(t *testing.T, minCache bool) *handlerContext { +func prepareHandlerContextBase(t *testing.T, cacheCfg *layer.CachesConfig) *handlerContext { key, err := keys.NewPrivateKey() require.NoError(t, err) @@ -153,15 +154,10 @@ func prepareHandlerContextBase(t *testing.T, minCache bool) *handlerContext { treeMock := tree.NewTree(memCli, zap.NewExample()) - cacheCfg := layer.DefaultCachesConfigs(l) - if minCache { - cacheCfg = getMinCacheConfig(l) - } - features := &layer.FeatureSettingsMock{} layerCfg := &layer.Config{ - Caches: cacheCfg, + Cache: layer.NewCache(cacheCfg), AnonKey: layer.AnonymousKey{Key: key}, Resolver: testResolver, TreeService: treeMock, @@ -194,6 +190,7 @@ func prepareHandlerContextBase(t *testing.T, minCache bool) *handlerContext { layerFeatures: features, treeMock: memCli, + cache: layerCfg.Cache, } } @@ -207,6 +204,7 @@ func getMinCacheConfig(logger *zap.Logger) *layer.CachesConfig { Logger: logger, Objects: minCacheCfg, ObjectsList: minCacheCfg, + SessionList: minCacheCfg, Names: minCacheCfg, Buckets: minCacheCfg, System: minCacheCfg, diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go index 6e1feb87..c10e98a6 100644 --- a/api/handler/object_list_test.go +++ b/api/handler/object_list_test.go @@ -1,17 +1,22 @@ package handler import ( + "context" "fmt" "net/http" "net/url" "sort" "strconv" "testing" + "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/cache" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/encryption" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func TestParseContinuationToken(t *testing.T) { @@ -60,6 +65,49 @@ func TestListObjectNullVersions(t *testing.T) { require.Equal(t, data.UnversionedObjectVersionID, result.Version[1].VersionID) } +func TestListObjectsContextCanceled(t *testing.T) { + layerCfg := layer.DefaultCachesConfigs(zaptest.NewLogger(t)) + layerCfg.SessionList.Lifetime = time.Hour + layerCfg.SessionList.Size = 1 + + hc := prepareHandlerContextBase(t, layerCfg) + + bktName := "bucket-versioning-enabled" + bktInfo := createTestBucket(hc, bktName) + + for i := 0; i < 4; i++ { + putObject(hc, bktName, "object"+strconv.Itoa(i)) + } + + result := listObjectsV1(hc, bktName, "", "", "", 2) + session := hc.cache.GetListSession(hc.owner, cache.CreateListSessionCacheKey(bktInfo.CID, "", result.NextMarker)) + // invoke list again to trigger cache eviction + // (use empty prefix to check that context canceled on replace) + listObjectsV1(hc, bktName, "", "", "", 2) + checkContextCanceled(t, session.Context) + + result2 := listObjectsV2(hc, bktName, "", "", "", "", 2) + session2 := hc.cache.GetListSession(hc.owner, cache.CreateListSessionCacheKey(bktInfo.CID, "", result2.NextContinuationToken)) + // invoke list again to trigger cache eviction + // (use non-empty prefix to check that context canceled on cache eviction) + listObjectsV2(hc, bktName, "o", "", "", "", 2) + checkContextCanceled(t, session2.Context) + + result3 := listObjectsVersions(hc, bktName, "", "", "", "", 2) + session3 := hc.cache.GetListSession(hc.owner, cache.CreateListSessionCacheKey(bktInfo.CID, "", result3.NextVersionIDMarker)) + // invoke list again to trigger cache eviction + listObjectsVersions(hc, bktName, "o", "", "", "", 2) + checkContextCanceled(t, session3.Context) +} + +func checkContextCanceled(t *testing.T, ctx context.Context) { + select { + case <-ctx.Done(): + case <-time.After(10 * time.Second): + } + require.ErrorIs(t, ctx.Err(), context.Canceled) +} + func TestListObjectsLatestVersions(t *testing.T) { hc := prepareHandlerContext(t) diff --git a/api/layer/cache.go b/api/layer/cache.go index b3c2c565..02f095a1 100644 --- a/api/layer/cache.go +++ b/api/layer/cache.go @@ -51,7 +51,7 @@ func NewCache(cfg *CachesConfig) *Cache { return &Cache{ logger: cfg.Logger, listsCache: cache.NewObjectsListCache(cfg.ObjectsList), - sessionListCache: cache.NewListSessionCache(cfg.ObjectsList), + sessionListCache: cache.NewListSessionCache(cfg.SessionList), objCache: cache.New(cfg.Objects), namesCache: cache.NewObjectsNameCache(cfg.Names), bucketCache: cache.NewBucketCache(cfg.Buckets), diff --git a/api/layer/layer.go b/api/layer/layer.go index ecf1df59..24870afe 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -69,7 +69,7 @@ type ( Config struct { GateOwner user.ID ChainAddress string - Caches *CachesConfig + Cache *Cache AnonKey AnonymousKey Resolver BucketResolver TreeService TreeService @@ -323,7 +323,7 @@ func NewLayer(log *zap.Logger, frostFS FrostFS, config *Config) Client { gateOwner: config.GateOwner, anonKey: config.AnonKey, resolver: config.Resolver, - cache: NewCache(config.Caches), + cache: config.Cache, treeService: config.TreeService, features: config.Features, } diff --git a/api/layer/versioning_test.go b/api/layer/versioning_test.go index 2bb25447..705ec00c 100644 --- a/api/layer/versioning_test.go +++ b/api/layer/versioning_test.go @@ -168,7 +168,7 @@ func prepareContext(t *testing.T, cachesConfig ...*CachesConfig) *testContext { user.IDFromKey(&owner, key.PrivateKey.PublicKey) layerCfg := &Config{ - Caches: config, + Cache: NewCache(config), AnonKey: AnonymousKey{Key: key}, TreeService: NewTreeService(), Features: &FeatureSettingsMock{}, diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index 684cdb1b..18160672 100644 --- a/cmd/s3-gw/app.go +++ b/cmd/s3-gw/app.go @@ -165,7 +165,7 @@ func (a *App) initLayer(ctx context.Context) { user.IDFromKey(&gateOwner, a.key.PrivateKey.PublicKey) layerCfg := &layer.Config{ - Caches: getCacheOptions(a.cfg, a.log), + Cache: layer.NewCache(getCacheOptions(a.cfg, a.log)), AnonKey: layer.AnonymousKey{ Key: randomKey, },