Skip to content

Commit

Permalink
Kill service spanning stat cache
Browse files Browse the repository at this point in the history 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
  • Loading branch information
@butonic
butonic committed Mar 1, 2024
1 parent 8f0bcdd commit 4b1c69a
Show file tree
Hide file tree
Showing 14 changed files with 48 additions and 161 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/kill-service-spanning-stat-cache.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Change: Drop unused service spanning stat cache

We removed the stat cache shared between gateway and storage providers. It is constantly invalidated and needs a different approach.

https://github.com/cs3org/reva/pull/4542
12 changes: 0 additions & 12 deletions internal/grpc/services/gateway/gateway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ type config struct {
DataTransfersFolder string `mapstructure:"data_transfers_folder"`
TokenManagers map[string]map[string]interface{} `mapstructure:"token_managers"`
AllowedUserAgents map[string][]string `mapstructure:"allowed_user_agents"` // map[path][]user-agent
StatCacheConfig cache.Config `mapstructure:"stat_cache_config"`
CreatePersonalSpaceCacheConfig cache.Config `mapstructure:"create_personal_space_cache_config"`
ProviderCacheConfig cache.Config `mapstructure:"provider_cache_config"`
UseCommonSpaceRootShareLogic bool `mapstructure:"use_common_space_root_share_logic"`
Expand Down Expand Up @@ -117,14 +116,6 @@ func (c *config) init() {
}

// caching needs to be explicitly enabled
if c.StatCacheConfig.Store == "" {
c.StatCacheConfig.Store = "noop"
}

if c.StatCacheConfig.Database == "" {
c.StatCacheConfig.Database = "reva"
}

if c.ProviderCacheConfig.Store == "" {
c.ProviderCacheConfig.Store = "noop"
}
Expand All @@ -146,7 +137,6 @@ type svc struct {
c *config
dataGatewayURL url.URL
tokenmgr token.Manager
statCache cache.StatCache
providerCache cache.ProviderCache
createPersonalSpaceCache cache.CreatePersonalSpaceCache
}
Expand Down Expand Up @@ -177,7 +167,6 @@ func New(m map[string]interface{}, _ *grpc.Server) (rgrpc.Service, error) {
c: c,
dataGatewayURL: *u,
tokenmgr: tokenManager,
statCache: cache.GetStatCache(c.StatCacheConfig),
providerCache: cache.GetProviderCache(c.ProviderCacheConfig),
createPersonalSpaceCache: cache.GetCreatePersonalSpaceCache(c.CreatePersonalSpaceCacheConfig),
}
Expand All @@ -190,7 +179,6 @@ func (s *svc) Register(ss *grpc.Server) {
}

func (s *svc) Close() error {
s.statCache.Close()
s.providerCache.Close()
s.createPersonalSpaceCache.Close()
return nil
Expand Down
14 changes: 5 additions & 9 deletions internal/grpc/services/gateway/ocmshareprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,17 @@ func (s *svc) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareRequest
}

status, err := s.addGrant(ctx, req.ResourceId, req.Grantee, req.AccessMethods[0].GetWebdavOptions().Permissions, req.Expiration, nil)
if err != nil {
switch {
case err != nil:
appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg(err.Error())
return nil, errors.Wrap(err, "gateway: error adding grant to storage")
}

switch status.Code {
case rpc.Code_CODE_OK:
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.ResourceId)
case rpc.Code_CODE_UNIMPLEMENTED:
case status.Code == rpc.Code_CODE_UNIMPLEMENTED:
appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg("storing grants not supported, ignoring")
default:
case status.Code != rpc.Code_CODE_OK:
appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg("storing grants is not successful")
return &ocm.CreateOCMShareResponse{
Status: status,
}, err
}, nil
}

return res, nil
Expand Down
34 changes: 3 additions & 31 deletions internal/grpc/services/gateway/publicshareprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@ package gateway
import (
"context"

userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
"github.com/cs3org/reva/v2/pkg/appctx"
ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
"github.com/pkg/errors"
)
Expand All @@ -39,15 +37,7 @@ func (s *svc) CreatePublicShare(ctx context.Context, req *link.CreatePublicShare
return nil, err
}

res, err := c.CreatePublicShare(ctx, req)
if err != nil {
return nil, err
}

if res.GetShare() != nil {
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), res.Share.ResourceId)
}
return res, nil
return c.CreatePublicShare(ctx, req)
}

func (s *svc) RemovePublicShare(ctx context.Context, req *link.RemovePublicShareRequest) (*link.RemovePublicShareResponse, error) {
Expand All @@ -58,13 +48,7 @@ func (s *svc) RemovePublicShare(ctx context.Context, req *link.RemovePublicShare
if err != nil {
return nil, err
}
res, err := driver.RemovePublicShare(ctx, req)
if err != nil {
return nil, err
}
// TODO: How to find out the resourceId? -> get public share first, then delete
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), nil)
return res, nil
return driver.RemovePublicShare(ctx, req)
}

func (s *svc) GetPublicShareByToken(ctx context.Context, req *link.GetPublicShareByTokenRequest) (*link.GetPublicShareByTokenResponse, error) {
Expand Down Expand Up @@ -137,17 +121,5 @@ func (s *svc) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicShare
}, nil
}

res, err := pClient.UpdatePublicShare(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "error updating share")
}
if res.GetShare() != nil {
s.statCache.RemoveStatContext(ctx,
&userprovider.UserId{
OpaqueId: res.Share.Owner.GetOpaqueId(),
},
res.Share.ResourceId,
)
}
return res, nil
return pClient.UpdatePublicShare(ctx, req)
}
17 changes: 0 additions & 17 deletions internal/grpc/services/gateway/storageprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorag

if res.Status.Code == rpc.Code_CODE_OK {
id := res.StorageSpace.Root
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), id)
s.providerCache.RemoveListStorageProviders(id)
}
return res, nil
Expand Down Expand Up @@ -363,7 +362,6 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
}

id := &provider.ResourceId{OpaqueId: req.Id.OpaqueId}
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), id)
s.providerCache.RemoveListStorageProviders(id)

if dsRes.Status.Code != rpc.Code_CODE_OK {
Expand Down Expand Up @@ -608,7 +606,6 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile
}
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return &gateway.InitiateFileUploadResponse{
Opaque: storageRes.Opaque,
Status: storageRes.Status,
Expand Down Expand Up @@ -645,7 +642,6 @@ func (s *svc) CreateContainer(ctx context.Context, req *provider.CreateContainer
}, nil
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand Down Expand Up @@ -688,7 +684,6 @@ func (s *svc) Delete(ctx context.Context, req *provider.DeleteRequest) (*provide
}, nil
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand All @@ -715,8 +710,6 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo

req.Source = sref
req.Destination = dref
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Source.ResourceId)
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Destination.ResourceId)
return c.Move(ctx, req)
}

Expand All @@ -739,7 +732,6 @@ func (s *svc) SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitra
return nil, errors.Wrap(err, "gateway: error calling SetArbitraryMetadata")
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand All @@ -761,7 +753,6 @@ func (s *svc) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArb
}
return nil, errors.Wrap(err, "gateway: error calling UnsetArbitraryMetadata")
}
s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)

return res, nil
}
Expand All @@ -785,7 +776,6 @@ func (s *svc) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provi
return nil, errors.Wrap(err, "gateway: error calling SetLock")
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand Down Expand Up @@ -826,7 +816,6 @@ func (s *svc) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest)
return nil, errors.Wrap(err, "gateway: error calling RefreshLock")
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand All @@ -847,12 +836,10 @@ func (s *svc) Unlock(ctx context.Context, req *provider.UnlockRequest) (*provide
return nil, errors.Wrap(err, "gateway: error calling Unlock")
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

// Stat returns the Resoure info for a given resource by forwarding the request to the responsible provider.
// TODO cache info
func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) {
c, _, ref, err := s.findAndUnwrapUnique(ctx, req.Ref)
if err != nil {
Expand Down Expand Up @@ -927,7 +914,6 @@ func (s *svc) RestoreFileVersion(ctx context.Context, req *provider.RestoreFileV
}, nil
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand Down Expand Up @@ -983,7 +969,6 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc
}, nil
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand All @@ -1006,7 +991,6 @@ func (s *svc) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleReques
}, nil
}

s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId)
return res, nil
}

Expand Down Expand Up @@ -1106,7 +1090,6 @@ func (s *svc) getStorageProviderClient(_ context.Context, p *registry.ProviderIn

return &cachedAPIClient{
c: c,
statCache: s.statCache,
createPersonalSpaceCache: s.createPersonalSpaceCache,
}, nil
}
Expand Down
85 changes: 29 additions & 56 deletions internal/grpc/services/gateway/storageprovidercache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package gateway

import (
"context"
"strings"

rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
Expand Down Expand Up @@ -84,40 +83,9 @@ func (c *cachedRegistryClient) GetHome(ctx context.Context, in *registry.GetHome

type cachedAPIClient struct {
c provider.ProviderAPIClient
statCache cache.StatCache
createPersonalSpaceCache cache.CreatePersonalSpaceCache
}

// Stat looks in cache first before forwarding to storage provider
func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, opts ...grpc.CallOption) (*provider.StatResponse, error) {
key := c.statCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId(), in.GetRef(), in.GetArbitraryMetadataKeys(), in.GetFieldMask().GetPaths())
if key != "" {
s := &provider.StatResponse{}
if err := c.statCache.PullFromCache(key, s); err == nil {
return s, nil
}
}

resp, err := c.c.Stat(ctx, in, opts...)
switch {
case err != nil:
return nil, err
case resp.Status.Code != rpc.Code_CODE_OK:
return resp, nil
case key == "":
return resp, nil
case strings.Contains(key, "sid:"+utils.ShareStorageProviderID):
// We cannot cache shares at the moment:
// we do not know when to invalidate them
// FIXME: find a way to cache/invalidate them too
return resp, nil
case utils.ReadPlainFromOpaque(resp.GetInfo().GetOpaque(), "status") == "processing":
return resp, nil
default:
return resp, c.statCache.PushToCache(key, resp)
}
}

// CreateHome caches calls to CreateHome locally - anyways they only need to be called once per user
func (c *cachedAPIClient) CreateHome(ctx context.Context, in *provider.CreateHomeRequest, opts ...grpc.CallOption) (*provider.CreateHomeResponse, error) {
key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId())
Expand All @@ -140,8 +108,37 @@ func (c *cachedAPIClient) CreateHome(ctx context.Context, in *provider.CreateHom
}
}

// CreateStorageSpace creates a storage space
func (c *cachedAPIClient) CreateStorageSpace(ctx context.Context, in *provider.CreateStorageSpaceRequest, opts ...grpc.CallOption) (*provider.CreateStorageSpaceResponse, error) {
if in.Type == "personal" {
key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId())
if key != "" {
s := &provider.CreateStorageSpaceResponse{}
if err := c.createPersonalSpaceCache.PullFromCache(key, s); err == nil {
return s, nil
}
}
resp, err := c.c.CreateStorageSpace(ctx, in, opts...)
switch {
case err != nil:
return nil, err
case resp.Status.Code != rpc.Code_CODE_OK && resp.Status.Code != rpc.Code_CODE_ALREADY_EXISTS:
return resp, nil
case key == "":
return resp, nil
default:
return resp, c.createPersonalSpaceCache.PushToCache(key, resp)
}
}
return c.c.CreateStorageSpace(ctx, in, opts...)
}

// methods below here are not cached, they just call the client directly

// Stat returns the Resoure info for a given resource
func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, opts ...grpc.CallOption) (*provider.StatResponse, error) {
return c.c.Stat(ctx, in, opts...)
}
func (c *cachedAPIClient) AddGrant(ctx context.Context, in *provider.AddGrantRequest, opts ...grpc.CallOption) (*provider.AddGrantResponse, error) {
return c.c.AddGrant(ctx, in, opts...)
}
Expand Down Expand Up @@ -229,29 +226,6 @@ func (c *cachedAPIClient) Unlock(ctx context.Context, in *provider.UnlockRequest
func (c *cachedAPIClient) GetHome(ctx context.Context, in *provider.GetHomeRequest, opts ...grpc.CallOption) (*provider.GetHomeResponse, error) {
return c.c.GetHome(ctx, in, opts...)
}
func (c *cachedAPIClient) CreateStorageSpace(ctx context.Context, in *provider.CreateStorageSpaceRequest, opts ...grpc.CallOption) (*provider.CreateStorageSpaceResponse, error) {
if in.Type == "personal" {
key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId())
if key != "" {
s := &provider.CreateStorageSpaceResponse{}
if err := c.createPersonalSpaceCache.PullFromCache(key, s); err == nil {
return s, nil
}
}
resp, err := c.c.CreateStorageSpace(ctx, in, opts...)
switch {
case err != nil:
return nil, err
case resp.Status.Code != rpc.Code_CODE_OK && resp.Status.Code != rpc.Code_CODE_ALREADY_EXISTS:
return resp, nil
case key == "":
return resp, nil
default:
return resp, c.createPersonalSpaceCache.PushToCache(key, resp)
}
}
return c.c.CreateStorageSpace(ctx, in, opts...)
}
func (c *cachedAPIClient) ListStorageSpaces(ctx context.Context, in *provider.ListStorageSpacesRequest, opts ...grpc.CallOption) (*provider.ListStorageSpacesResponse, error) {
return c.c.ListStorageSpaces(ctx, in, opts...)
}
Expand All @@ -261,7 +235,6 @@ func (c *cachedAPIClient) UpdateStorageSpace(ctx context.Context, in *provider.U
func (c *cachedAPIClient) DeleteStorageSpace(ctx context.Context, in *provider.DeleteStorageSpaceRequest, opts ...grpc.CallOption) (*provider.DeleteStorageSpaceResponse, error) {
return c.c.DeleteStorageSpace(ctx, in, opts...)
}

func (c *cachedAPIClient) TouchFile(ctx context.Context, in *provider.TouchFileRequest, opts ...grpc.CallOption) (*provider.TouchFileResponse, error) {
return c.c.TouchFile(ctx, in, opts...)
}
Loading

0 comments on commit 4b1c69a

Please sign in to comment.