Kill service spanning stat cache

changelog/unreleased/kill-service-spanning-stat-cache.md

@@ -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

internal/grpc/services/gateway/gateway.go

@@ -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"`
@@ -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"
  }
@@ -146,7 +137,6 @@ type svc struct {
  c *config
  dataGatewayURL url.URL
  tokenmgr token.Manager
- statCache cache.StatCache
  providerCache cache.ProviderCache
  createPersonalSpaceCache cache.CreatePersonalSpaceCache
 }
@@ -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),
  }
@@ -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

internal/grpc/services/gateway/ocmshareprovider.go

@@ -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

internal/grpc/services/gateway/publicshareprovider.go

@@ -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"
 )
@@ -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) {
@@ -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) {
@@ -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)
 }

internal/grpc/services/gateway/storageprovider.go

@@ -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
@@ -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 {
@@ -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,
@@ -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
 }
 
@@ -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
 }
 
@@ -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)
 }
 
@@ -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
 }
 
@@ -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
 }
@@ -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
 }
 
@@ -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
 }
 
@@ -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 {
@@ -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
 }
 
@@ -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
 }
 
@@ -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
 }
 
@@ -1106,7 +1090,6 @@ func (s *svc) getStorageProviderClient(_ context.Context, p *registry.ProviderIn
 
  return &cachedAPIClient{
  c: c,
- statCache: s.statCache,
  createPersonalSpaceCache: s.createPersonalSpaceCache,
  }, nil
 }

internal/grpc/services/gateway/storageprovidercache.go

@@ -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"
@@ -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())
@@ -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...)
 }
@@ -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...)
 }
@@ -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...)
 }