From 29ff1a4f155b0a6539d64132c17623e1c90eecb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 5 Jun 2023 00:13:23 +0200 Subject: [PATCH] add stat cache to sharesstorageprovider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/sharesstorageprovidercache.md | 5 ++ .../services/sharesstorageprovider/options.go | 56 +++++++++++++++++++ .../sharesstorageprovider.go | 48 +++++++++++++--- .../sharesstorageprovider_test.go | 5 +- 4 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 changelog/unreleased/sharesstorageprovidercache.md create mode 100644 internal/grpc/services/sharesstorageprovider/options.go diff --git a/changelog/unreleased/sharesstorageprovidercache.md b/changelog/unreleased/sharesstorageprovidercache.md new file mode 100644 index 0000000000..61c4f8ba1e --- /dev/null +++ b/changelog/unreleased/sharesstorageprovidercache.md @@ -0,0 +1,5 @@ +Enhancement: add sharesstorageprovider cache + +After disabling the cache in the gateway we added a statcache to the sharesstorageprovider to make the share jail more efficient. + +https://github.com/cs3org/reva/pull/3940 diff --git a/internal/grpc/services/sharesstorageprovider/options.go b/internal/grpc/services/sharesstorageprovider/options.go new file mode 100644 index 0000000000..b588c83e3c --- /dev/null +++ b/internal/grpc/services/sharesstorageprovider/options.go @@ -0,0 +1,56 @@ +// Copyright 2018-2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package sharesstorageprovider + +import ( + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" + "github.com/cs3org/reva/v2/pkg/storage/cache" +) + +// Option is used to pass options +type Option func(opts *Options) + +// Options represent options +type Options struct { + GatewayAPIClient gateway.GatewayAPIClient + CollaborationAPIClient collaboration.CollaborationAPIClient + StatCache cache.StatCache +} + +// WithGatewayAPIClient allows to set the gateway selector option +func WithGatewayAPIClient(v gateway.GatewayAPIClient) Option { + return func(o *Options) { + o.GatewayAPIClient = v + } +} + +// WithCollaborationAPIClient allows to set the opentelemetry tracer provider for grpc clients +func WithCollaborationAPIClient(v collaboration.CollaborationAPIClient) Option { + return func(o *Options) { + o.CollaborationAPIClient = v + } +} + +// WithStatCache allows to set the registry for service lookup +func WithStatCache(v cache.StatCache) Option { + return func(o *Options) { + o.StatCache = v + } +} diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 709631a042..1df4446132 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -23,7 +23,9 @@ import ( "fmt" "path/filepath" "strings" + "time" + "github.com/cs3org/reva/v2/pkg/storage/cache" "github.com/cs3org/reva/v2/pkg/storagespace" "google.golang.org/grpc" codes "google.golang.org/grpc/codes" @@ -57,16 +59,19 @@ func init() { } type config struct { - GatewayAddr string `mapstructure:"gateway_addr"` - UserShareProviderEndpoint string `mapstructure:"usershareprovidersvc"` + GatewayAddr string `mapstructure:"gateway_addr"` + UserShareProviderEndpoint string `mapstructure:"usershareprovidersvc"` + StatCache cache.Config `mapstructure:"statcache"` } type service struct { gateway gateway.GatewayAPIClient sharesProviderClient collaboration.CollaborationAPIClient + statCache cache.StatCache } func (s *service) Close() error { + s.statCache.Close() return nil } @@ -86,6 +91,13 @@ func NewDefault(m map[string]interface{}, _ *grpc.Server) (rgrpc.Service, error) return nil, err } + if c.StatCache.Store == "" { + c.StatCache.Store = "noop" + } + if c.StatCache.TTL == 0 { + c.StatCache.TTL = 30 // 30sec is the default desktop client poll interval + } + gateway, err := pool.GetGatewayServiceClient(sharedconf.GetGatewaySVC(c.GatewayAddr)) if err != nil { return nil, err @@ -96,14 +108,24 @@ func NewDefault(m map[string]interface{}, _ *grpc.Server) (rgrpc.Service, error) return nil, errors.Wrap(err, "sharesstorageprovider: error getting UserShareProvider client") } - return New(gateway, client) + return New( + WithGatewayAPIClient(gateway), + WithCollaborationAPIClient(client), + WithStatCache(cache.GetStatCache(c.StatCache.Store, c.StatCache.Nodes, c.StatCache.Database, "stat", time.Duration(c.StatCache.TTL)*time.Second, c.StatCache.Size)), + ) } // New returns a new instance of the SharesStorageProvider service -func New(gateway gateway.GatewayAPIClient, c collaboration.CollaborationAPIClient) (rgrpc.Service, error) { +func New(opts ...Option) (rgrpc.Service, error) { + options := Options{} + // first use selector options + for _, opt := range opts { + opt(&options) + } s := &service{ - gateway: gateway, - sharesProviderClient: c, + gateway: options.GatewayAPIClient, + sharesProviderClient: options.CollaborationAPIClient, + statCache: options.StatCache, } return s, nil } @@ -356,7 +378,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora var shareInfo map[string]*provider.ResourceInfo var err error if fetchShares { - receivedShares, shareInfo, err = s.fetchShares(ctx) + receivedShares, shareInfo, err = s.fetchShares(ctx) // we should cache the stat for the share jail per user if err != nil { return nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest") } @@ -1029,7 +1051,16 @@ func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedSha // convert backwards compatible share id rs.Share.ResourceId.StorageId, rs.Share.ResourceId.SpaceId = storagespace.SplitStorageID(rs.Share.ResourceId.StorageId) } - sRes, err := s.gateway.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: rs.Share.ResourceId}}) + ref := &provider.Reference{ResourceId: rs.Share.ResourceId} + key := s.statCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId(), ref, []string{}, []string{}) + if key != "" { + info := &provider.ResourceInfo{} + if err := s.statCache.PullFromCache(key, info); err == nil { + shareMetaData[rs.Share.Id.OpaqueId] = info + continue + } + } + sRes, err := s.gateway.Stat(ctx, &provider.StatRequest{Ref: ref}) if err != nil { appctx.GetLogger(ctx).Error(). Err(err). @@ -1044,6 +1075,7 @@ func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedSha Msg("ListRecievedShares: failed to stat the resource") continue } + _ = s.statCache.PushToCache(key, sRes.Info) shareMetaData[rs.Share.Id.OpaqueId] = sRes.Info } diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 555cf7968f..9bf44d457a 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go @@ -294,7 +294,10 @@ var _ = Describe("Sharesstorageprovider", func() { }) JustBeforeEach(func() { - p, err := provider.New(gw, sharesProviderClient) + p, err := provider.New( + provider.WithCollaborationAPIClient(sharesProviderClient), + provider.WithGatewayAPIClient(gw), + ) Expect(err).ToNot(HaveOccurred()) s = p.(sprovider.ProviderAPIServer) Expect(s).ToNot(BeNil())